Stack Layout implementation

[thedmd]

PR was recreated due to lost references after changing fork origin. That was not my intention, sorry. For comments please look at previous PR.

After searching for some layout related solution I decided to pull my own after everything I found was only issues listed on this repository (#97).

I think BeginHorizontal/EndHorizontal API was promissing, so I gave it a shot. There is how code look like and what results it produce.

            ImGui::BeginHorizontal("example_h1", spanToWindow ? bounds : ImVec2(0, 0));
                ImGui::TextUnformatted("Left");
                ImGui::Spring(middleWeight);
                ImGui::TextUnformatted("Middle");
                ImGui::Spring(1.0f - middleWeight);
                ImGui::TextUnformatted("Right");
            ImGui::EndHorizontal();

I tried to bend ImGui to help me layout widgets easly. This is what API I came up with:

    IMGUI_API void BeginHorizontal(const char* str_id, const ImVec2& size = ImVec2(0, 0));
    IMGUI_API void BeginHorizontal(const void* ptr_id, const ImVec2& size = ImVec2(0, 0));
    IMGUI_API void EndHorizontal();
    IMGUI_API void BeginVertical(const char* str_id, const ImVec2& size = ImVec2(0, 0));
    IMGUI_API void BeginVertical(const void* ptr_id, const ImVec2& size = ImVec2(0, 0));
    IMGUI_API void EndVertical();
    IMGUI_API void Spring(float weight = 1.0f, float spacing = -1.0f);

This code works by caching sizes of widgets in first frame and positioning them in next. Layout is evaluated only if something related to size changes.

Code was crafted in a few hours so it may be cruel in some places. I tried to be as consistent with ImGui code style. Please grab this code and try to use it. Let me know what you think about this solution. : )

Example code show one use case. There are however many more. Widgets can be interleaved by Springs which may have zero (sticking widgets together) weight or greater than zero. Free space will be divide according to them. There is an option to provide spacing which acts like in SameLine() function but works for both horizontal and vertical layouts. Layouts can be mixed together, you may put one in another as you can see on example gif.

[colesnicov]

Looks good. Will not be implemented?

[ocornut]

@colesnicov

Looks good. Will not be implemented?

imgui development is on hold for a few months and this is not a priority but it should eventually be merged in some way or another.

[mgerhardy]

There is a bom in imgui.cpp and imgui_internal.h (and maybe others) that shouldn't be there.

[sgf]

Im just going to raise this question then searched for this issue. This is a very important and useful feature.but im seem the code changes,it's a little complicated. looking forward to joining soon！

[b1tc0der]

@thedmd Any chance you could share the basic node graph code you used for the screenshot you posted on issue 746? https://github.com/ocornut/imgui/pull/746

[thedmd]

@b1tc0der I'm thinking about that. I need to find some time to update and wrap up code.

[b1tc0der]

@thedmd I think you've done an awesome job on this :-) If you need any help with wrapping up this code I am happy to help.

[francesco-cattoglio]

@thedmd I just found out the awesome job you made with the node editor. I am really interested in it because I was tasked with creating a small tool to help teaching design students the ropes of mathematics used in CG. I would love to have a blueprint-like interface for the software, but building something on top of the Unreal Engine would be complete overkill. Please let me know if there's any news about sharing the source code of your component!

[thedmd]

@francesco-cattoglio I just published all code related to Node Editor. You can find it here.

[francesco-cattoglio]

@thedmd ty so much! I'll keep you posted about any good or bad news that will show up in 2018!

[sherief]

It would be awesome if this could get merged into mainline. I'm trying to use @thedmd's awesome node editor, but its current code is built against v1.50 and my code base is all v1.60.

I'm hoping to clean up the node editor's code, have it compile against 1.60 (once this is merged), and submit a pull request to the node editor's repository. Having these layout options wouldn't hurt either.

@ocornut, any chance you might be able to look at this soon?

[ocornut]

@sherief Unfortunately I don't think this would realistically get merged soon, I have too much open topics on my plate and need to close the Nav then Viewport/Docking features before I embark into layout stuff.

I am however going to try replacing the 3 IndentX, GroupOffsetX, ColumnsOffsetX with ImVec2 (without using the .y coordinates at all) so ease with merging this code (those are the main reasons for conflicts afaik). I consider those 3 variables a mess at the moment, I don't mind adding 3 unused float per-window to facilitate this work. EDIT I tried that and it didn't look like it made much of a difference in term of the patch.

[ocornut]

Here's a quick attempt at merging this with current master (patch from master) Stack Layout implementation (merge).zip

It looks like there are some spacing bugs, I don't know what caused them. Maybe thedmd may want to look at it.

[sherief]

Totally understandable. Nav, viewports, and docking are also incredibly useful for everyone. I'll take a look at the merge attempt later today and try to update @thedmd's code to work against it - basically, I hope to keep the node editor in a state where it doesn't stray too far from the mainline imgui branch so that it's easier to use once you finally get around to merging the layout changes.

Thanks a ton!

[thedmd]

Bounds measurement code relied heavily on internal positioning code and cursor placement, if I recall correctly there were changes and unfortunately errors in spacing are expected.

I will look into that, since this is one of the features used in node editor. thedmd/imgui-node-editor#3

[thedmd]

Summary:

• Based on latest ImGui 1.61 (thanks for patch Omar!)
• Eliminated one-frame delay, results are immediate
• Eliminated flickering or jiggling

Scroll down for details.

(Spring drawing is debug mechanism and is not 100% accurate, has one frame delay).

I decided to not use BeginGroup()/EndGroup() to measure items, because they touched too many states and I'm still not comfortable with a way ImGui handle cursor movement. This time I'm using Indent() to hold layout elements in line and record cursor position inside layout internal data. Turns out it is easier to reason about that. Code dealing with composite layouts (layouts in layouts) was rewritten to eliminate need for multiple passes before everything stabilize. With current approach everything is in proper place after call to EndHorizontal()/EndVertical(). I started using references in internal functions to enforce correct usage. Is that ok Omar or should I change them back to pointers?

Layout items now record ImDrawList vertex buffer write index. This way I'm able to move drawn widgets to proper places and give immediate result. Logic code is mismatched with visual state for one frame. I think this is easier to swallow than errors in visual state. Anyway, layout is trying to reuse data from previous frame to handle spacing and place cursor for user to draw. If user drawn element change size layout can fixup widget geometry by translating it to proper position.

Example from imgui_demo.cpp was not ported back. My intent is to bring one you see above eventually.

[thedmd]

Rebased to resolve conflict with master branch.

[ocornut]

Hello @thedmd,

Today I was checking all the PR ahead of working on the slight-but-PR-breaking refactor discussed in #2036. Here's a few things to chew on:

---

(1)

I merged your branch as a test (there were 2 minor/obvious conflicts)

One noticeable issue are missing menus:

Due to this test in ItemSize():

if (!window->DC.CurrentLayout || window->DC.CurrentLayout->Type == ImGuiLayoutType_Vertical)

Because various piece of code are modifying window->DC.LayoutType in particular to set it to ImGuiLayout_Horizontal, but in this case the vertical path is taken. As a test I tried to make PushLayout()/PopLayout() set window->DC.LayoutType accordingly, which would be an impartial fix as the last PopLayout() would need to choose/overwrite a value (possible to fix that later). With that partial fix this is how the menus looks like:

Something with CurrentLineTextBaseOffset being lost in your modification of ItemSize() - the code in master lazily calls SameLine() which restore CurrentLineTextBaseOffset (should be called ~Baseline). I haven't investigated later.

---

2)

So you don't totally hate me for leaving this dangling, I have made some changes (see f9634feb667a4f1355a8c045c610383ef2f69653) which are designed to reduce the amount of code of your PR in places which are more likely to conflicts later. I used a ImVec1 type in a few place to facilitate a 1D->2D transition, so some of the code would be identical with/without the PR, but without affecting current imgui behavior.

Before merging this I would suggest to tackle 1) and first solve the issue using commit 220e6a55b7afaa1012b0ada43723a78052f59947 which is the commit just before f9634feb667a4f1355a8c045c610383ef2f69653. f9634feb667a4f1355a8c045c610383ef2f69653 will likely conflict but in an easy-to-resolve way (your side of the code probably should be selected everywhere).

[thedmd]

O, hello @ocornut

I will jump on first issue to after rebasing to 220e6a5. Do you want me to stop there to review changes? Or should I then move to HEAD?

By the way, I like the direction you're pushing ImGui with viewports and #2036.

[thedmd]

Hello @ocornut. Branch is now rebased on 220e6a5. I made a few changes:

• IndentX, GroupOffsetX and ColumnOffsetX was restored to their original form. They were required by first version of layout implementation, this none no longer relies on BeginGroup()/EndGroup() calls.
• ItemSize learned to honor DC.LayoutType, call to SameLine() is no longer necessary

Issue with Menu is gone. Should I move it to HEAD?

I see one potential problem. Widgets that assume vertical layout may not work as expected inside BeginHorizontal()/EndHorizontal(). Label of InputXXX may be affected by that. I'm going to test this. Edit: And yet they're working.

[thedmd]

I pushed version based on current master. One rebase onto 220e6a5 can be found there layouts-220e6a5

[thedmd]

@ocornut Everything is in sync.

[ocornut]

Great to see the patch looks much easier to track now!

[thedmd]

In sync again.

@ocornut Do you have any general plan for layouts? I wonder if this PR can fit in better and, for example, do not introduce an API that will make generalization harder.

[thedmd]

And we are now on v1.67 WIP (9ba202821f6d5).

[thedmd]

Bump to v1.67

[ocornut]

For own and future reference here's a minimal pastable demo:

static bool spanToWindow = true;
static float middleWeight = 0.30f;
ImGui::Checkbox("spanToWindow", &spanToWindow);
ImGui::DragFloat("middleWeight", &middleWeight, 0.01f, 0.0f, 1.0f);

ImGui::BeginHorizontal("example_h1", spanToWindow ? ImGui::GetContentRegionAvail() : ImVec2(0, 0));
ImGui::TextUnformatted("Left");
ImGui::Spring(middleWeight);
ImGui::TextUnformatted("Middle");
ImGui::Spring(1.0f - middleWeight);
ImGui::TextUnformatted("Right");
ImGui::EndHorizontal();

[ocornut]

Thanks thedmd and glad the patch is looking easier to maintain. Labelling paragraphs for easier answers.

Do you have any general plan for layouts?

(A) I don't have clear plans at the moment, just too overwhelmed with tasks. But I've looking at this in the background. I've stepped through your code today to get a better understanding of it. I don't have an good enough intuition of the use cases or problems. Is the fancier demo code somewhere? The code you posted on May 1? Do you have examples to point out (even shots) other than your imgui-node-editor ?

We have other desirable layout features and API wise things probably needs to be designed around each others so it's hard to pull one feature independently of others, however sane this PR is. For example the three rows of Checkbox under "Window Options" is a common case of something we ought to solve (which Spring don't solve, and Columns should solve but they currently solve it awkwardly - TL;DR; Columns need to be redone as a more first class citizen, and then we can decide how/where we use new terminology, because even BeginHorizontal may apply to other totally different uses).

(B) The use of TranslateLayoutItem is pretty ingenious, if not a little scary because perf wise it wouldn't scale so well (but at least there's a typical upper limit on vertex count which is ~~ what is possible to make visible at once on screen).

Are there case where CPU side clipping would clip vertices data before TranslateLayoutItem() has a chance to run?

(C) In the (older) #746 discussion, any reason why you are using leading and trailer Spring(0.0) in this example?

ImGui::BeginHorizontal("header");
    ImGui::Spring(0.0f);
    ImGui::TextUnformatted("Do N");
    ImGui::Spring();
    ImGui::Button("X");
    ImGui::Spring(0.0f);
ImGui::EndHorizontal();

(D) In the small blurb above if I replace ImGui::TextUnformatted("Right") with ImGui::Button("Right button", ImVec2(-1, 0)); it breaks, which is perhaps fine, I've just haven't gotten my head around varying use cases.

(E) As per your use of Indent (which is makes total sense given the current codebase): one of the task I'd like to tackle is replace window->DC.Indent, window->DC.ColumnsOffset by a more generic system where "working region" rectangles can be pushed/poped easily, as well as cleaning up the ContentsRegion mess. The hard problem to solve is enforcing clipping of variety of primitives while limiting draw calls, but I'm starting to think it may be healthier to just ignore this problem and work on this internal reoverhaul without involving clipping at all. At least we'll have a more consistent way to provide and manipulating boundaries.

(F) Maybe the align = 1.0f param of the BeginXXX functions should be called e.g. other_axis_align ?

(G) Items with mismatching text baseline don't seem properly aligned in an horizontal layout if align != 0.5f In this situation even without a leading ImGui::AlignTextToFramePadding(); the Right text after button would use an offset baseline. May be ok to ignore this problem for now.

(H) Attached a patch with minor/shallow coding convention tweaks. patch.zip

(I) You might want to use ImPool as a helper if you want (it is being used in the Docking branch, but it's already merged in master since I've been using it in my testing app) to store all ImGuiLayout of a window in a contiguous buffer, but this makes ImGuiLayout pointers invalidate when growing so some code need reworks. (You can probably ignore this suggestion now. I'm just letting you know this facility exists!)

(J) I just made a small change were I have reordered the value of ImGuiLayoutType_Horizontal/ImGuiLayoutType_Vertical so they can be used as indexes. This will create a minor conflict as you have a structure just under it. But it will be helpful to simplify some code, as blurbs like:

 if (layout.Type == ImGuiLayoutType_Horizontal)
     spacing = style_spacing.x;
else
    spacing = style_spacing.y;

Can be turned into:

spacing = style_spacing[layout.Type]

Provided layout.Type is valid. You can probably shorten some code duplication using this. I've been using this in the Docking branch using ImGuiAxis, the enums are now explicitely documented as guaranting matching the axis order. I think the viewport/docking have a const [] operator in ImVec2 that's not in master I could merge that if needed.

Thanks again!

[thedmd]

(A) Are you consider this to be fancier demo? This is test of composite layout (vertical, horizontal and vertical) where middle element is build from one shown in next row. Aim was to provide a way to test composition with ability to change settings at run time. It can be even fancier, but this gave me good enough test case to bash all issues. There is also this: Simple test which allowed me to make sure I got basics right. Things like moving cursor for next widget are not always trivial. Both test have 'Enable Sleep' checkbox which suspend thread for some time after any changing settings. This way I made sure layouts are correct at the call to EndHorizonal/Vertical and there is no one-frame lag present in first implementation. This time every frame is perfect.

Stack layouts were not mend for cases like "Window Options". At least it is not what I had in mind. I needed simple way to align widgets without burying myself in constant manual recalculations and measurements while drawing a node. Widget positioning is constrained. Maybe it is possible to extend code to support overflow or grid layouts. I didn't think about that yet. Columns tale is a long standing one. As you plan making them first class citizen I thought about same thing for layouts after seeing ImGuiLayoutType already present in internal header. I thought of layouts governing cursor navigation inside window instead of manually moving it in ItemSize. Was that what you had in mind?

(B) TranslateLayoutItem is called only to fixup widget placement in layout new or modified layout. Also if it run, it does only when required. I don't think it will hog cpu too much. Yet, it depends from what user place inside layout.

I did not consider clip rectangles. Tracking them is more complicated than vertex data. I leaved them as they is mostly because in next frame user widget will be drawn in position determined by layout with correct clipping. As for cases like text box with software clipping, things are better because whole geometry is moved and user will not see any artifacts.

I will stop there and answer to remaining points in next post.

[ocornut]

(A)

Are you consider this to be fancier demo?

Yes, I was wondering where was that code?

Stack layouts were not mend for cases like "Window Options".

Yes, sorry if I wasn't clear. I know it wasn't meant to do that. All I'm saying is from the end-user point of view, we are probably going to have several layout-related utilities and so from an API stability point of view, I cannot expose something called "Layout" that does1/3 of what we may need and ignore the others. Maybe we'll only be able to design the final API(s) when we have a clearer view of all the features. So I'm treating this PR as one component of a larger whole and probably don't expect to merge until we know more about the whole.

Columns tale is a long standing one. As you plan making them first class citizen I thought about same thing for layouts after seeing ImGuiLayoutType already present in internal header.

Columns/Table are on the list of things I will tackle this year as Blizzard wants it so this will be moving forward.

I thought of layouts governing cursor navigation inside window instead of manually moving it in ItemSize. Was that what you had in mind?

I don't have a super clear view. I know the ItemAdd/ItemSize are currently limiting because the widgets are locking down the widget position before calling ItemXXX, whereas we need our layout helpers (whatever) they are to be able to move the cursor. So their signature will change and it will be a good occasion to redesign some of that stuff and give more opportunity to an underlying layout system to do something with the widget pos/size.

(Another smaller issue with their signature is that I want the widget to be able to clip render while still proceeding, so while ItemAdd currently returns a bool we may need to change that to flags.)

If you want to open other issues with thought on another other layouting feature feel free to do that!

(B)

Yet, it depends from what user place inside layout.

Depending how this is exposed there will be a temptation for users to wrap lots of things in layout, so it is possible that 50% of a window will need processing.

Currently I imagine child window won't be offset. (As part of the thing I discussed in (E) I think in the future we will be able to benefit from most of the things we currently use BeginChild() for (clipping + scrolling + scrollbar) without a child window, but as per legacy support.)

As for cases like text box with software clipping, things are better because whole geometry is moved and user will not see any artifacts.

My thought is that some widgets may be clipping their contents when creating the vertices, and if we call TranslateLayoutItem() with negative X position the content may already be gone. However I don't have enough of a good understanding of the whole thing to judge if this is a real problem or not.

[thedmd]

(A)

Yes, I was wondering where was that code?

It is here: layouts-basic.cpp If you want it in imgui_demp.cpp I can port it as part of this PR.

Yes, sorry if I wasn't clear. I know it wasn't meant to do that. All I'm saying is from the end-user point of view, we are probably going to have several layout-related utilities and so from an API stability point of view, I cannot expose something called "Layout" that does1/3 of what we may need and ignore the others. Maybe we'll only be able to design the final API(s) when we have a clearer view of all the features. So I'm treating this PR as one component of a larger whole and probably don't expect to merge until we know more about the whole.

That's the reason I'm calling they "Stack Layout". I understand.

(Another smaller issue with their signature is that I want the widget to be able to clip render while still proceeding, so while ItemAdd currently returns a bool we may need to change that to flags.)

Do you have in mind software clipping like in with text or actual clip rects in draw list?

(B)

Yet, it depends from what user place inside layout.

Depending how this is exposed there will be a temptation for users to wrap lots of things in layout, so it is possible that 50% of a window will need processing.

This is true. Also processing at this point is an addition which can be vectorized if necessary. That being said, I understand concerns, right now I think it will be better to delegate this issue to the future and see if it surfaces.

Currently I imagine child window won't be offset. (As part of the thing I discussed in (E) I think in the future we will be able to benefit from most of the things we currently use BeginChild() for (clipping + scrolling + scrollbar) without a child window, but as per legacy support.)

I learned that child windows are their own beasts. I decided to not take them into account in layouts because they cannot be easily moved and reliably tracking their content would lead to too much code for the benefit of not waiting to next frame until all measurements are sorted out.

I find them troubling also in node editor development, because they look like cutout in the fabric of the window. Editor used one but I wanted it to behave like regular widget so I made imguiex_canvas.h extension. It acts like regular widget in the context of the window and provide infinite canvas where origin point (and scale) can be set by user. And visible rect queried both in canvas space and window space. It does work with widget clipping. Canvas lack direct support for scrollbars, they can be introduced with new composite widget.

As for cases like text box with software clipping, things are better because whole geometry is moved and user will not see any artifacts.

My thought is that some widgets may be clipping their contents when creating the vertices, and if we call TranslateLayoutItem() with negative X position the content may already be gone. However I don't have enough of a good understanding of the whole thing to judge if this is a real problem or not.

For software clipping this isn't an issue because every vertex emitted by widget is moved. Glitch will appear if widget is dealing with draw list clip rects. In this case geometry will be moved but clip rectangle not. This effect will last one frame. Maybe this is a good idea to make example covering this case.

(C)

In the (older) #746 discussion, any reason why you are using leading and trailer Spring(0.0) in this example?

Non-stretchable spring at the beginning of the layout act as spacer. Layout does not have internal padding, FramePadding exist to pad widget content. Layout however is not a widget. Maybe introducing LayoutPadding will be more elegant approach? This is how it looks now. In first implementation, if I can recall correctly, only widgets between springs could be moved around, so they acted also as a slider hinge. I guess usage pattern slipped in.

(D)

In the small blurb above if I replace ImGui::TextUnformatted("Right") with ImGui::Button("Right button", ImVec2(-1, 0)); it breaks, which is perhaps fine, I've just haven't gotten my head around varying use cases.

Stretched button occupies available space moving 'X' outside of window. Layout is correct, but value of CursorMaxPos was incorrectly reverted preventing horizontal scrollbar from appearing (assuming horizontal scrollbar is enabled).

(E)

As per your use of Indent (which is makes total sense given the current codebase): one of the task I'd like to tackle is replace window->DC.Indent, window->DC.ColumnsOffset by a more generic system where "working region" rectangles can be pushed/poped easily, as well as cleaning up the ContentsRegion mess. The hard problem to solve is enforcing clipping of variety of primitives while limiting draw calls, but I'm starting to think it may be healthier to just ignore this problem and work on this internal reoverhaul without involving clipping at all. At least we'll have a more consistent way to provide and manipulating boundaries.

I found out using Indent is currently the best way to change new line horizontal position without breaking anything (mentioned columns included). I hope new column API will be built on some low level primitives that layouts can also utilize.

As for clipping I think current approach is healthy in terms of complexity. I mean whole widget is drawn if even a tiny bit of it is visible and clipping inside widget should be handled by widget itself.

For column API clipping could be added retroactively. I had to face similar problem with canvas. I decided to push new draw command every time user begin to draw. This leaves me an open way to merge top most draw commands with same clip rects into single draw call. This isn't done in canvas code, I think such thing can be handled while building draw data.

(F)

Maybe the align = 1.0f param of the BeginXXX functions should be called e.g. other_axis_align ?

Will 'BeginHorizontal' with 'vertical_align' be sufficient? Public API can be more explicit.

(G) Well layout position widgets according to their bounding rects. All cursor manipulations are overwritten. To keep texts in line I think BeginGroup() with ImGui::AlignTextToFramePadding(); should be used to make bigger item. Is there some DC state I should update inside layout to keep widgets in line?

(H)

Attached a patch with minor/shallow coding convention tweaks. Apparently I missed quite a bit in terms of keeping style consistent. Thanks for a patch. Today I learned. : )

(I) Thanks. I didn't knew about ImPool. Currently layouts use free list to avoid excessive allocations. I may switch to pool if necessary, this will introduce indirection while pushing and popping layouts but can be handled. Would you like to see this now or wait until layout usage put more weight on the issue?

(J)

I just made a small change were I have reordered the value of ImGuiLayoutType_Horizontal/ImGuiLayoutType_Vertical so they can be used as indexes. There are few places where code can be now simplified. One that are more complicated I think I leave as they are. Code handling horizontal and vertical layout sometimes require pulling different levers.

Thank you for feedback!

[thedmd]

Issue in (D) was fixed and code style patch applied (thanks!).

[ocornut]

Thanks for the link/code!

(A)

(Another smaller issue with their signature is that I want the widget to be able to clip render while still proceeding, so while ItemAdd currently returns a bool we may need to change that to flags.) Do you have in mind software clipping like in with text or actual clip rects in draw list?

In this situation I am talking about widget code not having to submit their draw stuff. Right now clipping disable both the logic and the drawing but there are situations where it will be beneficial to run logic without submitting the draw stuff. That's a minor aspect of the other stuff discussed in the paragraph, it was probably noisy of me to even mention this idea here.

Another feature I should investigate in the following month is the possibility of doing right-to-left or bottom-to-level raster layouting, which would be a simple extention of the current system and yet very useful. Implementing this will require revisiting the signature for ItemAdd/ItemSize. From there we hopefully will have a little more flexibility for layout extension and I'll keep this PR in mind (will probably show you what I do before merging anyway) so the new signature can be both optimal and useful in many situation.

(E)

I found out using Indent is currently the best way to change new line horizontal position without breaking anything (mentioned columns included). I hope new column API will be built on some low level primitives that layouts can also utilize.

Yes. I think the mechanism of altering the "working rect" can be leveraged here, we would just change e.g. min.x, it wouldn't necessarily affect draw call/clipping.

I will also look into adding a bounding box in ImDrawCmd. Now that I have a system for measuring CPU perf I can consider looking at this.

(F)

Will 'BeginHorizontal' with 'vertical_align' be sufficient? Public API can be more explicit.

Yes that sounds good.

(G)

Is there some DC state I should update inside layout to keep widgets in line?

It is DC.CurrentLineTextBaseOffset but this is currently a bit of a mess (there are some ambiguous/hardcoded situations).

(I)

Would you like to see this now or wait until layout usage put more weight on the issue?

This is very minor in the grand scheme of thing. Only use if useful, otherwise feel free to ignore.

Most of those topics are not really pertaining to the Stack Layout PR, more like discussing future ideas. I'll tag you when I have new work in place regarding to layout and the more we have the best we will be able to tweak public APIs.

[thedmd]

A) By doing right-to-left etc. layouts are you thinking about generalizing cursor movement? Maybe an internal API to make it explicit (wink)? :) I'm interested.

E) What purpose a bounding box will have in ImDrawCmd?

F) Will do in next iteration.

G) I will look at DC.CurrentLineTextBaseOffset and consider how keep correct value inside layouts. This stuff is tricky. So I think I will keep this as separate commit. Will do in next iteration.

[ocornut]

A) I'm not sure what "generalizing cursor movement" means. I want to make it possible to push working rectangles, and the ItemAdd/ItemSize needs rework so they handle layout and output widget pos/size.

E)

What purpose a bounding box will have in ImDrawCmd?

It will allow to merge ImDrawCmd. This is very important as we go toward making columns very common and more practical. In most cases columns draw calls can be flattened into the parent draw call.

[thedmd]

A) By generalizing I mean ability to choose where cursor should be placed next. Right now the only option is moving it next to top right corner of the item.

[seanpaultaylor]

Any chance you will be adding the changes for imgui_demo.cpp to the PR to showcase this feature working ? I am also more interested in getting this working in the docking branch too.

[thedmd]

I will try to put examples in demo next time I rebase this PR.

[ocornut]

(1) Here's the PR rebased over master as of today. stack_layout.zip

I agree it'd be useful to add the demo code in imgui_demo.cpp to allow quick sanity test.

(2) (For feedback/thoughts only) I pushed another unrelated work branch with folds Indent/ColumnsOffset/GroupOffset into a single WorkReck, which can be modified by various systems, columns modify it. https://github.com/ocornut/imgui/tree/features/work_rect2

This specific branch was carefully made to avoid making changes to columns behavior, if you find anything let me know!

If you rebase your PR over this you'll get a few minor conflicts. The sorts of things that needs to be patched are:

ItemSize(): window->DC.CursorPos.x = (float)(int)(window->Pos.x + window->DC.Indent.x + window->DC.ColumnsOffset.x); becomes window->DC.CursorPos.x = (float)(int)(window->WorkRect.Min.x);

(GroupOffset was already weirdly folded into Indent when used, all of that is thankfyully gone here)

I think you may be able to alter WorkRect instead of calling SignedIndent and/or modifying Indent/Unindent. Overally this should all a little simpler.

Please note that the initial value of WorkRect is the same as ContentsRegionRect which is notoriously WRONG (aka what not you expect) when scrolling is involved. This only makes a difference when using patterns such as BeginChild(..., ImVec2(-1,1)) to use remaining size. Either way this should not be a problem specific to the current PR.

(Attached the files of what I think is the correct rebase over this branch. It's not a patch, I guess I could/should I make the diff between post-conflict state and those files if needed) stack_layout_over_workrect2.zip

I'll be merging this branch soonish unless I can find an issue. Feedback. welcome.

[thedmd]

(Ad 1) I turns out I made a rebase by myself before looking here. I'm picking up your patch instead of mine, I appears you also made some cleanups my version didn't have. Thank you. I made an example: Can you let me know if I managed to keep code style consistent?

(Ad 2) Looking at diff client rect manipulation looks a loot cleaner now. Bdw. isn't ClientRect more accurate name? I will try to use WorkRect instead of indents. I guess I could also remove dummies used for vertical alignment. Will post feedback soon.

(3) I'm thinking about adding feedback function which return maximum available space widget can occupy in layout. This way user will be able to fill empty space with content. One issue with that is return value will have one frame lag. Still looks like better than nothing. For example:

ImVec2 GetLastLayoutItemAvailableSize()

What do you think?

[ocornut]

Thanks for the update! Will look in more details later. May be wrong but I don't think you need the change in Indent/Unindent any more?

(2) I thought ClientRect would imply covering the whole window, while in this case the idea would be that any layout code can freely modify the WorkRect to restrict layout to a certain region. How far we can enforce this clipping-wise is yet to be defined, but that's the idea. So they inside a column the WorkRect covers that column.

I found a bug in the pushed code which makes Trees (now added a "Columns->Trees" demo) layout fails as TreeNode/Indent modify WorkRect.Min which is restored when starting the next column/row. My fix is to preserve modification to WorkRect per-column, just like we'd like to preserve ItemWidth eventually. It's not a trivial one-liner fix because as soon as you start storing/caching anything, existing columns sync behavior gets broken, but I'll try push the whole thing fixed for Columns V1 before resuming work on Columns/Table V2.

(3) I am not sure I understand the purpose precisely, but it might simply be that your layout system sets WorkRect.Max ? And then that query will go through standardized requests. (Trying to kill then ressucitate GetContentsRegionXXX as GetWorkRectXXX which will be absolute coordinates and handle those things).

[thedmd]

I made a rebase on work_rect2 in case it is needed: https://github.com/thedmd/imgui/tree/feature/work_rect2-layouts

I replaced calls to indent with direct work rect manipulation and it seems to work. Code is same for horizontal and vertical axes.

(ad 2) ClientRect - thanks for clarification

With a bug, do you mean stack layout somehow influence tree demo? I'm looking at demo right now on work_rect2-layouts branch and it seems to look ok.

I tried to make make Begin/EndLayout call be indistinguishable from call to ImGui::Dummy() with appropriate size, by leaving internal state in same state in both cases. Did I missed something? By the way, maybe this is good material for unit test?

(ad 3) Manipulating WorkRect.Max sound like good idea. That said reality is a bit more complex, because of the amount of moving parts involved. There are X pixels available, if four widgets in layout grow by X that would be bad. So there should be a way to tell if widget region can be stretchy. Adding extra constrains grow complexity and eventually will lead to journey of discovering constrain solvers like randrew/layout or amoeba. I think I will leave API as it is.

[ocornut]

(2) for the bug I mean the workrect2 has a bug compared to master, where tree indents are lost within column. Not stacklayout’s fault more a fault of workrect+columns (i have the fix but it comes with other changes i don’t want to push right now).

The automation/test system is experimental but I can grant you private access if you are interested in testing your stuff (and maybe we’ll get good feedback). Right now the app itself until works on Windows but it’d be easy to fix.

[thedmd]

I would like to play with that. Just right now there are some corner cases lost to the history I had tested but not preserved.

[thedmd]

It is all green now.

[xpenatan]

Any news to this? this is a really a important/nice feature to have.

[thedmd]

Make it green for 1.74

[rokups]

I rebased your branch on to latest master. Noticed few issues.

Spring sticks out of the window when scrollbar is not visible.

When scrollbar is visible spring is rendered under the scrollbar.

Window does not clip springs.

Unsure about this one, but seems to me there should be no empty space where A spring is. Also widget C sticks out of the border.

P.S. Since rebasing was trivial i wont share my branch as i rebased on current master which isnt 1.75 release.

[ocornut]

Heads up I moved some code over those 6 commits https://github.com/ocornut/imgui/commits/8c683de33f96349dc8cfa83b3915521306e921eb This will probably conflicts, best way ihmo to solve:

• first rebase over 095dc996b0e2e6440c945cdbc56bf80357624ce8 (before the moving commits)
• then rebase over 8c683de33f96349dc8cfa83b3915521306e921eb (or each iteratively) and move old blocks to new location

[thedmd]

Thank you. I will look into that soon.

[thedmd]

Soon™...

It is done:

• I moved STACK LAYOUT section after LAYOUT section
• ~~With the exception of getting current layout, my changes in ItemSize() was dropped~~ And it is back to fix complex layout. Overshoot bug was fixed.
• PR is on top of brand new 1.76 release

Built, tested. Layouts seems to do fine as far as I can tell.

Thank you for a rebase advice, it was helpful.

[thedmd]

@rokups:

• In current rebase I dealt with layout overshoot issues, thanks for noticing
• Fixed clip rect, so sprints are not drawn outside of window. Originally that was debug code to check if they are not drawn in some weird places

[ocornut]

Thanks again @thedmd! If you find it helpful, you potentially could use the automation/testing framework to test some aspect of that feature (for possible future regressions etc).

[thedmd]

You're welcome.

Adding more test is good idea. It will save me need to juggle branches in order to test more complex layouts.

Also visual proof things are ok:

[thedmd]

@ocornut Can you point me to where test framework is? After checking all branches have I failed to find it.

[thedmd]

Rebased on master (c8cde28cf3fb5d1e86faa80edf9b2c48ccc2f8dc, Wed May 20 17:56:08 2020 +0200).

Also per advice I introduced new define indicating stack layouts are available: #define IMGUI_HAS_STACK_LAYOUT 1 // Stack-Layout PR #846

[frink]

Looks like it needs another rebase again...

Thanks @thedmd. I like this a lot. 👍 @ocornut Any chance of merging this "Soon™"? 😉

[ocornut]

@ocornut Any chance of merging this "Soon™"? 😉

Probably not Soon™, there's so many large unfinished things on the back-burner. But I'm happy to help facilitating merging/rebasing thing when possible.

[thedmd]

@frink It is now rebased.

[frink]

Love it! Thank you so much!!!

[SadE54]

Hi,

could it be rebased for current 'official version' 1.79 ?

[thedmd]

@SadE54 Yes it can. :) Well, it is rebased on master. Hope that's ok.

@ocornut I removed changes to Indent/Unindent and simplified stack check.

[ocornut]

Is it common to wait 4+ years for a PR to get merged?

Definitively not

[SadE54]

So it could be the good time to get merged :)

[tlf30]

I would love to see this merged in

[zeyangl]

Could please @ocornut merge this if a 2-pass layout overhaul is not going to happen? ImGui is seriously great and is getting better at faster speed, but something like this should be really in the base code it's not even funny. If you merge this I could show case an internal tile editor we developed that would not be possible without ImGui, and would be better with this pull request!

[thedmd]

Branch is rebased on 1.84 (3512f2c2c283ec8).

[thedmd]

Rebased on 1.85 WIP (2d0a6a4969b4da5eae0704e740e1ae0e11b946d9)

[mnesarco]

I had a dream: That this branch was merged into master and then master was merged into docking 🤩. It was a happy dream.

[mnesarco]

Hi @thedmd , this layout looks amazing. I wonder if is it possible to move it to an independent file pair imgui_layout(.cpp/.hpp) so it can be used with vanilla ImGui. What do you think?

[ocornut]

There are currently 1-2 things that requires insertion in core functions and structures (ItemSize, state storage, etc) my plans when refactoring ItemSize was to find a way to make this sort of feature completely possible as an external extension. Needless to say many people are interested in stuff like this pr or right-to-left layout or other layout features, and many of them could be enabled by those refactors (i included this PR in my checklist of things that would be doable).

That said it may be interesting to move everything that is possible to move in a imgui_stacklayout pair of file, if only because it’ll make it very clear what changes are left in main files and therefore what we need to find a solution for.

[thedmd]

That said it may be interesting to move everything that is possible to move in a imgui_stacklayout pair of file, if only because it’ll make it very clear what changes are left in main files and therefore what we need to find a solution for.

I can do that.

[mnesarco]

I can do that.

Hi @thedmd have you had time to work on this? It would be great to have this feature available as an extension.

[thedmd]

@mnesarco I would like to make this an extension too. Before that happens, it is green again.

[thedmd]

There is an experimental branch with Stack Layouts moved to external files: feature/layout-external

Changes to ImGui:

• imgui.h - define IMGUI_HAS_STACK_LAYOUT=1
• imgui.h - #include "imgui_stacklayout.h"
• imgui.h/imgui.cpp - ImGuiStyle::LayoutAlign, style does not support extensions, new field need to be added
• imgui.cpp - ImGui::ItemSize calls internal API that returns current layout type
• imgui.cpp - ImGui::ItemSize implement horizontal layout support (previously vertical only + SameLine())
• imgui.cpp - ImGui::ItemAdd calls internal API that reports widget size
• imgui_internal.h - # include "imgui_stacklayout_internal.h"
• imgui_demo.cpp - add Stack Layout demo
• new: imgui_stacklayout.h - public API (BeginHorizontal, etc.)
• new: imgui_stacklayout_internal.h - internal API for ImGui::ItemSize and ImGui::ItemAdd
• new: imgui_stacklayout.cpp - all code and heavy lifting

This is a diff: https://github.com/ocornut/imgui/compare/master...thedmd:feature/layout-external?expand=1

@mnesarco - This is step forward, some work still need to be done @ocornut - Hope it will be easier to grasp and highlight places that need to be looked at in order to make stack layouts standalone extension

[ocornut]

Hope it will be easier to grasp and highlight places that need to be looked at in order to make stack layouts standalone extension

This is really good! Thank you again. Gives us visibility on what we can do on core's side. It also probably reassure current users this is an easy merge (in the meanwhile).

[aiekick]

for thoses who are interested i just merged the StackLayout Implementation In External to Imgui by following the explains of @thedmd https://github.com/aiekick/imgui/tree/docking_layout_external

[thedmd]

Rebased.

master r18718:

• layouts
• layout-external

docking r18718:

• docking-layout
• docking-layout-external

Thanks @aiekick. I added external layout for docking variant to the menagerie.

[aiekick]

Hello,

i have an issue with the docking_layout_external branch (i have the same with docking_layout)

i have a generic widget who si using two iother widget separeted by a ImGui::SameLine()

this widget can be used in any place and also in a MenuBar.

on normal ImGui branch, the ImGui::SameLine() have no visible effect to me

but with the docking_alyout, the second widget seem to be returned to next line i guess.

see this simple code :

ImGui::Begin("Hello, world!", nullptr, ImGuiWindowFlags_MenuBar);

if (ImGui::BeginMenuBar())
{
	ImGui::Button("AAA");
	ImGui::SameLine();
	ImGui::Button("BBB");
    
	ImGui::EndMenuBar();
}

ImGui::Button("AAA");
ImGui::SameLine();
ImGui::Button("BBB");

ImGui::End();

produce this result with normal imgui docking branch

but produce this result with docking_layout or docking_layout_external branchs

if i remove the ImGui::SameLine(); in the MenuBar section

ImGui::Begin("Hello, world!", nullptr, ImGuiWindowFlags_MenuBar);

if (ImGui::BeginMenuBar())
{
	ImGui::Button("AAA");
	//ImGui::SameLine();
	ImGui::Button("BBB");
    
	ImGui::EndMenuBar();
}

ImGui::Button("AAA");
ImGui::SameLine();
ImGui::Button("BBB");

ImGui::End();

its working again :

voila, not sure what is causing this issue :)

i fixed it in a naive and temporary way by just check if in MenuBar before calling ImGui::SameLine()

ImGuiWindow* window = ImGui::GetCurrentWindow();
if (!(window->Flags & ImGuiWindowFlags_MenuBar))
    ImGui::SameLine()

[thedmd]

This may be some unforseen consequences of touching layout code. I will investigate, thanks for the report. 🙂

[soufianekhiat]

Hi,

I found out a bug on feature/docking-layout-external:

Reprostep:

if ( ImGui::Begin( "DockWin" ) )
{
    ImGui::DockSpace( ImGui::GetID( "Dockspace" ) );

    if ( ImGui::Begin( "A" ) )
    {
        ImGui::End();
    }

    if ( ImGui::Begin( "B" ) )
    {
        ImGui::End();
    }

    ImGui::End();
}

And drag A on top of B to dock as a single window with tab:

Callstack: ImGui::ErrorCheckEndFrameSanityChecks() Line 8660 C++ ImGui::EndFrame() Line 5017 C++ ImGui::Render() Line 5105 C++

Here: IM_ASSERT_USER_ERROR(g.CurrentWindowStack.Size == 1, "Mismatched Begin/BeginChild vs End/EndChild calls: did you forget to call End/EndChild?");

[rokups]

You must call End() no matter what Begin() returns. This is inconsistent with another API and there are plans to fix it, but it is a big breaking change so it did not happen yet.

On June 16, 2022 00:01:09 SoufianeKHIAT @.***> wrote:

Hi, I found out a bug on feature/docking-layout-external: Reprostep: if ( ImGui::Begin( "DockWin" ) ) { ImGui::DockSpace( ImGui::GetID( "Dockspace" ) ); if ( ImGui::Begin( "A" ) ) { ImGui::End(); } if ( ImGui::Begin( "B" ) ) { ImGui::End(); } ImGui::End(); }

And drag A on top of B to dock as a single window with tab: Callstack: ImGui::ErrorCheckEndFrameSanityChecks() Line 8660 C++ ImGui::EndFrame() Line 5017 C++ ImGui::Render() Line 5105 C++Here: IM_ASSERT_USER_ERROR(g.CurrentWindowStack.Size == 1, "Mismatched Begin/BeginChild vs End/EndChild calls: did you forget to call End/EndChild?"); — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>