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: Automatic column width becomes zero when the number of columns changes #4046

Closed
ArekSredzki opened this issue Apr 16, 2021 · 11 comments

Comments

@ArekSredzki
Copy link

Version/Branch of Dear ImGui:

Version: 1.82 WIP 732cd83
Branch: docking

Back-end/Renderer/Compiler/OS

Not relevant

My Issue/Question:
One of the tables in my application has a variable number of columns.
Since the contents of each column can take up a relatively large amount of space, I need to have horizontal scrolling enabled with ImGuiTableFlags_ScrollX.

The width of each column is correctly computed on most frames.
However, the width of every column becomes 0 (+ padding) whenever the number of tables changes.
This results in an undesirable flicker that is very noticeable if the application is rendering at a lower frame rate.

I expected ImGui to at least initialize the column widths using the column label.
However, I think it would be desirable for ImGui to preserve the previously column width data when the column count changes, at least for the minimum common number of columns.

It's worth noting that the TableSetupColumn does not appear to provide a way to set a reasonable minimum width.
I attempted to use the following, but it ended up being the permanent width instead of just an initial default:

    const float init_width = ImGui::CalcTextSize(header_label.c_str()).x;
    ImGui::TableSetupColumn(header_label.c_str(), column_flags, init_width);

GIFs
imgui_column_sizing_issue-2021-04-15_23 08 06
imgui_column_sizing_issue-2021-04-15_23 19 53

Standalone, minimal, complete and verifiable example:

if (!ImGui::Begin("Hack"))
{
  ImGui::End();
  return;
}

// Add a new column every 0.5s, up to a max of 10 columns before rolling over back to 1.
constexpr double seconds_per_new_column = 0.5;
constexpr int max_num_columns = 10;
const int column_count = static_cast<int>(ImGui::GetTime() / seconds_per_new_column) % max_num_columns + 1;

if (ImGui::BeginTable("Variable Column Count Table",
                      column_count,
                      ImGuiTableFlags_Borders | ImGuiTableFlags_ScrollX))
{
  char label[32];
  for (int column = 0; column < column_count; column++)
  {
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wold-style-cast"
    ImFormatString(label, IM_ARRAYSIZE(label), "Column %d", column);
#pragma GCC diagnostic pop
    ImGui::TableSetupColumn(label);
  }
  ImGui::TableHeadersRow();

  constexpr int row_count = 10;
  for (int row = 0; row < row_count; row++)
  {
    ImGui::TableNextRow();
    for (int column = 0; column < column_count; column++)
    {
      ImGui::TableNextColumn();
      // Just some random contents that varies significantly from cell to cell.
      ImGui::Text("%d ^ %d = %d", column, row, static_cast<int>(pow(column, row)));
    }
  }
  ImGui::EndTable();
}

ImGui::End();
@ocornut
Copy link
Owner

ocornut commented Apr 16, 2021 via email

@ArekSredzki
Copy link
Author

Thanks for the speedy response as always @ocornut :)
As a stop-gap, I'm considering showing/hiding columns programmatically as you suggest. However, since that has its own complications, it'd be nice to fix it directly.

@ArekSredzki
Copy link
Author

ArekSredzki commented Apr 16, 2021

FYI, The same/similar issue occurs with ImGuiTableFlags_SizingStretchProp or ImGuiTableFlags_SizingFixedFit instead of ImGuiTableFlags_ScrollX

ocornut added a commit that referenced this issue Apr 16, 2021
…t changes. (#4046) + .ini skips columns with no data.
@ocornut
Copy link
Owner

ocornut commented Apr 16, 2021

Pushed fix 770f9da
Not very well exercised but the width restoration now has tests for this particular path.

I didn't attempt to preserve column order on column count change as it may be a more trickier (could be investigated later).

if (ImGui::Begin("Test #4046"))
{
    // Add a new column every 0.5s, up to a max of 10 columns before rolling over back to 1.
    double seconds_per_new_column = 0.5f;
    int max_num_columns = 10;
    const int column_count = static_cast<int>(ImGui::GetTime() / seconds_per_new_column) % max_num_columns + 1;

    if (ImGui::BeginTable("Variable Column Count Table", column_count,  ImGuiTableFlags_Borders | ImGuiTableFlags_ScrollX))
    {
        char label[32];
        for (int column = 0; column < column_count; column++)
        {
            sprintf(label, "Column %d", column);
            ImGui::TableSetupColumn(label);
        }
        ImGui::TableHeadersRow();

        int row_count = 10;
        for (int row = 0; row < row_count; row++)
        {
            ImGui::TableNextRow();
            for (int column = 0; column < column_count; column++)
            {
                ImGui::TableNextColumn();
                // Just some random contents that varies significantly from cell to cell.
                ImGui::Text("%d ^ %d = %d", column, row, (int)powf((float)column, (float)row));
            }
        }
        ImGui::EndTable();
    }
    ImGui::End();
}

@ocornut ocornut closed this as completed Apr 16, 2021
@ArekSredzki
Copy link
Author

@ocornut Thank you so much for the prompt fix! Would you mind merging it into the docking branch? Our build setup pulls ImGui from GitHub directly so having a merged commit hash would be very helpful :)

@ocornut
Copy link
Owner

ocornut commented Apr 16, 2021

Sorry I did a docking update yesterday, if you need a temporary commit hash you can fork + cherry-pick + push.

@ArekSredzki
Copy link
Author

No worries, I wasn't sure how frequently you usually update the branch.

The fix works nicely, though it would be nice for the column width to be initialized with at least the column name.
That would make it less jarring when a new column is added.
Would that be difficult to incorporate?

@ocornut
Copy link
Owner

ocornut commented Apr 16, 2021 via email

@ArekSredzki
Copy link
Author

I understand that doing it generically is difficult, but it would be nice to at least use an approximate version of the correct size rather than zero.

My thinking is that the min-width could be computed from the text size of the column label provided to TableSetupColumn.

However, being able to explicitly provide a minimum column width would be valuable. Perhaps I missed it but I didn't see a method of doing so in the current API.

@ArekSredzki
Copy link
Author

@ocornut Would it be reasonable for me to open a new ticket requesting the enhancements described in my previous comment or is there an existing method for achieving this? :)

@ocornut
Copy link
Owner

ocornut commented Apr 21, 2021

but it would be nice to at least use an approximate version of the correct size rather than zero.

Outside of your specific use (appending new columns while table a showing):

  • You'd still get moving column in the case of a table appearing after host window is appearing.
  • Because settings are saved this only happens once.
    Your use case is a little more specific because we do ditch those settings when resizing column count. If you simply hidden/shown column you wouldn't have this problem as much.

My thinking is that the min-width could be computed from the text size of the column label provided to TableSetupColumn.

In the case of a stretched down table this wouldn't be an adequate use.
That also wouldn't account for e.g. Sort markers.

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