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

Tables: SizingFixedFit flag not honored correctly for synched tables (widths of one bleeds into next one) #7218

Closed
Ifry opened this issue Jan 11, 2024 · 11 comments

Comments

@Ifry
Copy link

Ifry commented Jan 11, 2024

Version/Branch of Dear ImGui:

Version 1.90.1, Branch: master

Back-ends:

imgui_impl_dx11.cpp

Compiler, OS:

Windows 11 + MSVC 2022

Full config/build information:

No response

Details:

My Issue/Question:

When I use ImGuiTableFlags_SizingFixedFit without ImGuiTableFlags_Resizable, table columns are not synchronized and have different widths. It appears as if the widths are swapped between the first and second table.

The expected behavior is to have both tables with the same width of first column adjusted to fit the text "Very very long text 1"

Screenshots/Video:

изображение

Minimal, Complete and Verifiable Example code:

            ImGui::Begin("Synced tables test");

            if (ImGui::BeginTable("test_table", 2, ImGuiTableFlags_Borders | ImGuiTableFlags_SizingFixedFit))
            {
                ImGui::TableNextColumn();
                ImGui::Text("Very very long text 1");

                ImGui::TableNextColumn();
                ImGui::Text("Text 1");

                ImGui::TableNextColumn();
                ImGui::Text("Very very long text 2");

                ImGui::TableNextColumn();
                ImGui::Text("Text 2");

                ImGui::EndTable();
            }

            if (ImGui::BeginTable("test_table", 2, ImGuiTableFlags_Borders | ImGuiTableFlags_SizingFixedFit))
            {
                ImGui::TableNextColumn();
                ImGui::Text("Text 1");

                ImGui::TableNextColumn();
                ImGui::Text("Text 2");

                ImGui::TableNextColumn();
                ImGui::Text("Text 1");

                ImGui::TableNextColumn();
                ImGui::Text("Text 2");

                ImGui::EndTable();
            }

            ImGui::End();
@ScottDennison
Copy link

As a new user of ImGui, I thought I was going insane
But this exactly matches what I am seeing

@ocornut
Copy link
Owner

ocornut commented Mar 28, 2024

The issue here appears because they are tables sharing the same identifier.

If I change the code to use:

if (ImGui::BeginTable("test_table", 2, ImGuiTableFlags_Borders | ImGuiTableFlags_SizingFixedFit))
[...]
if (ImGui::BeginTable("test_table2", 2, ImGuiTableFlags_Borders | ImGuiTableFlags_SizingFixedFit))

Then it works as expected.

So effectively it is a bug in the feature that allows to share table settings between two tables.
I can totally investigate and aim to fix that but the correct result after a fix is that the first column of both tables will always be wide enough to hold the "Very very long text 1" bit, and I presume this isn't what you want. So in your specific situation I believe it would be more correct that you ensure your tables have different identifiers.

Read the FAQ Entry about ID Stack System if you need to submit many tables in a loop and you want them to have unique ids, then you can use e.g. PushID(loop_Index).

@ocornut ocornut changed the title Tables isn't synced with SizingFixedFit flag Tables: SizingFixedFit flag not honored correctly for synched tables (widths of one bleeds into next one) Mar 28, 2024
@ocornut ocornut added the bug label Mar 28, 2024
@Ifry
Copy link
Author

Ifry commented Mar 28, 2024

the correct result after a fix is that the first column of both tables will always be wide enough to hold the "Very very long text 1" bit

This is exactly what i wanted

@ScottDennison
Copy link

ScottDennison commented Mar 28, 2024

I can't answer for OP, but at least in my case

the correct result after a fix is that the first column of both tables will always be wide enough to hold the "Very very long text 1" bit, and I presume this isn't what you want

This is exactly what I wanted.
Edit: Heh, OP responding with the exact same sentence as I typed this

I am having the tables be children of treenodes (via TreeNodeEx with NoPush), with all tables being the same tree depth. The tables do not have borders (it just looks like columns of text), and I was aiming for the columns to have the same positioning.

@ocornut
Copy link
Owner

ocornut commented Mar 29, 2024

I have pushed a fix for this d3c3514
Will try to write a proper automated test for it later so I am keeping this open for now.

ocornut added a commit to ocornut/imgui_test_engine that referenced this issue Apr 3, 2024
@ocornut
Copy link
Owner

ocornut commented Apr 3, 2024

Pushed test for this too:
ocornut/imgui_test_engine@e7cf491

@ocornut ocornut closed this as completed Apr 3, 2024
@ScottDennison
Copy link

ScottDennison commented Apr 19, 2024

Unfortunately, having updated the codebase that I was experiencing this issue in to v1.90.5, the issue remains.

The only notable difference from OP's original issue, is for me, the short text is in the first table and the long text is in the second table. I'll see if simply reordering the rows from OP's MCVE is enough to reproduce it, and if so, report back here.

@ScottDennison
Copy link

In fact, I am seeing this in the imgui demo (which shows v1.90.5 at the top)
demo
Note that when expanding synced instance 1, synced instance 0 gains the extra blank space (good), but instance 1 does not and has any 2 digit cells truncated.

@ocornut ocornut reopened this Apr 20, 2024
@ScottDennison
Copy link

ScottDennison commented Apr 26, 2024

image
Confirming just swapping the table order in the original MCVE is enough to re-trigger the issue in v1.90.5

New MCVE:

            ImGui::Begin("Synced tables test");

            if (ImGui::BeginTable("test_table", 2, ImGuiTableFlags_Borders | ImGuiTableFlags_SizingFixedFit))
            {
                ImGui::TableNextColumn();
                ImGui::Text("Text 1");

                ImGui::TableNextColumn();
                ImGui::Text("Text 2");

                ImGui::TableNextColumn();
                ImGui::Text("Text 1");

                ImGui::TableNextColumn();
                ImGui::Text("Text 2");

                ImGui::EndTable();
            }

            if (ImGui::BeginTable("test_table", 2, ImGuiTableFlags_Borders | ImGuiTableFlags_SizingFixedFit))
            {
                ImGui::TableNextColumn();
                ImGui::Text("Very very long text 1");

                ImGui::TableNextColumn();
                ImGui::Text("Text 1");

                ImGui::TableNextColumn();
                ImGui::Text("Very very long text 2");

                ImGui::TableNextColumn();
                ImGui::Text("Text 2");

                ImGui::EndTable();
            }

            ImGui::End();

Sorry for the delay in producing the new MCVE, hectic week at work.

ocornut added a commit to ocornut/imgui_test_engine that referenced this issue Sep 17, 2024
@ocornut
Copy link
Owner

ocornut commented Sep 17, 2024

Sorry for my late reaction!
Pushed another fix e648dbb
And amended regression test ocornut/imgui_test_engine@3a02551

Hopefully this is right this time!

@ocornut
Copy link
Owner

ocornut commented Oct 8, 2024

Pushed a fix for a regression introduced by this #8045, fix f3d242a

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

3 participants