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

Last Table column cuts off a wide item in the first row when ScrollX, MultiSelect, and ImGuiListClipper are used together #8250

Open
bratpilz opened this issue Dec 19, 2024 · 5 comments

Comments

@bratpilz
Copy link

Version/Branch of Dear ImGui:

Version 1.91.7, Branch: master

Back-ends:

imgui_impl_glfw.cpp + imgui_impl_opengl3.cpp

Compiler, OS:

Linux + GCC 12.2.0

Full config/build information:

Dear ImGui 1.91.7 WIP (19162)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=201103
define: __linux__
define: __GNUC__=12
--------------------------------
io.BackendPlatformName: imgui_impl_glfw
io.BackendRendererName: imgui_impl_opengl3
io.ConfigFlags: 0x00000003
 NavEnableKeyboard
 NavEnableGamepad
io.ConfigNavCaptureKeyboard
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000000E
 HasMouseCursors
 HasSetMousePos
 RendererHasVtxOffset
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,128
io.DisplaySize: 1280.00,720.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
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:

Hello again!

I found a corner case in the interaction of MultiSelect, Tables, and ImGuiListClipper.
When ScrollX is enabled in the Table and I do not call TableHeadersRow, then the last column will cut off a wide item in the first row early. Furthermore, the first row will have a few pixels of extra vertical spacing at the bottom. If there are multiple rows of wide items at the beginning of the Table, then the last column no longer cuts off early, but the last of these multiple rows still has the extra vertical spacing at the bottom.
If I comment out the usage of ImGuiListClipper, then the last column no longer cuts off early.
If I comment out the usage of MultiSelect, then the last column no longer cuts off early and the extra vertical spacing at the bottom disappears.
Commenting out the call to TableSetupScrollFreeze has no effect either way.
I've modified the "Multi-Select (in a table)" example from the demo to demonstrate (see code and video 1).

Another interesting thing I've noticed during troubleshooting is that the problems I've described here disappear if I move the call of BeginMultiSelect above the submission of the first row (either via TableHeadersRow or via submitting other items). But this then causes another issue when the first row is frozen, namely that items get skipped while scrolling back up the table via BoxSelect (see video 2).

Kind regards.

Screenshots/Video:

table-wide-item-multiselect.mp4

This is what happens after moving the call of BeginMultiSelect above the submission of the first (frozen) row:

table-wide-item-multiselect-2.mp4

Minimal, Complete and Verifiable Example code:

// Here's some code anyone can copy and paste to reproduce your issue
	static const char* ExampleNames[] =
	{
	  "Artichoke", "Arugula", "Asparagus", "Avocado", "Bamboo Shoots", "Bean Sprouts", "Beans", "Beet", "Belgian Endive", "Bell Pepper",
	  "Bitter Gourd", "Bok Choy", "Broccoli", "Brussels Sprouts", "Burdock Root", "Cabbage", "Calabash", "Capers", "Carrot", "Cassava",
	  "Cauliflower", "Celery", "Celery Root", "Celcuce", "Chayote", "Chinese Broccoli", "Corn", "Cucumber"
	};

        if (ImGui::TreeNode("Multi-Select (in a table)"))
        {
            static ImGuiSelectionBasicStorage selection;

	    static int extra_table_flags = 0;
	    ImGui::CheckboxFlags("ScrollX", &extra_table_flags, ImGuiTableFlags_ScrollX);
	    static bool wide_items_instead_of_table_headers = false;
	    ImGui::Checkbox("Items instead of headers", &wide_items_instead_of_table_headers);
	    static int num_item_rows = 1;
	    if (wide_items_instead_of_table_headers)
	      {
		ImGui::SameLine();
		ImGui::SetNextItemWidth (ImGui::CalcTextSize ("X").x * 10.0f);
		ImGui::SliderInt("Rows", &num_item_rows, 0, 4);
	      }

            const int ITEMS_COUNT = 10000;
            ImGui::Text("Selection: %d/%d", selection.Size, ITEMS_COUNT);
            if (ImGui::BeginTable("##Basket", 2, extra_table_flags | ImGuiTableFlags_ScrollY | ImGuiTableFlags_RowBg | ImGuiTableFlags_BordersOuter))
            {
                ImGui::TableSetupColumn("Object");
                ImGui::TableSetupColumn("Action");
		ImGui::TableSetupScrollFreeze(0, wide_items_instead_of_table_headers ? num_item_rows : 1);

		if (wide_items_instead_of_table_headers)
		  {
		    for (int i = 0; i < num_item_rows; ++i)
		      {
			if (ImGui::TableNextColumn())
			  ImGui::TextUnformatted("A wide item in column 1");
			if (ImGui::TableNextColumn())
			  ImGui::TextUnformatted("A wide item in column 2");
		      }
		  }
		else
		  {
		    ImGui::TableHeadersRow();
		  }

                ImGuiMultiSelectFlags flags = ImGuiMultiSelectFlags_ClearOnEscape | ImGuiMultiSelectFlags_BoxSelect1d;
                ImGuiMultiSelectIO* ms_io = ImGui::BeginMultiSelect(flags, selection.Size, ITEMS_COUNT);
                selection.ApplyRequests(ms_io);

                ImGuiListClipper clipper;
                clipper.Begin(ITEMS_COUNT);
                if (ms_io->RangeSrcItem != -1)
		    clipper.IncludeItemByIndex((int)ms_io->RangeSrcItem); // Ensure RangeSrc item is not clipped.
                while (clipper.Step())
                {
		    for (int n = clipper.DisplayStart; n < clipper.DisplayEnd; n++)
		    {
		        ImGui::TableNextRow();
                        ImGui::TableNextColumn();
                        char label[64];
                        sprintf(label, "Object %05d: %s", n, ExampleNames[n % IM_ARRAYSIZE(ExampleNames)]);
                        bool item_is_selected = selection.Contains((ImGuiID)n);
                        ImGui::SetNextItemSelectionUserData(n);
                        ImGui::Selectable(label, item_is_selected, ImGuiSelectableFlags_SpanAllColumns | ImGuiSelectableFlags_AllowOverlap);
                        ImGui::TableNextColumn();
                        ImGui::SmallButton("hello");
                    }
                }

                ms_io = ImGui::EndMultiSelect();
                selection.ApplyRequests(ms_io);
                ImGui::EndTable();
            }
            ImGui::TreePop();
        }
@ocornut
Copy link
Owner

ocornut commented Dec 19, 2024

Thanks for your careful report.
They are two independent bugs and ideally both should be fixed.

The first bug appears to be that BeginMultiSelect() override window->DC.CursorMaxPos:

ms->ScopeRectMin = window->DC.CursorMaxPos = window->DC.CursorPos;

Which is used by TableEndRow()->TableEndCell() to measure the width of the last open cell, so that measurement is lost. Which is why this issue is fixed when you move the BeginMultiSelect() block above.

As for the second one, box-select is very sensitive to the fact that is uses a clipping rect and parent region and they must strictly match the one used by actual items. I'll investigate this as well.

@ocornut
Copy link
Owner

ocornut commented Dec 19, 2024

FYI The hacky workaround for the first bug is using the following:

if (wide_items_instead_of_table_headers)
{
    for (int i = 0; i < num_item_rows; ++i)
    {
        ImGui::TableNextRow();
        if (ImGui::TableSetColumnIndex(0))
            ImGui::TextUnformatted("A wide item in column 1");
        if (ImGui::TableSetColumnIndex(1))
            ImGui::TextUnformatted("A wide item in column 2");
        ImGui::TableSetColumnIndex(0);
    }
}
...

Aka changing column after the last submission, in order for measurement to not be lost by the BeginMultiSelect() call.
For this to rework I changed the logic to use TableNextRow() + TableSetColumnIndex(1). But you could as well just add TableSetColumnIndex(0) followed by TableSetColumnIndex(1) after the code you wrote.

@ocornut
Copy link
Owner

ocornut commented Dec 19, 2024

To clarify, ScrollX has no direct effect on those issues. But it looks different because without ScrollX the default Sizing Policy for columns is to Stretch Proportionally (ignoring measured contents width), and with ScrollX the default Sizing Policy is to Fit to Contents. Either way, the measured contents width for the last submitted column is overridden by BeginMultiSelect().

ocornut added a commit that referenced this issue Dec 19, 2024
…ured when calling BeginMultiSelect() while inside a table. (#8250)
@ocornut
Copy link
Owner

ocornut commented Dec 19, 2024

I am still working on exploring this further and writing tests (with led me to yakshave on yet another tangent) but I think this pushed workaround fd93229 is actually reasonable for various reasons (I checked all other rewrite of CursorMaxPos and this was the only one which had a reasonable chance to overlap cells, and it mimick similar calls in e.g. clipper).

@bratpilz
Copy link
Author

Thanks for taking a look at this so quickly! I just cherry-picked fd93229 to the docking branch for testing and it fixed the problem for me.
I also prefer to call BeginMultiSelect after submitting the frozen table rows instead of before, so I'm glad that seems to still be the way to go.

ocornut added a commit to ocornut/imgui_test_engine that referenced this issue Dec 20, 2024
matthew-mccall pushed a commit to matthew-mccall/imgui that referenced this issue Jan 1, 2025
…ured when calling BeginMultiSelect() while inside a table. (ocornut#8250)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants