New image handling and image types

[FrostKiwi]

So far Nuklear has ignored image aspect ratio and getting images to properly display has been by hacking your way around the layout system. This PR introduces enum nk_image_type in struct nk_image to remedy this and obsoletes PR #444. The new image types are demonstrated here:

image_types.mp4

The image type is a setting defined upon image creation and does not need to be supplied each time nk_image is called. The API for displaying the image stays the same. nk_image() generates the NK_COMMAND_IMAGE with correct sizes to honor aspect ratio based on the surround layout.

• NK_IMAGE_STRETCH: The default stretching, as required for styling to function properly
• NK_IMAGE_FILL: Honors the image's aspect ratio, fills out the entire target rect
• NK_IMAGE_FIT: Honors the image's aspect ratio, fits the entire image into the target rect
• NK_IMAGE_CENTER: 1:1 pixel-mapping, center the image
• NK_IMAGE_TILE: 1:1 pixel-mapping, sample image outside of UV range. It is required that the backend is set to texture tiling mode. (Which is the default OpenGL Texture behaviour anyways.). This will not work with nk_subimage in this PR's current form, as subimage just modifies UVs. This really has to be addressed, but it cannot happen inside of the backend agnostic nuklear.h. I think providing a utility function in nuklear_util.c to copy a the subimage rect into a new buffer, so the user may upload that buffer to create a new texture.

This PR is an API break: It requires the user to provide image-size upon image creation, no exceptions. Also the image type has to be provided upon nk_image creation. This is required for any of this to work and is a long overdue change IMO. This PR is also essential for the interface element that I'm porting to Nuklear "cards" to work. Since this API break is needed, I took the chance to rename some functions for more clarity. Eg. struct nk_image nk_image_ptr(void*); is now nk_image_from_ptr(void*, nk_ushort w, nk_ushort h, enum nk_image_type);, to make it clear, that this function is for generating a new nk_image from a pointer, not for providing the pointer of an nk_image.

Now there is no more new command type needed, as I did in PR #444, which is much nicer to maintain.

TODO:

• Fix demo files to speak the new API, so the Github's build pipline does not complain
• Make nk_subimage versions of all the new image types, except NK_IMAGE_TILE
• Introduce a function in 'nuklear_util.c' to solve the problem of nk_subimage being incompatible with tiled image drawing. (Unless someone has a better idea...)

[dumblob]

Generally this looks like a promising approach to me.

It requires the user to provide image-size upon image creation, no exceptions.

This though is a bit too strict in my opinion. I can not imagine myself tracking dimensions of each image, icon, ... I use in my app.

Why actually not emulate the current "old" API (which was extremely limited but did not require specifying dimensions) using this your new API?

Actually I dare to think this new approach could be implemented in a backwards compatible way (i.e. just extending the current API) without the need to introduce 5.x.y version.

What do you all think?

[FrostKiwi]

What do you all think?

Huh... Now that you mention it, maybe that is too strict. So keep the old commands, always defaulting to NK_IMAGE_STRETCH and create a set of new ones, where you have to specify size I guess?

I can not imagine myself tracking dimensions of each image, icon

Isn't that always automatically known when you load the image?

[dumblob]

Huh... Now that you mention it, maybe that is too strict. So keep the old commands, always defaulting to NK_IMAGE_STRETCH and create a set of new ones, where you have to specify size I guess?

Yes, right now that would be my preference. But I have a weird feeling I am missing something so please come up with other ideas if you have any 😉.

I can not imagine myself tracking dimensions of each image, icon

Isn't that always automatically known when you load the image?

Yes. What I meant though is the syntactical pollution of maintaining variables (scalars, arrays, maps, whatever...) holding the dimensions for no apparent benefit. Does that make more sense now?

[FrostKiwi]

Yes, right now that would be my preference. But I have a weird feeling I am missing something so please come up with other ideas if you have any 😉.

Created two sets of commands, one with type in the name, reverted changes to old commands, removed [5.x] tag from issue titled, since there is no more API break.

[dumblob]

Nice, thanks. Do you still plan to work on the last TODO items in the initial post above or shall we make our hands dirty with a full-scale review already?

[RobLoach]

I really like where this is going!

[FrostKiwi]

Note to self: Use Nuklear's internal floor(), instead of the math.h floor(). Creates an unresolved dependancy

[wuxxerno]

Is there anything I can help with to get this merged?

[FrostKiwi]

Is there anything I can help with to get this merged?

I've given you access to my repo and the branch, that this PR is connected to. Go wild, in case you have any ideas.

I'll revive my Nuklear efforts over the weekend. What it got stuck on last time I wanted to contribute, are the Krita design files + nk_card code, that I couldn't publish due to being derived from company internal design guideslines.

I'll recreate them over the weekend from FOSS material, post that and then return to this PR.

[wuxxerno]

I was able to start on this today. I updated the NK_IMAGE_CENTER case to use uv coords instead as it didnt work correctly on dynamic layout. I will continue with implementing the nk subimage as per @FrostKiwi todo notes.

[FrostKiwi]

I was able to start on this today. I updated the NK_IMAGE_CENTER case to use uv coords instead as it didnt work correctly on dynamic layout. I will continue with implementing the nk subimage as per @FrostKiwi todo notes.

Many many thanks for that excellent support. Finished my Pull Request regarding the skin design file #548 and will return to look at Nuklear next week.

[FrostKiwi]

as it didnt work with dynamic layouting

@wuxxerno Ohh wow, you were right. How did I miss that. Glad to have this figured out, I really didn't want to do the math there to get it to work without scissor^^

You made changes directly to nuklear.h, instead of the src/ source code of nuklear. That huge nuklear.h is auto-generated and should not be changed. You should do your changes in src/ and either include directly from the source or build the final nuklear.h with src/paq.sh or src/paq.bat on windows.
For instance I test in demo/glfw_opengl3, where I have a test.sh, which rebuilds the final nuklear.h and compiles the test program.

cd ../../src && sh paq.sh && cd - && make && ./bin/demo.exe

I moved your contributions over to with fc97bb9 to src/nuklear_vertex.c

[wuxxerno]

as it didnt work with dynamic layouting

@wuxxerno Ohh wow, you were right. How did I miss that. Glad to have this figured out, I really didn't want to do the math there to get it to work without scissor^^

You made changes directly to nuklear.h, instead of the src/ source code of nuklear. That huge nuklear.h is auto-generated and should not be changed. You should do your changes in src/ and either include directly from the source or build the final nuklear.h with src/paq.sh or src/paq.bat on windows. For instance I test in demo/glfw_opengl3, where I have a test.sh, which rebuilds the final nuklear.h and compiles the test program.

cd ../../src && sh paq.sh && cd - && make && ./bin/demo.exe

I moved your contributions over to with fc97bb9 to src/nuklear_vertex.c

I see! thank you for explaining and correcting that.

[wuxxerno]

I just finished implementing the nk_subimage handling. (All but NK_IMAGE_TILE)
Im not really sure on how to proceed with problem of tiling subimages

I think providing a utility function in nuklear_util.c to copy a the subimage rect into a new buffer, so the user may upload that buffer to create a new texture.

Did you mean something totally generic like a function that swaps and crops two user inputted buffers that the user then can use upload as a new texture for a new nk_image? @FrostKiwi

@dumblob
Im happy to work on any feedback if you would like to start reviewing.

[FrostKiwi]

I just finished implementing the nk_subimage handling. (All but NK_IMAGE_TILE)

That is HUGE! It was the quint essential challenge remaining to get this PR to an at least theoretically mergable state.

Im not really sure on how to proceed with problem of tiling subimages

In short: there is no solution from the position of a backend agnostic library, because the tile needs to tile (heh). Something only the backend can do. (Except rendering a ton of nk_images to tile window, resulting in horribly wasteful performance and blowing past all kinds of buffers on a high res screen with a small tile)

Did you mean something totally generic like a function that swaps and crops

Yes. A function which writes the content of an nk_subimage as defined by the subregion to a buffer, to a buffer specified by the user with a pointer. Similar to snprintf(), it returns the written size and if the user pointer is NULL, it returns just the needed size without writing anything, so the user may malloc() the needed amount.

The user may then setup tiling correctly. (By uploading as a separate texture, or bundle them in a texture array). It's less of a solution and more of a convenient tool to help the user setup tiling. (technically there is no reason this couldn't be done via a custom shader with frac() returning UVs in GLSL even without such an nk_util.h, but we should give at least some kind of universal solution, even if naive)

[malespiaut]

Hello,

I've tried to reproduce your (@FrostKiwi) example code using the SDL2 Surface backend, and the whole context crashes when trying to display NK_IMAGE_TILE.

record.mp4

Here's my modified nuklear/demo/sdl_renderer/main.c with the needed images.

sdl_renderer.zip

I hope this isn't a bug in my own implementation!

[FrostKiwi]

@malespiaut

Hello,

I've tried to reproduce your (@FrostKiwi) example code using the SDL2 Surface backend, and the whole context crashes when trying to display NK_IMAGE_TILE.

This should be really working. As mentioned in the doc, NK_IMAGE_TILE is a special case requiring cooperation from the rendering backend:

    /* Just draw the image with UVs for 1:1 pixel mapping. Image is draw outside
       it's bounds. What is drawn outside the bounds is determined by the
       backend. Eg. To get tiling behaviour with OpenGL, the texture should have
       been uploaded to the GPU with GL_REPEAT:
       glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_REPEAT);
       glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_REPEAT);
       (Which is the default OpenGL Texture behaviour anyways.) */

I followed your example and implemented it in SDL_renderer, loaded up the texture via SDL_Texture *texture = IMG_LoadTexture(renderer, "bin/bunny_wide.png"); and let it run in NK_IMAGE_TILE mode.
Same deal, the rendering disappears. Neither you video, nor my recreation resulted in a crash though, just the blue background.

Checked SDL_GetError() and here is the cause:
[Error] Values of 'uv' out of bounds 0.525253 1.420455 at 320/360
So the cause is clear. No crash, no mishandling, SDL2 (When handling the textures through its API) just refuses to draw images outside its UV bounds, which is required for the most simple implementation of tiling.

I always used GLFW for my projects and never used SDL2, so from here on out I can't say why SDL2 refuses drawing a texture outside it's bounds. Here is what ChatGPT had to say on the topic:

SDL itself doesn't directly support repeating textures (tiling) by setting texture coordinates outside this range as you might in OpenGL or Direct3D. This is because SDL abstracts away the underlying graphics API to provide a simple, cross-platform rendering API, which doesn't expose direct control over texture wrap modes.

PS: gave you commit access to this Pull Request, in-case you want to contribute a demo or fix.

[malespiaut]

I followed your example and implemented it in SDL_renderer, loaded up the texture via SDL_Texture *texture = IMG_LoadTexture(renderer, "bin/bunny_wide.png"); and let it run in NK_IMAGE_TILE mode. Same deal, the rendering disappears. Neither you video, nor my recreation resulted in a crash though, just the blue background.

There is no crash. It's just me quitting the application and starting it again to show the bug in three different way (resize, scroll, different window).

Sorry for my miscommunication.

PS: gave you commit access to this Pull Request, in-case you want to contribute a demo or fix.

Thank you very much, I'll do that!

[RobLoach]

It has been officially more than 2 years since this Pull Request was opened. Is there anything we can do to help push it forwards? What else is missing?