There's a issue when you set a window hidden (either manually set its NK_WINDOW_HIDDEN flag or do nk_window_show() ), it will only be hidden only for a period, only then to be reinitialized, which defeats the purpose of hiding and it being a flag in the first place
So this line of code in nk_clear function (defined in nuklear_context.c, is called everytime you render a frame)
https://github.com/Immediate-Mode-UI/Nuklear/blob/a329721db93eb336f8eaae20f9c822730dd2ed6c/src/nuklear_context.c#L137-L147
(do cleanups for 'active' window that isn't really active)
specifically the line ctx->begin = 0; (reset the start pointer of a linked list of windows)
the problem here is it assumes that the hidden window isn't at the 'ctx->begin' (where the 'win->prev' doesnt exist), which isn't always the case, but it set to that anyway without checking
then it choose the window before the hidden one, which may not exist, so it trigger the code and make Nuklear completely forget the window list,
and what worse is that the forgotten window (and the ones after it) won't be freed from memory by the later section of the code since its win->seq (I guess this is like its heartbeat) is valid with the ctx's
https://github.com/Immediate-Mode-UI/Nuklear/blob/a329721db93eb336f8eaae20f9c822730dd2ed6c/src/nuklear_context.c#L165-L171
since the reference is gone, the windows will live in the memory with the program, causing memory leak (thank you sleeptightAnsiC for clearing)
and in the next frame, nk_begin (how you start to draw things) can't find the existing window you're talkiing about (since the list reference is gone) so it will create new one with your spec, that means your hidden window is now reseted and live somewhere else in the memory, and when it's hidden and its 'win->prev' gets invalid (due to some internal reorder for Z indexing or something), that cleanup code (for invalid active window) will be triggered, now you're stuck in a loop where you hide window just to see it popup again
Solution? because i have no idea how do you edit someone's code on github, i've some suggestions:
-
nk_window_show()should clearctx->activeif it is the target window about to be hidden -
maybe 'nk_end' should clearctx->activeby default? -
nk_clearshould have some more considerations before doingctx->begin = 0;, and free the unused -
or use other approach on finding active window (I have a example code in comment)
-
automatically set
ctx->activeto0if current is hidden and letnk_beginin the next frame handles it?
(this is my first time doing this, im sorry for any mistakes or English errors, thank!)
Activity
30
sleeptightAnsiC8mo ago@QuanMz Thanks for all the details. There is only one thing that I would like to ask: which backend/demo have you used for testing this?
About the bug itself: This really smells like a regression. We should probably retest against some older commit before fixing it, and in case the issue doesn't occur there, it would be nice to git-bisect in order to understand what broke it.
it will only be hidden only for that single frame, only then to be reinitialized
I remember seeing something like this in the demos. After closing a window, it would reappear after some time, but in my observation it would take way longer than one frame (maybe even several seconds) so I thought it was intended. I wonder, if this bug has some specific condition that we're missing, or maybe that was something different entirely (idk)
since the reference is gone, the windows will live in the memory with the program, causing buffer overflow (or is it memory leak or stack overflow or something? i dont knoww??)
mem leak :)
because i have no idea how do you edit someone's code on github, i've some suggestions
I'll remember to tag you as Co-Author in case I reuse any of your suggested fix. (if any other contributor is reading this and wishes to fix it, please do the same)
In the mean time, you're welcome to send a patched file here. If I consider it correct, I will turn it into PR for you.
sleeptightAnsiC8mo ago@QuanMz Thanks for all the details. There is only one thing that I would like to ask: which backend/demo have you used for testing this?
About the bug itself: This really smells like a regression. We should probably retest against some older commit before fixing it, and in case the issue doesn't occur there, it would be nice to git-bisect in order to understand what broke it.
it will only be hidden only for that single frame, only then to be reinitialized
I remember seeing something like this in the demos. After closing a window, it would reappear after some time, but in my observation it would take way longer than one frame (maybe even several seconds) so I thought it was intended. I wonder, if this bug has some specific condition that we're missing, or maybe that was something different entirely (idk)
since the reference is gone, the windows will live in the memory with the program, causing buffer overflow (or is it memory leak or stack overflow or something? i dont knoww??)
mem leak :)
because i have no idea how do you edit someone's code on github, i've some suggestions
I'll remember to tag you as Co-Author in case I reuse any of your suggested fix. (if any other contributor is reading this and wishes to fix it, please do the same)
In the mean time, you're welcome to send a patched file here. If I consider it correct, I will turn it into PR for you.
sleeptightAnsiC8mo ago@QuanMz Thanks for all the details. There is only one thing that I would like to ask: which backend/demo have you used for testing this?
About the bug itself: This really smells like a regression. We should probably retest against some older commit before fixing it, and in case the issue doesn't occur there, it would be nice to git-bisect in order to understand what broke it.
it will only be hidden only for that single frame, only then to be reinitialized
I remember seeing something like this in the demos. After closing a window, it would reappear after some time, but in my observation it would take way longer than one frame (maybe even several seconds) so I thought it was intended. I wonder, if this bug has some specific condition that we're missing, or maybe that was something different entirely (idk)
since the reference is gone, the windows will live in the memory with the program, causing buffer overflow (or is it memory leak or stack overflow or something? i dont knoww??)
mem leak :)
because i have no idea how do you edit someone's code on github, i've some suggestions
I'll remember to tag you as Co-Author in case I reuse any of your suggested fix. (if any other contributor is reading this and wishes to fix it, please do the same)
In the mean time, you're welcome to send a patched file here. If I consider it correct, I will turn it into PR for you.
sleeptightAnsiC8mo ago@QuanMz Thanks for all the details. There is only one thing that I would like to ask: which backend/demo have you used for testing this?
About the bug itself: This really smells like a regression. We should probably retest against some older commit before fixing it, and in case the issue doesn't occur there, it would be nice to git-bisect in order to understand what broke it.
it will only be hidden only for that single frame, only then to be reinitialized
I remember seeing something like this in the demos. After closing a window, it would reappear after some time, but in my observation it would take way longer than one frame (maybe even several seconds) so I thought it was intended. I wonder, if this bug has some specific condition that we're missing, or maybe that was something different entirely (idk)
since the reference is gone, the windows will live in the memory with the program, causing buffer overflow (or is it memory leak or stack overflow or something? i dont knoww??)
mem leak :)
because i have no idea how do you edit someone's code on github, i've some suggestions
I'll remember to tag you as Co-Author in case I reuse any of your suggested fix. (if any other contributor is reading this and wishes to fix it, please do the same)
In the mean time, you're welcome to send a patched file here. If I consider it correct, I will turn it into PR for you.
sleeptightAnsiC8mo ago@QuanMz Thanks for all the details. There is only one thing that I would like to ask: which backend/demo have you used for testing this?
About the bug itself: This really smells like a regression. We should probably retest against some older commit before fixing it, and in case the issue doesn't occur there, it would be nice to git-bisect in order to understand what broke it.
it will only be hidden only for that single frame, only then to be reinitialized
I remember seeing something like this in the demos. After closing a window, it would reappear after some time, but in my observation it would take way longer than one frame (maybe even several seconds) so I thought it was intended. I wonder, if this bug has some specific condition that we're missing, or maybe that was something different entirely (idk)
since the reference is gone, the windows will live in the memory with the program, causing buffer overflow (or is it memory leak or stack overflow or something? i dont knoww??)
mem leak :)
because i have no idea how do you edit someone's code on github, i've some suggestions
I'll remember to tag you as Co-Author in case I reuse any of your suggested fix. (if any other contributor is reading this and wishes to fix it, please do the same)
In the mean time, you're welcome to send a patched file here. If I consider it correct, I will turn it into PR for you.
QuanMz8mo ago@sleeptightAnsiC I used SDL3_renderer (the one recently made!), glfw_opengl3, d3d11 to test this, but i bet it's not their fault since Nuklear is the one make commands and manage windows (i eventually do it anyway just to be sure lol) i also see this weird behavior in demo, i thought it's a intended feature so that you can test, untill i actually try myself, about that 1 frame, i maybe wrong here, i was thinking if the cleanup happen right after render finishs, so maybe the next frame will just reinitialize the windows immediately, but anyway yeah it does feel way longer than that, but sometime in a split of second? also thank you for pointing out my mistake! im a little confused by the terms, my badd
after trying to actually find solution i found out that my 2 previous suggestions about clearing ctx->active was actually a really bad idea as it will make the program unresponsive, so... here's the patched file that i hoped you go check a look! you can comment/uncomment my changes (on the top of file) to see it! It simply goes thru the list to find visible window
nuklear.c (please change it to .h because for some reasson github doesn't support uploading it) you can run this with the demo of your prefered render egine or something
by the way, what do you mean by Co-Author or PR? A source code modification? Im not used to git, sorry..
QuanMz8mo ago@sleeptightAnsiC I used SDL3_renderer (the one recently made!), glfw_opengl3, d3d11 to test this, but i bet it's not their fault since Nuklear is the one make commands and manage windows (i eventually do it anyway just to be sure lol) i also see this weird behavior in demo, i thought it's a intended feature so that you can test, untill i actually try myself, about that 1 frame, i maybe wrong here, i was thinking if the cleanup happen right after render finishs, so maybe the next frame will just reinitialize the windows immediately, but anyway yeah it does feel way longer than that, but sometime in a split of second? also thank you for pointing out my mistake! im a little confused by the terms, my badd
after trying to actually find solution i found out that my 2 previous suggestions about clearing ctx->active was actually a really bad idea as it will make the program unresponsive, so... here's the patched file that i hoped you go check a look! you can comment/uncomment my changes (on the top of file) to see it! It simply goes thru the list to find visible window
nuklear.c (please change it to .h because for some reasson github doesn't support uploading it) you can run this with the demo of your prefered render egine or something
by the way, what do you mean by Co-Author or PR? A source code modification? Im not used to git, sorry..
QuanMz8mo ago@sleeptightAnsiC I used SDL3_renderer (the one recently made!), glfw_opengl3, d3d11 to test this, but i bet it's not their fault since Nuklear is the one make commands and manage windows (i eventually do it anyway just to be sure lol) i also see this weird behavior in demo, i thought it's a intended feature so that you can test, untill i actually try myself, about that 1 frame, i maybe wrong here, i was thinking if the cleanup happen right after render finishs, so maybe the next frame will just reinitialize the windows immediately, but anyway yeah it does feel way longer than that, but sometime in a split of second? also thank you for pointing out my mistake! im a little confused by the terms, my badd
after trying to actually find solution i found out that my 2 previous suggestions about clearing ctx->active was actually a really bad idea as it will make the program unresponsive, so... here's the patched file that i hoped you go check a look! you can comment/uncomment my changes (on the top of file) to see it! It simply goes thru the list to find visible window
nuklear.c (please change it to .h because for some reasson github doesn't support uploading it) you can run this with the demo of your prefered render egine or something
by the way, what do you mean by Co-Author or PR? A source code modification? Im not used to git, sorry..
QuanMz8mo ago@sleeptightAnsiC I used SDL3_renderer (the one recently made!), glfw_opengl3, d3d11 to test this, but i bet it's not their fault since Nuklear is the one make commands and manage windows (i eventually do it anyway just to be sure lol) i also see this weird behavior in demo, i thought it's a intended feature so that you can test, untill i actually try myself, about that 1 frame, i maybe wrong here, i was thinking if the cleanup happen right after render finishs, so maybe the next frame will just reinitialize the windows immediately, but anyway yeah it does feel way longer than that, but sometime in a split of second? also thank you for pointing out my mistake! im a little confused by the terms, my badd
after trying to actually find solution i found out that my 2 previous suggestions about clearing ctx->active was actually a really bad idea as it will make the program unresponsive, so... here's the patched file that i hoped you go check a look! you can comment/uncomment my changes (on the top of file) to see it! It simply goes thru the list to find visible window
nuklear.c (please change it to .h because for some reasson github doesn't support uploading it) you can run this with the demo of your prefered render egine or something
by the way, what do you mean by Co-Author or PR? A source code modification? Im not used to git, sorry..
MrOneTwo8mo agoI'm also looking at this issue so I'll do my best to help.
/* remove hotness from hidden or closed windows*/
if (((iter->flags & NK_WINDOW_HIDDEN) ||
(iter->flags & NK_WINDOW_CLOSED)) &&
iter == ctx->active) {
ctx->active = iter->prev;
ctx->end = iter->prev;
if (!ctx->end)
ctx->begin = 0;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;
}If we get to this code we can safely assume iter is valid. I guess here we assume that when a window gets hidden or closed we make the previous window active, but as @QuanMz pointed out we don't handle the case where the iter is the first element.
The only case for iter->prev to be NULL is when it's the first element of the linked list, right?
Wouldn't it then be enough to change to:
if (!ctx->end)
ctx->begin = iter;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;We can't have a node that's a legit part of the linked list, without a prev pointer set UNLESS it's the first element, no?
MrOneTwo8mo agoI'm also looking at this issue so I'll do my best to help.
/* remove hotness from hidden or closed windows*/
if (((iter->flags & NK_WINDOW_HIDDEN) ||
(iter->flags & NK_WINDOW_CLOSED)) &&
iter == ctx->active) {
ctx->active = iter->prev;
ctx->end = iter->prev;
if (!ctx->end)
ctx->begin = 0;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;
}If we get to this code we can safely assume iter is valid. I guess here we assume that when a window gets hidden or closed we make the previous window active, but as @QuanMz pointed out we don't handle the case where the iter is the first element.
The only case for iter->prev to be NULL is when it's the first element of the linked list, right?
Wouldn't it then be enough to change to:
if (!ctx->end)
ctx->begin = iter;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;We can't have a node that's a legit part of the linked list, without a prev pointer set UNLESS it's the first element, no?
MrOneTwo8mo agoI'm also looking at this issue so I'll do my best to help.
/* remove hotness from hidden or closed windows*/
if (((iter->flags & NK_WINDOW_HIDDEN) ||
(iter->flags & NK_WINDOW_CLOSED)) &&
iter == ctx->active) {
ctx->active = iter->prev;
ctx->end = iter->prev;
if (!ctx->end)
ctx->begin = 0;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;
}If we get to this code we can safely assume iter is valid. I guess here we assume that when a window gets hidden or closed we make the previous window active, but as @QuanMz pointed out we don't handle the case where the iter is the first element.
The only case for iter->prev to be NULL is when it's the first element of the linked list, right?
Wouldn't it then be enough to change to:
if (!ctx->end)
ctx->begin = iter;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;We can't have a node that's a legit part of the linked list, without a prev pointer set UNLESS it's the first element, no?
MrOneTwo8mo agoor maybe this code shouldn't modify the linked list when dealing with HIDDEN windows? I don't get why should we modify the linked list whenever the window is hidden. Is it because the order of the linked list implies Z order? (edit: ok I have checked the code and focusing the window is literally removing the window from the linked list and appending it at the end so yeah the order is important).
For closed windows we already do nk_remove_window, which boots the window from the linked list.
This code could be split if we don't have to modify the linked list.
/* garbage collector */
iter = ctx->begin;
while (iter) {
/* make sure valid minimized windows do not get removed */
if ((iter->flags & NK_WINDOW_MINIMIZED) &&
!(iter->flags & NK_WINDOW_CLOSED) &&
iter->seq == ctx->seq) {
iter = iter->next;
continue;
}
/* remove hotness from hidden or closed windows*/
if (((iter->flags & NK_WINDOW_HIDDEN) ||
(iter->flags & NK_WINDOW_CLOSED)) &&
iter == ctx->active) {
ctx->active = iter->prev;
ctx->end = iter->prev;
if (!ctx->end)
ctx->begin = 0;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;
}Here we split the handling but not modifying the linked list, except for the active field.
/* garbage collector */
iter = ctx->begin;
while (iter) {
/* make sure valid minimized windows do not get removed */
if ((iter->flags & NK_WINDOW_MINIMIZED) &&
!(iter->flags & NK_WINDOW_CLOSED) &&
iter->seq == ctx->seq) {
iter = iter->next;
continue;
}
/* remove hotness from hidden or closed windows*/
if (((iter->flags & NK_WINDOW_HIDDEN) ||
(iter->flags & NK_WINDOW_CLOSED)) &&
iter == ctx->active) {
ctx->active = iter->prev;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;
}
// This needs to happen for when it's a HIDDEN window but it implies that we're not handling
// both HIDDEN and CLOSED... it's definitely possible to handle all of this with a bit more concise logic.
if (iter->flags & NK_WINDOW_HIDDEN) {
iter = iter->next;
continue;
}
...
/* window itself is not used anymore so free */
if (iter->seq != ctx->seq || iter->flags & NK_WINDOW_CLOSED) {
next = iter->next;
nk_remove_window(ctx, iter);
nk_free_window(ctx, iter);
iter = next;
} else iter = iter->next;or maybe this code shouldn't modify the linked list when dealing with HIDDEN windows? I don't get why should we modify the linked list whenever the window is hidden. Is it because the order of the linked list implies Z order? (edit: ok I have checked the code and focusing the window is literally removing the window from the linked list and appending it at the end so yeah the order is important).
For closed windows we already do nk_remove_window, which boots the window from the linked list.
This code could be split if we don't have to modify the linked list.
/* garbage collector */
iter = ctx->begin;
while (iter) {
/* make sure valid minimized windows do not get removed */
if ((iter->flags & NK_WINDOW_MINIMIZED) &&
!(iter->flags & NK_WINDOW_CLOSED) &&
iter->seq == ctx->seq) {
iter = iter->next;
continue;
}
/* remove hotness from hidden or closed windows*/
if (((iter->flags & NK_WINDOW_HIDDEN) ||
(iter->flags & NK_WINDOW_CLOSED)) &&
iter == ctx->active) {
ctx->active = iter->prev;
ctx->end = iter->prev;
if (!ctx->end)
ctx->begin = 0;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;
}Here we split the handling but not modifying the linked list, except for the active field.
/* garbage collector */
iter = ctx->begin;
while (iter) {
/* make sure valid minimized windows do not get removed */
if ((iter->flags & NK_WINDOW_MINIMIZED) &&
!(iter->flags & NK_WINDOW_CLOSED) &&
iter->seq == ctx->seq) {
iter = iter->next;
continue;
}
/* remove hotness from hidden or closed windows*/
if (((iter->flags & NK_WINDOW_HIDDEN) ||
(iter->flags & NK_WINDOW_CLOSED)) &&
iter == ctx->active) {
ctx->active = iter->prev;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;
}
// This needs to happen for when it's a HIDDEN window but it implies that we're not handling
// both HIDDEN and CLOSED... it's definitely possible to handle all of this with a bit more concise logic.
if (iter->flags & NK_WINDOW_HIDDEN) {
iter = iter->next;
continue;
}
...
/* window itself is not used anymore so free */
if (iter->seq != ctx->seq || iter->flags & NK_WINDOW_CLOSED) {
next = iter->next;
nk_remove_window(ctx, iter);
nk_free_window(ctx, iter);
iter = next;
} else iter = iter->next;or maybe this code shouldn't modify the linked list when dealing with HIDDEN windows? I don't get why should we modify the linked list whenever the window is hidden. Is it because the order of the linked list implies Z order? (edit: ok I have checked the code and focusing the window is literally removing the window from the linked list and appending it at the end so yeah the order is important).
For closed windows we already do nk_remove_window, which boots the window from the linked list.
This code could be split if we don't have to modify the linked list.
/* garbage collector */
iter = ctx->begin;
while (iter) {
/* make sure valid minimized windows do not get removed */
if ((iter->flags & NK_WINDOW_MINIMIZED) &&
!(iter->flags & NK_WINDOW_CLOSED) &&
iter->seq == ctx->seq) {
iter = iter->next;
continue;
}
/* remove hotness from hidden or closed windows*/
if (((iter->flags & NK_WINDOW_HIDDEN) ||
(iter->flags & NK_WINDOW_CLOSED)) &&
iter == ctx->active) {
ctx->active = iter->prev;
ctx->end = iter->prev;
if (!ctx->end)
ctx->begin = 0;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;
}Here we split the handling but not modifying the linked list, except for the active field.
/* garbage collector */
iter = ctx->begin;
while (iter) {
/* make sure valid minimized windows do not get removed */
if ((iter->flags & NK_WINDOW_MINIMIZED) &&
!(iter->flags & NK_WINDOW_CLOSED) &&
iter->seq == ctx->seq) {
iter = iter->next;
continue;
}
/* remove hotness from hidden or closed windows*/
if (((iter->flags & NK_WINDOW_HIDDEN) ||
(iter->flags & NK_WINDOW_CLOSED)) &&
iter == ctx->active) {
ctx->active = iter->prev;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;
}
// This needs to happen for when it's a HIDDEN window but it implies that we're not handling
// both HIDDEN and CLOSED... it's definitely possible to handle all of this with a bit more concise logic.
if (iter->flags & NK_WINDOW_HIDDEN) {
iter = iter->next;
continue;
}
...
/* window itself is not used anymore so free */
if (iter->seq != ctx->seq || iter->flags & NK_WINDOW_CLOSED) {
next = iter->next;
nk_remove_window(ctx, iter);
nk_free_window(ctx, iter);
iter = next;
} else iter = iter->next;or maybe this code shouldn't modify the linked list when dealing with HIDDEN windows? I don't get why should we modify the linked list whenever the window is hidden. Is it because the order of the linked list implies Z order? (edit: ok I have checked the code and focusing the window is literally removing the window from the linked list and appending it at the end so yeah the order is important).
For closed windows we already do nk_remove_window, which boots the window from the linked list.
This code could be split if we don't have to modify the linked list.
/* garbage collector */
iter = ctx->begin;
while (iter) {
/* make sure valid minimized windows do not get removed */
if ((iter->flags & NK_WINDOW_MINIMIZED) &&
!(iter->flags & NK_WINDOW_CLOSED) &&
iter->seq == ctx->seq) {
iter = iter->next;
continue;
}
/* remove hotness from hidden or closed windows*/
if (((iter->flags & NK_WINDOW_HIDDEN) ||
(iter->flags & NK_WINDOW_CLOSED)) &&
iter == ctx->active) {
ctx->active = iter->prev;
ctx->end = iter->prev;
if (!ctx->end)
ctx->begin = 0;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;
}Here we split the handling but not modifying the linked list, except for the active field.
/* garbage collector */
iter = ctx->begin;
while (iter) {
/* make sure valid minimized windows do not get removed */
if ((iter->flags & NK_WINDOW_MINIMIZED) &&
!(iter->flags & NK_WINDOW_CLOSED) &&
iter->seq == ctx->seq) {
iter = iter->next;
continue;
}
/* remove hotness from hidden or closed windows*/
if (((iter->flags & NK_WINDOW_HIDDEN) ||
(iter->flags & NK_WINDOW_CLOSED)) &&
iter == ctx->active) {
ctx->active = iter->prev;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;
}
// This needs to happen for when it's a HIDDEN window but it implies that we're not handling
// both HIDDEN and CLOSED... it's definitely possible to handle all of this with a bit more concise logic.
if (iter->flags & NK_WINDOW_HIDDEN) {
iter = iter->next;
continue;
}
...
/* window itself is not used anymore so free */
if (iter->seq != ctx->seq || iter->flags & NK_WINDOW_CLOSED) {
next = iter->next;
nk_remove_window(ctx, iter);
nk_free_window(ctx, iter);
iter = next;
} else iter = iter->next;or maybe this code shouldn't modify the linked list when dealing with HIDDEN windows? I don't get why should we modify the linked list whenever the window is hidden. Is it because the order of the linked list implies Z order? (edit: ok I have checked the code and focusing the window is literally removing the window from the linked list and appending it at the end so yeah the order is important).
For closed windows we already do nk_remove_window, which boots the window from the linked list.
This code could be split if we don't have to modify the linked list.
/* garbage collector */
iter = ctx->begin;
while (iter) {
/* make sure valid minimized windows do not get removed */
if ((iter->flags & NK_WINDOW_MINIMIZED) &&
!(iter->flags & NK_WINDOW_CLOSED) &&
iter->seq == ctx->seq) {
iter = iter->next;
continue;
}
/* remove hotness from hidden or closed windows*/
if (((iter->flags & NK_WINDOW_HIDDEN) ||
(iter->flags & NK_WINDOW_CLOSED)) &&
iter == ctx->active) {
ctx->active = iter->prev;
ctx->end = iter->prev;
if (!ctx->end)
ctx->begin = 0;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;
}Here we split the handling but not modifying the linked list, except for the active field.
/* garbage collector */
iter = ctx->begin;
while (iter) {
/* make sure valid minimized windows do not get removed */
if ((iter->flags & NK_WINDOW_MINIMIZED) &&
!(iter->flags & NK_WINDOW_CLOSED) &&
iter->seq == ctx->seq) {
iter = iter->next;
continue;
}
/* remove hotness from hidden or closed windows*/
if (((iter->flags & NK_WINDOW_HIDDEN) ||
(iter->flags & NK_WINDOW_CLOSED)) &&
iter == ctx->active) {
ctx->active = iter->prev;
if (ctx->active)
ctx->active->flags &= ~(unsigned)NK_WINDOW_ROM;
}
// This needs to happen for when it's a HIDDEN window but it implies that we're not handling
// both HIDDEN and CLOSED... it's definitely possible to handle all of this with a bit more concise logic.
if (iter->flags & NK_WINDOW_HIDDEN) {
iter = iter->next;
continue;
}
...
/* window itself is not used anymore so free */
if (iter->seq != ctx->seq || iter->flags & NK_WINDOW_CLOSED) {
next = iter->next;
nk_remove_window(ctx, iter);
nk_free_window(ctx, iter);
iter = next;
} else iter = iter->next;This really smells like a regression. We should probably retest against some older commit before fixing it, and in case the issue doesn't occur there, it would be nice to git-bisect in order to understand what broke it.
I found some very old commits at https://github.com/vurtun/nuklear where it does not seem to happen, but it was so far ago, literally back when this library was called zahnrad in 2016, so I abandoned the idea of bisecting this.
I think we can just assume this feature never worked and fix it with whatever seems right.
sleeptightAnsiC8mo agoThis really smells like a regression. We should probably retest against some older commit before fixing it, and in case the issue doesn't occur there, it would be nice to git-bisect in order to understand what broke it.
I found some very old commits at https://github.com/vurtun/nuklear where it does not seem to happen, but it was so far ago, literally back when this library was called zahnrad in 2016, so I abandoned the idea of bisecting this.
I think we can just assume this feature never worked and fix it with whatever seems right.
@QuanMz Thanks for all the details. There is only one thing that I would like to ask: which backend/demo have you used for testing this?
About the bug itself: This really smells like a regression. We should probably retest against some older commit before fixing it, and in case the issue doesn't occur there, it would be nice to git-bisect in order to understand what broke it.
I remember seeing something like this in the demos. After closing a window, it would reappear after some time, but in my observation it would take way longer than one frame (maybe even several seconds) so I thought it was intended. I wonder, if this bug has some specific condition that we're missing, or maybe that was something different entirely (idk)
mem leak :)
I'll remember to tag you as Co-Author in case I reuse any of your suggested fix. (if any other contributor is reading this and wishes to fix it, please do the same)
In the mean time, you're welcome to send a patched file here. If I consider it correct, I will turn it into PR for you.