v 4.10.0: Multiple PRs merged #160

Nielsbishere · 2020-07-06T15:19:03Z

Multiple PRs:

Various changes such as:

Fixing clicking through multiple overlapping windows
Allowing subpixel rendering
Allowing add_window to return a Window pointer to read out information useful to an abstraction that isn't immediate mode over nuklear.
Fixed radio buttons style
Fixed compile issues with C++17 (-Werr -W3)
Trees can now have 'child' nodes, which are supposed to be the final stop for childless nodes. This isn't fully true, but it can be used to have another tree node type. It doesn't have a symbol as a file should as opposed to a folder.

merge from upstream

… for vertices (they check correctly now). Paq.sh now automatically outputs to nuklear.h.

keharriso's Prevent clicking more than one (overlapping) button per frame. However, this has a couple of other changes that should fix dropdowns from intefering as well.

…nto fix_click

- Sets click to 0 every start of frame - Input was still down for that frame, so it triggered the next event Now it waits for your button to go back up before resetting the clicked variable, so as long as your mouse is down, only one item is clicked.

…checkbox! With README changes that show off more features and the updated style. Slightly modified.

…nto fix_radio_buttons

…nto fix_click

…and use a 32-bit uint instead.

RobLoach · 2021-12-14T04:27:28Z

Looks like there's a few git conflicts 👀

dumblob · 2021-12-14T09:11:40Z

I'm sorry for us not getting earlier to this. I think this PR is quite valuable and was a lot of work to put it all together. We should merge (parts of?) it after a thorough review. Would anyone be willing to update it to fix the merge conflicts? I'm constantly fighting my tighter and tighter schedule...

Nielsbishere · 2021-12-15T19:57:45Z

There we go

dumblob · 2021-12-15T20:42:26Z

I've skimmed it (really no review yet) and it actually looks surprisingly good to me. @RobLoach could you please thoroughly review it? @Hejsil would you also find some time to take a look at this rather sensitive PR? The more eyes the better.

Hejsil

I tried, but it is very hard to review this as I have to context switch all the time to figure out what each change relates to. I would personally prefer we don't do these "merge 5 things together" pull requests for a few reasons:

It's very hard to review
The git history is a mess, and doing a "Squash and Merge" is not really a good idea as we want multiple commits, one for each fix/feature.
Change log for this version is a bit of a mess. I would rather we have patch and minor bumps have just one thing they do. Major bumps can be a mess.

Hejsil · 2021-12-15T22:09:22Z

doc/nuklear.html

+- 2020/07/06 (4.04.0) - Added subpixel API from itsuhane. Allowing you to turn on 
+                        NK_ENABLE_SUBPIXEL_API to position window and elements in subpixels
+                        Changed nk_begin to return a nk_window so you can get the position and other useful info.
+                        Fixed radio buttons' style.
+                        Fixed click cascading through multiple buttons; it should only click the upper button.
+                        Strict c++17 now compiles: only declaring memset, memcpy if they are used.
+                        Only asserting index range if ushorts are used for indices.
+                        Made paq.sh identical to paq.bat (outputs to nuklear.h).
+                        Replaced NK_ASSERT(0 && ...) with NK_ERROR(), to ensure it compiles on strict C++17.
+                        Replaced const char** by const char * const * in combobox, to ensure const List&lt;const char*&gt; can be used.
+                        Added nk_tree_type for allowing the symbol next to a tree element to be hidden (for children) if it's not an image.
 - 2020/04/09 (4.02.1) - Removed unused nk_sqrt function to fix compiler warnings
                      - Fixed compiler warnings if you bring your own methods for


Hmm. The versioning in here seems quite out of date

I think we'll fix the versioning after we finally know which of the (many) other PRs get merged first.

Hejsil · 2021-12-15T22:15:05Z

Readme.md


 ```c
 /* init gui state */
 struct nk_context ctx;


Not sure why we are updating the example in the readme. IMO readme examples should be very small and give the reader a taste of how nuklear is to use. Not sure this update really makes it any better

It shows elements of nuklear that are often going to be used in applications, but I guess it's preference

I usually tend to prefer readme examples to be very small. In our case we're pretty much covered by everything in example/ directory. So here I won't advocate for extending the current example.

I'm though fine with extending if it's just about 1-5 SLOC which add and I quote "often used elements". That's not the current case though.

Hejsil · 2021-12-15T22:15:42Z

clib.json

+  "src": [
+    "nuklear.h"
+  ]
 }


Let's not change the formatting of the clib.json. Only bump the version

Any reason? To me it seems easier to read this way

I'm indifferent here. But generally speaking if such changes shall further extend big PRs I'd beg everyone to leave them out for the sake of clarity.

I would personally also like it to be formatted this way, as then it would be inline with my json formatter (4 spaces would be nice too). If enough people agree, then sure we can change it, but I don't want to end up en a situation where we change the clib.json format in every PR, if you know what I mean :)

Hejsil · 2021-12-15T22:15:59Z

doc/nuklear.html

@@ -1,4 +1,4 @@
-<meta charset='utf-8' emacsmode='-*- markdown -*-'>
+<meta charset='utf-8' emacsmode='-*- markdown -*-'>


What changed on this line?

Unsure, hopefully not line endings

Hejsil · 2021-12-15T22:18:40Z

src/nuklear_combo.c

 }
 NK_API int
-nk_combo_callback(struct nk_context *ctx, void(*item_getter)(void*, int, const char**),
+nk_combo_callback(struct nk_context *ctx, void(*item_getter)(void*, int, const char* const *),


Pretty sure this is a breaking change as old void (*)(void *, int, const char**) functions passed here will now be a compiler error.

That's right, but that's why there's a major bump and not a minor one. If this change isn't enforced then it won't work for some cases.

Bumping to 4.10 is a minor bump. We would need a 5.0 to be a major one.

There is actually no major bump (only minor) in this PR as of now 😢.

But more importantly I'd definitely prefer if major bumping features (i.e. those affecting backwards compatibility) were in a separate PR. It's true that this PR is quite convoluted though I think I'd merge it after it passes our review. But the split due to a major bump is something important for several other reasons (one of which is release engineering).

@Hejsil my bad, it's major.minor.bugfix? Then I might have to bump it

Hejsil · 2021-12-15T22:18:57Z

src/nuklear_combo.c

 NK_API void
 nk_combobox_callback(struct nk_context *ctx,
-    void(*item_getter)(void* data, int id, const char **out_text),
+    void(*item_getter)(void* data, int id, const char * const *out_text),


Hejsil · 2021-12-15T22:25:12Z

src/nuklear_draw.c

+NK_API void
+nk_push_scissor_subpixel(struct nk_command_buffer *b, struct nk_rect r)
+{
+    struct nk_command_scissor *cmd;
+    NK_ASSERT(b);
+    if (!b) return;
+
+    b->clip.x = r.x;
+    b->clip.y = r.y;
+    b->clip.w = r.w;
+    b->clip.h = r.h;
+    cmd = (struct nk_command_scissor*)
+        nk_command_buffer_push(b, NK_COMMAND_SCISSOR, sizeof(*cmd));
+
+    if (!cmd) return;
+    cmd->x = r.x;
+    cmd->y = r.y;
+    cmd->w = NK_MAX(0, r.w);
+    cmd->h = NK_MAX(0, r.h);
+}


There are a lot of these new subpixel apis that are just copies of the original but without the casts to short. Their signature are the same, so should nk_push_scissor just support subpixels if enabled instead of having two functions?

Not sure if this was a bad auto merge or for backwards compatibility.

so should nk_push_scissor just support subpixels if enabled instead of having two functions?

Interesting idea. I think it's worth exploring. @Nielsbishere would you find some time to try to rewrite this PR to see if it solves some of the issues we have here and if it makes the PR leaner?

Hejsil · 2021-12-15T22:28:30Z

src/nuklear_style.c

+    tab->sym_minimize       = NK_SYMBOL_PLUS;
+    tab->sym_maximize       = NK_SYMBOL_MINUS;
+    tab->sym_child          = NK_SYMBOL_NONE;


I think this is a breaking change visually for people who use the default style

Generally I'm indifferent on this one. A quick google search seems to suggest that triangles are a tiny bit more modern than + - though. So I think this can be left out of this big PR.

Hejsil · 2021-12-15T22:28:56Z

src/nuklear_toggle.c

+nk_toggle_behavior(struct nk_input *in, struct nk_rect select,
+    nk_flags *state, int active)


Pretty sure this should be a bool and not an int

Hejsil · 2021-12-15T22:30:40Z

src/paq.sh

+rm ../nuklear.h
+
+python3 build.py --macro NK --intro HEADER --pub nuklear.h --priv1 nuklear_internal.h,nuklear_math.c,nuklear_util.c,nuklear_color.c,nuklear_utf8.c,nuklear_buffer.c,nuklear_string.c,nuklear_draw.c,nuklear_vertex.c --extern stb_rect_pack.h,stb_truetype.h --priv2 nuklear_font.c,nuklear_input.c,nuklear_style.c,nuklear_context.c,nuklear_pool.c,nuklear_page_element.c,nuklear_table.c,nuklear_panel.c,nuklear_window.c,nuklear_popup.c,nuklear_contextual.c,nuklear_menu.c,nuklear_layout.c,nuklear_tree.c,nuklear_group.c,nuklear_list_view.c,nuklear_widget.c,nuklear_text.c,nuklear_image.c,nuklear_9slice.c,nuklear_button.c,nuklear_toggle.c,nuklear_selectable.c,nuklear_slider.c,nuklear_progress.c,nuklear_scrollbar.c,nuklear_text_editor.c,nuklear_edit.c,nuklear_property.c,nuklear_chart.c,nuklear_color_picker.c,nuklear_combo.c,nuklear_tooltip.c --outro LICENSE,CHANGELOG,CREDITS > ../nuklear.h
+
+if [ -s ../nuklear.h ] ; then
+	echo -- nulear exported
+else
+	echo -- nuklear export failed
+fi


When would this script fail, so that we would need these error messages? Also, I think these messages should be emitted by build.py instead of here

If python3 is not installed (or if the build script fails)

Generally I think the only potentially valuable change to paq.sh could be to print an error message in case of a failure. But only in that case and it has to be to stderr. I.e.:

#!/bin/sh set -e python build.py --macro NK --intro HEADER --pub nuklear.h --priv1 nuklear_internal.h,nuklear_math.c,nuklear_util.c,nuklear_color.c,nuklear_utf8.c,nuklear_buffer.c,nuklear_string.c,nuklear_draw.c,nuklear_vertex.c --extern stb_rect_pack.h,stb_truetype.h --priv2 nuklear_font.c,nuklear_input.c,nuklear_style.c,nuklear_context.c,nuklear_pool.c,nuklear_page_element.c,nuklear_table.c,nuklear_panel.c,nuklear_window.c,nuklear_popup.c,nuklear_contextual.c,nuklear_menu.c,nuklear_layout.c,nuklear_tree.c,nuklear_group.c,nuklear_list_view.c,nuklear_widget.c,nuklear_text.c,nuklear_image.c,nuklear_9slice.c,nuklear_button.c,nuklear_toggle.c,nuklear_selectable.c,nuklear_slider.c,nuklear_progress.c,nuklear_scrollbar.c,nuklear_text_editor.c,nuklear_edit.c,nuklear_property.c,nuklear_chart.c,nuklear_color_picker.c,nuklear_combo.c,nuklear_tooltip.c --outro LICENSE,CHANGELOG,CREDITS > ../nuklear.h || { ret=$? printf 'ERR nuklear.h wrongly generated (or not generated at all)\n' >&2 exit $ret }

(and if others will approve any such adjustments, then paq.bat also has to be adjusted to behave identically!)

Nielsbishere · 2021-12-15T22:54:28Z

@Hejsil I agree, but it was requested I merge this a long time ago iirc.

dumblob · 2021-12-16T09:07:09Z

@Nielsbishere yep, we thought that merging small changes will make it more appropriate. As it turns out, not all the changes are as small as anticipated. Do you think you could help us with splitting the PR (at least the major bump versus other changes) and cleaning it as discussed above?

aganm · 2021-12-25T02:39:27Z

I double checked this branch to see if all works well in case it ends up being pulled in. There's only one thing that bugs me, I'm not sure if it's on purpose but the color picker widget isn't following my mouse cursor anymore.

You see when I click, I'm holding the mouse, but the color cursor isn't following the mouse cursor:

This is the expected behaviour:

Nielsbishere · 2021-12-25T02:48:25Z

Oh yeah I will get to this PR in a bit, just busy.
@aganm I'm guessing the event is being consumed before it reaches it for some reason.
@dumblob I think I should probably apply those fixes. I'm not sure how to split it in different branches without rewriting it.

dumblob · 2021-12-28T13:27:35Z

@aganm thanks for the quick test! That's very important to do. Feel free to continue your endeavor if you find some time for it. We'll be thankful for that!

@dumblob I think I should probably apply those fixes. I'm not sure how to split it in different branches without rewriting it.

Yeah, different fixes will be necessary. Regarding splitting, what do you mean by "rewriting it"? Just take few related commits into a separate branch and then consolidate the result with one new commit in that new branch (fixing the discrepancies which are to be expected). Do this until you run out of all the commits from this PR 😉. This PR will then get closed as superseeded by all the new PRs (for each new branch). Sounds tedious but in this case it's really valuable and we'll be greateful if you managed. Take your time.

Thanks!

mecwerks · 2022-11-22T15:57:47Z

I double checked this branch to see if all works well in case it ends up being pulled in. There's only one thing that bugs me, I'm not sure if it's on purpose but the color picker widget isn't following my mouse cursor anymore.

Found a fix that worked for me. I'm not sure what else this may affect but it seemed to retain the click through fix

diff --git a/src/nuklear_input.c b/src/nuklear_input.c
index 6bcb97d..a8f5e05 100644
--- a/src/nuklear_input.c
+++ b/src/nuklear_input.c
@@ -154,7 +154,7 @@ nk_input_has_mouse_click_in_rect(const struct nk_input *i, enum nk_buttons id,
     struct nk_rect b)
 {
     const struct nk_mouse_button *btn;
-    if (!i || i->mouse.clicked) return nk_false;
+    if (!i) return nk_false;
     btn = &i->mouse.buttons[id];
     if (!NK_INBOX(btn->clicked_pos.x,btn->clicked_pos.y,b.x,b.y,b.w,b.h))
         return nk_false;

aganm · 2022-11-22T16:44:11Z

@mecwerks Thanks for that, I have looked into it and this change comes from commit

8e89387 Merge branch 'fix_click' of https://github.com/Nielsbishere/Nuklear into fix_radio_buttons

It seems like the reason for this change was to fix click related to radio buttons, but, I have tested your proposed fix and it seems to work, both color pickers and radio buttons seem to work perfectly with this fix!

Fix which is a revert of this one specific line

Nuklear/src/nuklear_input.c

Line 157 in 8e89387

if (!i || i->mouse.clicked) return nk_false;

back to what it was before this PR

Nuklear/src/nuklear_input.c

Line 151 in 3a0aafb

if (!i) return nk_false;

With this fix I would be happy if we could merge this PR.

Nielsbishere · 2022-11-22T16:53:43Z

Oh hey it's my old branch. @aganm @mecwerks the reason for this change was quite important; if you have a dropdown window over a radio button, it'll push through the window into the radio button. The proper fix would be to figure out why the color picker has this behavior. Maybe this check is in the wrong place; preventing that behavior? But letting the radio button be clicked while the input event is already consumed is not good imo

aganm · 2022-11-22T17:14:46Z

Oh hey it's my old branch. @aganm @mecwerks the reason for this change was quite important; if you have a dropdown window over a radio button, it'll push through the window into the radio button. The proper fix would be to figure out why the color picker has this behavior. Maybe this check is in the wrong place; preventing that behavior? But letting the radio button be clicked while the input event is already consumed is not good imo

It seems both color picker and radio button rely on the same underlying nk_button_behavior function. But the difference is that the radio button calls nk_button_behavior with a behavior parameter of NK_BUTTON_DEFAULT and the color picker calls nk_button_behavior with a behavior parameter of NK_BUTTON_REPEATED. The function nk_button_behavior seems to not handle the repeated mode correctly.

Nielsbishere · 2022-11-22T17:34:24Z

@aganm sounds like a solution is to only check if it's not repeated then. I'm not continuing this branch as my current project doesn't use it. Maybe if I ever use it again, but I don't have any time left to do this rn

RobLoach · 2025-11-04T16:57:07Z

3 years 😉 Thanks for your push on this. Are there any individual PRs you believe we should bring in? A large change like this can be difficult to review, as demonstrated. Smaller code changes are easier to bring in.

Nielsbishere · 2025-11-04T20:47:26Z

yeah I was going through my open PRs and noticed this was still open. I'm not sure what happened since then in terms of development, but the original was smaller PRs and was asked to be merged into one bigger PR.

Feature subpixel api #50
Window improvements: callbacks, ids #35
Fix radio buttons #15
Fix click #14
Strict c++17 now compiles #13
So it might be worth looking what's worth salvaging from there if these features haven't made it in in these 3 years.

itsuhane and others added 30 commits June 16, 2019 15:19

Add subpixel API.

8d11c1b

Merge pull request Immediate-Mode-UI#1 from vurtun/master

7965ce2

merge from upstream

Merge branch 'master' into subpixel

4aaad16

Add forward-declarations for subpixel-drawing APIs.

a61e94e

Now detecting if memcpy and memset are actually needed. And same goes…

e4133b7

… for vertices (they check correctly now). Paq.sh now automatically outputs to nuklear.h.

https://github.com/vurtun/nuklear/pull/803

0919f70

keharriso's Prevent clicking more than one (overlapping) button per frame. However, this has a couple of other changes that should fix dropdowns from intefering as well.

Merge branch 'fix_cpp17' of https://github.com/Nielsbishere/Nuklear i…

c9980c2

…nto fix_click

vurtun/nuklear#653 zhouxs1023's Modify the appearance for option and …

11aa9a3

…checkbox! With README changes that show off more features and the updated style. Slightly modified.

Fixed readme

842fd02

Fixed logical mistake I made in readme

f4d97f5

Update nuklear_internal.h

014cd2d

Update nuklear_util.c

0de31b9

Repacked nuklear.h and updated package.json

be3f371

Delete package-lock.json

2c6afa7

Merged with fixed_cpp17

31492dc

Moved ctx to the stack

4ede715

Merge branch 'fix_click' of https://github.com/Nielsbishere/Nuklear i…

8e89387

…nto fix_radio_buttons

Merge with fix_click and bump version

9f8a941

Update changelog

871f277

Fixed indenting and updated changelog

15aff92

Merge branch 'fix_click' of https://github.com/Nielsbishere/Nuklear i…

9b05321

…nto fix_radio_buttons

Updated changelog

498c3c1

Now refreshing nuklear.h everytime nuklear rebuilds

7926983

Removing memset and memcpy functions if they don't exist anymore

6023f8d

Memset/memcpy

8ca23c3

Merge https://github.com/Immediate-Mode-UI/Nuklear into fix_cpp17

2c6db66

Updated to work with clang

2a415e8

Moved to linux line endings.

792931d

Merge branch 'fix_cpp17' of https://github.com/Nielsbishere/Nuklear i…

9aa9729

…nto fix_click

Nielsbishere changed the title ~~Feature subpixel api~~ v 4.04.0: Multiple PRs merged Jul 6, 2020

Nielsbishere mentioned this pull request Jul 6, 2020

"File" (childless) node in nk_tree #159

Closed

Nielsbishere added 2 commits July 20, 2020 16:46

Added "nk_tree_push_from_hash" to avoid having to push a const char* …

88a34cc

…and use a 32-bit uint instead.

Fixed strict C++20 compile errors.

f6ac703

Nielsbishere added 4 commits December 15, 2021 20:38

Merge https://github.com/Immediate-Mode-UI/Nuklear into HEAD

72108de

Merge https://github.com/Immediate-Mode-UI/Nuklear into HEAD

4c3562c

Fix build

e974b01

Fix build 2?

0abaf32

Nielsbishere changed the title ~~v 4.04.0: Multiple PRs merged~~ v 4.10.0: Multiple PRs merged Dec 15, 2021

Fix build 3?

43b6a84

Hejsil requested changes Dec 15, 2021

View reviewed changes

RobLoach marked this pull request as draft December 22, 2021 19:16

aganm mentioned this pull request Dec 25, 2021

Revert "Fix layout bounds not accounting for padding" #387

Merged

Nielsbishere closed this Nov 4, 2025

		@@ -1,4 +1,4 @@
		<meta charset='utf-8' emacsmode='-- markdown --'>
		<meta charset='utf-8' emacsmode='-- markdown --'>

		nk_toggle_behavior(struct nk_input *in, struct nk_rect select,
		nk_flags *state, int active)

v 4.10.0: Multiple PRs merged #160

v 4.10.0: Multiple PRs merged #160

Uh oh!

Conversation

Nielsbishere commented Jul 6, 2020

Uh oh!

RobLoach commented Dec 14, 2021

Uh oh!

dumblob commented Dec 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nielsbishere commented Dec 15, 2021

Uh oh!

dumblob commented Dec 15, 2021

Uh oh!

Hejsil left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nielsbishere Dec 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dumblob Dec 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nielsbishere commented Dec 15, 2021

Uh oh!

dumblob commented Dec 16, 2021

Uh oh!

aganm commented Dec 25, 2021

Uh oh!

dumblob commented Dec 14, 2021 •

edited

Loading

Nielsbishere Dec 15, 2021 •

edited

Loading

dumblob Dec 16, 2021 •

edited

Loading

dumblob commented Dec 28, 2021 •

edited

Loading

mecwerks commented Nov 22, 2022 •

edited

Loading