`BeginGroup`/`EndGroup` with table inside reports incorrect item size

[thedmd]

Version/Branch of Dear ImGui:

Version 1.90.6 WIP (r19053), Branch: master

Back-ends:

imgui_impl_win32.cpp + imgui_impl_dx11.cpp

Compiler, OS:

Windows 11, MSVC 2022, x64

Full config/build information:

Dear ImGui 1.90.6 WIP (19053)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=199711
define: _WIN32
define: _WIN64
define: _MSC_VER=1939
define: _MSVC_LANG=201402
--------------------------------
io.BackendPlatformName: imgui_impl_win32
io.BackendRendererName: imgui_impl_dx11
io.ConfigFlags: 0x00000000
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000000E
 HasMouseCursors
 HasSetMousePos
 RendererHasVtxOffset
--------------------------------
io.Fonts: 2 fonts, Flags: 0x00000000, TexSize: 1024,1024
io.DisplaySize: 1905.33,987.33
io.DisplayFramebufferScale: 1.50,1.50
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

Details:

My understanding is that groups should surround containing widgets, which isn't the case with tables.

Table surrounded by BeginGroup/EndGroup appears to be measured incorrectly.

Problem can be reproduced simply by resizing Test2 window. Green rect is expected to cover whole table.
Push shift key to enable workaround, which place 0 size dummy widget at the same line as table.

Digging into the code, table OuterRect seems to be "stuck" on minimum size. At the moment I cannot tell if this is an desired behavior or not.

Screenshots/Video:

Current	Expected

Minimal, Complete and Verifiable Example code:

ImGui::Begin("Test2");
ImGui::BeginGroup();
if (ImGui::BeginTable("table2", 2))
{
    ImGui::TableNextColumn();
    ImGui::Text("Thing 1");
    ImGui::TableNextColumn();
    ImGui::Text("Thing 2");
    ImGui::EndTable();
}

if (ImGui::GetIO().KeyShift)
{
    ImGui::PushStyleVar(ImGuiStyleVar_FramePadding, ImVec2(0.0f, 0.0f));
    ImGui::SameLine(0.0f, 0.0f);
    ImGui::Dummy(ImVec2(0.0f, 0.0f));
    ImGui::PopStyleVar();
}
ImGui::EndGroup();
ImGui::GetWindowDrawList()->AddRectFilled(ImGui::GetItemRectMin(), ImGui::GetItemRectMax(), IM_COL32(0, 255, 0, 64), 4.0f);
ImGui::End();

[MartinClementson]

I am seeing issues with tables being inside a Group. The end cursor gets set according to the position of the last row, not where the table ends.

My table has a height of -60. If I put it in a group, the buttons that are drawn after EndTable() and EndGroup() will be drawn inside the table. In the image below you can also see the purple WorkRect of the table:

If I remove BeginGroup/EndGroup. It behaves the way I'd expect:

I have found that it's caused by having a negative height on the table. If I set a fixed size, the cursor will move to the correct position, which is below the table. However, in this situation, I need to use a negative height as the table is inside a resizable window. It seems to me that it's a bug, as I would expect the group to correctly calculate the endpos regardless of whether the table size is static or dynamically calculated. But maybe I am missing a crucial flag in this case?

[ocornut]

As Michał reported there's definitively an issue and yours may be the same, but I am not sure I understand why you would want to surround the table in your screenshot by a group?

[ocornut]

FYI the culprit for Michał's case is this code in EndTable():

else if (temp_data->UserOuterSize.x <= 0.0f)
{
    // Some references for this: #7651 + tests "table_reported_size", "table_reported_size_outer" equivalent Y block
    // - Checking for ImGuiTableFlags_ScrollX/ScrollY flag makes us a frame ahead when disabling those flags.
    // - FIXME-TABLE: Would make sense to pre-compute expected scrollbar visibility/sizes to generally save a frame of feedback.
    const float inner_content_max_x = table->OuterRect.Min.x + table->ColumnsAutoFitWidth; // Slightly misleading name but used for code symmetry with inner_content_max_y
    const float decoration_size = table->TempData->AngledHeadersExtraWidth + ((table->Flags & ImGuiTableFlags_ScrollY) ? inner_window->ScrollbarSizes.x : 0.0f);
    outer_window->DC.IdealMaxPos.x = ImMax(outer_window->DC.IdealMaxPos.x, inner_content_max_x + decoration_size - temp_data->UserOuterSize.x);
    outer_window->DC.CursorMaxPos.x = ImMax(backup_outer_max_pos.x, ImMin(table->OuterRect.Max.x, inner_content_max_x + decoration_size));
}
else
{
    outer_window->DC.CursorMaxPos.x = ImMax(backup_outer_max_pos.x, table->OuterRect.Max.x);
}

And I suspect the equivalent but slightly different Y code block is related to the issue for Martin.

Specifically the question is why the last line:

outer_window->DC.CursorMaxPos.x = ImMax(backup_outer_max_pos.x, ImMin(table->OuterRect.Max.x, inner_content_max_x + decoration_size));

isn't

outer_window->DC.CursorMaxPos.x = ImMax(backup_outer_max_pos.x, table->OuterRect.Max.x);

Following the addition of IdealMaxPos I did that change: ae0d2dd

The simplest way to understand is to try to make the change and run imgui_test_suite:

Need further investigation...

[ocornut]

If I investigate table_padding which is the simplest test failing, I'm starting to understand the issue:

In theory outer_window->DC.CursorMaxPos.x = ImMax(backup_outer_max_pos.x, table->OuterRect.Max.x); seems correct.
Note the two code paths:

    else if (temp_data->UserOuterSize.x <= 0.0f)
    {
        outer_window->DC.CursorMaxPos.x = ImMax(backup_outer_max_pos.x, ImMin(table->OuterRect.Max.x, inner_content_max_x + decoration_size));
    }
    else
    {
        outer_window->DC.CursorMaxPos.x = ImMax(backup_outer_max_pos.x, table->OuterRect.Max.x);
    }

However if BeginTable(..ImVec2(0,0)) ... EndTable() reports its actual width into CursorMaxPos.x then it would create a feedback loop where table would fill a window, auto-resizing window would fit to table: auto-resizing window would never be able to shrink down if table contents gets smaller. As CursorMaxPos is the way for widgets to convey to the window how much space is being used this also affects EndGroup() which uses the same data.

Current workaround

• As in same post, a SameLine(0,0); Dummy(0,0) may be enough.
• Instead of passing -60 you could workaround the issue by using GetContentRegionAvail().y - 60.0f.

I don't want to imply that changing the table behavior is impossible but it's likely going to be a long and tricky change.
It's possible that we could find a fix that would benefit EndGroup() without changing table's behavior in regards to reporting its used size to the container window.

[ocornut]

The table_reported_size_2 test is also failing for the same reason:
You can right-click -> "Run GUI Func" to see the window:

// ## Test that resizable column report their current size
t = IM_REGISTER_TEST(e, "table", "table_reported_size_2");
t->GuiFunc = [](ImGuiTestContext* ctx)
{
    ImGui::Begin("Test Window", NULL, ImGuiWindowFlags_NoSavedSettings);
    ImGui::PushStyleVar(ImGuiStyleVar_CellPadding, ImVec2(0.0f, 0.0f));
    if (ctx->IsFirstGuiFrame())
        TableDiscardInstanceAndSettings(ImGui::GetID("table1"));
    if (ImGui::BeginTable("table1", 3, ImGuiTableFlags_Resizable | ImGuiTableFlags_SizingFixedFit))
    {
        ImGui::TableSetupColumn("", 0, 50.0f);
        ImGui::TableSetupColumn("", 0, 100.0f);
        ImGui::TableSetupColumn("", 0, 50.0f);
        ImGui::TableNextColumn();
        ImGui::Text("Hello");
        ImGui::TableSetBgColor(ImGuiTableBgTarget_CellBg, IM_COL32(255, 0, 0, 50), 0);
        ImGui::TableSetBgColor(ImGuiTableBgTarget_CellBg, IM_COL32(0, 255, 0, 50), 1);
        ImGui::TableSetBgColor(ImGuiTableBgTarget_CellBg, IM_COL32(0, 0, 255, 50), 2);
        ImGui::EndTable();
    }
    ImGui::PopStyleVar();
    ImGui::End();
};
t->TestFunc = [](ImGuiTestContext* ctx)
{
    ctx->SetRef("Test Window");
    ImGuiTable* table = ImGui::TableFindByID(ctx->GetID("table1"));

    // FIXME: Expose nicer helpers?
    const float w_extra = (table->OuterPaddingX * 2.0f) + (table->CellSpacingX1 + table->CellSpacingX2) * (table->ColumnsEnabledCount - 1) + (table->CellPaddingX * 2.0f) * table->ColumnsEnabledCount;
    ctx->TableResizeColumn("table1", 1, 100.0f);
    IM_CHECK_EQ(table->OuterWindow->ContentSize.x, 50.0f + 100.0f + 50.0f + w_extra);

    ctx->TableResizeColumn("table1", 1, 80.0f);
    IM_CHECK_EQ(table->OuterWindow->ContentSize.x, 50.0f + 80.0f + 50.0f + w_extra);
};

If it's even more offending failure here because in this example, even though the table itself is auto-resizing (omitted size in BeginTable), there are columns with hard-coded fixed size.

At heart the problem is the data used for window auto-sizing is the same one used by EndGroup() for measurement.

Potential fixes would involve:

• Maintaining a third field (along with CursorMaxPos and IdealMaxPos) used for current frame measurement that's guaranteed to never be used to next-frame feedback. In theory window auto-sizing would use IdealMaxPos and not ImMax(CursorMaxPos, IdealMaxPos) and then we'd be free to set CursorMaxPos "correctly" but our layout system makes this too difficult to follow on.
• As a hack/workaround, EndGroup() might query Last Item data to include that in bounds. This would definitively fix both your problems but might creates a little bit of an inconsistency if there are other items, though interestingly a vast majority of "other items following a table" would extend the rectangle (this is what the Dummy(0,0) is doing, it's really going to be ok in a vast majority of cases.

[ocornut]

@thedmd @MartinClementson my proposed solution (last item of previous message) would be in EndGroup() to change

ImRect group_bb(group_data.BackupCursorPos, ImMax(window->DC.CursorMaxPos, group_data.BackupCursorPos));

window->DC.CursorPos = group_data.BackupCursorPos;
window->DC.CursorPosPrevLine = group_data.BackupCursorPosPrevLine;
window->DC.CursorMaxPos = ImMax(group_data.BackupCursorMaxPos, window->DC.CursorMaxPos);

to

ImRect group_bb(group_data.BackupCursorPos, ImMax(ImMax(window->DC.CursorMaxPos, g.LastItemData.Rect.Max), group_data.BackupCursorPos));

window->DC.CursorPos = group_data.BackupCursorPos;
window->DC.CursorPosPrevLine = group_data.BackupCursorPosPrevLine;
window->DC.CursorMaxPos = ImMax(group_data.BackupCursorMaxPos, group_bb.Max);

[MartinClementson]

Thank you Omar,

I had a look into EndTable and EndGroup and it seems like your proposed solution might be the right way to go. I'm in a situation where it would be risky to make internal imgui changes, as I wouldn't have enough time to test that it doesn't break something else in the project. Almost all of the widgets in the project utilize groups to some extent.

In my case, the initial workaround was calling "ImGui::SetCursorPosY(ImGui::GetCurrentWindow()->Size.y - 60);". After EndGroup(). The reason for the group is that I draw all widgets in a wrapper function that draws a label on the left-hand side of the widget and handles vertical text alignment, among other things. For that, I'm creating a group for each widget. The table in the screenshot had no label, but it was still wrapped in a group. The simplest fix for me is to adjust my wrapper function so that it won't create a group if there is no label for the widget.

[thedmd]

This patch made items leak into empty groups. Is this a desired behavior?

Before	After

{
    ImGui::Begin("Group Test");

    ImGui::BeginGroup();
    ImGui::EndGroup();
    auto group_a_min = ImGui::GetItemRectMin();
    auto group_a_max = ImGui::GetItemRectMax();

    ImGui::BeginGroup();
    ImGui::Text("Hello");
    ImGui::EndGroup();
    auto group_b_min = ImGui::GetItemRectMin();
    auto group_b_max = ImGui::GetItemRectMax();

    ImGui::BeginGroup();
    ImGui::EndGroup();
    auto group_c_min = ImGui::GetItemRectMin();
    auto group_c_max = ImGui::GetItemRectMax();

    ImGui::GetWindowDrawList()->AddRect(group_a_min, group_a_max, IM_COL32(255, 0, 0, 255));
    ImGui::GetWindowDrawList()->AddRect(group_b_min, group_b_max, IM_COL32(0, 255, 0, 255));
    ImGui::GetWindowDrawList()->AddRect(group_c_min, group_c_max, IM_COL32(0, 0, 255, 255));

    ImGui::End();
}