[Libreoffice-commits] core.git: Branch 'libreoffice-7-0' - vcl/skia

[Luboš Luňák (via logerrit)]

 vcl/skia/gdiimpl.cxx |   52 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 16 deletions(-)

New commits:
commit 4de9f35aef7a96a3c034e8a7ed5006ab36e5984a
Author:     Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Wed Nov 18 16:30:52 2020 +0100
Commit:     Caolán McNamara <caolanm at redhat.com>
CommitDate: Mon Nov 23 21:39:53 2020 +0100

    avoid Skia floating point position fixups for rectangles (tdf#137329)
    
    Avoid to toSkX()/toSkY() tweaks for rectangular areas, so with AA
    enabled it leads to fuzzy edges, and rectangles should line up
    perfectly with all pixels even without tweaks.
    
    Change-Id: I45bf5a57a9f2d941eb7ec224992fc452481a2f98
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106060
    Tested-by: Jenkins
    Reviewed-by: Luboš Luňák <l.lunak at collabora.com>
    (cherry picked from commit 5b292e878703ebc32e875406f4116cba145a9042)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106434
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index 40bd2181c9b1..bd497d7f8f6e 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -50,7 +50,8 @@ namespace
 // bottom-most line of pixels of the bounding rectangle (see
 // https://lists.freedesktop.org/archives/libreoffice/2019-November/083709.html).
 // So be careful with rectangle->polygon conversions (generally avoid them).
-void addPolygonToPath(const basegfx::B2DPolygon& rPolygon, SkPath& rPath)
+void addPolygonToPath(const basegfx::B2DPolygon& rPolygon, SkPath& rPath,
+                      bool* hasOnlyOrthogonal = nullptr)
 {
     const sal_uInt32 nPointCount(rPolygon.count());
 
@@ -85,6 +86,11 @@ void addPolygonToPath(const basegfx::B2DPolygon& rPolygon, SkPath& rPath)
         else if (!bHasCurves)
         {
             rPath.lineTo(aCurrentPoint.getX(), aCurrentPoint.getY());
+            // If asked for, check whether the polygon has a line that is not
+            // strictly horizontal or vertical.
+            if (hasOnlyOrthogonal != nullptr && aCurrentPoint.getX() != aPreviousPoint.getX()
+                && aCurrentPoint.getY() != aPreviousPoint.getY())
+                *hasOnlyOrthogonal = false;
         }
         else
         {
@@ -105,6 +111,8 @@ void addPolygonToPath(const basegfx::B2DPolygon& rPolygon, SkPath& rPath)
             rPath.cubicTo(aPreviousControlPoint.getX(), aPreviousControlPoint.getY(),
                           aCurrentControlPoint.getX(), aCurrentControlPoint.getY(),
                           aCurrentPoint.getX(), aCurrentPoint.getY());
+            if (hasOnlyOrthogonal != nullptr)
+                *hasOnlyOrthogonal = false;
         }
         aPreviousPoint = aCurrentPoint;
         nPreviousIndex = nCurrentIndex;
@@ -115,7 +123,8 @@ void addPolygonToPath(const basegfx::B2DPolygon& rPolygon, SkPath& rPath)
     }
 }
 
-void addPolyPolygonToPath(const basegfx::B2DPolyPolygon& rPolyPolygon, SkPath& rPath)
+void addPolyPolygonToPath(const basegfx::B2DPolyPolygon& rPolyPolygon, SkPath& rPath,
+                          bool* hasOnlyOrthogonal = nullptr)
 {
     const sal_uInt32 nPolygonCount(rPolyPolygon.count());
 
@@ -124,7 +133,7 @@ void addPolyPolygonToPath(const basegfx::B2DPolyPolygon& rPolyPolygon, SkPath& r
 
     for (const auto& rPolygon : rPolyPolygon)
     {
-        addPolygonToPath(rPolygon, rPath);
+        addPolygonToPath(rPolygon, rPath, hasOnlyOrthogonal);
     }
 }
 
@@ -815,36 +824,47 @@ void SkiaSalGraphicsImpl::performDrawPolyPolygon(const basegfx::B2DPolyPolygon&
     preDraw();
 
     SkPath polygonPath;
-    addPolyPolygonToPath(aPolyPolygon, polygonPath);
+    bool hasOnlyOrthogonal = true;
+    addPolyPolygonToPath(aPolyPolygon, polygonPath, &hasOnlyOrthogonal);
     polygonPath.setFillType(SkPathFillType::kEvenOdd);
     addXorRegion(polygonPath.getBounds());
 
     SkPaint aPaint;
     aPaint.setAntiAlias(useAA);
-    // We normally use pixel at their center positions, but slightly off (see toSkX/Y()).
-    // With AA lines that "slightly off" causes tiny changes of color, making some tests
-    // fail. Since moving AA-ed line slightly to a side doesn't cause any real visual
-    // difference, just place exactly at the center. tdf#134346
-    const SkScalar posFix = useAA ? toSkXYFix : 0;
+
+    // For lines we use toSkX()/toSkY() in order to pass centers of pixels to Skia,
+    // as that leads to better results with floating-point coordinates
+    // (e.g. https://bugs.chromium.org/p/skia/issues/detail?id=9611).
+    // But that means that we generally need to use it also for areas, so that they
+    // line up properly if used together (tdf#134346).
+    // On the other hand, with AA enabled and rectangular areas, this leads to fuzzy
+    // edges (tdf#137329). But since rectangular areas line up perfectly to pixels
+    // everywhere, it shouldn't be necessary to do this for them.
+    // So if AA is enabled, avoid this fixup for rectangular areas.
+    if (!useAA || !hasOnlyOrthogonal)
+    {
+        // We normally use pixel at their center positions, but slightly off (see toSkX/Y()).
+        // With AA lines that "slightly off" causes tiny changes of color, making some tests
+        // fail. Since moving AA-ed line slightly to a side doesn't cause any real visual
+        // difference, just place exactly at the center. tdf#134346
+        const SkScalar posFix = useAA ? toSkXYFix : 0;
+        polygonPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, nullptr);
+    }
     if (mFillColor != SALCOLOR_NONE)
     {
-        SkPath path;
-        polygonPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, &path);
         aPaint.setColor(toSkColorWithTransparency(mFillColor, fTransparency));
         aPaint.setStyle(SkPaint::kFill_Style);
         // HACK: If the polygon is just a line, it still should be drawn. But when filling
         // Skia doesn't draw empty polygons, so in that case ensure the line is drawn.
-        if (mLineColor == SALCOLOR_NONE && path.getBounds().isEmpty())
+        if (mLineColor == SALCOLOR_NONE && polygonPath.getBounds().isEmpty())
             aPaint.setStyle(SkPaint::kStroke_Style);
-        getDrawCanvas()->drawPath(path, aPaint);
+        getDrawCanvas()->drawPath(polygonPath, aPaint);
     }
     if (mLineColor != SALCOLOR_NONE)
     {
-        SkPath path;
-        polygonPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, &path);
         aPaint.setColor(toSkColorWithTransparency(mLineColor, fTransparency));
         aPaint.setStyle(SkPaint::kStroke_Style);
-        getDrawCanvas()->drawPath(path, aPaint);
+        getDrawCanvas()->drawPath(polygonPath, aPaint);
     }
     postDraw();
 #if defined LINUX