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: Angled headers: fix border position, hit box, and overflowing contents in some cases #7416

Closed
wants to merge 3 commits into from

Conversation

cfillion
Copy link
Contributor

@cfillion cfillion commented Mar 18, 2024

Bug 1: Border hit box extends beyond non-scrollable tables. table->AngledHeadersHeight was added to the top and bottom coordinates instead of just the top.

Hit box too big

Bug 2: Borders remain offset after the user stops calling TableAngleHeadersRow. table->AngledHeadersHeight was not reset.

Border Y not reset

Bug 3: Table contents overflow the table's outer box when an angled header is used in combination with a list clipper. Angled header's row height could have a fractional part, while this line in ItemSize truncates window->DC.CursorMaxPos.y, leading to accumulating error at each subsequent row.

The root issue remains if the user does eg. ImGui::TableNextRow(0, 78.396423);, but this fixes the case with TableAngleHeadersRow.

Overflowing contents

This also improves rendering quality because header row's border lines are a bit fuzzy with a fractional part.

Before After

Duplication code

auto table = [] (const char *id, const bool angled, const float angle) {
  ImGuiStyle &style { ImGui::GetStyle() };
  const float oldAngle { style.TableAngledHeadersAngle };
  style.TableAngledHeadersAngle = angle * (IM_PI / 180.0f); // should be a StyleVar...
  if(!ImGui::BeginTable(id, 3, ImGuiTableFlags_Borders | ImGuiTableFlags_Resizable)) {
    style.TableAngledHeadersAngle = oldAngle;
    return;
  }
  ImGui::TableSetupColumn("Lorem Ipsum",    ImGuiTableColumnFlags_AngledHeader);
  ImGui::TableSetupColumn("Dolor Sit Amet", ImGuiTableColumnFlags_AngledHeader);
  ImGui::TableSetupColumn("Foo Bar Baz",    ImGuiTableColumnFlags_AngledHeader);
  if(angled)
    ImGui::TableAngledHeadersRow();
  ImGui::TableHeadersRow();
  for(int row = 0; row < 8; ++row) {
    ImGui::TableNextRow();
    for(int column = 0; column < 3; ++column) {
      ImGui::TableNextColumn();
      ImGui::Text("%d.%d", row, column);
    }
  }
  ImGui::EndTable();
  style.TableAngledHeadersAngle = oldAngle; // should be PopStyleVar
};

ImGui::Begin("Hello, world!");

static bool angled1 { true };
ImGui::Checkbox("Angled header##1", &angled1);
table("table1", angled1, 50.f);

static bool angled2 { true };
ImGui::Checkbox("Angled header##2", &angled2);
table(io.KeyShift ? "table1" : "table2", angled2, 35.f);

// (the following is not fixed by this PR)
if(ImGui::BeginTable("table3", 3, ImGuiTableFlags_Borders)) {
  ImGui::TableNextRow(0, 78.396423); // a possible value used by TableAngledHeadersRowEx
  ImGui::TableNextColumn();
  ImGui::Text("Foo");
  ImGuiListClipper clipper;
  clipper.Begin(32);
  while(clipper.Step()) {
    for(int row { clipper.DisplayStart }; row < clipper.DisplayEnd; ++row) {
      ImGui::TableNextRow();
      for(int column {}; column < 3; ++column) {
        ImGui::TableNextColumn();
        ImGui::Text("%.02fx%.02f", ImGui::GetCursorPosX(), ImGui::GetCursorPosY());
      }
    }
  }
  ImGui::EndTable();
}
ImGui::Text("After table");

ImGui::End();

ocornut pushed a commit that referenced this pull request Mar 26, 2024
ocornut pushed a commit that referenced this pull request Mar 26, 2024
ocornut pushed a commit that referenced this pull request Mar 26, 2024
@ocornut
Copy link
Owner

ocornut commented Mar 26, 2024

Thank you Christian for the quality PR and explanations. I've merged all three commits now.

About the clipper rounding issue, mapping the rowHeight to a slider e.g. SliderFloat("rowHeight", &rowHeight, 78.0f, 79.0f); facilitate visualizing the issue.
It mostly manifests in:

static void ImGuiListClipper_SeekCursorForItem(ImGuiListClipper* clipper, int item_n)
{
    // StartPosY starts from ItemsFrozen hence the subtraction
    // Perform the add and multiply with double to allow seeking through larger ranges
    ImGuiListClipperData* data = (ImGuiListClipperData*)clipper->TempData;
    float pos_y = (float)((double)clipper->StartPosY + data->LossynessOffset + (double)(item_n - data->ItemsFrozen) * clipper->ItemsHeight);
    ImGuiListClipper_SeekCursorAndSetupPrevLine(pos_y, clipper->ItemsHeight);
}

Where the rounding is not taken into account.

I have a bit of an issue right now changing that function because I think support for non-float should probably indeed be done elsewhere (e.g. in ItemSize ItemAdd functions), so I'll work on that later.

Thanks a lot!

(PS: I have to cherry-pick and amend with changelog etc, in vast majority of PR i don't expect user to fill in changelog as it would conflicts or be misplaced if not merged right away, so that's normal), but as I result actual PR is not marked as merged but all commits are rightly attributed to you)

@ocornut ocornut closed this Mar 26, 2024
pull bot pushed a commit to TeamREPENTOGON/imgui that referenced this pull request Mar 27, 2024
pull bot pushed a commit to TeamREPENTOGON/imgui that referenced this pull request Mar 27, 2024
pull bot pushed a commit to TeamREPENTOGON/imgui that referenced this pull request Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants