Minor issue with table clip rect

[BobbyAnguelov]

Really minor thing but for tables that have vertical scrolling the table contents are drawn over the borders.

If you run the examples and go to the tables section in the demo and look at the "vertical scrolling, with clipping" example, you can see the issue.

Zoomed in for more clarity:

[v-ein]

Try to replace ImGuiTableFlags_BordersOuter with ImGuiTableFlags_BordersOuterH and see if the issue persists. I bet it will only happen when you have both horizontal and vertical outer borders. That's because the table is optimized to render such borders as a rectangle, but the rectangle gets wrong coordinates :).

[v-ein]

BTW the issue might become more of a nuisance when you place opaque widgets into the table - they get painted over the borders, making the right-side and bottom borders piecewise... depending on the contents, might look pretty ugly.

[v-ein]

So... to make it clear - here's how the table renders its outer borders:

if ((table->Flags & ImGuiTableFlags_BordersOuter) == ImGuiTableFlags_BordersOuter)
{
    inner_drawlist->AddRect(outer_border.Min, outer_border.Max, outer_col, 0.0f, 0, border_size);
}
else if (table->Flags & ImGuiTableFlags_BordersOuterV)
{
    inner_drawlist->AddLine(outer_border.Min, ImVec2(outer_border.Min.x, outer_border.Max.y), outer_col, border_size);
    inner_drawlist->AddLine(ImVec2(outer_border.Max.x, outer_border.Min.y), outer_border.Max, outer_col, border_size);
}
else if (table->Flags & ImGuiTableFlags_BordersOuterH)
{
    inner_drawlist->AddLine(outer_border.Min, ImVec2(outer_border.Max.x, outer_border.Min.y), outer_col, border_size);
    inner_drawlist->AddLine(ImVec2(outer_border.Min.x, outer_border.Max.y), outer_border.Max, outer_col, border_size);
}

You see, when both borders are enabled, the border is rendered as a rectangle with bottom-right corner in outer_border.Max - only that the bottom-right corner is exclusive (contrary to top-left which is inclusive), and therefore outer_border.Max actually lies outside of the rectangle frame. When only horizontal or only vertical borders are enabled, the line is drawn through outer_border.Max, making it inclusive. This causes a 1-pixel offset for right and bottom sides, depending on which flags are used.

This seems to render the borders properly:

inner_drawlist->AddRect(outer_border.Min, outer_border.Max + ImVec2(1, 1), outer_col, 0.0f, 0, border_size);

Note: I've only tested this on Windows and not sure how it behaves on other backens, including high-DPI screens.

[ocornut]

Hello,

I investigated this and your fix @v-ein seems right, pushed it as a340718. Thank you!

Apologies I didn't investigate this and #3752 earlier: some of the border code in tables is subtly tricky because it closely relates to cliprect matching and ImDrawCmd merging constraints. However making this specific changes passes my regression tests in ImGuiTestSuite and didn't alter ImDrawCmd count.

While investigating this I also spotted another long-time bug there in some situations the top-most outer border would be drawn twice, once with TableBorderLight (incorrectly), once with TableBorderStrong (correctly). The earlier would show underneath when TableBorderStrong has alpha under 1.0f. This is fixed with 5a483c2.

One remaining subtle issue that CellBg would be displayed underneath the left-most and top-most border, again only visible when TableBorderStrong.a < 1.0f.

[BobbyAnguelov]

Would the remaining issues account for this?

It basically still draws the cell contents on top of the border in the example app.

It's not so bad in the demo app, but it my case (with my theme colors), it gives the impression a bunch of my UI is kinda broken:

[v-ein]

I believe it's a different issue. Horizontal borders are drawn within the cells, with top border of each row being potentially overlaid by cell contents. In most cases cell padding prevents contents from overlaying the border. However, if you set vertical padding to zero, or like in your case, scroll the table, there's a good chance that the border gets painted over.

In contrast to that, vertical borders are rendered outside the cells, i.e. the layout mechanism does account for space taken by borders between columns and by vertical sides of the outer border. Therefore they are not overlaid by contents.

Here's a picture I recently made explaining the difference between how vertical and horizontal borders are rendered:

Here, every cell contains a button (either "Lorem ipsum" or just "X"), and I modify padding a little. Item spacing is also set to 0, 0 to demonstrate overlapping with subsequent widgets (the button labeled "More test").

[ocornut]

@v-ein Your explanation was not the reason for this.

Unfortunately it is/was more complicated and had to do a rather long deep-dive to come with the right fix. As mentioned in #6765 (comment) for those two borders there is the additional complexity related to how draw commands are merged related to clipping.

If you alter the HostClipRect (offset by border size) then after the TableMergeDrawChannels() channel the affected columns's ClipRect won't match parent ClipRect and won't be merged so that's a waste. If you alter InnerClipRect then when draw command merging occurs contents can now stray off expected rectangle. As the problem can only occur on scrolling tables however the first solution can work if only applied in this case. This is my fix a8bdbfd, I'm hoping it doesn't break anything, it is passing tests including "table_draw_calls" but arguably this one is largely incomplete.

The "dumb" workaround would be to offset the border itself to be e.g. 1 px above and left of the contents, but that would be visibility inconsistent in term of layout and possibly even more visible than original bug in many condition.

On the way to solve this I also stumbled and fixed a random unrelated issue 8db02ef where in some instance a draw commands was wasted with ScrollX enabled.

(PS: not merging into docking before a few days to reduce merge-commit spam but I verified it does cherry-pick in docking.)

[ocornut]

The previous fix caused an issue #7428

I've reverted it (revert: a131c3e) and I'm proposing another fix (864a2bf), you can see me here toggling the change on/off (after the revert which is ):

Toggling the change on/off before the revert (so you can see the line moving to)

It's unfortunately a little bit complicated for many reasons so I'm hoping I didn't break anything else, but I want a tested with that toggle in many areas and it seems generally betters, and it passes my tests for draw-call matching/merging.
Hope I did get it right this time!