Fix rasterization errors in aliased cosmetic drawPolyLine

If a cosmetic polyline contained a 1-pixel segment, the next segment
would be drawn with wrong starting point.

The original fix for QTBUG-26156 had some unwanted side effects
(QTBUG-31579 and now QTBUG-42398). It tried to skip start-point update
if stroke() did not actually draw anything (because the segment was
too small). However, to determine that, it tested for a change in
lastPixel. But that was not failsafe; in some cases (1-pixel segment),
lastPixel could be unchanged even though the segment had been
drawn. With this change, we instead test directly whether stroke()
skipped the segment or not.

Task-number: QTBUG-42398
Change-Id: Id751db69a18cd1af4f45070db9d5698aa532d22a
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@theqtcompany.com>

2 files changed, 45 insertions, 12 deletions

diff --git a/src/gui/painting/qcosmeticstroker.cpp b/src/gui/painting/qcosmeticstroker.cpp
index d179002127..c416b9e044 100644
--- a/src/gui/painting/qcosmeticstroker.cpp
+++ b/src/gui/painting/qcosmeticstroker.cpp
@@ -601,8 +601,7 @@ void QCosmeticStroker::drawPath(const QVectorPath &path)
             if (!closed && drawCaps && points == end - 2)
                 caps |= CapEnd;
 
-            QCosmeticStroker::Point last = this->lastPixel;
-            bool unclipped = stroke(this, p.x(), p.y(), p2.x(), p2.y(), caps);
+            bool moveNextStart = stroke(this, p.x(), p.y(), p2.x(), p2.y(), caps);
 
             /* fix for gaps in polylines with fastpen and aliased in a sequence
                of points with small distances: if current point p2 has been dropped
@@ -612,14 +611,8 @@ void QCosmeticStroker::drawPath(const QVectorPath &path)
                still need to update p to avoid drawing the line after this one from
                a bad starting position.
             */
-            if (fastPenAliased && unclipped) {
-                if (last.x != lastPixel.x || last.y != lastPixel.y
-                    || points == begin + 2 || points == end - 2) {
-                    p = p2;
-                }
-            } else {
+            if (!fastPenAliased || moveNextStart || points == begin + 2 || points == end - 2)
                 p = p2;
-            }
             points += 2;
             caps = NoCaps;
         }
@@ -726,8 +719,9 @@ template<DrawPixel drawPixel, class Dasher>
 static bool drawLine(QCosmeticStroker *stroker, qreal rx1, qreal ry1, qreal rx2, qreal ry2, int caps)
 {
     if (stroker->clipLine(rx1, ry1, rx2, ry2))
-        return false;
+        return true;
 
+    bool didDraw = false;
     const int half = stroker->legacyRounding ? 31 : 0;
     int x1 = toF26Dot6(rx1) + half;
     int y1 = toF26Dot6(ry1) + half;
@@ -812,6 +806,7 @@ static bool drawLine(QCosmeticStroker *stroker, qreal rx1, qreal ry1, qreal rx2,
                 dasher.adjust();
                 x += xinc;
             } while (++y < ys);
+            didDraw = true;
         }
     } else {
         // horizontal
@@ -886,10 +881,11 @@ static bool drawLine(QCosmeticStroker *stroker, qreal rx1, qreal ry1, qreal rx2,
                 dasher.adjust();
                 y += yinc;
             } while (++x < xs);
+            didDraw = true;
         }
     }
     stroker->lastPixel = last;
-    return true;
+    return didDraw;
 }
 
 
@@ -897,7 +893,7 @@ template<DrawPixel drawPixel, class Dasher>
 static bool drawLineAA(QCosmeticStroker *stroker, qreal rx1, qreal ry1, qreal rx2, qreal ry2, int caps)
 {
     if (stroker->clipLine(rx1, ry1, rx2, ry2))
-        return false;
+        return true;
 
     int x1 = toF26Dot6(rx1);
     int y1 = toF26Dot6(ry1);
diff --git a/tests/auto/gui/painting/qpainter/tst_qpainter.cpp b/tests/auto/gui/painting/qpainter/tst_qpainter.cpp
index 8a0705630d..bf19a04203 100644
--- a/tests/auto/gui/painting/qpainter/tst_qpainter.cpp
+++ b/tests/auto/gui/painting/qpainter/tst_qpainter.cpp
@@ -290,6 +290,10 @@ private slots:
 
     void RasterOp_NotDestination();
     void drawTextNoHinting();
+
+    void drawPolyline_data();
+    void drawPolyline();
+
 private:
     void fillData();
     void setPenColor(QPainter& p);
@@ -4828,6 +4832,39 @@ void tst_QPainter::drawTextNoHinting()
     QVERIFY(true);
 }
 
+void tst_QPainter::drawPolyline_data()
+{
+    QTest::addColumn< QVector<QPointF> >("points");
+
+    QTest::newRow("basic") << (QVector<QPointF>() << QPointF(10, 10) << QPointF(20, 10) << QPointF(20, 20) << QPointF(10, 20));
+    QTest::newRow("clipped") << (QVector<QPointF>() << QPoint(-10, 100) << QPoint(-1, 100) << QPoint(-1,  -2) << QPoint(100, -2) << QPoint(100, 40)); // QTBUG-31579
+    QTest::newRow("shortsegment") << (QVector<QPointF>() << QPoint(20, 100) << QPoint(20, 99) << QPoint(21, 99) << QPoint(21, 104)); // QTBUG-42398
+}
+
+void tst_QPainter::drawPolyline()
+{
+    QFETCH(QVector<QPointF>, points);
+    QImage images[2];
+
+    for (int r = 0; r < 2; r++) {
+        images[r] = QImage(150, 150, QImage::Format_ARGB32);
+        images[r].fill(Qt::transparent);
+        QPainter p(images + r);
+        QPen pen(Qt::red, 0, Qt::SolidLine, Qt::FlatCap);
+        p.setPen(pen);
+        QVERIFY(p.pen().isCosmetic());
+        if (r) {
+            for (int i = 0; i < points.count()-1; i++) {
+                p.drawLine(points.at(i), points.at(i+1));
+            }
+        } else {
+            p.drawPolyline(points);
+        }
+    }
+
+    QCOMPARE(images[0], images[1]);
+}
+
 QTEST_MAIN(tst_QPainter)
 
 #include "tst_qpainter.moc"