Stack Layout implementation #746
Conversation
| @@ -381,6 +411,10 @@ struct ImGuiContext | |||
| ImVector<ImFont*> FontStack; // Stack for PushFont()/PopFont() | |||
| ImVector<ImGuiPopupRef> OpenPopupStack; // Which popups are open (persistent) | |||
| ImVector<ImGuiPopupRef> CurrentPopupStack; // Which level of BeginPopup() we are in (reset every frame) | |||
| ImGuiLayout* CurrentLayout; | |||
| ImVector<ImGuiLayout*> LayoutStack; | |||
| ImVector<ImGuiLayout*> HorizontalLayouts; | |||
There was a problem hiding this comment.
Actually split to Horizontal and Vertical layouts is not necessary. These can be held in one vector.
|
Thanks Michał, this looks very interesting (and it isn't often that we receive non-trivial PR here). Looking forward to look into it as it seems very cool! Might merge some small parts early (changing some floats to ImVec2) if only to make this PR not break as easily with future changes. (As a minor niggle I would appreciate if you used xxxx_xxxx casing for local variables.) |
|
No rush. I will really appreciate your comments on this code. I have hard time to grasp how ImGui handle cursor positioning. I have areas of code which should move it vertically. Undo last padding without using style (as style may change). These parts of code may need more attention. I don't want to break stuff I do not touch. Demo appears to work. 'ItemSize()' and 'SameLine()' are hard to understand. Every time I think I got it, there is one more variable I have trouble to grasp. I will be more than happy to see simplified logic which hide quirks of managing cursor position. For example I will keep updating code. I already found some bugs when combining multiple layouts and I'm bashing them. And sure, I will change format of local variables. Bdw. ImGui is quite nice library. Thank you for that. |
|
Your patch is great if only because it may force me to look at those areas in details (not many people have ventured there after from me), and it probably needs a little cleanup. One of the confusing thing is the FYI I have this local patch to fix SameLine(X) where X is a >0 in the context of the context of a group. That may tie to your changes somehow. |
|
Thanks for a patch. If it fixes a problem with
// drawRect() and fillItemBounds() are at very end of this comment
// if you want to copy&paste this example
drawRect(600, 40, ImColor(255, 0, 0, 128), 2); // 2 is by how many pixel rect should be expanded
ImGui::BeginGroup();
ImGui::Dummy(ImVec2(200, 40));
fillItemBounds(ImColor(255, 128, 128)); // red rect
ImGui::SameLine(0, 25);
ImGui::SameLine(0, 25);
ImGui::Dummy(ImVec2(350, 40));
fillItemBounds(ImColor(128, 255, 128)); // green rect
ImGui::EndGroup();
// Vertical
drawRect(40, 400, ImColor(255, 0, 0, 128), 2);
ImGui::BeginGroup();
ImGui::Dummy(ImVec2(40, 100));
ImGui::SameColumn(0, 25);
ImGui::SameColumn(0, 25);
fillItemBounds(ImColor(255, 128, 128));
ImGui::Dummy(ImVec2(40, 250));
fillItemBounds(ImColor(128, 255, 128));
ImGui::EndGroup();Mentionend utility functions. Make sure you include auto fillItemBounds = [](ImColor color, float expand = 0.0f)
{
ImGui::GetWindowDrawList()->AddRectFilled(
ImGui::GetItemRectMin() - ImVec2(expand, expand),
ImGui::GetItemRectMax() + ImVec2(expand, expand),
color);
};
auto drawRect = [](float w, float h, ImColor color, float expand = 0.0f)
{
ImGui::GetWindowDrawList()->AddRect(
ImGui::GetCursorScreenPos() - ImVec2(expand, expand),
ImGui::GetCursorScreenPos() + ImVec2(w + expand, h + expand),
color);
};To avoid question being lost. Does SameLine()/SameColumn() should stack if first argument is zero? |
That's undefined doc-wise but to me it was never question of it "stacking". SameLine() is a sort of "undo newline + move X cursor" so by this definition it shouldn't really add up? |
|
I think I understand what it does. To enforce stacking I can always add dummy widget without size after calling SameLine(). Thanks for pointing me that out. |
|
I think layouts are now done. Sorry for early rebase. I rashed that out, please do not mind that. What this PR now represent is complete implementation of horizontal and vertical layouts. How it works. Layout control position of all elements it contains. Elements are:
Let's use horizontal layout as an example. Minimum possible width is sum of widths of all items and sizes of all springs. Minimum height is maximum height of all items. Layout is always trying to generate smallest possible result. However user may provide larger rectangle where layout should fit. This extra free space will be used by springs and distributed equally based on their weighs. If weights of all springs are equal, spaces between items will also be equal. Items inside layout may have different height. By default they are centered vertically on available space (as on image). Alignment can be controlled using Springs are special for layouts. They have weight which control how much of free space will be assigned to them. Also they allow to override spacing. By default Layouts can stack and automatically use free space from parent layout if any is present. Layouts are calculated once at the end of frame if any meaningful change is detected. Most of the time values stay as calculated on first pass. Nested layouts may some times require additional pass. Maximum number of frames required to calculate all layouts is equal to number of nested layouts. Changing of alignment inside layout does not require recalculation.
I think I bashed out all bugs. I tried to not touch original ImGui code when possible. Also I fixed naming of local variables. : ) I hope you will find time for feedback. Bonus: All this work was done to enable me to create node editor which is not only easy to use but also easy to craft from code. For example node visible in image is generated using this code: ImGui::BeginVertical("node");
ImGui::BeginHorizontal("header");
ImGui::Spring(0.0f);
ImGui::TextUnformatted("Do N");
ImGui::Spring();
ImGui::Button("X");
ImGui::Spring(0.0f);
ImGui::EndHorizontal();
drawItemRect(ImColor(0, 0, 0));
ImGui::Spring(0,4);
ImGui::BeginHorizontal("content");
ImGui::Spring(0, 4);
ImGui::PushStyleVar(ImGuiStyleVar_LayoutAlign, 0.0f);
ImGui::BeginVertical("inputs");
ImGui::Button("X"); ImGui::SameLine(); ImGui::TextUnformatted("Enter");
ImGui::Spring(0.0f);
ImGui::Button("X"); ImGui::SameLine(); ImGui::TextUnformatted("N");
ImGui::Spring(0.0f);
ImGui::Button("X"); ImGui::SameLine(); ImGui::TextUnformatted("Reset");
ImGui::Spring(1.0f, 0.0f);
ImGui::EndVertical();
fillItemRect(ImColor(0, 255, 0, 128));
ImGui::PopStyleVar();
ImGui::Spring();
ImGui::PushStyleVar(ImGuiStyleVar_LayoutAlign, 1.0f);
ImGui::BeginVertical("outputs");
ImGui::TextUnformatted("Exit"); ImGui::SameLine(); ImGui::Button("X");
ImGui::Spring(0.0f);
ImGui::TextUnformatted("Counter"); ImGui::SameLine(); ImGui::Button("X");
ImGui::Spring(1.0f);
ImGui::EndVertical();
fillItemRect(ImColor(0, 255, 0, 128));
ImGui::PopStyleVar();
ImGui::Spring(0, 4);
ImGui::EndHorizontal();
ImGui::EndVertical();
drawItemRect(ImColor(0, 0, 0), 2); |
|
Most complex layouts I have are now working as expected. Finally. |
|
Changes:
|
|
@ocornut What is your opinion about SuspendLayout() / ResumeLayout() API? |
|
I'm sorry I probably won't be able to look at this seriously until a long time might be several months. My mind is totally elsewhere, too many open tasks, and I can't judge this in an hour as it is a serious matter, I'd need to seriously make use of it. I will try to make a quick first evaluation sooner if only so that we can have more of a conversation here. I'm very interested in this and every of your comment/thought as those posted above important so don't hesitate to keep posting here. (this is also why I'm tempted to merge some of the float>ImVec2 to keep it up to date more easily and keep you around :). My concern from just skimming briefly at the code is that there may be a meaningful cost right now, it is quite heavier than what ImGui typically do. So it can't be added if we imply that the default user behavior should be to use those functions around everything. Nevertheless it is definitively very useful so we should strive to a) optimize it and b) make it clear what the cost are. Quick bits:
|
|
I understand you're busy. No rush. Layout is recalculated when size of item inside change or it's bounds change. After that it is sitting idle doing nothing.
If you look at BeginLayout()/EndLayout() and BeginLayoutItem()/EndLayoutItem() you will notice groups are used. Layout is build on top of groups. They are handy to capture size of widgets.
Groups are useful and provide what such simple layout need to work, bounding rectangles. If its purpose is defied by layouts any other primitive which allow measurements will suffice.
Good idea. I will do this.
There is no particular reason why these vectors are in the context itself. I had to put them somewhere. Actually moving them to window (together with g.CurrentLayout) can work better and prevent stealing layout when creating child windows. Should I move them to window itself or DC?
Layouts are keeping local item arrays to preserve more state between frames. If I use ten layouts one after another and add new item in first one all ten will be invalidated and need recalculation. When keeping items locally only first one need to be rebuild, remaining nine will not change. (Note that change of position of layout does not invalidate it, only change of size). Place for fixed number of items may be allocated together with ImGuiLayout structure and dynamic allocation will kick in only for more complex usages. That would be possible with buffer for ImVector you mention earlier. It isn't obvious in for what usage code should be optimized. Allocations are performed on first pass, then memory is reused. I should add means of removing cached layouts or marking is as free for reuse. Maybe we should see how layouts behave in real code and optimize them based on profiling later? I mean optimizations which are biased to some kind of usage. Summary of to do:
|
The overhead is probably not layout calculation but adding items to arrays, touching memory, etc.
The misnamed DC is meant to store things that in theory could be ditched if the window isn't alive. In practice it isn't well organized, but you could store it in DC yes.
The idea is mostly that touching more random memory (most commonly when chasing heap pointers) generally leads to lower perfs than touching contiguous memory that could fit in same cache line. As for keeping the number of heap allocations down is it mostly keeping a nice hygiene and a nice image but doesn't matter as much as the actual memory usage patterns. Current implementation of |
|
I got the idea. More local data is, faster code may run. I will think about that and how to minimize number of allocations further. Ad for When you talk about introducing local buffer to ImVector is something like small string optimization in std::string? Have place for a few elements and allocate if capacity limit is reached? |
Yes exactly. Most of the vectors are used for stacks for which we can give an optimal static size that is unlikely to be reach, and only use heap on overflow. Most of those stacks will rarely ever have more than say, 4 values. |
|
About my patch above: So SameLine() with non-zero value could add both GroupOffsetX and ColumnsOffsetX: But at this point is it becoming clear we have a problem where too much is hard-coded, and columns are ignoring group offset, etc. So we could clean that out and store our offset (better: min-max ranges?) within a stack with a single semantic. (Sorry this is not directly related to your commit, but it will probably conflict a little once fixed) |
|
I have pushed the half-assed fix in the meantime because it gets us much closer to intended behavior. If anyone was mislead thinking the X value was always window-relative (unlikely to be a problem, but it could) it is better to correct it sooner than later. Full cleanup to make group/columns simpler and recursive will have to wait a little. |
|
I rebased source to master. First commit was cleaned up and still vectorize few variables like Indent. Rest remain unchanged. |
|
This can now be done thanks to using layouts and #768. Enjoy! Nodes are relaying heavily on layouts. Notice that links (bezier curves) can be selected too.
|
…them. This change is an bootstrap for working with layouts as we need to make cursor move horizontally and vertically. Keeping only X coordinate prevent correct measurements.
…cal layout API with springs and inner item alignment.
Add loop friendly API to begin layouts.
…gn may be override for single layout scope. Alignment is not inherited by nested layouts. Add SuspendLayout()/ResumeLayout() to restore original ImGui positioning inside layout. This make it possible to create a regular group or display more complicated controls like Combo.
… applied until layout tree became stable for first time. Without that weird visual artifact may be spoted when layout aligned to center grow to full size and actually overshot parent layout for one frame. There is no legitimate way in ImGui to move widgets after they are drawn, what is actually required to fix this issue for good. With 'IsStable' at least initial experience isn't weird.
|
Wow is that node graph/patch panel example code available somewhere? I would love to try it out! |
|
PR was recreated due to lost references after changing fork origin. That was not my intention, sorry. |
|
@thedmd Do you implement The implementation of Unity3D's IMGUI uses two frames: one for building the stack, another one for calculating positions and sizes. And I'm wondering if that can be achieved in only one frame. |
|
Short answer is no, I did not. Initially springs lengths are zero and they are calculated at the end of the frame to be used in next one. Non trivial layouts cannot be done in one pass as far as I know. I see a possibility now to handle to correct widgets placement on screen in very same frame. Logic will follow in next one. This require adding transformation stack to ImDrawList. However, tempering with draw list can introduce problems for custom draw calls and draws that depends on absolute positions on screen (I couldn't think of any in ImGui). I did not used this for layouts yet. |
|
@thedmd Is it possible to remove the first id paramter from |
|
Yes, and no. Yes, it is possible but API will be very easy to use incorrectly. Actually it is necessary to make unique ID only for top level layout, all nested layouts will be isolated because ID acts like scope in C++. From the top of my head, you can define macros: # define BeginHorizontal2() ImGui::BeginHorizontal(#__LINE__)
# define BeginVertical2() ImGui::BeginHorizontal(#__LINE__)And use In reality you need unique ID only for top level layout, as I stated before. Like: ImGui::BeginVertical(&m_MyItem);
ImGui::BeginHorizontal("header");
ImGui::Text("+ %s +", m_MyItem.Name);
ImGui::Spring(1.0f);
ImGui::EndHorizontal();
ImGui::BeginHorizontal("content");
ImGui::Spring(1.0f);
ImGui::Text("%s", m_MyItem.Value);
ImGui::Spring(1.0f);
ImGui::EndHorizontal();
ImGui::EndVertical();You can also use constant with top level and write |
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.
I tried to bend ImGui to help me layout widgets easly. This is what API I came up with:
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.