always use region band in Skia's setClipRegion (tdf#133208)

Do like other VCL backends do.

Change-Id: I64b5d5a2fb131b41c70aa63eaf84022e9aa9fab5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/94702
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

6 files changed, 191 insertions, 15 deletions

diff --git a/vcl/Library_vcl.mk b/vcl/Library_vcl.mk
index 8e309d24dc86..0e861442218f 100644
--- a/vcl/Library_vcl.mk
+++ b/vcl/Library_vcl.mk
@@ -462,6 +462,7 @@ $(eval $(call gb_Library_add_exception_objects,vcl,\
     vcl/source/uitest/uno/uiobject_uno \
     vcl/source/uitest/uno/uitest_uno \
     vcl/backendtest/outputdevice/bitmap \
+    vcl/backendtest/outputdevice/clip \
     vcl/backendtest/outputdevice/common \
     vcl/backendtest/outputdevice/gradient \
     vcl/backendtest/outputdevice/line \
diff --git a/vcl/backendtest/VisualBackendTest.cxx b/vcl/backendtest/VisualBackendTest.cxx
index fb39156714a7..5daa9a642d55 100644
--- a/vcl/backendtest/VisualBackendTest.cxx
+++ b/vcl/backendtest/VisualBackendTest.cxx
@@ -90,7 +90,7 @@ class VisualBackendTestWindow : public WorkWindow
 private:
     Timer maUpdateTimer;
     std::vector<std::chrono::high_resolution_clock::time_point> mTimePoints;
-    static constexpr unsigned char gnNumberOfTests = 8;
+    static constexpr unsigned char gnNumberOfTests = 9;
     unsigned char mnTest;
     bool mbAnimate;
     ScopedVclPtr<VirtualDevice> mpVDev;
@@ -435,6 +435,43 @@ public:
         }
     }
 
+    static void testClip(vcl::RenderContext& rRenderContext, int nWidth, int nHeight)
+    {
+        tools::Rectangle aRectangle;
+        size_t index = 0;
+
+        std::vector<tools::Rectangle> aRegions = setupRegions(2, 2, nWidth, nHeight);
+
+        aRectangle = aRegions[index++];
+        {
+            vcl::test::OutputDeviceTestClip aOutDevTest;
+            Bitmap aBitmap = aOutDevTest.setupClipRectangle();
+            assertAndSetBackground(vcl::test::OutputDeviceTestClip::checkClip(aBitmap), aRectangle, rRenderContext);
+            drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext);
+        }
+        aRectangle = aRegions[index++];
+        {
+            vcl::test::OutputDeviceTestClip aOutDevTest;
+            Bitmap aBitmap = aOutDevTest.setupClipPolygon();
+            assertAndSetBackground(vcl::test::OutputDeviceTestClip::checkClip(aBitmap), aRectangle, rRenderContext);
+            drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext);
+        }
+        aRectangle = aRegions[index++];
+        {
+            vcl::test::OutputDeviceTestClip aOutDevTest;
+            Bitmap aBitmap = aOutDevTest.setupClipPolyPolygon();
+            assertAndSetBackground(vcl::test::OutputDeviceTestClip::checkClip(aBitmap), aRectangle, rRenderContext);
+            drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext);
+        }
+        aRectangle = aRegions[index++];
+        {
+            vcl::test::OutputDeviceTestClip aOutDevTest;
+            Bitmap aBitmap = aOutDevTest.setupClipB2DPolyPolygon();
+            assertAndSetBackground(vcl::test::OutputDeviceTestClip::checkClip(aBitmap), aRectangle, rRenderContext);
+            drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext);
+        }
+    }
+
     virtual void Paint(vcl::RenderContext& rRenderContext, const tools::Rectangle& /*rRect*/) override
     {
         if (mnTest % gnNumberOfTests == gnNumberOfTests - 1)
@@ -535,6 +572,10 @@ public:
         }
         else if (mnTest % gnNumberOfTests == 6)
         {
+            testClip(rRenderContext, nWidth, nHeight);
+        }
+        else if (mnTest % gnNumberOfTests == 7)
+        {
             std::vector<tools::Rectangle> aRegions = setupRegions(3, 1, nWidth, nHeight);
 
             aRectangle = aRegions[index++];
diff --git a/vcl/backendtest/outputdevice/clip.cxx b/vcl/backendtest/outputdevice/clip.cxx
new file mode 100644
index 000000000000..86064b583610
--- /dev/null
+++ b/vcl/backendtest/outputdevice/clip.cxx
@@ -0,0 +1,83 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ */
+
+#include <test/outputdevice.hxx>
+
+namespace vcl::test
+{
+Bitmap OutputDeviceTestClip::setupClipRectangle()
+{
+    initialSetup(13, 13, constBackgroundColor);
+
+    tools::Rectangle rectangle = maVDRectangle;
+    rectangle.shrink(2);
+    mpVirtualDevice->SetClipRegion(vcl::Region(rectangle));
+    mpVirtualDevice->SetBackground(constFillColor);
+    mpVirtualDevice->Erase(maVDRectangle);
+
+    return mpVirtualDevice->GetBitmap(maVDRectangle.TopLeft(), maVDRectangle.GetSize());
+}
+
+Bitmap OutputDeviceTestClip::setupClipPolygon()
+{
+    initialSetup(13, 13, constBackgroundColor);
+
+    tools::Rectangle rectangle = maVDRectangle;
+    rectangle.shrink(2);
+    mpVirtualDevice->SetClipRegion(vcl::Region(tools::Polygon(rectangle)));
+    mpVirtualDevice->SetBackground(constFillColor);
+    mpVirtualDevice->Erase(maVDRectangle);
+
+    return mpVirtualDevice->GetBitmap(maVDRectangle.TopLeft(), maVDRectangle.GetSize());
+}
+
+Bitmap OutputDeviceTestClip::setupClipPolyPolygon()
+{
+    initialSetup(13, 13, constBackgroundColor);
+
+    tools::Rectangle rectangle = maVDRectangle;
+    rectangle.shrink(2);
+    mpVirtualDevice->SetClipRegion(vcl::Region(tools::PolyPolygon(rectangle)));
+    mpVirtualDevice->SetBackground(constFillColor);
+    mpVirtualDevice->Erase(maVDRectangle);
+
+    return mpVirtualDevice->GetBitmap(maVDRectangle.TopLeft(), maVDRectangle.GetSize());
+}
+
+Bitmap OutputDeviceTestClip::setupClipB2DPolyPolygon()
+{
+    initialSetup(13, 13, constBackgroundColor);
+
+    tools::Rectangle rectangle = maVDRectangle;
+    rectangle.shrink(2);
+    mpVirtualDevice->SetClipRegion(vcl::Region(basegfx::B2DPolyPolygon(basegfx::B2DPolygon{
+        basegfx::B2DPoint(rectangle.getX(), rectangle.getY()),
+        basegfx::B2DPoint(rectangle.getX(), rectangle.getY() + rectangle.getHeight()),
+        basegfx::B2DPoint(rectangle.getX() + rectangle.getWidth(),
+                          rectangle.getY() + rectangle.getHeight()),
+        basegfx::B2DPoint(rectangle.getX() + rectangle.getWidth(), rectangle.getY()),
+    })));
+    mpVirtualDevice->SetBackground(constFillColor);
+    mpVirtualDevice->Erase(maVDRectangle);
+
+    return mpVirtualDevice->GetBitmap(maVDRectangle.TopLeft(), maVDRectangle.GetSize());
+}
+
+TestResult OutputDeviceTestClip::checkClip(Bitmap& aBitmap)
+{
+    std::vector<Color> aExpected{ constBackgroundColor, constBackgroundColor, constFillColor,
+                                  constFillColor,       constFillColor,       constFillColor,
+                                  constFillColor };
+    return checkRectangles(aBitmap, aExpected);
+}
+
+} // end namespace vcl::test
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/inc/test/outputdevice.hxx b/vcl/inc/test/outputdevice.hxx
index f2f4784d086c..b6cf70c22bd0 100644
--- a/vcl/inc/test/outputdevice.hxx
+++ b/vcl/inc/test/outputdevice.hxx
@@ -200,6 +200,18 @@ public:
     Bitmap setupRadialGradient();
 };
 
+class VCL_DLLPUBLIC OutputDeviceTestClip : public OutputDeviceTestCommon
+{
+public:
+    Bitmap setupClipRectangle();
+    Bitmap setupClipPolygon();
+    Bitmap setupClipPolyPolygon();
+    Bitmap setupClipB2DPolyPolygon();
+
+    static TestResult checkClip(Bitmap& rBitmap);
+};
+
+
 }} // end namespace vcl::test
 
 #endif
diff --git a/vcl/qa/cppunit/BackendTest.cxx b/vcl/qa/cppunit/BackendTest.cxx
index 7fa586b7049a..b34dda5262ec 100644
--- a/vcl/qa/cppunit/BackendTest.cxx
+++ b/vcl/qa/cppunit/BackendTest.cxx
@@ -455,6 +455,46 @@ public:
             CPPUNIT_ASSERT(eResult != vcl::test::TestResult::Failed);
     }
 
+    void testClipRectangle()
+    {
+        vcl::test::OutputDeviceTestClip aOutDevTest;
+        Bitmap aBitmap = aOutDevTest.setupClipRectangle();
+        auto eResult = vcl::test::OutputDeviceTestClip::checkClip(aBitmap);
+        exportImage("09-01_clip_rectangle_test.png", aBitmap);
+        if (SHOULD_ASSERT)
+            CPPUNIT_ASSERT(eResult != vcl::test::TestResult::Failed);
+    }
+
+    void testClipPolygon()
+    {
+        vcl::test::OutputDeviceTestClip aOutDevTest;
+        Bitmap aBitmap = aOutDevTest.setupClipPolygon();
+        auto eResult = vcl::test::OutputDeviceTestClip::checkClip(aBitmap);
+        exportImage("09-02_clip_polygon_test.png", aBitmap);
+        if (SHOULD_ASSERT)
+            CPPUNIT_ASSERT(eResult != vcl::test::TestResult::Failed);
+    }
+
+    void testClipPolyPolygon()
+    {
+        vcl::test::OutputDeviceTestClip aOutDevTest;
+        Bitmap aBitmap = aOutDevTest.setupClipPolyPolygon();
+        auto eResult = vcl::test::OutputDeviceTestClip::checkClip(aBitmap);
+        exportImage("09-03_clip_polypolygon_test.png", aBitmap);
+        if (SHOULD_ASSERT)
+            CPPUNIT_ASSERT(eResult != vcl::test::TestResult::Failed);
+    }
+
+    void testClipB2DPolyPolygon()
+    {
+        vcl::test::OutputDeviceTestClip aOutDevTest;
+        Bitmap aBitmap = aOutDevTest.setupClipB2DPolyPolygon();
+        auto eResult = vcl::test::OutputDeviceTestClip::checkClip(aBitmap);
+        exportImage("09-04_clip_b2dpolypolygon_test.png", aBitmap);
+        if (SHOULD_ASSERT)
+            CPPUNIT_ASSERT(eResult != vcl::test::TestResult::Failed);
+    }
+
     void testTdf124848()
     {
         ScopedVclPtr<VirtualDevice> device = VclPtr<VirtualDevice>::Create(DeviceFormat::DEFAULT);
@@ -530,6 +570,11 @@ public:
     CPPUNIT_TEST(testDrawBlend);
     CPPUNIT_TEST(testDrawXor);
 
+    CPPUNIT_TEST(testClipRectangle);
+    CPPUNIT_TEST(testClipPolygon);
+    CPPUNIT_TEST(testClipPolyPolygon);
+    CPPUNIT_TEST(testClipB2DPolyPolygon);
+
     CPPUNIT_TEST(testTdf124848);
 
     CPPUNIT_TEST_SUITE_END();
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index 051243ad2a8d..b280f732425f 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -415,20 +415,14 @@ void SkiaSalGraphicsImpl::setCanvasClipRegion(SkCanvas* canvas, const vcl::Regio
 {
     SkiaZone zone;
     SkPath path;
-    // Handle polygons last, since rectangle->polygon area conversions
-    // are problematic (see addPolygonToPath() comment).
-    if (region.getRegionBand())
-    {
-        RectangleVector rectangles;
-        region.GetRegionRectangles(rectangles);
-        for (const tools::Rectangle& rectangle : rectangles)
-            path.addRect(SkRect::MakeXYWH(rectangle.getX(), rectangle.getY(), rectangle.GetWidth(),
-                                          rectangle.GetHeight()));
-    }
-    else if (!region.IsEmpty())
-    {
-        addPolyPolygonToPath(region.GetAsB2DPolyPolygon(), path);
-    }
+    // Always use region rectangles, regardless of what the region uses internally.
+    // That's what other VCL backends do, and trying to use addPolyPolygonToPath()
+    // in case a polygon is used leads to off-by-one errors such as tdf#133208.
+    RectangleVector rectangles;
+    region.GetRegionRectangles(rectangles);
+    for (const tools::Rectangle& rectangle : rectangles)
+        path.addRect(SkRect::MakeXYWH(rectangle.getX(), rectangle.getY(), rectangle.GetWidth(),
+                                      rectangle.GetHeight()));
     path.setFillType(SkPathFillType::kEvenOdd);
     canvas->clipPath(path);
 }