Possible error in calculating #682

New issue

Closed

opened 2016-05-31 20:28:52 +00:00 by JaapSuter · 4 comments

JaapSuter commented

2016-05-31 20:28:52 +00:00

(Migrated from github.com)

Looking near line 4206 in imgui.cpp: ocornut/imgui@45dacbf084/imgui.cpp (L4206)

window->ContentsRegionRect.Min.x = -window->Scroll.x + window->WindowPadding.x;
window->ContentsRegionRect.Min.x = -window->Scroll.y + window->TitleBarHeight() + window->MenuBarHeight() + window->WindowPadding.y;

It appears to me like you assign to Min.X twice in a row, which seems to me like the second line perhaps should've been assigned to Min.Y instead (similar to how Max.X and `Max.Y are assigned to immediately after.)

However, I'm not entirely sure. I stumbled upon this while trying to figure out a problem with a run-away scrollbar (it keeps growing downward by 4 pixels every frame) ever since I integrated Paniq's imgui_dock code. Changing that second Min.X to Min.Y didn't do anything to fix that though. So I'll keep digging, but in the meantime I figured I'd at least ask if that code is as intended.

Great project, thank you so much Ocornut!

Looking near line 4206 in imgui.cpp: https://github.com/ocornut/imgui/blob/45dacbf084e897c317d034275fbd070ed71c0709/imgui.cpp#L4206 ``` window->ContentsRegionRect.Min.x = -window->Scroll.x + window->WindowPadding.x; window->ContentsRegionRect.Min.x = -window->Scroll.y + window->TitleBarHeight() + window->MenuBarHeight() + window->WindowPadding.y; ``` It appears to me like you assign to `Min.X` twice in a row, which seems to me like the second line perhaps should've been assigned to `Min.Y` instead (similar to how `Max.X` and `Max.Y are assigned to immediately after.) However, I'm not entirely sure. I stumbled upon this while trying to figure out a problem with a run-away scrollbar (it keeps growing downward by 4 pixels every frame) ever since I integrated Paniq's imgui_dock code. Changing that second `Min.X` to `Min.Y` didn't do anything to fix that though. So I'll keep digging, but in the meantime I figured I'd at least ask if that code is as intended. Great project, thank you so much Ocornut!

ocornut commented

2016-05-31 20:35:06 +00:00

(Migrated from github.com)

OUCH, totally a bug I introduced a few commits ago.
Interestingly it wasn't having much effect because core imgui doesn't use the Y value for this field, BUT the user could in theory retrieve it (even if it's rare).
Thanks!

Let me know if you fnid more about your other issue.

OUCH, totally a bug I introduced a few commits ago. Interestingly it wasn't having much effect because core imgui doesn't use the Y value for this field, BUT the user could in theory retrieve it (even if it's rare). Thanks! Let me know if you fnid more about your other issue.

JaapSuter commented

2016-05-31 21:01:19 +00:00

(Migrated from github.com)

    ImGui::SetNextWindowSize(ImVec2(float(width), float(height)));
    ImGui::PushStyleVar(ImGuiStyleVar_WindowRounding, 0);
    ImGui::PushStyleVar(ImGuiStyleVar_WindowPadding, ImVec2(0, 0));

    ImGui::Begin("Workspace", nullptr,
                 ImGuiWindowFlags_NoTitleBar |
                 ImGuiWindowFlags_NoResize |
                 ImGuiWindowFlags_NoMove |
                 ImGuiWindowFlags_NoCollapse |
                 ImGuiWindowFlags_NoScrollWithMouse |
                 ImGuiWindowFlags_MenuBar |
                 ImGuiWindowFlags_NoSavedSettings |
                 ImGuiWindowFlags_NoBringToFrontOnFocus |
                 ImGuiWindowFlags_NoFocusOnAppearing);

    ImGui::PopStyleVar();
    ImGui::PopStyleVar();

    igBeginWorkspace(width, height);

            ....

                 ImGui::BeginDock("foo");
                 ImGui::EndDock();

    igEndWorkspace();    

    ImGui::PushStyleVar(ImGuiStyleVar_WindowRounding, 0);
    ImGui::PushStyleVar(ImGuiStyleVar_WindowPadding, ImVec2(0, 0));

    ImGui::End();

    ImGui::PopStyleVar();
    ImGui::PopStyleVar();

To be clear, I'm not expecting you to debug my problems. Nor do I assume you know much of Paniq's code (though I did see your involvement in the Dock Layout discussion, leaving me with some small amount of hope that you are somewhat aware of it).

I started stepping through the debugger, and I narrowed things down to a narrow place where before and after a vertical size.y value keeps increasing by 4. From there, I figured I trace back where that increment of 4 comes from. I used data-breakpoints to see whom is writing to what, which led me to some kind of circular interplay between the window size.y and the window content_avail.y.

In ImGui::BeginChild near line 3550, the code updates size.y using content_avail.y like so:

size.y = ImMax(content_avail.y, 4.0f) - fabsf(size.y);

And indeed, every frame tick the value of content_avail.y is 4 greater. So working back, I see that content_avail.y comes from ContentsRegionRect.Max, then putting a data breakpoint on that, leads me to it being updated in ImGui::Begin near line 4215 in this code:

    window->ContentsRegionRect.Max.y = -window->Scroll.y - window->WindowPadding.y + (window->SizeContentsExplicit.y != 0.0f ? window->SizeContentsExplicit.y : ImMax(window->SizeContents.y, window->Size.y - window->ScrollbarSizes.y));

Most of the values in that statement are zero. The thing that is increasing by 4 every frame is, guess what... (suspense 😄...), window->size.y. But that was precisely the thing that got updated by ContentsRegionRect.Max previously.

So I'm not clear on the relationship between these two variables. Perhaps one is referring to the parent window and the other to the child window. Or do they belong to the same window and is one from the previous frame?

Again, I'm not really expecting you to read this and have a solution for me. I'm just writing this down in the hope it may clear my own head and make it obvious what I'm doing wrong. If you have any thoughts at all, that'd be great.

If I'm unable to resolve this I'll probably try to make a minimal reproduction and fire it your way. Then I'll probably discover the problem is either in Paniq's code (not likely), or my own code (very likely).

Thanks again,

Jaap Suter

Let me know if instead of continuing here you'd rather I open up a new issue, but I'm a bit stumped on this vertical window growing thing. I've trimmed it down to three windows being created (the outer default Debug one, and then one window that Paniq creates in BeginWorkspace, and then one docked window that I create in there using BeginDock). I've put no widgets on any of it. The outermost window is created as follows (I took this from Paniq's code): ``` ImGui::SetNextWindowSize(ImVec2(float(width), float(height))); ImGui::PushStyleVar(ImGuiStyleVar_WindowRounding, 0); ImGui::PushStyleVar(ImGuiStyleVar_WindowPadding, ImVec2(0, 0)); ImGui::Begin("Workspace", nullptr, ImGuiWindowFlags_NoTitleBar | ImGuiWindowFlags_NoResize | ImGuiWindowFlags_NoMove | ImGuiWindowFlags_NoCollapse | ImGuiWindowFlags_NoScrollWithMouse | ImGuiWindowFlags_MenuBar | ImGuiWindowFlags_NoSavedSettings | ImGuiWindowFlags_NoBringToFrontOnFocus | ImGuiWindowFlags_NoFocusOnAppearing); ImGui::PopStyleVar(); ImGui::PopStyleVar(); igBeginWorkspace(width, height); .... ImGui::BeginDock("foo"); ImGui::EndDock(); igEndWorkspace(); ImGui::PushStyleVar(ImGuiStyleVar_WindowRounding, 0); ImGui::PushStyleVar(ImGuiStyleVar_WindowPadding, ImVec2(0, 0)); ImGui::End(); ImGui::PopStyleVar(); ImGui::PopStyleVar(); ``` To be clear, I'm not expecting you to debug my problems. Nor do I assume you know much of Paniq's code (though I did see your involvement in the Dock Layout discussion, leaving me with some small amount of hope that you are somewhat aware of it). I started stepping through the debugger, and I narrowed things down to a narrow place where before and after a vertical size.y value keeps increasing by 4. From there, I figured I trace back where that increment of 4 comes from. I used data-breakpoints to see whom is writing to what, which led me to some kind of circular interplay between the window `size.y` and the window `content_avail.y`. In `ImGui::BeginChild` near line 3550, the code updates `size.y` using `content_avail.y` like so: ``` size.y = ImMax(content_avail.y, 4.0f) - fabsf(size.y); ``` And indeed, every frame tick the value of `content_avail.y` is 4 greater. So working back, I see that `content_avail.y` comes from `ContentsRegionRect.Max`, then putting a data breakpoint on that, leads me to it being updated in `ImGui::Begin` near line 4215 in this code: ``` window->ContentsRegionRect.Max.y = -window->Scroll.y - window->WindowPadding.y + (window->SizeContentsExplicit.y != 0.0f ? window->SizeContentsExplicit.y : ImMax(window->SizeContents.y, window->Size.y - window->ScrollbarSizes.y)); ``` Most of the values in that statement are zero. The thing that is increasing by 4 every frame is, guess what... (suspense :smile:...), `window->size.y`. But that was precisely the thing that got updated by `ContentsRegionRect.Max` previously. So I'm not clear on the relationship between these two variables. Perhaps one is referring to the parent window and the other to the child window. Or do they belong to the same window and is one from the previous frame? Again, I'm not really expecting you to read this and have a solution for me. I'm just writing this down in the hope it may clear my own head and make it obvious what I'm doing wrong. If you have any thoughts at all, that'd be great. If I'm unable to resolve this I'll probably try to make a minimal reproduction and fire it your way. Then I'll probably discover the problem is either in Paniq's code (not likely), or my own code (very likely). Thanks again, Jaap Suter

ocornut commented

2016-05-31 21:30:29 +00:00

(Migrated from github.com)

Please post as new issue with nice subject line, thanks!

Sent from my fax machine

On 31 May 2016, at 23:01, Jaap Suter notifications@github.com wrote:

Let me know if instead of continuing here you'd rather I open up a new issue, but I'm a bit stumped on this vertical window growing thing. I've trimmed it down to three windows being created (the outer default Debug one, and then one window that Paniq creates in BeginWorkspace, and then one docked window that I create in there using BeginDock). I've put no widgets on any of it. The outermost window is created as follows (I took this from Paniq's code):
ImGui::SetNextWindowSize(ImVec2(float(width), float(height)));
ImGui::PushStyleVar(ImGuiStyleVar_WindowRounding, 0);
ImGui::PushStyleVar(ImGuiStyleVar_WindowPadding, ImVec2(0, 0));

ImGui::Begin("Workspace", nullptr,
             ImGuiWindowFlags_NoTitleBar |
             ImGuiWindowFlags_NoResize |
             ImGuiWindowFlags_NoMove |
             ImGuiWindowFlags_NoCollapse |
             ImGuiWindowFlags_NoScrollWithMouse |
             ImGuiWindowFlags_MenuBar |
             ImGuiWindowFlags_NoSavedSettings |
             ImGuiWindowFlags_NoBringToFrontOnFocus |
             ImGuiWindowFlags_NoFocusOnAppearing);

ImGui::PopStyleVar();
ImGui::PopStyleVar();

igBeginWorkspace(width, height);

        ....

             ImGui::BeginDock("foo");
             ImGui::EndDock();

igEndWorkspace();    

ImGui::PushStyleVar(ImGuiStyleVar_WindowRounding, 0);
ImGui::PushStyleVar(ImGuiStyleVar_WindowPadding, ImVec2(0, 0));

ImGui::End();

ImGui::PopStyleVar();
ImGui::PopStyleVar();
To be clear, I'm not expecting you to debug my problems. Nor do I assume you know much of Paniq's code (though I did see your involvement in the Dock Layout discussion, leaving me with some small amount of hope that you are somewhat aware of it).

I started stepping through the debugger, and I narrowed things down to a narrow place where before and after a vertical size.y value keeps increasing by 4. From there, I figured I trace back where that increment of 4 comes from. I used data-breakpoints to see whom is writing to what, which led me to some kind of circular interplay between the window size.y and the window content_avail.y.

In ImGui::BeginChild near line 3550, the code updates size.y using content_avail.y like so:

size.y = ImMax(content_avail.y, 4.0f) - fabsf(size.y);
And indeed, every frame tick the value of content_avail.y is 4 greater. So working back, I see that content_avail.y comes from ContentsRegionRect.Max, then putting a data breakpoint on that, leads me to it being updated in ImGui::Begin near line 4215 in this code:
window->ContentsRegionRect.Max.y = -window->Scroll.y - window->WindowPadding.y + (window->SizeContentsExplicit.y != 0.0f ? window->SizeContentsExplicit.y : ImMax(window->SizeContents.y, window->Size.y - window->ScrollbarSizes.y));
Most of the values in that statement are zero. The thing that is increasing by 4 every frame is, guess what... (suspense 😄...), window->size.y. But that was precisely the thing that got updated by ContentsRegionRect.Max previously.

So I'm not clear on the relationship between these two variables. Perhaps one is referring to the parent window and the other to the child window. Or do they belong to the same window and is one from the previous frame?

Again, I'm not really expecting you to read this and have a solution for me. I'm just writing this down in the hope it may clear my own head and make it obvious what I'm doing wrong. If you have any thoughts at all, that'd be great.

If I'm unable to resolve this I'll probably try to make a minimal reproduction and fire it your way. Then I'll probably discover the problem is either in Paniq's code (not likely), or my own code (very likely).

Thanks again,

Jaap Suter

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub, or mute the thread.

Please post as new issue with nice subject line, thanks! Sent from my fax machine > On 31 May 2016, at 23:01, Jaap Suter notifications@github.com wrote: > > Let me know if instead of continuing here you'd rather I open up a new issue, but I'm a bit stumped on this vertical window growing thing. I've trimmed it down to three windows being created (the outer default Debug one, and then one window that Paniq creates in BeginWorkspace, and then one docked window that I create in there using BeginDock). I've put no widgets on any of it. The outermost window is created as follows (I took this from Paniq's code): > > ``` > ImGui::SetNextWindowSize(ImVec2(float(width), float(height))); > ImGui::PushStyleVar(ImGuiStyleVar_WindowRounding, 0); > ImGui::PushStyleVar(ImGuiStyleVar_WindowPadding, ImVec2(0, 0)); > > ImGui::Begin("Workspace", nullptr, > ImGuiWindowFlags_NoTitleBar | > ImGuiWindowFlags_NoResize | > ImGuiWindowFlags_NoMove | > ImGuiWindowFlags_NoCollapse | > ImGuiWindowFlags_NoScrollWithMouse | > ImGuiWindowFlags_MenuBar | > ImGuiWindowFlags_NoSavedSettings | > ImGuiWindowFlags_NoBringToFrontOnFocus | > ImGuiWindowFlags_NoFocusOnAppearing); > > ImGui::PopStyleVar(); > ImGui::PopStyleVar(); > > igBeginWorkspace(width, height); > > .... > > ImGui::BeginDock("foo"); > ImGui::EndDock(); > > igEndWorkspace(); > > ImGui::PushStyleVar(ImGuiStyleVar_WindowRounding, 0); > ImGui::PushStyleVar(ImGuiStyleVar_WindowPadding, ImVec2(0, 0)); > > ImGui::End(); > > ImGui::PopStyleVar(); > ImGui::PopStyleVar(); > ``` > > To be clear, I'm not expecting you to debug my problems. Nor do I assume you know much of Paniq's code (though I did see your involvement in the Dock Layout discussion, leaving me with some small amount of hope that you are somewhat aware of it). > > I started stepping through the debugger, and I narrowed things down to a narrow place where before and after a vertical size.y value keeps increasing by 4. From there, I figured I trace back where that increment of 4 comes from. I used data-breakpoints to see whom is writing to what, which led me to some kind of circular interplay between the window size.y and the window content_avail.y. > > In ImGui::BeginChild near line 3550, the code updates size.y using content_avail.y like so: > > size.y = ImMax(content_avail.y, 4.0f) - fabsf(size.y); > And indeed, every frame tick the value of content_avail.y is 4 greater. So working back, I see that content_avail.y comes from ContentsRegionRect.Max, then putting a data breakpoint on that, leads me to it being updated in ImGui::Begin near line 4215 in this code: > > ``` > window->ContentsRegionRect.Max.y = -window->Scroll.y - window->WindowPadding.y + (window->SizeContentsExplicit.y != 0.0f ? window->SizeContentsExplicit.y : ImMax(window->SizeContents.y, window->Size.y - window->ScrollbarSizes.y)); > ``` > > Most of the values in that statement are zero. The thing that is increasing by 4 every frame is, guess what... (suspense 😄...), window->size.y. But that was precisely the thing that got updated by ContentsRegionRect.Max previously. > > So I'm not clear on the relationship between these two variables. Perhaps one is referring to the parent window and the other to the child window. Or do they belong to the same window and is one from the previous frame? > > Again, I'm not really expecting you to read this and have a solution for me. I'm just writing this down in the hope it may clear my own head and make it obvious what I'm doing wrong. If you have any thoughts at all, that'd be great. > > If I'm unable to resolve this I'll probably try to make a minimal reproduction and fire it your way. Then I'll probably discover the problem is either in Paniq's code (not likely), or my own code (very likely). > > Thanks again, > > Jaap Suter > > — > You are receiving this because you modified the open/close state. > Reply to this email directly, view it on GitHub, or mute the thread.

ocornut commented

2016-05-31 21:54:25 +00:00

(Migrated from github.com)

Are you sure that the 4.0f comes from that hardcoded value (which is an arbitrary minimum size for a child window because 0.0f causes problem)? And not from the 4.0 of style.ItemSpacing.y ? Which you can change to another value to identify the cause.

OK this is all a little confusing but I would eventually look. If you can expand the workspacing/dock code into a ImGui-only repro it would probably make me look faster but otherwise I will dig in it. Screenshots/video would also help just to grasp the situation. Normally I'd look faster but this week has and is going to be very tough/busy as I am announcing my game :) Are you sure that the 4.0f comes from that hardcoded value (which is an arbitrary minimum size for a child window because 0.0f causes problem)? And not from the 4.0 of `style.ItemSpacing.y` ? Which you can change to another value to identify the cause.