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 crash with > 31 columns + ScrollY + ScrollFreezeTopRow #3058

Closed
GrigoryGraborenko opened this issue Mar 13, 2020 · 7 comments
Closed

Comments

@GrigoryGraborenko
Copy link

Version: 1.76 WIP (17502)
Branch: imgui/tree/tables

Back-ends: Qt + https://github.com/seanchas116/qtimgui
Compiler: Visual Studio 2019
Operating System: Windows 10

Hi! I'm part of a team writing a 3D subsea pipeline inspection utility for Fugro. We're currently in the process of creating a prototype replacement for our Unity/C# version. One of the business requirements is that human-created tags/events on pipes are visualized in a spreadsheet-style way. Unfortunately, some of this data is imported, and has a large number of critical fields - some of them more than 32,

At the moment, creating a table with 32 or more columns will trigger an assertion as soon as you start scrolling down. This happens in particular when I have ImGuiTableFlags_ScrollFreezeTopRow enabled. Sorry I haven't attached a call stack, but even zipped, it's too big for github.

Also, is there any chance that the 64 column limit I've heard about can be set by a preprocessor flag? The data we have to deal with is ill-defined and we might need more than that.

Here's a minimal example:

ImGui::Begin("Example Bug");
ImGuiTableFlags flags =
    ImGuiTableFlags_ScrollY
    | ImGuiTableFlags_ScrollFreezeTopRow
    ;

const double itemsHeight = 20;
std::vector<std::string> headers;
for (int i = 0; i < 32; i++) {
    headers.push_back("Header");
}
if (ImGui::BeginTable("EventTable", headers.size(), flags, ImVec2(500, 500))) {
    for (auto header = headers.begin(); header != headers.end(); header++)
    {
        ImGui::TableSetupColumn(header->c_str(), ImGuiTableColumnFlags_DefaultSort);
    }
    ImGui::TableAutoHeaders();

    for (int i = 0; i < 100; i++) {
        ImGui::TableNextRow(0, itemsHeight);
        for(auto header = headers.begin(); header != headers.end(); header++)
        {
            ImGui::Text("Data");
            ImGui::TableNextCell();
        }
    }
    ImGui::EndTable();
}

ImGui::End();

@ocornut
Copy link
Owner

ocornut commented Mar 13, 2020

Thanks you Grigory. I could confirm there's a bug in TableDrawMergeChannels() with >31 columns and freezing enabled, as the number of draw channel 1+count*2 exceed 64 and this function makes heavy use of 64-bit flags. I'll look into the fix.

Also, is there any chance that the 64 column limit I've heard about can be set by a preprocessor flag?

That's a bigger change unfortunately, but I'll keep it in mind.

@ocornut
Copy link
Owner

ocornut commented Mar 13, 2020

Should be fixed now, by 339ffd2 + 96a2c46 mostly (and minor 4918a64 3bfc9e8)

@ocornut ocornut closed this as completed Mar 13, 2020
@ocornut
Copy link
Owner

ocornut commented Mar 13, 2020

Also fixed bug where we asserted on 64 columns (limit was incorrectly 63 instead of 64) and added a basic regression tests in our automation framework.

@GrigoryGraborenko
Copy link
Author

That was amazingly fast turnaround - wasn't expecting that at all! Thank you!

@GrigoryGraborenko
Copy link
Author

@ocornut Just a heads up - this fix seems to have seriously impacted performance for large number of rows

@ocornut
Copy link
Owner

ocornut commented Mar 16, 2020 via email

@GrigoryGraborenko
Copy link
Author

Sorry, my bad, maybe not - likely to have been some code changes on my end. If it is because a particular commit of ImGui I'll create a new issue, but it's definitely not this bugfix that's responsible.

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