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

Wrong width of collapsing header or selectable when added in side by side childs or table #6170

Open
lesauxvi opened this issue Feb 16, 2023 · 10 comments
Labels

Comments

@lesauxvi
Copy link

Dear ImGui 1.89.2
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=201703
define: __APPLE__
define: __GNUC__=4
define: __clang_version__=13.0.0 (clang-1300.0.29.30)
define: IMGUI_HAS_VIEWPORT
define: IMGUI_HAS_DOCK
--------------------------------
io.BackendPlatformName: imgui_impl_glfw
io.BackendRendererName: imgui_impl_vulkan
io.ConfigFlags: 0x00000000
io.ConfigViewportsNoDecoration
io.ConfigMacOSXBehaviors
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000140E
 HasMouseCursors
 HasSetMousePos
 PlatformHasViewports
 RendererHasVtxOffset
 RendererHasViewports
--------------------------------
io.Fonts: 2 fonts, Flags: 0x00000000, TexSize: 512,128
io.DisplaySize: 1200.00,800.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 10.00,6.00
style.WindowBorderSize: 1.00
style.FramePadding: 8.00,7.00
style.FrameRounding: 9.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 5.00,5.00
style.ItemInnerSpacing: 5.00,4.00

Version/Branch of Dear ImGui:

Version: 1.89.2
Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_vulkan.cpp + imgui_impl_glfw.cpp
Compiler: clang
Operating System: MacOS 12.6

My Issue/Question:

I would like to have two panels side by side. Each panel has at least one CollapsingHeader element. This is very easy to do, using the BeginChild API or using a table. However, the size of the CollapsingHeader is not consistent, whatever the approach I tried. You will find below one working example based using a BeginChild approach and a picture giving the results I obtained.

I would like to get something very similar to the results presented on the window "Test 4" but with the correct width of the CollapsingHeader elements (similar to the one on window Test 2) and presenting an indent between the CollapsingHeader and the content below (see Test 2), plus with the correct padding.

I tried something with the ImGuiWindowFlags_AlwaysUseWindowPadding flag, but in that case the padding is too important (see picture Test 3).

What am I doing wrong or missing?

Screenshots/Video

Capture d’écran 2023-02-16 à 15 40 52

Standalone, minimal, complete and verifiable example:

// Here's the goal
ImGui::Begin("Test 2");
if (ImGui::CollapsingHeader("Hello", ImGuiTreeNodeFlags_DefaultOpen)) {
   ImGui::Text("My other text");
   ImGui::Text("My other text");
}
ImGui::End();


// Here's an example with two panels side by side. The width of the CollapsingHeader is not correct
ImGui::Begin("Test 4");
ImGui::BeginChild("Test 5", ImVec2(ImGui::GetContentRegionAvail().x / 2.f, -1.f), false);
if (ImGui::CollapsingHeader("Hello 3", ImGuiTreeNodeFlags_DefaultOpen)) {
    ImGui::Text("My text ");
    ImGui::Text("My text");
    ImGui::Text("My text");
}
ImGui::EndChild();
ImGui::SameLine();
ImGui::BeginChild("Test 6", ImVec2(ImGui::GetContentRegionAvail().x, -1.f), false);
if (ImGui::CollapsingHeader("Hello 4", ImGuiTreeNodeFlags_DefaultOpen)) {
    ImGui::Text("My text");
    ImGui::Text("My text");
    ImGui::Text("My text");
}
ImGui::EndChild();
ImGui::End();


// Here is another example that fixes the width of the CollapsingHeader item, but the padding is way too important
ImGui::Begin("Test 3");
ImGui::BeginChild("Test 3", ImVec2(ImGui::GetContentRegionAvail().x / 2.f, ImGui::GetContentRegionAvail().y), false, ImGuiWindowFlags_AlwaysUseWindowPadding);
if (ImGui::CollapsingHeader("Hello 3", ImGuiTreeNodeFlags_DefaultOpen)) {
    ImGui::Text("My text");
    ImGui::Text("My text");
    ImGui::Text("My text");
}
ImGui::EndChild();
ImGui::SameLine();
ImGui::BeginChild("Test 4", ImVec2(ImGui::GetContentRegionAvail().x, ImGui::GetContentRegionAvail().y), false, ImGuiWindowFlags_AlwaysUseWindowPadding);
if (ImGui::CollapsingHeader("Hello 4", ImGuiTreeNodeFlags_DefaultOpen)) {
    ImGui::Text("My text");
    ImGui::Text("My text");
    ImGui::Text("My text");
}
ImGui::EndChild();
ImGui::End();
@ocornut
Copy link
Owner

ocornut commented Feb 16, 2023

Hello,

You mention using table API which should be the natural way of doing this two-columns layout, yet none of your example use the BeginTable() API so it is difficult to provide an answer that is valid for tables....

By default CollapsingHeader() extend a little bit to the left and the right of the working rectangle (this is what you see in Test 2).

It naturally cannot be done if the host window has no padding (& no initial indent). This is what you see in Test 4 and what you would also see in Test 2 if you altered style so that style.WindowPadding == 0.0f;

Test 3 is "correct" in the sense that:

  • Parent window add WindowPadding.x amount of indent.
  • Child window also add another WindowPadding.x amount of indent.
  • CollapsingHeader() extend by WindowPadding.x*0.5f to the left and right.

The way to do this if you need to use BeginChild() to split that window in two may be to use a child with zero padding, then inside CollapsingHeader() followed by an Indent() call.

However, as mentioned, it may ultimately be different with tables.

@lesauxvi
Copy link
Author

Thank you very much for your reactivity!

Yes, I did not had the Table example, sorry. Here it is :

ImGui::Begin("Test with Table");
    ImGuiTableFlags table_flags = ImGuiTableFlags_Resizable | ImGuiTableFlags_ScrollY | ImGuiTableFlags_BordersInnerV | ImGuiTableFlags_NoPadOuterX ;
    if (ImGui::BeginTable("##tableau_sequence_tracking", 2, table_flags)) {
        ImGui::TableNextRow();
        ImGui::TableNextColumn();
        if (ImGui::CollapsingHeader("Hello 7",  ImGuiTreeNodeFlags_DefaultOpen)) {
           ImGui::Text("My text");
           ImGui::Text("My text");
           ImGui::Text("My text");
        }
       ImGui::TableNextColumn();
    if (ImGui::CollapsingHeader("Hello 8", ImGuiTreeNodeFlags_DefaultOpen)) {
        ImGui::Text("My text");
        ImGui::Text("My text");
        ImGui::Text("My text");
    }
    ImGui::EndTable();
}
ImGui::End();

This produces the same result as the one presented on figure "Test 4".

To sum up your response : before the call to the child (but after the call to Begin(xxx)), set style.WindowPadding == 0.0f and then add some indent. That would be a working solution, but that requires come changes on the existing code (actually, I'm just doing some modification of the general layout of the window, and if I can do that without changing too much the actual code, that would be great - but I there is no other solution, so be it!). But if you have a solution based on the Table API, that would be great!

@lesauxvi
Copy link
Author

I managed to have a working solution for the child based code:

Capture d’écran 2023-02-17 à 09 00 42

Here is the solution that works:

auto &style = ImGui::GetStyle();
auto vec = style.WindowPadding;
style.WindowPadding = {0.f, 0.f};
ImGui::Begin("Child based solution");
style.WindowPadding = vec;
ImGui::BeginChild("##child_1", ImVec2(ImGui::GetContentRegionAvail().x / 2.f, ImGui::GetContentRegionAvail().y), false, ImGuiWindowFlags_AlwaysUseWindowPadding);
if (ImGui::CollapsingHeader("Child left", ImGuiTreeNodeFlags_DefaultOpen)) {
    ImGui::Text("My text");
    ImGui::Text("My text");
    ImGui::Text("My text");
}
ImGui::EndChild();
ImGui::SameLine();
ImGui::BeginChild("##child_2", ImVec2(ImGui::GetContentRegionAvail().x, ImGui::GetContentRegionAvail().y), false, ImGuiWindowFlags_AlwaysUseWindowPadding);
if (ImGui::CollapsingHeader("Child right", ImGuiTreeNodeFlags_DefaultOpen)) {
    ImGui::Text("My text");
    ImGui::Text("My text");
    ImGui::Text("My text");
}
ImGui::EndChild();
ImGui::End();

I tried something similar with the Tables, but nothing worked. Here is the lastest version of the code:

style.WindowPadding = vec;
ImGui::Begin("Table based solution");
ImGuiTableFlags table_flags = ImGuiTableFlags_Resizable | ImGuiTableFlags_ScrollY | ImGuiTableFlags_BordersInnerV | ImGuiTableFlags_NoPadOuterX ;
style.WindowPadding = {0, 0};
if (ImGui::BeginTable("##tableau_sequence", 2, table_flags)) {
   ImGui::TableNextRow();
   ImGui::TableNextColumn();
   if (ImGui::CollapsingHeader("Table left", ImGuiTreeNodeFlags_DefaultOpen)) {
       ImGui::Text("My text");
       ImGui::Text("My text");
       ImGui::Text("My text");
   }
    ImGui::TableNextColumn();
    if (ImGui::CollapsingHeader("Table right", ImGuiTreeNodeFlags_DefaultOpen)) {
        ImGui::Text("My text");
        ImGui::Text("My text");
        ImGui::Text("My text");
    }
    ImGui::EndTable();
}
style.WindowPadding = vec;
ImGui::End();

and here is the corresponding result
Capture d’écran 2023-02-17 à 09 06 26

If someone has a working solution and can share it, I would be very grateful as the Table solution brings the moving separator between the columns, which is a very nice feature.

@ocornut
Copy link
Owner

ocornut commented Feb 17, 2023

There are many tangled factors here. Sorry this is going to be involved :)
I also redirected #6173 to here as many of the ideas are the same.

First of all, in case it isn't evident, in the case of BeginChild() (or BeginTable() with scrolling enabled, which is essentially a child) we need to draw within the boundary of the child window mostly for performances reasons (more on that later). So if the window has no padding and we start submitting item from window relative x=0, we can't extend to the left. We could technically push clip rects but it would be too costly if done per item and would generate draw calls. We could also allocate a specific layer but it's involved (but not impossible).

In the case of table we have a specialized system/channel to optimally honor features such as ImGuiSelectableFlags_SpanAllColumns:

    else if (span_all_columns && g.CurrentTable)
        TablePushBackgroundChannel();

So a Selectable can cover multiple rows.
We could imagine adding a flag to say "I want this item to extend beyond the boundaries of my host window" and offer a similar feature but it's not simple and would not be correct if it was on by default (so should be opt-in).


Another issue specific with CollapsingHeader() is that it is extruding using window->WindowPadding (window padding of host window as locked at the time of Begin()) rather than style.WindowPadding.

if (display_frame)
{
    // Framed header expand a little outside the default padding, to the edge of InnerClipRect
    // (FIXME: May remove this at some point and make InnerClipRect align with WindowPadding.x instead of WindowPadding.x*0.5f)
    frame_bb.Min.x -= IM_FLOOR(window->WindowPadding.x * 0.5f - 1.0f);
    frame_bb.Max.x += IM_FLOOR(window->WindowPadding.x * 0.5f);
}

This has been the case since prehistoric times: 824cf5a added the latch in Begin() and started having CollapsingHeader() use it. There are two remaining widgets location using that (CollapsingHeader() and TabBar). The reason is that they are trying to match the window clipping limit assuming we are intending to cover the width of our window. It's wrong in many ways but there's no trivial action to make, rather a few ones.

  • I think we should first eliminate adding half WindowPadding.x to default clipping limits (Cannot draw over horizontal padding area when child's border size is non zero #3312)
  • The extension of CollapsingHeader() to the left and right is problematic but at least should use style.WindowPadding or style.FramePadding to be coherent with other things.
  • However even with that, it doesn't guarantee that FramePadding < WindowPadding especially with your non-border tables.
  • It would be simpler and senseful to completely remove that extruding for CollapsingHeader.
    @lesauxvi try changing the code to
    if (display_frame && !g.IO.KeyShift)
    {
        // Framed header expand a little outside the default padding, to the edge of InnerClipRect
        // (FIXME: May remove this at some point and make InnerClipRect align with WindowPadding.x instead of WindowPadding.x*0.5f)
        frame_bb.Min.x -= IM_FLOOR(style.WindowPadding.x * 0.5f - 1.0f);
        frame_bb.Max.x += IM_FLOOR(style.WindowPadding.x * 0.5f);
    }

And press SHIFT to compare.


Selectable()'s own flavor of the same issue is that it extends by ItemSpacing*0.5f on each side, the intent being that selectables next to each others should leave no clicking gap and no visual gap. That solution has however generated lots of issues and I would like to redesign this. However, from the POV of Selectable() filling a window, if ItemSpacing < WindowPadding that Selectable rectangle will be clipped as seen in #6173.

@ocornut ocornut added the layout label Feb 17, 2023
@lesauxvi
Copy link
Author

lesauxvi commented Feb 17, 2023

I tried what you suggested @ocornut and here are the results. First, if I do not press the SHIFT key, we have

Capture d’écran 2023-02-17 à 16 10 07

If I press the SHIFT key, then we have

Capture d’écran 2023-02-17 à 16 10 18

The headers are perfectly aligned with the rest of the elements and the results are consistent in that sense that whatever the approach I consider, the visual output feels the same.

@ypujante
Copy link
Contributor

@ocornut in #6173, you said: "TL;DR; you could add window padding to your child"

I am not sure how you do this exactly.

I tried:

      auto windowPadding = ImGui::GetStyle().WindowPadding;
      if(ImGui::BeginChild("Content"))
      {
        ImGui::PushStyleVar(ImGuiStyleVar_WindowPadding, windowPadding);
        ImGui::Selectable("select1", true);
        ImGui::Selectable("select2", true);
        ImGui::PopStyleVar();
      }

and that doesn't do anything

@ocornut
Copy link
Owner

ocornut commented Feb 17, 2023

WindowPadding is applied at the time of Begin()/BeginChild().
Child without visible borders don't have padding by default unless you use ImGuiWindowFlags_AlwaysUseWindowPadding.

@ypujante
Copy link
Contributor

Using ImGuiWindowFlags_AlwaysUseWindowPadding fixes the problem of the Separable (the eye icon in my screenshot) but then actually introduces padding which I don't want because a) it's wasteful, b) it's not aligned anymore.

Screen Shot 2023-02-17 at 07 26 34

Is there any way to go around this?

@ocornut
Copy link
Owner

ocornut commented Feb 17, 2023

This is being discussed in details in this topic. There's no single simple answer.

@ocornut ocornut changed the title Wrong width of collapsing header when added in side by side panels or table Wrong width of collapsing header or selectable when added in side by side childs or table Feb 17, 2023
@ocornut
Copy link
Owner

ocornut commented Feb 17, 2023

Disabling padding and adding the corresponding Indent(padding); call is probably the simpler answer for your Selectable() call (may not work for CollapsingHeader()).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants