[Libreoffice-commits] core.git: vcl/qa vcl/skia

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

 vcl/qa/cppunit/skia/skia.cxx |   30 ++++++++++++++++++++++++++++++
 vcl/skia/gdiimpl.cxx         |   20 ++++++++++++++++++--
 2 files changed, 48 insertions(+), 2 deletions(-)

New commits:
commit ff1cfaf87ce0aa9673e1c3f92308cde6a2c6aa69
Author:     Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Wed Mar 17 16:03:12 2021 +0100
Commit:     Luboš Luňák <l.lunak at collabora.com>
CommitDate: Wed Mar 17 18:01:00 2021 +0100

    round polygon points before merging them for Skia drawing (tdf#140848)
    
    basegfx::utils::mergeToSinglePolyPolygon() appears to have rounding
    problems. Point coordinates are in pixels anyway.
    
    Change-Id: I9880cc32f934a08923a5c59278f6aa07852c05f9
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/112647
    Tested-by: Jenkins
    Reviewed-by: Luboš Luňák <l.lunak at collabora.com>

diff --git a/vcl/qa/cppunit/skia/skia.cxx b/vcl/qa/cppunit/skia/skia.cxx
index d8e103d1a431..6ed0b884955d 100644
--- a/vcl/qa/cppunit/skia/skia.cxx
+++ b/vcl/qa/cppunit/skia/skia.cxx
@@ -43,6 +43,7 @@ public:
     void testMatrixQuality();
     void testDelayedScale();
     void testTdf137329();
+    void testTdf140848();
 
     CPPUNIT_TEST_SUITE(SkiaTest);
     CPPUNIT_TEST(testBitmapErase);
@@ -53,6 +54,7 @@ public:
     CPPUNIT_TEST(testMatrixQuality);
     CPPUNIT_TEST(testDelayedScale);
     CPPUNIT_TEST(testTdf137329);
+    CPPUNIT_TEST(testTdf140848);
     CPPUNIT_TEST_SUITE_END();
 
 private:
@@ -395,6 +397,34 @@ void SkiaTest::testTdf137329()
     CPPUNIT_ASSERT_EQUAL(COL_BLACK, device->GetPixel(Point(4, 4)));
 }
 
+void SkiaTest::testTdf140848()
+{
+    if (!SkiaHelper::isVCLSkiaEnabled())
+        return;
+    ScopedVclPtr<VirtualDevice> device = VclPtr<VirtualDevice>::Create(DeviceFormat::DEFAULT);
+    device->SetOutputSizePixel(Size(1300, 400));
+    device->SetBackground(Wallpaper(COL_BLACK));
+    device->SetAntialiasing(AntialiasingFlags::Enable);
+    device->Erase();
+    device->SetLineColor();
+    device->SetFillColor(COL_WHITE);
+    basegfx::B2DPolygon p1 = { { 952.73121259842514519, 102.4599685039370911 },
+                               { 952.73121259842514519, 66.55445669291347599 },
+                               { 1239.9753070866140661, 66.554456692913390725 },
+                               { 1239.9753070866140661, 138.36548031496062094 },
+                               { 952.73121259842514519, 138.36548031496070621 } };
+    basegfx::B2DPolygon p2 = { { 1168.1642834645670064, 210.17650393700790801 },
+                               { 1168.1642834645670064, 66.554456692913404936 },
+                               { 1239.9753070866140661, 66.554456692913390725 },
+                               { 1239.9753070866142934, 353.79855118110236845 },
+                               { 1168.1642834645670064, 353.79855118110236845 } };
+    device->DrawPolyPolygon(basegfx::B2DPolyPolygon(p1));
+    device->DrawPolyPolygon(basegfx::B2DPolyPolygon(p2));
+    //savePNG("/tmp/tdf140848.png", device);
+    // Rounding errors caused the overlapping part not to be drawn.
+    CPPUNIT_ASSERT_EQUAL(COL_WHITE, device->GetPixel(Point(1200, 100)));
+}
+
 } // namespace
 
 CPPUNIT_TEST_SUITE_REGISTRATION(SkiaTest);
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index aa934f1e4942..be31e0ee649a 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -1015,6 +1015,20 @@ bool SkiaSalGraphicsImpl::delayDrawPolyPolygon(const basegfx::B2DPolyPolygon& aP
     return true;
 }
 
+// Tdf#140848 - basegfx::utils::mergeToSinglePolyPolygon() seems to have rounding
+// errors that sometimes cause it to merge incorrectly.
+static void roundPolygonPoints(basegfx::B2DPolyPolygon& polyPolygon)
+{
+    for (basegfx::B2DPolygon& polygon : polyPolygon)
+    {
+        polygon.makeUnique();
+        for (sal_uInt32 i = 0; i < polygon.count(); ++i)
+            polygon.setB2DPoint(i, basegfx::B2DPoint(basegfx::fround(polygon.getB2DPoint(i))));
+        // Control points are saved as vectors relative to points, so hopefully
+        // there's no need to round those.
+    }
+}
+
 void SkiaSalGraphicsImpl::checkPendingDrawing()
 {
     if (mLastPolyPolygonInfo.polygons.size() != 0)
@@ -1026,10 +1040,12 @@ void SkiaSalGraphicsImpl::checkPendingDrawing()
         if (polygons.size() == 1)
             performDrawPolyPolygon(polygons.front(), transparency, true);
         else
-            // TODO: tdf#136222 shows that basegfx::utils::mergeToSinglePolyPolygon() is unreliable
-            // in corner cases, possibly either a bug or rounding errors somewhere.
+        {
+            for (basegfx::B2DPolyPolygon& p : polygons)
+                roundPolygonPoints(p);
             performDrawPolyPolygon(basegfx::utils::mergeToSinglePolyPolygon(polygons), transparency,
                                    true);
+        }
     }
 }