nk_button_image does not center image

[diggit]

As title says, nk_button_image does not center align provided image, on the other hand nk_button_label does center align text. Is there any way how to achieve same behavior for nk_button_image?

[dumblob]

I don't know the answer, but search the source in the spirit of other single header libs :wink:.

[diggit]

I already checked code. There does not seem to be straightforward solution. Question is: Is that intended behavior? I can change it to be centered and open PR if there is chance of merging...

[BenjiBoy926]

I'm really glad that someone noticed this because I also ran into this issue and have been wondering about a "proper" solution. I was already brainstorming ways of getting around it in my own code, but it'd be nice if a robust fix could be added to the library itself

[dumblob]

There is definitely chance of merging (I also have the rights for that). The expectation is, that it's unnecessarily complicated to do it outside of Nuklear (i.e. in the app which uses Nuklear), that the library change will be as much compatible with the current API as possible, that if it breaks anything in the repository (demos etc.), then it'll be corrected in the pull request, and that it behaves "the same as text".

The reason why general bitmap support is the most basic one, but text support is way more extensive seems to boil down to the fact, that text support had to be that complicated (from baking up to styling, wrapping etc.) to bring it to a minimal usable state.

[BenjiBoy926]

So you're saying that's why it's possible to align text but not images, since text actually needs all of those features to function properly?

[dumblob]

@BenjiBoy926 don't be so surprised - that's exactly how software development works - adding features doesn't mean they were planned from the very beginning. By the way there is a significant different between font rendering and general vector rendering, so keep in mind we can't "just use it for images as well".

We're eager to see proposals how to unify Nuklear API and make it more consistent and we're willing to merge them if viable.

[diggit]

Hmm, in which level should be image centering done? Eg. you can achieve image centering by modifying lowest function...

NK_API void
nk_draw_image(struct nk_command_buffer *b, struct nk_rect r,
    const struct nk_image *img, struct nk_color col)
{
    ...
    cmd->x = (short)r.x;
    cmd->y = (short)r.y;
    cmd->w = (unsigned short)NK_MAX(0, r.w);
    cmd->h = (unsigned short)NK_MAX(0, r.h);
    ...
}

to

NK_API void
nk_draw_image(struct nk_command_buffer *b, struct nk_rect r,
    const struct nk_image *img, struct nk_color col)
{
    ...
    cmd->w = (unsigned short)NK_MAX(0, r.w);
    cmd->h = (unsigned short)NK_MAX(0, r.h);
    cmd->x = (short)r.x + (cmd->w - img->w)/2;
    cmd->y = (short)r.y + (cmd->h - img->h)/2;
    ...
}

but this affects backgrounds too and IDK if it is desired...

Another option would be some wrapper function like:

NK_API void
nk_draw_image_centered(...)
{
    ...
}

and calling only from places where it makes sense.

[dumblob]

nk_draw_image can be changed IMHO, but as I wrote, the rest of the library, examples and demos shall be updated to accommodate for this change. But if writing your own wrapper is easier than changing Nuklear, then I'll hesitate to merge the pull.

[diggit]

By writing wrapper, I meant adding possibility to call nk_draw_image_centered instead of nk_draw_image in some widgets like buttons trees and such, but preserving current behavior for calls where image is meant as background etc.

[BenjiBoy926]

I like the idea of a wrapper to let the user decide how they want to align the image. Couldn't we even take this a step further with a function that takes in the alignment as an argument?

NK_API void nk_image(struct nk_context*, struct nk_image);
NK_API void nk_image_color(struct nk_context*, struct nk_image, struct nk_color);
// Here's the new function!
NK_API void nk_image_align(struct nk_context*, struct nk_image, nk_flags alignment);

We could then let the nk_flags argument trickle down into those lower functions like nk_draw_image, which might now look something like:

NK_API void
nk_draw_image(struct nk_command_buffer *b, struct nk_rect r,
    const struct nk_image *img, struct nk_color col, nk_flags align)
{
    ...
    cmd->w = (unsigned short)NK_MAX(0, r.w);
    cmd->h = (unsigned short)NK_MAX(0, r.h);
    cmd->x = some_function_of_alignment(align);
    cmd->y = some_function_of_alignment(align);
    ...
}

Then the user could call the function like:

nk_image_align(ctx, img, NK_TEXT_CENTERED);

Although admittedly this is problematic since NK_TEXT_CENTERED doesn't have to do with images

[dumblob]

Yep, the "wrapper" with arguments designating what to do is the way to go. What about though making the API even more general to accommodate for possible future things like affine transformations (if ever implemented) - see e.g. https://github.com/vurtun/nuklear/issues/841 ?

/* probably not C89, so don't forget to change it... */
union nk_affine_transform {
  struct {
    enum {
      NK_TRANSFORM_ABS = 0,
      NK_TRANSFORM_REL_VCENTER = 1 << 0,
      NK_TRANSFORM_REL_HCENTER = 1 << 1,
      NK_TRANSFORM_REL_VTOP = 1 << 2,
      NK_TRANSFORM_REL_VBOTTOM = 1 << 3,
      NK_TRANSFORM_REL_HLEFT = 1 << 4,
      NK_TRANSFORM_REL_HRIGHT = 1 << 5
    } flags;
    nk_int h;
    nk_int v;
  } translate;
  struct {
    ...
  } rotate;
  ...
}
enum nk_affine_transform_kind {
  NK_TRANSFORM_TRANSLATE,
  NK_TRANSFORM_ROTATE,
  ...,
}
NK_API void nk_image_transformed(struct nk_context*, struct nk_image,
    enum nk_affine_transform_kind, union nk_affine_transform);

(just an idea, so feel free to solve it differently/better)

[BenjiBoy926]

I'd like to check on something before I end up rewriting code that already exists within nuklear. Do generic alignment functions already exist in the codebase? The kind of function I'm talking about might have a signature like this:

NK_LIB struct nk_vec2 nk_align(struct nk_rect parent, struct nk_vec2 child_dimensions, enum nk_align);

Where the return value nk_vec2 is the x-y coordinate of the child. I could easily implement it myself, but I just want to double check to see if it already exists

[dumblob]

@BenjiBoy926 no, there are no generic alignment functions in Nuklear. They are usually also not needed as there is support for text alignment in all functions/places in Nuklear and everything accommodate to these text alignment parameters (e.g. button icon shall go automatically to the right if text alignment is left). There is everywhere also support for padding of more or less any visual object.

Therefore I'm still not convinced there is place for generic alignment. If I'm not mistaken, this would be for the first time a "meta" function was introduced to Nuklear - immediate mode is rather about being direct without any "meta" functionality. Another issue with "meta" stuff is, that it won't be clear at all why we can use this "meta" function only for very limited number of Nuklear functions...

On the other hand this topic is just (?) about image in a button (i.e. nothing complicated) and I think we can introduce an aligned variant of the function, but then for consistency reasons we shall introduce aligned variants of all similar functions in Nuklear (basically everywhere where there is one or more visual objects "contained" in some "rectangle").

[BenjiBoy926]

I think I may have misinterpreted something, so just to clarify, are we going to add alignment to the plain nk_image functions that do nothing but draw an image? Or are we only concerned here with nk_button_image? Because I do think it would be useful to be able to align even a standalone image inside of the space it's being laid out in.

Another question. The nk_image struct has members unsigned short w, h, but the user only initializes them if they use nk_subimage_xxx initializers. So do w, h represent the actual width / height of the image, or is it something more complicated that has to do with subimages?

[dumblob]

are we going to add alignment to the plain nk_image functions that do nothing but draw an image? Or are we only concerned here with nk_button_image?

Rule of thumb says the only thing which matters is the stuff which is expected to be used by a regular programmer (i.e. not a programmer who wants to hack the library). This generally boils down to stuff tagged with NK_API. So these functions shall support direct alignment.

Subimage is just a rectangular cutout (useful e.g. in games for texture mapping). See nk_draw_list_add_image as that's the only place in Nuklear where subimages are used. If you won't find answer to your questions, just let me know.

[BenjiBoy926]

Hey @dumblob I found a bug that I fixed unrelated to this issue and I want to start a pull request. Could you add me to the list of contributors, or whatever it is you do to give me permission to publish a new branch?

[dumblob]

Could you add me to the list of contributors, or whatever it is you do to give me permission to publish a new branch?

We're trying to live a review culture, so don't hesitate to make a pull request and we'll try to review it as soon as possible (I get every second day a notification about new pull requests in this repository, so you can look at them to see what pops up during reviews :wink:).

[BenjiBoy926]

@dumblob I must be misunderstanding something about github then, because for some reason whenever I try to publish my new branch, it says: Permission to vurtun/nuklear.git denied to BenjiBoy926. I double-checked my password and username input and everything and still nothin'

[dumblob]

That's weird. Try cloning the repo again, create a new branch, copy the changed files from your current working repo, try to commit and try the push. It might lead to issues if you cloned the repo using https, but cloning using ssh shall "never" make the repo read-only.

[BenjiBoy926]

Oh, I think I actually did clone the repo using https. How do you clone a repository using ssh? I've never done that, before

[dumblob]

Oh, I think I actually did clone the repo using https.

To avoid misunderstanding, cloning it using https shall work as well, but under certain situations it might lead to "difficulties" like a different master branch link etc. Regarding ssh access, please refer to https://help.github.com/en/articles/connecting-to-github-with-ssh .

[diggit]

Hi @BenjiBoy926, You don't have necessary permissions to push to this repo (and that is fine). You can find workflow for example here or here. In short:

• create fork of this repo and clone it
• create branch with meaningful name
• commit your changes
• test everything ;)
• open pull request to merge your changes into vurtun/nuklear
• wait for review, do requested changes - if any (pushing to that branch updates pull request)
• profit?

Also, please open new issues with such questions/problems. It is kinda off-topic to this issue.