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

Table API Synced tabled bugs #7933

Closed
lailoken opened this issue Aug 29, 2024 · 4 comments
Closed

Table API Synced tabled bugs #7933

lailoken opened this issue Aug 29, 2024 · 4 comments
Labels
bug tables/columns test wanted Should add corresponding test to ImGuiTestSuite

Comments

@lailoken
Copy link

Version/Branch of Dear ImGui:

1.91-docking

Back-ends:

sdl

Compiler, OS:

Linux GCC

Full config/build information:

No response

Details:

I've been having some sporadic issues with synced tables and have been able to reproduce it in a slightly modified version of the interactive manual.

It all works fine with vertical layout, but horizontally (SameLine) tables break both resizing and hovered row/column function calls.

Both tables have otherwise the same metrics (width) etc.

PS: Resizing (seems to) work again if I render them from right to left... not sure about the hovering functions.

Regards,
Marius.

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

void DebugTableSync()
{
    ImGui::SetNextWindowSize(ImVec2(1000, 800), ImGuiCond_FirstUseEver);
    if (ImGui::Begin("Debug Table Sync"))
    {
        static bool show_additional = false;
        static bool show_horizontal = false;
        ImGui::Checkbox("Show additional table", &show_additional);
        ImGui::Checkbox("Show horizontal", &show_horizontal);
        ImGuiTableFlags flags = ImGuiTableFlags_Resizable | ImGuiTableFlags_Hideable | ImGuiTableFlags_RowBg |
                                ImGuiTableFlags_SizingFixedFit | ImGuiTableFlags_HighlightHoveredColumn;
        int hovered_row = -1;
        int hovered_col = -1;
        for (int n = 0; n < 1 + show_additional; n++)
        {
            char buf[32];
            sprintf(buf, "Synced Table %d", n);
            if (show_horizontal && n > 0)
                ImGui::SameLine();
            if (ImGui::BeginTable("Table", 3, flags, ImVec2(450.0f, ImGui::GetTextLineHeightWithSpacing() * 5)))
            {
                ImGui::TableSetupColumn("One");
                ImGui::TableSetupColumn("Two");
                ImGui::TableSetupColumn("Three");
                ImGui::TableHeadersRow();
                for (int cell = 0; cell < 9; cell++)
                {
                    ImGui::TableNextColumn();
                    ImGui::Text("this cell %d", cell);
                }
                if (hovered_row == -1)
                    hovered_row = ImGui::TableGetHoveredRow();
                if (hovered_col == -1)
                    hovered_col = ImGui::TableGetHoveredColumn();
                ImGui::EndTable();
            }
        }

        ImGui::Text("Hovered row %d col %d", hovered_row, hovered_col);
    }
    ImGui::End();
}
@ocornut
Copy link
Owner

ocornut commented Sep 3, 2024

Thanks, I can confirm that there is an issue.

If you display the values of TableGetHoveredColumn()/TableGetHoveredRow() for each instance it's also noticeable that something is wrong here.

void DebugTableSync()
{
    ImGui::SetNextWindowSize(ImVec2(1000, 800), ImGuiCond_FirstUseEver);
    if (ImGui::Begin("Debug Table Sync"))
    {
        static bool show_additional = false;
        static bool show_horizontal = false;
        ImGui::Checkbox("Show additional table", &show_additional);
        ImGui::Checkbox("Show horizontal", &show_horizontal);
        ImGuiTableFlags flags = ImGuiTableFlags_Resizable | ImGuiTableFlags_Hideable | ImGuiTableFlags_RowBg |
            ImGuiTableFlags_SizingFixedFit | ImGuiTableFlags_HighlightHoveredColumn;
        int hovered_cols[2];
        int hovered_rows[2];
        for (int n = 0; n < 1 + show_additional; n++)
        {
            char buf[32];
            sprintf(buf, "Synced Table %d", n);
            if (show_horizontal && n > 0)
                ImGui::SameLine();
            if (ImGui::BeginTable("Table", 3, flags, ImVec2(450.0f, ImGui::GetTextLineHeightWithSpacing() * 5)))
            {
                ImGui::TableSetupColumn("One");
                ImGui::TableSetupColumn("Two");
                ImGui::TableSetupColumn("Three");
                ImGui::TableHeadersRow();
                for (int cell = 0; cell < 9; cell++)
                {
                    ImGui::TableNextColumn();
                    ImGui::Text("this cell %d", cell);
                }
                hovered_cols[n] = ImGui::TableGetHoveredColumn();
                hovered_rows[n] = ImGui::TableGetHoveredRow();
                ImGui::EndTable();
            }
        }

        for (int n = 0; n < 1 + show_additional; n++)
            ImGui::Text("Table %d: Hovered row %d col %d", n, hovered_rows[n], hovered_cols[n]);
    }
    ImGui::End();
}

I will try to investigate this soon.

@ocornut ocornut added the test wanted Should add corresponding test to ImGuiTestSuite label Sep 4, 2024
ocornut added a commit that referenced this issue Sep 4, 2024
…ple synched instances layed out at different X positions. (#7933)

Was reading ClipRect from last frame.
@ocornut
Copy link
Owner

ocornut commented Sep 4, 2024

I have pushed a first fix 8dd3383 but this is not enough for this issue.

The remaining issue is caused by the TableGetMaxColumnWidth() code which does this:

max_width = table->WorkRect.Max.x - ..... - column->MinX;

At a point in time where table->WorkRect correspond the current instance and column->MinX correspond to the previous instance.

ocornut added a commit that referenced this issue Sep 4, 2024
…that are layed out at different X positions. (#7933)

TableGetMaxColumnWidth() was using MinX from previous column. Storing info in column. Still incorrect interleaved data for multi-instances but it covers majority of use cases.
@ocornut
Copy link
Owner

ocornut commented Sep 4, 2024

Pushed a good enough fix f75cf62

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

ocornut commented Sep 4, 2024

We had code for an automated test for this and it was indeed disabled as this was marked as broken, enabled it now:
ocornut/imgui_test_engine@8412ecb

image

@ocornut ocornut closed this as completed Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug tables/columns test wanted Should add corresponding test to ImGuiTestSuite
Projects
None yet
Development

No branches or pull requests

2 participants