QTableView: set correct clip on QPainter when using spans

As it stood, QTableView would sometimes show drawing
artifacts when sections (rows or columns) where
rearranged, in combination with spans.

The reason is that the current code would go through the
currently visible cells in the viewport to check if any of
them where affected by a span. But the stored spans (in
QSpanCollection) is kept in sync with the column layout in
the model, and doesn't know anything about the rearranged
columns in the view. But a column with spans that is moved
to the left of the viewport can affect the painting of
the viewport as well, and needs to be taken into consideration.
For that reason, the current solution that uses
QSpanCollection::spanAt(x, y), will in that case fail.

Since it seems sensible that QSpanCollection is kept in sync with
the model (it makes model changes that affect spans easier to
handle), this patch will not change QSpanCollection. Instead, it
will iterate through all the spans (which is assumed to normally be
a limited size), convert them to the column layout of the view
(visual instead of logical), and find the ones that overlap.
Overlapping spans will, like before, be added as clip rects
to the QPainter.

Fixes: QTBUG-91896
Pick-to: 6.4 6.3 6.2
Change-Id: Iabe20df13cbd41a68f1104d3d9e24b89c4b88913
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

1 files changed, 23 insertions, 4 deletions

diff --git a/src/widgets/itemviews/qtableview.cpp b/src/widgets/itemviews/qtableview.cpp
index 520a6f3afc..2f46842712 100644
--- a/src/widgets/itemviews/qtableview.cpp
+++ b/src/widgets/itemviews/qtableview.cpp
@@ -882,10 +882,29 @@ void QTableViewPrivate::drawAndClipSpans(const QRegion &area, QPainter *painter,
         visibleSpans = spans.spansInRect(logicalColumn(firstVisualColumn), logicalRow(firstVisualRow),
                                          lastVisualColumn - firstVisualColumn + 1, lastVisualRow - firstVisualRow + 1);
     } else {
-        for(int x = firstVisualColumn; x <= lastVisualColumn; x++)
-            for(int y = firstVisualRow; y <= lastVisualRow; y++)
-                visibleSpans.insert(spans.spanAt(x,y));
-        visibleSpans.remove(nullptr);
+        // Any cell outside the viewport, on the top or left, can still end up visible inside the
+        // viewport if is has a span. Calculating if a spanned cell overlaps with the viewport is
+        // "easy" enough when the columns (or rows) in the view are aligned with the columns
+        // in the model; In that case you know that if a column is outside the viewport on the
+        // right, it cannot affect the drawing of the cells inside the viewport, even with a span.
+        // And under that assumption, the spansInRect() function can be used (which is optimized
+        // to only iterate the spans that are close to the viewport).
+        // But when the view has rearranged the columns (or rows), this is no longer true. In that
+        // case, even if a column, according to the model, is outside the viewport on the right, it
+        // can still overlap with the viewport. This can happen if it was moved to the left of the
+        // viewport and one of its cells has a span. In that case we need to take the theoretically
+        // slower route and iterate through all the spans, and check if any of them overlaps with
+        // the viewport.
+        const auto spanList = spans.spans;
+        for (QSpanCollection::Span *span : spanList) {
+            const int spanVisualLeft = visualColumn(span->left());
+            const int spanVisualTop = visualRow(span->top());
+            const int spanVisualRight = spanVisualLeft + span->width() - 1;
+            const int spanVisualBottom = spanVisualTop + span->height() - 1;
+            if ((spanVisualLeft <= lastVisualColumn && spanVisualRight >= firstVisualColumn)
+                    && (spanVisualTop <= lastVisualRow && spanVisualBottom >= firstVisualRow))
+                visibleSpans.insert(span);
+        }
     }
 
     for (QSpanCollection::Span *span : qAsConst(visibleSpans)) {