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

Clipping for differing row heights #6042

Open
JJCUBER opened this issue Jan 3, 2023 · 10 comments
Open

Clipping for differing row heights #6042

JJCUBER opened this issue Jan 3, 2023 · 10 comments
Labels

Comments

@JJCUBER
Copy link
Contributor

JJCUBER commented Jan 3, 2023

Version/Branch of Dear ImGui:

Version: 1.89.1
Branch: master

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_sdl.cpp + imgui_impl_opengl3.cpp (using GLEW)
Operating System: Windows 10

My Issue/Question:

I am trying to implement clipping behavior for a table with rows of different heights. (Some of my rows are single-line and some are multi-line; I would like to avoid pagination, if possible.) From what I can tell, the ImGuiListClipper assumes that all the rows are the same height.

Is there a reasonable way to emulate some sort of clipper for my requirements with the current tools Dear ImGui provides (without rewriting a lot of what Dear ImGui does under the hood)? I was thinking of doing something along the lines of caching the size/geometry information of each row, eliminating the need for Dear ImGui to recalculate it every time and avoiding extra conversions of my data to string representations (I would store this size/geometry information with each of my rows and update it for a given row whenever it is modified); however, I am unsure how I would go about "providing" that information to Dear ImGui.

Simple Screenshot of the Differing Heights:

image

@ocornut ocornut changed the title Table Clipping for differing row heights Clipping for differing row heights Jan 3, 2023
@ocornut ocornut added the clipper label Jan 3, 2023
@ocornut
Copy link
Owner

ocornut commented Jan 4, 2023

Here how I would approach it:

  • You need to know the height of your data-set.
  • Use ImGuiListClipper::Begin() with items_count = total_height, and items_height = 1.0f
  • From that point. the clipper will give you indices in pixels unit instead of items unit.
  • You'll need a way to provide random access (from a Y pixel position) into your data set, which can be done with "seeking" forward. Depending on your data-set set, caching the height of each entry + seeking into a Y position may be enough.

Pseudo-code

// Keep state to incrementally seek ahead
int item_current = 0;
float item_current_y1 = 0.0f;

clipper.Begin(data_set.total_height, 1.0f);
while (clipper.Step())
{
    float pos_min_y = (float)clipper.DisplayStart;
    float pos_max_y = (float)clipper.DisplayStart;
    int item_min = data_set.SeekToPos(pos_min_y, &item_current, &item_current_y1); // Advance item_current and item_current_y1 until reaching pos_min_y
    int item_max = data_set.SeekToPos(pos_max_y, &item_current, &item_current_y1); // Advance item_current and item_current_y1 until reaching pos_max_y
    for (int item_n = item_min; item_n < item_max; item_n++)
    {
        // Display item
    } 
}
clipper.End();

@JJCUBER
Copy link
Contributor Author

JJCUBER commented Jan 4, 2023

To get the height of a given row (in order to cache it on the first passthrough), I have tried looking into a couple variables/functions, but none of them seem to give me noticeably different numbers for taller rows. Am I misusing them and/or looking at the wrong things? (I've tried calling these before and after creating the next row.)

I've tried:

// not the full code, just the relevant parts I've looked at
ImGuiContext& g = *GImGui;
ImGuiTable* table = g.CurrentTable;
float height = table->RowPosY2 - table->RowPosY1;
float height = ImGui::GetFrameHeight();
float height = ImGui::TableGetCellBgRect(table, 3).GetHeight();

@ocornut
Copy link
Owner

ocornut commented Jan 5, 2023

It’s not possible to obtain a cell height before the contents has been submitted. It’d be more natural that you compute the height of your text/content + add CellPadding.y to it.

@JJCUBER
Copy link
Contributor Author

JJCUBER commented Jan 6, 2023

What I've landed on is something like this for calculating the height:

ImGuiContext& g = *GImGui;
ImGuiTable* table = g.CurrentTable;
ImGuiWindow* window = g.CurrentWindow;

const float wrapWidth = ImGui::CalcWrapWidthForPos(window->DC.CursorPos, window->DC.TextWrapPos);
const ImVec2 textSize = ImGui::CalcTextSize(s.c_str(), s.c_str() + s.size(), false, wrapWidth);

// alternate way to calculate?  (index is hard-coded for testing)
const ImVec2 sameTextSize = ImGui::CalcTextSize(s.c_str(), s.c_str() + s.size(), false, table->Columns[3].WidthGiven);

// I will eventually be calculating this on every cell in a row and storing the max of them, along with a cumulative sum or absolute position
const float absolutePosition = window->DC.CursorPos.y + textSize.y + table->CellPaddingY * 2;

From what I can tell, this does properly give the absolute starting position of the next row. (Once I get the chance, I will try integrating this with my data structures and the pseudo-code you gave above. I will also try sharing relevant parts of my final code once it works; this way, it can serve as pointers for anyone else down the line who wants to do something similar to this.)

My main question is this:

Should ImGui::CalcWrapWidthForPos(window->DC.CursorPos, window->DC.TextWrapPos) and table->Columns[3].WidthGiven always give the same value (assuming they are called in the same cell)? Should one be preferred over the other?

@JJCUBER
Copy link
Contributor Author

JJCUBER commented Jan 13, 2023

I am running into an issue while doing something along the lines of your pseudocode. Rows at the top of the view always render in their entirety, so getting to the bottom of the table can cause the last row or two to not be visible if there are some taller rows before it. Similarly, having a tall row as the first row visible means that scrolling does nothing until it is completely out of view.

Basically, I am having issues rendering only part of the first visible row when scrolling (while using this clipper method). Is there a way to solve this issue? (Everything else seems to be working well aside from this, even with a massive amount of rows.)


Edit:

I still need to do more testing, but I believe this seems to solve it for the most part(?):

if(i == startI)
    ImGui::SetCursorPosY(ImGui::GetCursorPosY() - (ImGui::GetScrollY() - rowStart));

I've found that this has to get called on every cell in the first visible row; this basically offsets everything else based on the scroll.

Unfortunately, the alternating colors of the rows don't stay consistent while scrolling and it ends up flickering back and forth. I fixed this with:

ImGui::TableSetBgColor(ImGuiTableBgTarget_RowBg0, ImGui::GetColorU32(i % 2 ? ImGuiCol_TableRowBgAlt : ImGuiCol_TableRowBg));

(where i is the current absolute row, as in, including rows outside the clipper range)

There also seems to be some weird behavior of the clipper; after a selectable has been selected at any point in time, the clipper will submit an extra region/iteration of the while loop when scrolling past said selectable (even after unselecting it). It will keep submitting the same erroneous region which is near where the selectable was, but it does not include the row of the selectable in its range. When clicking on a new selectable, it will stop doing this until said new selectable is out of view. This feels more like a bug in the interaction between the clipper and selectables, as opposed to some bug in my code. I am still in the process of converting this specific project to the latest release of Dear ImGui, so it is possible that it has already been fixed. I will do some more testing.


Edit 2:

This issue seems to be caused by this in the source code of the clipper:

            // Add focused/active item
            ImRect nav_rect_abs = ImGui::WindowRectRelToAbs(window, window->NavRectRel[0]);
            if (g.NavId != 0 && window->NavLastIds[0] == g.NavId)
                data->Ranges.push_back(ImGuiListClipperRange::FromPositions(nav_rect_abs.Min.y, nav_rect_abs.Max.y, 0, 0));

I have tried skipping the extra range given and it does work, but I am having trouble determining which range is the extra one.
(Temporarily setting g.navId to 0 while stepping the clipper also works?) This also feels very hacky and likely has some unintended side-effects.

Sorry for all of the edits and posts, but I want to provide as many details as possible for resolving this.

@JJCUBER
Copy link
Contributor Author

JJCUBER commented Jan 18, 2023

@ocornut Sorry to bother you again, but I was wondering if you happened to have any insight on this (regarding the Dear ImGui clipper range getting submitted for off-screen focus being incorrect and/or whether my way of "disabling" it is valid).

(I am also in the process of creating a relevant code outline of how I ended up implementing everything; I will post this once I have cleaned it up so that others with similar clipper wants/needs can use/take inspiration from it.)

If trying to help resolve this would take up too much time, I understand.

@JJCUBER
Copy link
Contributor Author

JJCUBER commented Jan 19, 2023

This is the gist of the relevant code for anyone else wanting a clipper which handles rows of differing heights:

  • Relevant structs and functions
template <class T>
struct RowHeight
{
    float height;
    float cumulativeHeight;
    const T& row;
};

bool hasTableResized(float prevColumnWidths[], int columnCt)
{
    ImGuiTable* table = ImGui::GetCurrentTable();

    bool hasResized{};
    for (int i = 0; i < columnCt; i++)
    {
        const float currentWidth = table->Columns[i].WidthGiven;
        if (prevColumnWidths[i] != currentWidth)
            hasResized = true;
        prevColumnWidths[i] = currentWidth;
    }

    return hasResized;
}
  • Rendering
// shouldRecalcClipper is a variable you might want to set in the event that you have (re)filtered or (re)sorted your table
if (hasTableResized(prevColumnWidths, columnCt) || shouldRecalcClipper)
{
    shouldRecalcClipper = false;

    rowHeights.clear();

    ImGuiTable* table = ImGui::GetCurrentTable();

    float cumulativeHeight{};
    for (const TableType& row : rows)
    {
        // this assumes that your table type has a member variable that keeps the state of whether a given row is filtered out; feel free to remove/modify this if it isn't applicable to your use case
        if (row.isFilteredOut)
            continue;
        
        float maxHeight{};
        // I start this at 1 since my id column is skipped (I render it separately as a selectable; you might need to change the code slightly depending on how you handle it)
        int col{1};

        // Convert each of your cells in a row to its string representation, then run this on each cell of the current row (I use a lot of lambda passthroughs to make my code very generic/reusable, but that would probably be hard to interpret and doesn't apply to how most people will likely handle this)
        const std::string& s = /* your string representation */;
        const char* cStr = s.c_str();
        float height = ImGui::CalcTextSize(cStr, cStr + s.size(), false, prevColumnWidths[col++]).y + table->CellPaddingY * 2;
        if (height > maxHeight)
            maxHeight = height;
        
        cumulativeHeight += maxHeight;

        rowHeights.emplace_back(maxHeight, cumulativeHeight, row);
    }
}

// nothing to draw
if (rowHeights.empty())
    return;

ImGuiListClipper clipper;
clipper.Begin((int)rowHeights.back().cumulativeHeight, 1.0f);

// hacky way to disable clipper.Step() submitting a range for an offscreen row that has focus
ImGuiContext& g = *ImGui::GetCurrentContext();
ImGuiID navId = g.NavId;
g.NavId = 0;

while(clipper.Step())
{
    float minY = (float)clipper.DisplayStart;
    float maxY = (float)clipper.DisplayEnd;

    const auto startRowIt = std::lower_bound(rowHeights.begin(), rowHeights.end(), minY, [](const RowHeight<TableType>& rowHeight, float f) { return f > rowHeight.cumulativeHeight; });
    int startI = startRowIt - rowHeights.begin();

    for (int i = startI; i < rowHeights.size() && (!i || maxY > rowHeights[i - 1].cumulativeHeight); i++)
    {
        ImGui::TableNextRow();

        ImGui::TableNextColumn();

        float rowStart = rowHeights[i].cumulativeHeight - rowHeights[i].height;
        if (i == startI)
            // this MUST be called after every ImGui::TableNextColumn() call in the first row, not just for the first column
            ImGui::SetCursorPosY(ImGui::GetCursorPosY() - (ImGui::GetScrollY() - rowStart));

        ImGui::TableSetBgColor(ImGuiTableBgTarget_RowBg0, ImGui::GetColorU32(i % 2 ? ImGuiCol_TableRowBgAlt : ImGuiCol_TableRowBg));


        /* Handle rendering your cells for the current row here */
    }
}

g.NavId = navId;

@ocornut
Copy link
Owner

ocornut commented Dec 21, 2023

Also see #3823

@Stefan13-13
Copy link

Stefan13-13 commented Jan 11, 2024

Thanks you @JJCUBER
I'm new to ImGui and trying to build something.

How did you define rowHeights (the type) in your code above, like: std::vector<RowHeight<rowType>>? But what about rowType, is it required or can it be omitted?
And how can I easily get prevColumnWidths from my table?

@JJCUBER
Copy link
Contributor Author

JJCUBER commented Feb 6, 2024

@Stefan13-13
Sorry for the delayed response, I didn't see the notification.

  1. It's defined as std::vector<RowHeight<TableType>>& rowHeights (it's templated).
  2. I'm unsure what you mean by if it can be omitted. You can have it be a fixed type (instead of templated) if you only need to worry about one type of table.
  3. prevColumnWidths is just a simple array of floats which are calculated/updated within hasTableResized(). To get them, you would just call the function (to make sure they are up to date), then you can access the given index of the array.

Do note that I wrote this a very long time ago, so I can't make any guarantee about how well it works on the latest version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants