Pixel-based Clipper in Table: Assertion during scrolling

[bratpilz]

Version/Branch of Dear ImGui:

Version 1.92.2b (19222), Branch: master

Back-ends:

imgui_impl_glfw.cpp + imgui_impl_opengl3.cpp

Compiler, OS:

Linux + GCC

Full config/build information:

Dear ImGui 1.92.2b (19222)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=201103
define: __linux__
define: __GNUC__=12
--------------------------------
io.BackendPlatformName: imgui_impl_glfw (3308)
io.BackendRendererName: imgui_impl_opengl3
io.ConfigFlags: 0x00000003
 NavEnableKeyboard
 NavEnableGamepad
io.ConfigNavCaptureKeyboard
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000001E
 HasMouseCursors
 HasSetMousePos
 RendererHasVtxOffset
 RendererHasTextures
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,128
io.Fonts->FontLoaderName: stb_truetype
io.DisplaySize: 1280.00,800.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

Details:

My Issue/Question:
I have a table where the rows are different heights. In addition, the table contains many such rows, so it is important to use the ListClipper to limit the rows that get drawn to just the visible ones (plus sometimes the one that holds the Nav focus).
To accomplish this, I set items_height to 1, and thus use the ListClipper "in pixel mode" instead of the normal "element mode", so to speak.
That then means that DisplayStart/DisplayEnd represent a range of pixels instead of a range of rows/elements.
Since for example DisplayStart could be in the middle of an element, I have to correct the cursor y position in that case (included for completeness, the assertion happens even without this step).
Since this commit, I get an assertion while scrolling in my table while an earlier row has the Nav focus:

imgui_tables.cpp:2068: void ImGui::TableEndRow(ImGuiTable*): Assertion `table->IsUnfrozenRows == false' failed.

The reason for this is that "row_increase" can turn negative in this special pixel-based setup. The code assumes that item_height is the same as the height of the rows in the table, and that they are of uniform height in the first place. The negative value of row_increase then gets added onto Table->CurrentRow, which then causes us to arrive at CurrentRow == 0 multiple times in the same frame, which in turn causes the assertion to trigger.

I know this is a very special use case. Is there maybe an easier/better way to perform this kind of pixel based clipping in a table?

Screenshots/Video:

UnfrozenRows.mp4

Minimal, Complete and Verifiable Example code:

// Here's some code anyone can copy and paste to reproduce your issue
	{
	  using namespace ImGui;

	  static bool do_cursor_correction = true;

	  auto TableClipperMoveCursorY = [&] (int dy) -> int
	    {
	      if (!do_cursor_correction)
		return 0;
	      auto tbl = GetCurrentTable();
	      auto pos = GetCursorScreenPos();
	      pos.y += dy;
	      // We need to modify both the cursor position and RowPosY2, just like the
	      // clipper does (see ImGuiListClipper_SeekCursorAndSetupPrevLine).
	      SetCursorScreenPos (pos);
	      tbl->RowPosY2 = pos.y;
	      return dy;
	    };

	  SetNextWindowSize (ImVec2 (320.0f, 400.0f), ImGuiCond_Appearing);
	  Begin ("Test");
	  Checkbox ("Cursor correction", &do_cursor_correction);
	  if (BeginTable ("Table", 2, ImGuiTableFlags_ScrollY))
	    {
	      TableSetupColumn ("Idx");
	      TableSetupColumn ("Elem");
	      TableHeadersRow();

	      // Set up 50 rows, with individual heights [20, 21, 22, ..., 69].
	      static int const N = 50;
	      struct { int idx, y0, y1; } elems[N];
	      int total_height = 0;
	      for (int i = 0; i < N; ++i)
		{
		  auto &elem = elems[i];
		  elem.idx = i;
		  elem.y0 = total_height;
		  elem.y1 = elem.y0 + 20 + i;
		  total_height = elem.y1;
		}

	      // Find first item that overlaps a y coordinate.
	      auto elem_at_y = [&] (float y) {
		int idx = 0;
		while (idx < N && elems[idx].y1 < y)
		  ++idx;
		return idx; };

	      ImGuiListClipper clipper;
	      clipper.Begin (total_height, 1); // "items_height" is 1 because we are clipping by pixels instead of rows.
	      int max_pixel = 0;
	      while (clipper.Step())
		{
		  // DisplayStart and DisplayEnd represent a pixel range in the y axis.
		  // Both of them probably fall in the middle of an elem instead of directly at the top/bottom of it.
		  // That means that we have to adjust the cursor position accordingly (see TableClipperMoveCursorY).
		  // If we do not do this, we can only scroll entire rows at a time (uncheck "Cursor correction" to see this in action).
		  int start_pixel = clipper.DisplayStart;
		  if (start_pixel < max_pixel)
		    start_pixel += TableClipperMoveCursorY (max_pixel - start_pixel);
		  if (start_pixel < clipper.DisplayEnd)
		    {
		      int idx = elem_at_y (start_pixel);
		      int pixel = start_pixel;
		      while (idx < N)
			{
			  auto &elem = elems[idx];
			  if (elem.y0 >= clipper.DisplayEnd)
			    break;
			  if (elem.y0 != pixel)
			    pixel += TableClipperMoveCursorY (elem.y0 - pixel); // Move cursor up to start of elem.
			  int height = elem.y1 - elem.y0;
			  TableNextColumn();
			  Text ("%u (%u px)", elem.idx, height);
			  TableNextColumn();
			  Dummy (ImVec2 (100, height));
			  GetForegroundDrawList()->AddRect (GetItemRectMin(), GetItemRectMax(), IM_COL32 (255, 0, 255, 255));
			  pixel = elem.y1;
			  ++idx;
			}
		      max_pixel = pixel;
		    }
		}
	      clipper.End();
	      EndTable();
	    }
	  End();
	}

[ocornut]

I know this is a very special use case.

Thanks for your report and post. I am generally willing to support this use case.
I will investigate.

The code assumes that item_height is the same as the height of the rows in the table, [...]

Right.
Before the row_increase change you outlined, did things work alright for you?
I would assume that row freezing and row bg did not work correctly with this scheme?

Side notes:

• linking to Wrong row returned in TableGetHoveredRow() #7350, is there a way to know when a table cell is hovered? #6588, Need a better way to detect when hovering over table cells #6250 for reference.
• you could call ImGuiListClipper::SeekCursorForItem() in theory, tho that won't solve your issue.

[ocornut]

I initially wanted to make the clipper and table code more resilient but eventually it seemed foolish and misleading, at core the issue it indeed the assumption that 1 clipper item == 1 table row.

With aa2f40c

• I have added better asserts to detect this issue.
• Introduced a ImGuiListClipperFlags_NoSetTableRowCounters flag. It's tagged as supposedly internal because there's no neat API to set it yet, but you can simply do clipper.Flags |= ImGuiListClipperFlags_NoSetTableRowCounters; before clipper.Begin().

[bratpilz]

Thanks! Using this new flag prevents the assert as expected.
The frozen rows feature has in fact been working the whole time (I use 1 frozen row filled with custom header items).
I hadn't enabled the row coloring before, but I just tried it out and it in fact does not work with this use case, as expected. The color of the rows changes while scrolling, since the row index can't be relied upon.

[ocornut]

Right, come to think of it, the frozen row would work as they bypass the clipper.

You can override the internal counter used for RowBg coloring if you like.

[ocornut]

I'll close this as solved for now but don't hesitate to reply here if you have more issues or questions.