Add SDL3 #779
Add SDL3 #779
Conversation
|
Sure! I'll take a look in a few hours. Thanks |
|
Also made it thread-safe. There are a couple things that I think could improve. I'm not 100% sure it's cleaning up the memory correctly tho |
demo/sdl3/nuklear_sdl3_renderer.h
Outdated
| char *str = 0; | ||
| (void)usr; | ||
| if (!len) return; | ||
| str = (char*)SDL_malloc((size_t)len+1); |
There was a problem hiding this comment.
In case of multi-byte text, like SDL TextInput which is unicode, len is count grapheme clusters or glyphs, not bytes.
| return SDL_Fail(); | ||
| } | ||
|
|
||
| SDL_StartTextInput(appContext->window); |
There was a problem hiding this comment.
On desktop platforms, SDL_StartTextInput() is implicitly called on SDL window creation. On some, typically non desktop, platforms this call implicitly enables and shows virtual keyboard.
There was a problem hiding this comment.
In SDL3, this needs to be explicitly called on Desktop platforms. I think adding a comment about the behavior of calling this on mobile is probably in order
There was a problem hiding this comment.
Hmm, is there a cleaner way we could accomplish this? Perhaps a callback when a textbox on focus/blur?
There was a problem hiding this comment.
According to SDL docs, call to SDL_StartTextInput needs to be paired with SDL_StopTextInput, probably implying that TextInput should NOT be active all the time https://wiki.libsdl.org/SDL3/SDL_StartTextInput#remarks
For reference, take a look at how one of SDL tests implements this: https://github.com/libsdl-org/SDL/blob/cc9937201e421ec55b12ad3f07ff2268f15096e8/test/checkkeys.c#L372-L395
demo/sdl3/nuklear_sdl3_renderer.h
Outdated
|
|
||
| case SDL_EVENT_MOUSE_MOTION: | ||
| if (ctx->input.mouse.grabbed) { | ||
| int x = (int)ctx->input.mouse.prev.x, y = (int)ctx->input.mouse.prev.y; |
There was a problem hiding this comment.
SDL3 (mouse) movement events were changed from integer to float. Nuklear uses float internally too. Is the integer cast truncation intentional?
There was a problem hiding this comment.
This was a hack on my part, changing to float and removing the cast should be fine
There was a problem hiding this comment.
Know the significance of ctx->input.mouse.grabbed? Looking at the other implementations, I'm not quite sure how they'd apply here.
There was a problem hiding this comment.
It seems as if while window is moved around by grabbing it with mouse, then absolute mouse pointer x,y cannot be trusted and thus relative x,y are being used.
The SDL documentation says in this scenario SDL_GetGlobalMouseState() is supposed to be used.
But actually the situation is more complex. Typically a full screen or borderless window SDL application would force relative mouse movement, and locked mouse into window by SDL_SetRelativeMouseMode. This mode grabs the mouse pointer by the window and hides the system cursor or something like that. And when the application switches to windowed mode, then relative mouse mode is disabled and mouse is only locked otionally into window when window gets focus and mouse gets unlocked when window loses focus.
There was a problem hiding this comment.
After some testing, I think what I've put in there seems to be working with or without window grabbing.
case SDL_EVENT_MOUSE_MOTION:
ctx->input.mouse.prev = ctx->input.mouse.pos;
ctx->input.mouse.delta.x = evt->motion.xrel;
ctx->input.mouse.delta.y = evt->motion.yrel;
ctx->input.mouse.pos.x = evt->motion.x;
ctx->input.mouse.pos.y = evt->motion.y;
return 1;I played around with SetRelativeMouseMode too, and it didn't seem to have any real affect...
case SDL_EVENT_MOUSE_MOTION:
{
struct nk_sdl* sdl = (struct nk_sdl*)ctx->userdata.ptr;
ctx->input.mouse.prev = ctx->input.mouse.pos;
ctx->input.mouse.delta.x = evt->motion.xrel;
ctx->input.mouse.delta.y = evt->motion.yrel;
ctx->input.mouse.pos.x = evt->motion.x;
ctx->input.mouse.pos.y = evt->motion.y;
if (ctx->input.mouse.grabbed) {
SDL_SetWindowRelativeMouseMode(sdl->win, true);
ctx->input.mouse.pos.x += evt->motion.x;
ctx->input.mouse.pos.y += evt->motion.y;
}
else {
SDL_SetWindowRelativeMouseMode(sdl->win, false);
ctx->input.mouse.pos.x = evt->motion.x;
ctx->input.mouse.pos.y = evt->motion.y;
}
return 1;
}Overall, I think what's in there is fine and don't want to over-complicate it 🤷 ... What do you think?
There was a problem hiding this comment.
I agree, it is fine as it is.
SDL_SetWindowRelativeMouseMode() is best to use when the application is in full screen or borderless full screen modes. The mouse cursor is confined into the whole screen (so app window), the OS / HW mouse pointer is hidden (but software cursor could be used), and mouse motion events are rapidly reporting mouse motion position and delta position. This is fit for most action games where you control a player character. Or in case of emulators where cursor is also emulated, e.g. DOSBox.
The way you used that API is somewhat wrong as when relative mode is enabled, the current event does not reflect the changes yet. In relative mode we would only use evt->motion.xrel/yrel. In normal mode we would use only the absolute position so evt->motion.x/y.
| case SDL_EVENT_TEXT_INPUT: | ||
| { | ||
| nk_glyph glyph; | ||
| SDL_memcpy(glyph, evt->text.text, NK_UTF_SIZE); |
There was a problem hiding this comment.
evt->text.text is UTF-8 encoded const char* nul terminated string. Based on limited research it is not guaranteed that a single glyph is reported by this event. Could it happen that text input gets cropped as the UTF-8 input is not passed per glyph to nk_input _glyph in a loop?
|
I don't have push access to this branch, but this patch fixes the casting klei mentioned: index 5a8c589..e819169 100644
--- a/demo/sdl3/nuklear_sdl3_renderer.h
+++ b/demo/sdl3/nuklear_sdl3_renderer.h
@@ -308,10 +308,10 @@ nk_sdl_handle_event(struct nk_context* ctx, SDL_Event *evt)
case SDL_EVENT_MOUSE_MOTION:
if (ctx->input.mouse.grabbed) {
- int x = (int)ctx->input.mouse.prev.x, y = (int)ctx->input.mouse.prev.y;
- nk_input_motion(ctx, x + evt->motion.xrel, y + evt->motion.yrel);
+ float x = ctx->input.mouse.prev.x, y = ctx->input.mouse.prev.y;
+ nk_input_motion(ctx, (int)(x + evt->motion.xrel), (int)(y + evt->motion.yrel));
}
- else nk_input_motion(ctx, evt->motion.x, evt->motion.y);
+ else nk_input_motion(ctx, (int)evt->motion.x, (int)evt->motion.y);
return 1;
case SDL_EVENT_TEXT_INPUT:
@@ -323,7 +323,7 @@ nk_sdl_handle_event(struct nk_context* ctx, SDL_Event *evt)
return 1;
case SDL_EVENT_MOUSE_WHEEL:
- nk_input_scroll(ctx,nk_vec2((float)evt->wheel.x,(float)evt->wheel.y));
+ nk_input_scroll(ctx, nk_vec2(evt->wheel.x, evt->wheel.y));
return 1;
}
return 0;The first casting change is technically unnecessary, but it allows adding the prev and rel values as floats before converting them back to ints. |
demo/sdl3/nuklear_sdl3_renderer.h
Outdated
|
|
||
| case SDL_EVENT_MOUSE_MOTION: | ||
| if (ctx->input.mouse.grabbed) { | ||
| int x = (int)ctx->input.mouse.prev.x, y = (int)ctx->input.mouse.prev.y; |
There was a problem hiding this comment.
After some testing, I think what I've put in there seems to be working with or without window grabbing.
case SDL_EVENT_MOUSE_MOTION:
ctx->input.mouse.prev = ctx->input.mouse.pos;
ctx->input.mouse.delta.x = evt->motion.xrel;
ctx->input.mouse.delta.y = evt->motion.yrel;
ctx->input.mouse.pos.x = evt->motion.x;
ctx->input.mouse.pos.y = evt->motion.y;
return 1;I played around with SetRelativeMouseMode too, and it didn't seem to have any real affect...
case SDL_EVENT_MOUSE_MOTION:
{
struct nk_sdl* sdl = (struct nk_sdl*)ctx->userdata.ptr;
ctx->input.mouse.prev = ctx->input.mouse.pos;
ctx->input.mouse.delta.x = evt->motion.xrel;
ctx->input.mouse.delta.y = evt->motion.yrel;
ctx->input.mouse.pos.x = evt->motion.x;
ctx->input.mouse.pos.y = evt->motion.y;
if (ctx->input.mouse.grabbed) {
SDL_SetWindowRelativeMouseMode(sdl->win, true);
ctx->input.mouse.pos.x += evt->motion.x;
ctx->input.mouse.pos.y += evt->motion.y;
}
else {
SDL_SetWindowRelativeMouseMode(sdl->win, false);
ctx->input.mouse.pos.x = evt->motion.x;
ctx->input.mouse.pos.y = evt->motion.y;
}
return 1;
}Overall, I think what's in there is fine and don't want to over-complicate it 🤷 ... What do you think?
| return SDL_Fail(); | ||
| } | ||
|
|
||
| SDL_StartTextInput(appContext->window); |
There was a problem hiding this comment.
Hmm, is there a cleaner way we could accomplish this? Perhaps a callback when a textbox on focus/blur?
| struct nk_sdl* sdl; | ||
| NK_ASSERT(ctx); | ||
| sdl = (struct nk_sdl*)ctx->userdata.ptr; |
There was a problem hiding this comment.
One worry I have about this strategy is that the SDL renderer is taking over the nk_context userdata to store information required by the renderer. If the application is making use of the userdata, there will be a conflicts unless they change to use nk_sdl_set_userdata() instead.
Perhaps we introduce another renderer-specific userdata property? Something like void* renderer_data or something? As an added benefit, that could be used in some of the other renderers too (glfw_opengl2, sfml_gl3, gdi, etc).
There was a problem hiding this comment.
I assume the renderer backends are still examples only and not part of the UI library, which is a single header. E.g. I change nk_sdl_handle_event to add more keys, etc. As integrator, I am in full control over the userdata. E.g. I do not have nk_sdl sdl at all, I embed the stuff from it into my own context thingy.
There was a problem hiding this comment.
So you use your own userdata elsewhere outside of the nuklear context?
There was a problem hiding this comment.
Um, I do not use nuklear_sdl3_renderer.h and APIs like nk_sdl_render(struct nk_context* ctx at all. I use the demo as an example and adapt it to my needs. E.g. my own version of nk_sdl_render() gets not a Nuklear context, it gets an application context that embeds the Nuklear context too. What I wanted to communicate above is that the life cycle of the Nuklear context is shorter then the application context in my case, and that I consider nuklear_sdl3_renderer.h to be an example, not the official API.
So the answer to your question is actually no. I do not have a specific need for userdata enabled by NK_INCLUDE_COMMAND_USERDATA, it can be used by nuklear_sdl3_renderer.h and I will adapt that header content as I need.
-
Text processing: In case of text processing I use freetype2, different texture packer, custom font atlas, and custom cached glyph encoding library that are not exclusive to
nk_user_font, and Nuklear. So the life cycle of my text rendering sub system is typically longer than the life cycle of the Nuklear context. The font atlas is based onSDL_surfaceobjects where I could do palette color manipulations, which is not a typical use case I admit, and when the Nuklear context is created and render backend is selected the font atlas layers are converted toSDL_textrureobjects in native texture format. I actually intend to use SDL3 GPU API not the SDL3 renderer. That is important as my whole design revolves around SDL3 GPU's ability to transparently select any backend that is available. I am not deciding which backend to use so my font atlas needs to remain backend agnostic (e.g. usesSDL_surfacenot textures that have backend dependent pixel formats). -
GUI: my application context is not a sub feature of Nuklear, instead Nuklear's context is the sub feature of my application context. The same way as for text processing, I tried to shoot for a design which separates the GUI library and its render backend and the application core logic even if this is very difficult with the immediate mode pattern. Nuklear requires a lot of boilerplate and that boilerplate is interleaved into the core application logic which is difficult to maintain so I try to encapsulate, structure and generalize things for which I created an abstraction level above Nuklear similar to a c++ object oriented wrapper would do, but focusing on features (buttons, lists, hotkeys, sound effects, ...) instead of API coverage. I use a custom GUI layout manager that is loading parameters and asset or resource IDs from json files above and independent of Nuklear. Assets are in custom file formats using LZ4 compression and are loaded into renderer agnostic
SDL_surfaceobjects. Later on when the system specific render backend is known, D3D/OGL/etc, then these surfaces are converted and loaded into GPU memory intoSDL_textureobjects, using native texture formats, with assets packed as most optimal for particular screen content expected to be rendered. I only use Nuklear STATIC layout and position every top level (custom) widgets inside the application frontend or menu screens. Nuklear's render target is just anSDL_texturenot the swapchain texture or whatever. In short my application's context is one order or abstraction level higher than Nuklear's context. The Nuklear context is only created after the render backend is determined. I am sure there is a better way to do all of this, but this is what i came up with for my particular needs.
Co-authored-by: Rob Loach <[email protected]>
[SDL3] Use built-in motion handler to circumvent FPS-limit/VSync issue
|
What is the state of this PR? I'm happy to help with any remaining items |
|
I believe all that's really missing is...
|
|
Cool, for Sam told me in the SDL Discord to "Remember [that] this will bring up the on-screen keyboard on some platforms, so you should only do this when you're ready for text input". We could macro guard it or put a comment above it so people are aware of its behavior. I treat the source files for the backends as examples, but the header files for the backends as Nuklear vendored, so IMO it's not unreasonable to just put a disclaimer since it's "just an example". Alternatively, you could place the start and stop calls inside the focus/defocus events for text inputs or windows, but that might interfere with some edge cases. |
|
Actually, Sam also said that:
So it sounds like the latter is the best option. Any ideas on where we should hook into the input focus? I'm not too familiar on whether that should lie at a window focus or input focus boundary. Are there any cases of keyboard navigation that would make sense on desktop but not on other platforms? |
|
I've fully integrated this into my own project, and it's working nicely so far except for one minor issue. This is observable in Also, I started looking into the SDL TextInput stuff and it looks like the most trivial solution would be to somehow insert the calls into |
Agreed. Not an elegant solution currently. Are you proposing some kind of event callback system for them?
The other thing I'd like to see incorporated into this is a clearer implementation of detailing with the global object state. Or something like encorporating #799 into here. |
That's the first thing that comes to mind. I don't have deep knowledge of the Nuklear codebase, so I wasn't sure if that's done in other places, or what the best way to integrate something like that would be. Do you have any opinions?
This is interesting. Personally, I'm fine with the global state for my purposes. Does it make more sense to integrate these changes here, or have #799 tackle it for all of the SDL demos, once we have nailed down the API?
Did you have any thoughts on why this may be? |
Maybe a strict callback isn't necessary as that would incur a small but present overhead for all backends. I could see it being 2 (mutually inclusive) optional macros that are called if they exist at appropriate places in Nuklear. It wouldn't just be nk_edit_focus/nk_edit_unfocus, because that's just for the manually forcing focus/unfocus, not from the user actually clicking on an edit field or something else. I'm not sure what the best name is but something like NK_ON_EDIT_FOCUS()/UNFOCUS() might work. Not sure about any necessary parameters either. Just my two cents till I actually have the chance to review/test this and have a more informed opinion. |
apply 779.diff from Immediate-Mode-UI#779 but without Cmake files and without NK_MALLOC/NK_MFREE macros Compiled and tested with: $ cd Nuklear/demo/sdl3 $ gcc nuklear_sdl3_renderer.c -lm -ldl -lSDL3 -I./../.. and it works just fine except the issue mentioned in comment: Immediate-Mode-UI#779 (comment) Hopefully, this is the last sdl3_renderer PR I will find...
| NK_ASSERT(renderer); | ||
| sdl = SDL_malloc(sizeof(struct nk_sdl)); | ||
| NK_ASSERT(sdl); | ||
| nk_zero(sdl, sizeof(struct nk_sdl)); |
There was a problem hiding this comment.
nk_zero() is a private/internal symbol that comes from src/nuklear_util.c (defined in src/nuklear_internal.h). This causes compilation to fail when the amalgamated header isn't used.
You could probably replace this line with variation of SDL_zero*
EDIT: I can confirm that SDL_zerop fixes this issue for me:
- nk_zero(sdl, sizeof(struct nk_sdl));
+ SDL_zerop(sdl);Fixes issue mentioned in: Immediate-Mode-UI#779 (comment)
|
|
||
| #ifndef NK_INCLUDE_COMMAND_USERDATA | ||
| #error "nuklear_sdl3 requires the NK_INCLUDE_COMMAND_USERDATA define" | ||
| #endif |
There was a problem hiding this comment.
Not sure if this is useful but there should be more errors, for NK_INCLUDE_VERTEX_BUFFER_OUTPUT and NK_INCLUDE_FONT_BAKING (and probably more), as I was unable to compile without those dependencies. Perhaps, some functions could be defined conditionally depending on those macros.
| switch(evt->button.button) | ||
| { | ||
| case SDL_BUTTON_LEFT: | ||
| if (evt->button.clicks > 1) |
There was a problem hiding this comment.
- if (evt->button.clicks > 1)
+ if (evt->button.clicks == 2)This isn't perfect but should help with the issue mentioned in: #779 (comment)
There was a problem hiding this comment.
Nice, I'll go ahead and test this. When you say it isn't perfect, are you still experiencing some undesirable behavior?
There was a problem hiding this comment.
Per our discussion on Discord, this is how SDL2 demo handles double-click and this is what works the best:
Nuklear/demo/sdl_renderer/nuklear_sdl_renderer.h
Lines 337 to 341 in 171090c
So better fix would be:
diff --git a/demo/sdl3/nuklear_sdl3_renderer.h b/demo/sdl3/nuklear_sdl3_renderer.h
index c57a423..b9b3a29 100644
--- a/demo/sdl3/nuklear_sdl3_renderer.h
+++ b/demo/sdl3/nuklear_sdl3_renderer.h
@@ -332,8 +332,7 @@ nk_sdl_handle_event(struct nk_context* ctx, SDL_Event *evt)
case SDL_BUTTON_LEFT:
if (evt->button.clicks > 1)
nk_input_button(ctx, NK_BUTTON_DOUBLE, x, y, down);
- else
- nk_input_button(ctx, NK_BUTTON_LEFT, x, y, down);
+ nk_input_button(ctx, NK_BUTTON_LEFT, x, y, down);
break;
case SDL_BUTTON_MIDDLE: nk_input_button(ctx, NK_BUTTON_MIDDLE, x, y, down); break;
case SDL_BUTTON_RIGHT: nk_input_button(ctx, NK_BUTTON_RIGHT, x, y, down); break;Basically, repetitive clicks always send single-click + double-click. However, I'm unsure how widgets with special double-click interactions would work in some edge-cases.
The best approach would be to use timer/delay and custom logic, but this would be way harder to implement, especially with new SDL_app abstraction. Btw, GLFW demos do use delay in this case:
Nuklear/demo/glfw_opengl2/nuklear_glfw_gl2.h
Lines 280 to 286 in 171090c
There was a problem hiding this comment.
Have you had the chance to test this? It wouldn't be too hard to add a similar delay tracking variable into this demo, assuming the double click does have unintended interactions with widgets
There was a problem hiding this comment.
Have you had the chance to test this?
I've only tested the SDL2-demo way. Haven't messed with the timer.
It wouldn't be too hard to add a similar delay tracking variable into this demo
I think that would be too "specific" for a demo. The easiest way to implement this is by using SDL_GetTicks. To be honest, I don't really like how GLFW-demo implements this, as it may detect two quick clicks with movement between them as double-click.
|
How about the dependency on This PR tries to solve this by providing EDIT: The patch bellow works with patch
diff --git a/demo/sdl3/nuklear_sdl3_renderer.h b/demo/sdl3/nuklear_sdl3_renderer.h
index c57a423..4b67310 100644
--- a/demo/sdl3/nuklear_sdl3_renderer.h
+++ b/demo/sdl3/nuklear_sdl3_renderer.h
@@ -16,6 +16,12 @@
#error "nuklear_sdl3_renderer requires at least SDL 3.0.0"
#endif
+/* We have to redefine it because demos do not include any headers
+ * This is the same default value as the one from "src/nuklear_internal.h" */
+#ifndef NK_BUFFER_DEFAULT_INITIAL_SIZE
+ #define NK_BUFFER_DEFAULT_INITIAL_SIZE (4*1024)
+#endif
+
NK_API struct nk_context* nk_sdl_init(SDL_Window *win, SDL_Renderer *renderer);
NK_API struct nk_font_atlas* nk_sdl_font_stash_begin(struct nk_context* ctx);
NK_API void nk_sdl_font_stash_end(struct nk_context* ctx);
@@ -79,6 +85,27 @@ NK_API void nk_sdl_set_userdata(struct nk_context* ctx, nk_handle userdata) {
sdl->userdata = userdata;
}
+NK_INTERN void *
+nk_sdl_alloc(nk_handle user, void *old, nk_size size)
+{
+ NK_UNUSED(user);
+ NK_UNUSED(old);
+ return SDL_malloc(size);
+}
+
+NK_INTERN void
+nk_sdl_free(nk_handle user, void *old)
+{
+ NK_UNUSED(user);
+ SDL_free(old);
+}
+
+NK_GLOBAL const struct nk_allocator nk_sdl_allocator = {
+ .userdata = NULL,
+ .alloc = nk_sdl_alloc,
+ .free = nk_sdl_free,
+};
+
NK_INTERN void
nk_sdl_device_upload_atlas(struct nk_context* ctx, const void *image, int width, int height)
{
@@ -146,8 +173,8 @@ nk_sdl_render(struct nk_context* ctx, enum nk_anti_aliasing AA)
config.line_AA = AA;
/* convert shapes into vertexes */
- nk_buffer_init_default(&vbuf);
- nk_buffer_init_default(&ebuf);
+ nk_buffer_init(&vbuf, &nk_sdl_allocator, NK_BUFFER_DEFAULT_INITIAL_SIZE);
+ nk_buffer_init(&ebuf, &nk_sdl_allocator, NK_BUFFER_DEFAULT_INITIAL_SIZE);
nk_convert(&sdl->ctx, &sdl->ogl.cmds, &vbuf, &ebuf, &config);
/* iterate over and execute each draw command */
@@ -230,12 +257,12 @@ nk_sdl_init(SDL_Window *win, SDL_Renderer *renderer)
nk_zero(sdl, sizeof(struct nk_sdl));
sdl->win = win;
sdl->renderer = renderer;
- nk_init_default(&sdl->ctx, 0);
+ nk_init(&sdl->ctx, &nk_sdl_allocator, NULL);
sdl->ctx.userdata = nk_handle_ptr((void*)sdl);
sdl->ctx.clip.copy = nk_sdl_clipboard_copy;
sdl->ctx.clip.paste = nk_sdl_clipboard_paste;
sdl->ctx.clip.userdata = nk_handle_ptr(0);
- nk_buffer_init_default(&sdl->ogl.cmds);
+ nk_buffer_init(&sdl->ogl.cmds, &nk_sdl_allocator, NK_BUFFER_DEFAULT_INITIAL_SIZE);
return &sdl->ctx;
}
@@ -246,7 +273,7 @@ nk_sdl_font_stash_begin(struct nk_context* ctx)
NK_ASSERT(ctx);
sdl = (struct nk_sdl*)ctx->userdata.ptr;
NK_ASSERT(sdl);
- nk_font_atlas_init_default(&sdl->atlas);
+ nk_font_atlas_init(&sdl->atlas, &nk_sdl_allocator);
nk_font_atlas_begin(&sdl->atlas);
return &sdl->atlas;
} |
This comment was marked as outdated.
This comment was marked as outdated.
|
@sleeptightAnsiC I'm glad to see someone else thinking about this PR! IMO the biggest unknown right now is solving how we can call From the original thread, I believe everything else is reasonably resolved except for UTF-8 support in SDL_EVENT_TEXT_INPUT. |
agreed, and your solution seems reasonable. @RobLoach thoughts? |
Interesting, although likely not relevant to this exact PR (but it is a new feature in SDL3 iirc) You might have luck looking at the atlas implementation, but there are definitely a few downsides to using this: (from the docs)
|
| }; | ||
|
|
||
| struct nk_sdl { | ||
| SDL_Window *win; |
There was a problem hiding this comment.
Does it make sense to keep reference to SDL_Window?
This demo does not use it anywhere (and even if it would, it probably shouldn't). Back in SDL2 demos, they were using it in nk_sdl_handle_grab for SDL_WarpMouseInWindow (link)
Unless this reference is meant to be used in the future, maybe we can simply remove it?
There was a problem hiding this comment.
If it's unused, I would remove it
Just to confirm that it's only being assigned but never really used anywhere:
grep
/tmp/Nuklear $ grep -n '[^a-z]win[^a-z]' ./demo/sdl3/nuklear_sdl3_renderer.h
19:NK_API struct nk_context* nk_sdl_init(SDL_Window *win, SDL_Renderer *renderer);
58: SDL_Window *win;
223:nk_sdl_init(SDL_Window *win, SDL_Renderer *renderer)
226: NK_ASSERT(win);
231: sdl->win = win;Working patch:
diff
diff --git a/demo/sdl3/nuklear_sdl3_renderer.c b/demo/sdl3/nuklear_sdl3_renderer.c
index b407af0..bc3433c 100644
--- a/demo/sdl3/nuklear_sdl3_renderer.c
+++ b/demo/sdl3/nuklear_sdl3_renderer.c
@@ -126,7 +126,7 @@ SDL_AppResult SDL_AppInit(void** appstate, int argc, char* argv[]) {
font_scale = scale_y;
}
- struct nk_context* ctx = nk_sdl_init(appContext->window, appContext->renderer);
+ struct nk_context* ctx = nk_sdl_init(appContext->renderer);
appContext->ctx = ctx;
{
diff --git a/demo/sdl3/nuklear_sdl3_renderer.h b/demo/sdl3/nuklear_sdl3_renderer.h
index c57a423..69cc474 100644
--- a/demo/sdl3/nuklear_sdl3_renderer.h
+++ b/demo/sdl3/nuklear_sdl3_renderer.h
@@ -16,7 +16,7 @@
#error "nuklear_sdl3_renderer requires at least SDL 3.0.0"
#endif
-NK_API struct nk_context* nk_sdl_init(SDL_Window *win, SDL_Renderer *renderer);
+NK_API struct nk_context* nk_sdl_init(SDL_Renderer *renderer);
NK_API struct nk_font_atlas* nk_sdl_font_stash_begin(struct nk_context* ctx);
NK_API void nk_sdl_font_stash_end(struct nk_context* ctx);
NK_API int nk_sdl_handle_event(struct nk_context* ctx, SDL_Event *evt);
@@ -55,7 +55,6 @@ struct nk_sdl_vertex {
};
struct nk_sdl {
- SDL_Window *win;
SDL_Renderer *renderer;
struct nk_sdl_device ogl;
struct nk_context ctx;
@@ -220,15 +219,13 @@ nk_sdl_clipboard_copy(nk_handle usr, const char *text, int len)
}
NK_API struct nk_context*
-nk_sdl_init(SDL_Window *win, SDL_Renderer *renderer)
+nk_sdl_init(SDL_Renderer *renderer)
{
struct nk_sdl* sdl;
- NK_ASSERT(win);
NK_ASSERT(renderer);
sdl = SDL_malloc(sizeof(struct nk_sdl));
NK_ASSERT(sdl);
nk_zero(sdl, sizeof(struct nk_sdl));
- sdl->win = win;
sdl->renderer = renderer;
nk_init_default(&sdl->ctx, 0);
sdl->ctx.userdata = nk_handle_ptr((void*)sdl);There was a problem hiding this comment.
Ahhh... looking through other PRs/Issues such as #772 (comment) #608 #609 gave me more idea about why the reference was there in the first place.
Maybe we should actually add nk_sdl_handle_grab back to this demo? Removing reference to SDL_Window still wouldn't be a problem, since we could ask for it when calling nk_sdl_handle_grab
There was a problem hiding this comment.
I can check when I get back to my PC, but iirc the SDL2 demo had some broken grabbing behavior, and I think I have that modified/disabled in my working copy
There was a problem hiding this comment.
Note: we will also need Window reference for SDL_StartTextInput and SDL_StopTextInput, if we ever use them.
EDIT: we should also make a check in nk_sdl_handle_event to ensure that the Window sending event is the same as the one in nk_sdl (and make an early bail, if it's not the same)
|
Something that I noticed while looking at other similar PR #825 (comment) :
So we forgot to set |
Got it! 😃 I was able to implement this by recreating the atlas texture with SDL_RenderDebugText. The code is not perfect and needs some additional work but should be enough for anyone who wants to display simple debug font. screenshot
code
diff --git a/demo/sdl3/nuklear_sdl3_renderer.c b/demo/sdl3/nuklear_sdl3_renderer.c
index b407af0..1d63d7b 100644
--- a/demo/sdl3/nuklear_sdl3_renderer.c
+++ b/demo/sdl3/nuklear_sdl3_renderer.c
@@ -129,6 +129,7 @@ SDL_AppResult SDL_AppInit(void** appstate, int argc, char* argv[]) {
struct nk_context* ctx = nk_sdl_init(appContext->window, appContext->renderer);
appContext->ctx = ctx;
+#if 0
{
struct nk_font_atlas *atlas;
struct nk_font_config config = nk_font_config(0);
@@ -152,6 +153,9 @@ SDL_AppResult SDL_AppInit(void** appstate, int argc, char* argv[]) {
/*nk_style_load_all_cursors(ctx, atlas->cursors);*/
nk_style_set_font(ctx, &font->handle);
}
+#else
+ nk_sdl_style_set_font_debug(ctx);
+#endif
return SDL_APP_CONTINUE;
}
diff --git a/demo/sdl3/nuklear_sdl3_renderer.h b/demo/sdl3/nuklear_sdl3_renderer.h
index c57a423..dc22ded 100644
--- a/demo/sdl3/nuklear_sdl3_renderer.h
+++ b/demo/sdl3/nuklear_sdl3_renderer.h
@@ -24,6 +24,7 @@ NK_API void nk_sdl_render(struct nk_context* ctx, enum nk_anti_a
NK_API void nk_sdl_shutdown(struct nk_context* ctx);
NK_API nk_handle nk_sdl_userdata(struct nk_context* ctx);
NK_API void nk_sdl_set_userdata(struct nk_context* ctx, nk_handle userdata);
+NK_API void nk_sdl_style_set_font_debug(struct nk_context* ctx);
#endif /* NK_SDL3_RENDERER_H_ */
@@ -371,7 +372,8 @@ void nk_sdl_shutdown(struct nk_context* ctx)
sdl = (struct nk_sdl*)ctx->userdata.ptr;
NK_ASSERT(sdl);
- nk_font_atlas_clear(&sdl->atlas);
+ if (sdl->atlas.font_num > 0)
+ nk_font_atlas_clear(&sdl->atlas);
nk_buffer_free(&sdl->ogl.cmds);
if (sdl->ogl.font_tex != NULL) {
@@ -384,5 +386,116 @@ void nk_sdl_shutdown(struct nk_context* ctx)
nk_free(ctx);
}
+/* FIXME: width/height of the debug font atlas (integer)
+ * Probably needs better name... */
+#define WH (10)
+
+NK_INTERN float
+nk_sdl_query_debug_font_width(nk_handle handle, float height,
+ const char *text, int len)
+{
+ NK_UNUSED(handle);
+ NK_UNUSED(text);
+ /* FIXME: does this thing need to handle newlines and tabs ??? */
+ return height * len;
+}
+
+NK_INTERN void
+nk_sdl_query_debug_font_glypth(nk_handle handle, float font_height,
+ struct nk_user_font_glyph *glyph,
+ nk_rune codepoint, nk_rune next_codepoint)
+{
+ char ascii;
+ int idx, x, y;
+ NK_UNUSED(next_codepoint);
+
+ ascii = (codepoint > (nk_rune)'~' || codepoint < (nk_rune)' ')
+ ? '?' : (char)codepoint;
+ NK_ASSERT(ascii <= '~' && ascii >= ' ');
+
+ idx = (int)(ascii - ' ');
+ NK_ASSERT(idx >= 0 && idx <= (WH * WH));
+
+ x = idx / WH;
+ y = idx % WH;
+
+ glyph->height = font_height;
+ glyph->width = font_height;
+ glyph->xadvance = font_height;
+ glyph->uv[0].x = (float)(x + 0) / WH;
+ glyph->uv[0].y = (float)(y + 0) / WH;
+ glyph->uv[1].x = (float)(x + 1) / WH;
+ glyph->uv[1].y = (float)(y + 1) / WH;
+ glyph->offset.x = 0.0f;
+ glyph->offset.y = 0.0f;
+}
+
+NK_API void
+nk_sdl_style_set_font_debug(struct nk_context* ctx)
+{
+ struct nk_user_font* font;
+ struct nk_sdl* sdl;
+ SDL_Surface *surface;
+ SDL_Renderer *renderer;
+ char buf[2];
+ int x, y;
+ bool success;
+ NK_ASSERT(ctx);
+
+ /* FIXME: For now, use another software Renderer just to make sure
+ * that we won't change any state in the main Renderer,
+ * but it would be nice to reuse the main one */
+ surface = SDL_CreateSurface(WH * SDL_DEBUG_TEXT_FONT_CHARACTER_SIZE,
+ WH * SDL_DEBUG_TEXT_FONT_CHARACTER_SIZE,
+ SDL_PIXELFORMAT_RGBA32);
+ NK_ASSERT(surface);
+ renderer = SDL_CreateSoftwareRenderer(surface);
+ NK_ASSERT(renderer);
+ success = SDL_SetRenderDrawColor(renderer, 0xFF, 0xFF, 0xFF, 0xFF);
+ NK_ASSERT(success);
+
+ /* SPACE is the first printable ASCII character */
+ buf[0] = ' ';
+ buf[1] = '\0';
+ for (x = 0; x < WH; x++)
+ {
+ for (y = 0; y < WH; y++)
+ {
+ success = SDL_RenderDebugText(renderer,
+ (float)(x * SDL_DEBUG_TEXT_FONT_CHARACTER_SIZE),
+ (float)(y * SDL_DEBUG_TEXT_FONT_CHARACTER_SIZE),
+ buf);
+ NK_ASSERT(success);
+
+ buf[0]++;
+ /* TILDE is the last printable ASCII character */
+ if (buf[0] > '~')
+ break;
+ }
+ }
+ success = SDL_RenderPresent(renderer);
+ NK_ASSERT(success);
+
+ font = SDL_malloc(sizeof(*font));
+ NK_ASSERT(font);
+ font->userdata.ptr = NULL;
+ font->height = SDL_DEBUG_TEXT_FONT_CHARACTER_SIZE;
+ font->width = &nk_sdl_query_debug_font_width;
+ font->query = &nk_sdl_query_debug_font_glypth;
+
+ /* HACK: nk_sdl_device_upload_atlas turns pixels into SDL_Texture
+ * and sets said Texture into sdl->ogl.font_tex
+ * then nk_sdl_render expects same Texture at font->texture */
+ sdl = (struct nk_sdl*)ctx->userdata.ptr;
+ nk_sdl_device_upload_atlas(&sdl->ctx, surface->pixels, surface->w, surface->h);
+ font->texture.ptr = sdl->ogl.font_tex;
+
+ /* FIXME: font is passed but never free'd (memleak) */
+ nk_style_set_font(ctx, font);
+
+ SDL_DestroyRenderer(renderer);
+ SDL_DestroySurface(surface);
+}
+
#endif /* NK_SDL3_RENDERER_IMPLEMENTATION_ONCE */
#endif /* NK_SDL3_RENDERER_IMPLEMENTATION */ |
| sdl = (struct nk_sdl*)ctx->userdata.ptr; | ||
| NK_ASSERT(sdl); | ||
|
|
||
| nk_font_atlas_clear(&sdl->atlas); |
There was a problem hiding this comment.
When I was messing with custom font (mentioned here), this line would lead to the crash unless I make a check for any existing font:
- nk_font_atlas_clear(&sdl->atlas);
+ if (sdl->atlas.font_num > 0)
+ nk_font_atlas_clear(&sdl->atlas);However, this feels like something that should be fixed inside of nk_font_atlas_clear
This code was developed by me, mentioned in comment here: Immediate-Mode-UI#779 (comment)
This is NOT a proper fix, but stops crashes for me. This issue was spotted and mentioned here (by me): Immediate-Mode-UI#779 (comment)
Mentioned by me, here: Immediate-Mode-UI#779 (comment)
I've started experimenting with this and found small solution. patch
diff --git a/demo/sdl3/nuklear_sdl3_renderer.h b/demo/sdl3/nuklear_sdl3_renderer.h
index c57a423..1eb61f1 100644
--- a/demo/sdl3/nuklear_sdl3_renderer.h
+++ b/demo/sdl3/nuklear_sdl3_renderer.h
@@ -274,6 +274,20 @@ nk_sdl_handle_event(struct nk_context* ctx, SDL_Event *evt)
NK_ASSERT(ctx);
NK_ASSERT(evt);
+ static bool was_active = false;
+ const bool active = (ctx->active && ctx->active->edit.active);
+ SDL_Window *win = SDL_GetWindowFromEvent(evt);
+ struct nk_sdl* sdl = (struct nk_sdl*)ctx->userdata.ptr;
+ if (active != was_active && win == sdl->win)
+ {
+ if (!was_active && active)
+ SDL_StartTextInput(win);
+ else if (was_active && !active)
+ SDL_StopTextInput(win);
+ was_active = active;
+ SDL_LogInfo(SDL_LOG_CATEGORY_APPLICATION, "text input %s", active ? "started" : "stopped");
+ }
+
switch(evt->type)
{
case SDL_EVENT_KEY_UP: /* KEYUP & KEYDOWN share same routine */video
2025-09-10_02-49-37.mp4 |
@zoogies I've removed the sdl_gpu stuff, since it wasn't there yet, and cleaned up the CMake a bit. Mind a review?