Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stack Layout implementation #846

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Stack Layout implementation #846

wants to merge 1 commit into from

Conversation

thedmd
Copy link
Contributor

@thedmd thedmd commented Sep 26, 2016

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();

stack_layout

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
Copy link

Looks good. Will not be implemented?

@ocornut
Copy link
Owner

ocornut commented Jan 9, 2017

@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
Copy link

mgerhardy commented Mar 7, 2017

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

@sgf
Copy link

sgf commented Jun 21, 2017

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
Copy link

@thedmd Any chance you could share the basic node graph code you used for the screenshot you posted on issue 746?
#746

@thedmd
Copy link
Contributor Author

thedmd commented Jul 15, 2017

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

@b1tc0der
Copy link

@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
Copy link

@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
Copy link
Contributor Author

thedmd commented Dec 10, 2017

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

@francesco-cattoglio
Copy link

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

@sherief
Copy link

sherief commented Mar 5, 2018

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
Copy link
Owner

ocornut commented Mar 5, 2018

@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
Copy link
Owner

ocornut commented Mar 5, 2018

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
Copy link

sherief commented Mar 6, 2018

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
Copy link
Contributor Author

thedmd commented Mar 6, 2018

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
Copy link
Contributor Author

thedmd commented May 1, 2018

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).
imgui-layout-2 0

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
Copy link
Contributor Author

thedmd commented May 9, 2018

Rebased to resolve conflict with master branch.

@thedmd thedmd force-pushed the layouts branch 2 times, most recently from 2ad726b to 437adbe Compare May 17, 2018 20:08
@thedmd
Copy link
Contributor Author

thedmd commented Jul 5, 2024

@giovancris
It is doable thanks to ImDrawListSplitter. It is a draft and I will look into how to integrate change without impacting performance.
image

@pthom
Thanks for PR! I will review & integrate it after rebase.

If you don't mind I'm going to borrow your idea of checking ID's. : )

@pthom
Copy link
Contributor

pthom commented Jul 5, 2024

If you don't mind I'm going to borrow your idea of checking ID's. : )

Please do :-)

Note: if you do use it, there is an additional related commit that is needed:
pthom@97622b0

(TempInputScalar and TempInputText are the only places that do reuse the same Id)

@thedmd
Copy link
Contributor Author

thedmd commented Jul 6, 2024

1.91.0 WIP is in the works. PR is rebased on it which is very close 1.90.9 release.

Changes:

  • layout now does clip content (as requested by @giovancris)
  • calling layout with same ID twice in same frame trigger an assertion (inspired by @pthom changes)

master 1.91.0 WIP r19093:

docking ImGui 1.91.0 WIP r19093:

Notice: layouts and docking-layout will be dropped in favor of layout-external and docking-layout-external respectively.
This will require users to explicitly include imgui_stacklayout.cpp in their project.

@pthom Turns out, checking for missuses took one line sice all information is already there. :)

@pthom
Copy link
Contributor

pthom commented Jul 9, 2024

@thedmd :

Many thanks for your update!

However, I'm afraid it has a little issue as far as clipping is concerned, when mixed with your node editor and springs.

Let me explain:

In the example below you will see

  • a node from imgui-node-editor
  • This node contains a vertical layout, which contains two horizontal layouts.
  • Inside each horizontal there is ImGui::Text("Hello"), followed by ImGui::Spring(), followed by ImGui::Text("World")

With the previous version, I had this rendering which was okay:
image

And with the new version, I will have this output:

image

i.e. the part after the spring is clipped out in the first horizontal layout.

Simple repro code (it uses ImGui Bundle, but this can be reproduced elsewhere): you just need to call Gui()

#include "imgui.h"
#include "immapp/immapp.h"
#include "imgui-node-editor/imgui_node_editor.h"

namespace ed = ax::NodeEditor;


void GuiNode()
{
    // Basically a vertical layout with 2 inner horizontal layouts that contain text elements + a spring element
    ImGui::BeginVertical("V");
    {
        ImGui::BeginHorizontal("H");
        {
            ImGui::Text("Hello");
            ImGui::Spring();
            ImGui::Text("world");  // With the new version, this is clipped out!
        }
        ImGui::EndHorizontal();

        ImGui::BeginHorizontal("H2");
        {
            ImGui::Text("Hello");
            ImGui::Spring();
            ImGui::Text("world");
            ImGui::Text("And again");
        }
        ImGui::EndHorizontal();
    }
    ImGui::EndVertical();
}


void Gui()
{
    // A simple node editor with a single node
    ed::Begin("Node editor");
    ed::BeginNode(ed::NodeId(1));
    {
        GuiNode();
    }
    ed::EndNode();
    ed::End();
}


int main(int, char**)
{
    // Run the application using ImmApp (from ImGui Bundle)
    // This is simply a way to quickly setup an application with ImGui + Node Editor
    HelloImGui::RunnerParams runnerParams;
    runnerParams.callbacks.ShowGui = Gui;
    ImmApp::AddOnsParams addOnsParams;
    addOnsParams.withNodeEditor = true;
    ImmApp::Run(runnerParams, addOnsParams);
    return 0;
}

I will temporarily revert to the old version.

I'm afraid I cannot help efficiently on the correction, because I did not study the stacklayout code sufficiently. I hope the above repro will be enough for you to study it when you have time.

Cheers

pthom pushed a commit to pthom/imgui that referenced this pull request Jul 9, 2024
See ocornut#846 (comment) :

clipping has an issue, when mixed with your node editor and springs.
pthom added a commit to pthom/imgui_bundle that referenced this pull request Jul 9, 2024
@thedmd
Copy link
Contributor Author

thedmd commented Jul 21, 2024

Rebased on 19097, which does include Omar's multi-selection work (awesome!).

Changes:

  • layout clipping is now fixed thanks to @pthom report

master 1.91.0 WIP r19097:

docking ImGui 1.91.0 WIP r19097:

Notice: layouts and docking-layout will be dropped in favor of layout-external and docking-layout-external respectively.
This will require users to explicitly include imgui_stacklayout.cpp in their project.

@wkster
Copy link

wkster commented Aug 13, 2024

Hi @thedmd

Thanks your contribution. I'm encountering an issue with layouts where a button doesn't appear when I have nested BeginHorizontal calls within the same layout. Here’s a simplified version of the code I'm working with:

void Inspector::showDemo() {
    static float x, y, z;
    static const float FIXED_SIZE = 28;
    static const float HEIGHT = 28;
    auto width = ImGui::GetContentRegionAvail().x;
    auto column_width = (width - FIXED_SIZE) / 2;

    ImGui::BeginHorizontal("##showDemoh1", ImVec2(width, HEIGHT));
        ImGui::BeginHorizontal("##showDemoh2", ImVec2(column_width, HEIGHT), 0);
            ImGui::Text("Scale");
            ImGui::Spring();
            ImGui::Button("ICO");
        ImGui::EndHorizontal();

        // ImGui::BeginHorizontal("##showDemoh3", ImVec2(column_width, HEIGHT), 1);
        //   ImGui::Text("X");
        //   ImGui::InputFloat("##X", &x);
        //   ImGui::Text("Y");
        //   ImGui::InputFloat("##Y", &y);
        //   ImGui::Text("Z");
        //   ImGui::InputFloat("##Z", &z);
        // ImGui::EndHorizontal();

        ImGui::Button("RESET");
        ImGui::Spring();
    ImGui::EndHorizontal();
}

When these nested blocks are commented out, the button appears as expected. Do you have any insights or suggestions on how to resolve this issue?

Thank you for your time and assistance!

@wyxather
Copy link

wyxather commented Sep 6, 2024

hey i've been playing around with this PR, is there any reason why you use PushID instead of PushOverrideID in imgui_stacklayout.cpp->ImGui::BeginLayout?

using ImGui ID Stack Tool it shows this example below, since the generated ID get pass again to window->GetID(...) (first on BeginHorizontal, second on BeginLayout PushID)

Seed PushID Result
0x00000000 "Window" 0xDEADBEEF
0xDEADBEEF 0912345612 0xDEADCODE
0xDEADCODE "Item" 0xFFFF0000

while changing it to PushOverrideID, it shows the correct str_id given to BeginHorizontal, eg

Seed PushID Result
0x00000000 "Window" 0xDEADBEEF
0xDEADBEEF "Horizontal" 0xDEADCODE
0xDEADCODE "Item" 0xFFFF0000

its not much of an issue i guess, but even then we technically reduce a call to ImHashData.

@altschuler
Copy link

Hi @thedmd

Thank you for creating this! It's very, very useful for taking imgui beyond simple layout without jumping hoops every time :)

I was wondering whether there's a way to utilize the space that a spring would otherwise use, or said another way; get the size that a spring would have, to replace the area with an item. I might be missing something basic, but given this simple horizontal layout:

| [Spring] [Item] |

Is there some way to replace the spring area with an item? If there was an alternative to ImGui::Spring that returned the size of the spring and did not move the cursor that might be enough, although the user would have to be careful to exactly fill the entire area so the cursor ends up in the correct position again.

Thanks!

@wyxather
Copy link

wyxather commented Sep 9, 2024

Hi @thedmd

Thank you for creating this! It's very, very useful for taking imgui beyond simple layout without jumping hoops every time :)

I was wondering whether there's a way to utilize the space that a spring would otherwise use, or said another way; get the size that a spring would have, to replace the area with an item. I might be missing something basic, but given this simple horizontal layout:

| [Spring] [Item] |

Is there some way to replace the spring area with an item? If there was an alternative to ImGui::Spring that returned the size of the spring and did not move the cursor that might be enough, although the user would have to be careful to exactly fill the entire area so the cursor ends up in the correct position again.

Thanks!

my workaround for this issue:
since a "Spring" used ItemAdd, you can either use SetCursorScreenPos to GetItemRectMin or implement a custom widget using Behaviour API.

@altschuler
Copy link

my workaround for this issue: since a "Spring" used ItemAdd, you can either use SetCursorScreenPos to GetItemRectMin or implement a custom widget using Behaviour API.

Thank you! It works well with setting cursor pos, although it's somewhat cumbersome, so I'm curious what you mean by implementing a custom widget using Behaviour API?

@wyxather
Copy link

wyxather commented Sep 9, 2024

my workaround for this issue: since a "Spring" used ItemAdd, you can either use SetCursorScreenPos to GetItemRectMin or implement a custom widget using Behaviour API.

Thank you! It works well with setting cursor pos, although it's somewhat cumbersome, so I'm curious what you mean by implementing a custom widget using Behaviour API?

since im using it for my custom widget this is how i do it

Spring();
if (!IsItemVisible()) [[unlikely]] {
    return;
}

auto& g = *GImGui;
auto& window = *g.CurrentWindow;
auto& item = g.LastItemData; // This is Spring

item.ID  =  window.GetID("ToggleText");  // Spring use Dummy which has 0 id, we need to change it.

KeepAliveID(item.ID); // This is probably not needed if you want single press. but if you use sliderbehaviour, you need this.

if (ButtonBehavior(item.Rect, item.ID, nullptr, nullptr)) [[unlikely]] {
    value = !value;
    MarkItemEdited(item.ID);
}

RenderToggleText(item.Rect, item.ID, value, text);

pthom pushed a commit to pthom/imgui that referenced this pull request Sep 10, 2024
See ocornut#846 (comment) :

clipping has an issue, when mixed with your node editor and springs.
pthom pushed a commit to pthom/imgui that referenced this pull request Sep 10, 2024
See ocornut#846 (comment) :

clipping has an issue, when mixed with your node editor and springs.
pthom pushed a commit to pthom/imgui that referenced this pull request Sep 10, 2024
See ocornut#846 (comment) :

clipping has an issue, when mixed with your node editor and springs.
@thedmd
Copy link
Contributor Author

thedmd commented Sep 15, 2024

is there any reason why you use PushID instead of PushOverrideID in imgui_stacklayout.cpp->ImGui::BeginLayout?

I think there is no other that this function appeared around 3 years after this PR was open 😅
Switching to PushOverrideID in BeginLayout does look like a good idea at the first glance. Thanks!

@thedmd
Copy link
Contributor Author

thedmd commented Sep 15, 2024

@altschuler @wyxather

Measuring springs

I'm toying with an idea of an API that does return size of a spring. What is holding me up is breaking that perfect frame behavior. Notice that whatever you throw in stack layout it is in correct place in very same frame. There will be no flickering.

The problem is spring size information is available after top most stack layout ends. This mean returned size will be lagging one frame, because you do need it while doing layouts. This in turn can influence layout measurements and start feedback look of updates that will launch layout size to infinity if not played correctly.

I will give such API a try with next update/rebase. We will see how practical it will be. I think I will make it internal first because of that one frame lag.

@altschuler
Copy link

Yeah that makes sense. I implemented versions of @wyxather idea, calling them BeginSpring and EndSpring, where begin adds a spring, records its size and sets the cursor back to the starting point and returns the size. EndSpring resets the cursors.

It generally works well, but I ran into both the issues you mention. The layout is off by a frame, which is mostly fine for static layouts, but once you start resizing a window it obviously looks a little strange.

I also had the infinite size problem, eg using a standard ImGui::Selectable() adds some padding, so if you try to size it to the returned spring size it grows every frame. In this particular case using ImGuiSelectableFlags_NoPadWithHalfSpacing flag solves it, but it does highlight the problem of infinite size. I was wondering whether using clipping or setting item size explicitly in EndSpring could help, but I didn't dig much further.

pthom pushed a commit to pthom/imgui that referenced this pull request Oct 27, 2024
See ocornut#846 (comment) :

clipping has an issue, when mixed with your node editor and springs.
@jstreibel
Copy link

@ocornut Omar plz merge this! Signed an ImGui fan

pthom pushed a commit to pthom/imgui that referenced this pull request Nov 21, 2024
See ocornut#846 (comment) :

clipping has an issue, when mixed with your node editor and springs.
insanebytes pushed a commit to insanebytes/imgui that referenced this pull request Dec 13, 2024
insanebytes pushed a commit to insanebytes/imgui that referenced this pull request Dec 13, 2024
pthom pushed a commit to pthom/imgui that referenced this pull request Dec 18, 2024
See ocornut#846 (comment) :

clipping has an issue, when mixed with your node editor and springs.
pthom pushed a commit to pthom/imgui that referenced this pull request Dec 22, 2024
See ocornut#846 (comment) :

clipping has an issue, when mixed with your node editor and springs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.