[Libreoffice-commits] core.git: Branch 'feature/skia' - 3 commits - vcl/backendtest vcl/inc vcl/qa vcl/skia

Luboš Luňák (via logerrit) logerrit at kemper.freedesktop.org
Fri Nov 8 11:47:45 UTC 2019


Rebased ref, commits from common ancestor:
commit bf0b822522ffdd6619a0195921dc12347dc71e89
Author:     Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Fri Nov 1 13:44:20 2019 +0100
Commit:     Luboš Luňák <l.lunak at collabora.com>
CommitDate: Fri Nov 8 12:46:11 2019 +0100

    use different line and fill color in vcl backendtest
    
    Having them the same can hide problems with them fixed up incorrectly.
    And it also shows that drawPolygon() with line color unset does not
    draw the right-most and bottom-most line, which is what all underlying
    graphics systems do, so the test is kind of wrong and I've added
    a compensation to make it visually correct (and match the checked
    expected result).
    
    Change-Id: I333f41210232c74ba55bd5c92ef5fda917ce3e59

diff --git a/vcl/backendtest/VisualBackendTest.cxx b/vcl/backendtest/VisualBackendTest.cxx
index c501eb09879f..a1134df5f0da 100644
--- a/vcl/backendtest/VisualBackendTest.cxx
+++ b/vcl/backendtest/VisualBackendTest.cxx
@@ -88,7 +88,7 @@ class VisualBackendTestWindow : public WorkWindow
 private:
     Timer maUpdateTimer;
     std::vector<std::chrono::high_resolution_clock::time_point> mTimePoints;
-    static constexpr unsigned char gnNumberOfTests = 6;
+    static constexpr unsigned char gnNumberOfTests = 7;
     unsigned char mnTest;
     bool mbAnimate;
     ScopedVclPtr<VirtualDevice> mpVDev;
@@ -225,29 +225,61 @@ public:
         aRectangle = aRegions[index++];
         {
             vcl::test::OutputDeviceTestRect aOutDevTest;
-            Bitmap aBitmap = aOutDevTest.setupFilledRectangle();
-            assertAndSetBackground(vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap), aRectangle, rRenderContext);
+            Bitmap aBitmap = aOutDevTest.setupFilledRectangle(false);
+            assertAndSetBackground(vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap, false), aRectangle, rRenderContext);
             drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext);
         }
 
         aRectangle = aRegions[index++];
         {
             vcl::test::OutputDeviceTestPolygon aOutDevTest;
-            Bitmap aBitmap = aOutDevTest.setupFilledRectangle();
-            assertAndSetBackground(vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap), aRectangle, rRenderContext);
+            Bitmap aBitmap = aOutDevTest.setupFilledRectangle(false);
+            assertAndSetBackground(vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap, false), aRectangle, rRenderContext);
             drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext);
         }
 
         aRectangle = aRegions[index++];
         {
             vcl::test::OutputDeviceTestPolyPolygon aOutDevTest;
-            Bitmap aBitmap = aOutDevTest.setupFilledRectangle();
-            assertAndSetBackground(vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap), aRectangle, rRenderContext);
+            Bitmap aBitmap = aOutDevTest.setupFilledRectangle(false);
+            assertAndSetBackground(vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap, false), aRectangle, rRenderContext);
             drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext);
         }
 
         aRectangle = aRegions[index++];
         {
+            vcl::test::OutputDeviceTestRect aOutDevTest;
+            Bitmap aBitmap = aOutDevTest.setupFilledRectangle(true);
+            assertAndSetBackground(vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap, true), aRectangle, rRenderContext);
+            drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext);
+        }
+
+        aRectangle = aRegions[index++];
+        {
+            vcl::test::OutputDeviceTestPolygon aOutDevTest;
+            Bitmap aBitmap = aOutDevTest.setupFilledRectangle(true);
+            assertAndSetBackground(vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap, true), aRectangle, rRenderContext);
+            drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext);
+        }
+
+        aRectangle = aRegions[index++];
+        {
+            vcl::test::OutputDeviceTestPolyPolygon aOutDevTest;
+            Bitmap aBitmap = aOutDevTest.setupFilledRectangle(true);
+            assertAndSetBackground(vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap, true), aRectangle, rRenderContext);
+            drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext);
+        }
+    }
+
+    static void testDiamonds(vcl::RenderContext& rRenderContext, int nWidth, int nHeight)
+    {
+        tools::Rectangle aRectangle;
+        size_t index = 0;
+
+        std::vector<tools::Rectangle> aRegions = setupRegions(3, 1, nWidth, nHeight);
+
+        aRectangle = aRegions[index++];
+        {
             vcl::test::OutputDeviceTestPolygon aOutDevTest;
             Bitmap aBitmap = aOutDevTest.setupDiamond();
             assertAndSetBackground(vcl::test::OutputDeviceTestCommon::checkDiamond(aBitmap), aRectangle, rRenderContext);
@@ -441,14 +473,18 @@ public:
         }
         else if (mnTest % gnNumberOfTests == 2)
         {
-            testLines(rRenderContext, nWidth, nHeight);
+            testDiamonds(rRenderContext, nWidth, nHeight);
         }
         else if (mnTest % gnNumberOfTests == 3)
         {
-            testBitmaps(rRenderContext, nWidth, nHeight);
+            testLines(rRenderContext, nWidth, nHeight);
         }
         else if (mnTest % gnNumberOfTests == 4)
         {
+            testBitmaps(rRenderContext, nWidth, nHeight);
+        }
+        else if (mnTest % gnNumberOfTests == 5)
+        {
             std::vector<tools::Rectangle> aRegions = setupRegions(3, 2, nWidth, nHeight);
 
             aRectangle = aRegions[index++];
diff --git a/vcl/backendtest/outputdevice/bitmap.cxx b/vcl/backendtest/outputdevice/bitmap.cxx
index 58b7c5f03ff0..66a0a64adf49 100644
--- a/vcl/backendtest/outputdevice/bitmap.cxx
+++ b/vcl/backendtest/outputdevice/bitmap.cxx
@@ -107,7 +107,7 @@ Bitmap OutputDeviceTestBitmap::setupDrawMask()
 
     initialSetup(13, 13, constBackgroundColor);
 
-    mpVirtualDevice->DrawMask(Point(2, 2), aBitmap, constFillColor);
+    mpVirtualDevice->DrawMask(Point(2, 2), aBitmap, constLineColor);
 
     return mpVirtualDevice->GetBitmap(maVDRectangle.TopLeft(), maVDRectangle.GetSize());
 }
diff --git a/vcl/backendtest/outputdevice/common.cxx b/vcl/backendtest/outputdevice/common.cxx
index 00c7b473e88a..34e37e771f1f 100644
--- a/vcl/backendtest/outputdevice/common.cxx
+++ b/vcl/backendtest/outputdevice/common.cxx
@@ -220,7 +220,7 @@ TestResult checkDiamondLine(Bitmap& rBitmap, int aLayerNumber, Color aExpectedCo
 
 const Color OutputDeviceTestCommon::constBackgroundColor(COL_LIGHTGRAY);
 const Color OutputDeviceTestCommon::constLineColor(COL_LIGHTBLUE);
-const Color OutputDeviceTestCommon::constFillColor(COL_LIGHTBLUE);
+const Color OutputDeviceTestCommon::constFillColor(COL_BLUE);
 
 OutputDeviceTestCommon::OutputDeviceTestCommon()
 {}
@@ -366,12 +366,13 @@ TestResult OutputDeviceTestCommon::checkRectangleAA(Bitmap& aBitmap)
     return checkRectangles(aBitmap, aExpected);
 }
 
-TestResult OutputDeviceTestCommon::checkFilledRectangle(Bitmap& aBitmap)
+TestResult OutputDeviceTestCommon::checkFilledRectangle(Bitmap& aBitmap, bool useLineColor)
 {
     std::vector<Color> aExpected
     {
         constBackgroundColor, constBackgroundColor,
-        constFillColor, constFillColor, constFillColor, constFillColor, constFillColor
+        useLineColor ? constLineColor : constFillColor,
+        constFillColor, constFillColor, constFillColor, constFillColor
     };
     return checkRectangles(aBitmap, aExpected);
 }
diff --git a/vcl/backendtest/outputdevice/line.cxx b/vcl/backendtest/outputdevice/line.cxx
index ff03a777771e..a43c72a62dc0 100644
--- a/vcl/backendtest/outputdevice/line.cxx
+++ b/vcl/backendtest/outputdevice/line.cxx
@@ -48,7 +48,7 @@ Bitmap OutputDeviceTestLine::setupDiamond()
 {
     initialSetup(11, 11, constBackgroundColor);
 
-    mpVirtualDevice->SetLineColor(constFillColor);
+    mpVirtualDevice->SetLineColor(constLineColor);
     mpVirtualDevice->SetFillColor();
 
     Point aPoint1, aPoint2, aPoint3, aPoint4;
diff --git a/vcl/backendtest/outputdevice/outputdevice.cxx b/vcl/backendtest/outputdevice/outputdevice.cxx
index 408ce09551f7..4f31501b6f3b 100644
--- a/vcl/backendtest/outputdevice/outputdevice.cxx
+++ b/vcl/backendtest/outputdevice/outputdevice.cxx
@@ -55,7 +55,12 @@ Bitmap OutputDeviceTestAnotherOutDev::setupXOR()
 
 TestResult OutputDeviceTestAnotherOutDev::checkDrawOutDev(Bitmap& rBitmap)
 {
-    return checkFilledRectangle(rBitmap);
+    std::vector<Color> aExpected
+    {
+        constBackgroundColor, constBackgroundColor,
+        constFillColor, constFillColor, constFillColor, constFillColor, constFillColor
+    };
+    return checkRectangles(rBitmap, aExpected);
 }
 
 TestResult OutputDeviceTestAnotherOutDev::checkXOR(Bitmap& rBitmap)
diff --git a/vcl/backendtest/outputdevice/polygon.cxx b/vcl/backendtest/outputdevice/polygon.cxx
index 4a0fac9f92ed..f0977abfb5f6 100644
--- a/vcl/backendtest/outputdevice/polygon.cxx
+++ b/vcl/backendtest/outputdevice/polygon.cxx
@@ -16,13 +16,17 @@ namespace test {
 namespace
 {
 
-void drawPolygonOffset(OutputDevice& rDevice, tools::Rectangle const & rRect, int nOffset)
+void drawPolygonOffset(OutputDevice& rDevice, tools::Rectangle const & rRect, int nOffset, int nFix = 0)
 {
+    // Note: According to https://lists.freedesktop.org/archives/libreoffice/2019-November/083709.html
+    // filling polygons always skips the right-most and bottom-most pixels, in order to avoid
+    // overlaps when drawing adjacent polygons. Specifying nFix = 1 allows to visually compensate
+    // for this by making the polygon explicitly larger.
     tools::Polygon aPolygon(4);
     aPolygon.SetPoint(Point(rRect.Left()  + nOffset, rRect.Top()    + nOffset), 0);
-    aPolygon.SetPoint(Point(rRect.Right() - nOffset, rRect.Top()    + nOffset), 1);
-    aPolygon.SetPoint(Point(rRect.Right() - nOffset, rRect.Bottom() - nOffset), 2);
-    aPolygon.SetPoint(Point(rRect.Left()  + nOffset, rRect.Bottom() - nOffset), 3);
+    aPolygon.SetPoint(Point(rRect.Right() - nOffset + nFix, rRect.Top()    + nOffset), 1);
+    aPolygon.SetPoint(Point(rRect.Right() - nOffset + nFix, rRect.Bottom() - nOffset + nFix), 2);
+    aPolygon.SetPoint(Point(rRect.Left()  + nOffset, rRect.Bottom() - nOffset + nFix), 3);
     aPolygon.Optimize(PolyOptimizeFlags::CLOSE);
 
     rDevice.DrawPolygon(aPolygon);
@@ -43,13 +47,16 @@ Bitmap OutputDeviceTestPolygon::setupRectangle(bool bEnableAA)
     return mpVirtualDevice->GetBitmap(maVDRectangle.TopLeft(), maVDRectangle.GetSize());
 }
 
-Bitmap OutputDeviceTestPolygon::setupFilledRectangle()
+Bitmap OutputDeviceTestPolygon::setupFilledRectangle(bool useLineColor)
 {
     initialSetup(13, 13, constBackgroundColor);
 
-    mpVirtualDevice->SetLineColor();
+    if(useLineColor)
+        mpVirtualDevice->SetLineColor(constLineColor);
+    else
+        mpVirtualDevice->SetLineColor();
     mpVirtualDevice->SetFillColor(constFillColor);
-    drawPolygonOffset(*mpVirtualDevice, maVDRectangle, 2);
+    drawPolygonOffset(*mpVirtualDevice, maVDRectangle, 2, useLineColor ? 0 : 1);
 
     return mpVirtualDevice->GetBitmap(maVDRectangle.TopLeft(), maVDRectangle.GetSize());
 }
diff --git a/vcl/backendtest/outputdevice/polypolygon.cxx b/vcl/backendtest/outputdevice/polypolygon.cxx
index f36db1e9fd65..e5a527042bdc 100644
--- a/vcl/backendtest/outputdevice/polypolygon.cxx
+++ b/vcl/backendtest/outputdevice/polypolygon.cxx
@@ -17,13 +17,17 @@ namespace test {
 namespace
 {
 
-tools::Polygon createPolygonOffset(tools::Rectangle const & rRect, int nOffset)
+tools::Polygon createPolygonOffset(tools::Rectangle const & rRect, int nOffset, int nFix = 0)
 {
+    // Note: According to https://lists.freedesktop.org/archives/libreoffice/2019-November/083709.html
+    // filling polygons always skips the right-most and bottom-most pixels, in order to avoid
+    // overlaps when drawing adjacent polygons. Specifying nFix = 1 allows to visually compensate
+    // for this by making the polygon explicitly larger.
     tools::Polygon aPolygon(4);
     aPolygon.SetPoint(Point(rRect.Left()  + nOffset, rRect.Top()    + nOffset), 0);
-    aPolygon.SetPoint(Point(rRect.Right() - nOffset, rRect.Top()    + nOffset), 1);
-    aPolygon.SetPoint(Point(rRect.Right() - nOffset, rRect.Bottom() - nOffset), 2);
-    aPolygon.SetPoint(Point(rRect.Left()  + nOffset, rRect.Bottom() - nOffset), 3);
+    aPolygon.SetPoint(Point(rRect.Right() - nOffset + nFix, rRect.Top()    + nOffset), 1);
+    aPolygon.SetPoint(Point(rRect.Right() - nOffset + nFix, rRect.Bottom() - nOffset + nFix), 2);
+    aPolygon.SetPoint(Point(rRect.Left()  + nOffset, rRect.Bottom() - nOffset + nFix), 3);
     aPolygon.Optimize(PolyOptimizeFlags::CLOSE);
     return aPolygon;
 }
@@ -46,15 +50,18 @@ Bitmap OutputDeviceTestPolyPolygon::setupRectangle(bool bEnableAA)
     return mpVirtualDevice->GetBitmap(maVDRectangle.TopLeft(), maVDRectangle.GetSize());
 }
 
-Bitmap OutputDeviceTestPolyPolygon::setupFilledRectangle()
+Bitmap OutputDeviceTestPolyPolygon::setupFilledRectangle(bool useLineColor)
 {
     initialSetup(13, 13, constBackgroundColor);
 
-    mpVirtualDevice->SetLineColor();
+    if(useLineColor)
+        mpVirtualDevice->SetLineColor(constLineColor);
+    else
+        mpVirtualDevice->SetLineColor();
     mpVirtualDevice->SetFillColor(constFillColor);
 
     tools::PolyPolygon aPolyPolygon(1);
-    aPolyPolygon.Insert(createPolygonOffset(maVDRectangle, 2));
+    aPolyPolygon.Insert(createPolygonOffset(maVDRectangle, 2, useLineColor ? 0 : 1));
 
     mpVirtualDevice->DrawPolyPolygon(aPolyPolygon);
 
diff --git a/vcl/backendtest/outputdevice/polypolygon_b2d.cxx b/vcl/backendtest/outputdevice/polypolygon_b2d.cxx
index 808e8637c3d8..977f73dcc1f4 100644
--- a/vcl/backendtest/outputdevice/polypolygon_b2d.cxx
+++ b/vcl/backendtest/outputdevice/polypolygon_b2d.cxx
@@ -16,13 +16,17 @@ namespace test
 {
 namespace
 {
-basegfx::B2DPolygon createPolygonOffset(tools::Rectangle const& rRect, int nOffset)
+basegfx::B2DPolygon createPolygonOffset(tools::Rectangle const& rRect, int nOffset, int nFix = 0)
 {
+    // Note: According to https://lists.freedesktop.org/archives/libreoffice/2019-November/083709.html
+    // filling polygons always skips the right-most and bottom-most pixels, in order to avoid
+    // overlaps when drawing adjacent polygons. Specifying nFix = 1 allows to visually compensate
+    // for this by making the polygon explicitly larger.
     basegfx::B2DPolygon aPolygon{
         basegfx::B2DPoint(rRect.Left() + nOffset, rRect.Top() + nOffset),
-        basegfx::B2DPoint(rRect.Right() - nOffset, rRect.Top() + nOffset),
-        basegfx::B2DPoint(rRect.Right() - nOffset, rRect.Bottom() - nOffset),
-        basegfx::B2DPoint(rRect.Left() + nOffset, rRect.Bottom() - nOffset),
+        basegfx::B2DPoint(rRect.Right() - nOffset + nFix, rRect.Top() + nOffset),
+        basegfx::B2DPoint(rRect.Right() - nOffset + nFix, rRect.Bottom() - nOffset + nFix),
+        basegfx::B2DPoint(rRect.Left() + nOffset, rRect.Bottom() - nOffset + nFix),
     };
     aPolygon.setClosed(true);
     return aPolygon;
@@ -46,14 +50,18 @@ Bitmap OutputDeviceTestPolyPolygonB2D::setupRectangle(bool bEnableAA)
     return mpVirtualDevice->GetBitmap(maVDRectangle.TopLeft(), maVDRectangle.GetSize());
 }
 
-Bitmap OutputDeviceTestPolyPolygonB2D::setupFilledRectangle()
+Bitmap OutputDeviceTestPolyPolygonB2D::setupFilledRectangle(bool useLineColor)
 {
     initialSetup(13, 13, constBackgroundColor);
 
-    mpVirtualDevice->SetLineColor();
+    if (useLineColor)
+        mpVirtualDevice->SetLineColor(constLineColor);
+    else
+        mpVirtualDevice->SetLineColor();
     mpVirtualDevice->SetFillColor(constFillColor);
 
-    basegfx::B2DPolyPolygon aPolyPolygon(createPolygonOffset(maVDRectangle, 2));
+    basegfx::B2DPolyPolygon aPolyPolygon(
+        createPolygonOffset(maVDRectangle, 2, useLineColor ? 0 : 1));
 
     mpVirtualDevice->DrawPolyPolygon(aPolyPolygon);
 
diff --git a/vcl/backendtest/outputdevice/rectangle.cxx b/vcl/backendtest/outputdevice/rectangle.cxx
index f861cc2f05bb..033ebb3c54ae 100644
--- a/vcl/backendtest/outputdevice/rectangle.cxx
+++ b/vcl/backendtest/outputdevice/rectangle.cxx
@@ -31,11 +31,14 @@ namespace
 
 } // end anonymous namespace
 
-Bitmap OutputDeviceTestRect::setupFilledRectangle()
+Bitmap OutputDeviceTestRect::setupFilledRectangle(bool useLineColor)
 {
     initialSetup(13, 13, constBackgroundColor);
 
-    mpVirtualDevice->SetLineColor();
+    if(useLineColor)
+        mpVirtualDevice->SetLineColor(constLineColor);
+    else
+        mpVirtualDevice->SetLineColor();
     mpVirtualDevice->SetFillColor(constFillColor);
 
     drawRectOffset(*mpVirtualDevice, maVDRectangle, 2);
diff --git a/vcl/inc/test/outputdevice.hxx b/vcl/inc/test/outputdevice.hxx
index fb625d7987f3..520436f66fea 100644
--- a/vcl/inc/test/outputdevice.hxx
+++ b/vcl/inc/test/outputdevice.hxx
@@ -50,7 +50,7 @@ public:
 
     static TestResult checkRectangle(Bitmap& rBitmap);
     static TestResult checkRectangleAA(Bitmap& rBitmap);
-    static TestResult checkFilledRectangle(Bitmap& rBitmap);
+    static TestResult checkFilledRectangle(Bitmap& rBitmap, bool useLineColor);
     static TestResult checkLines(Bitmap& rBitmap);
     static TestResult checkAALines(Bitmap& rBitmap);
     static TestResult checkDiamond(Bitmap& rBitmap);
@@ -155,7 +155,7 @@ public:
     OutputDeviceTestRect() = default;
 
     Bitmap setupRectangle(bool bEnableAA);
-    Bitmap setupFilledRectangle();
+    Bitmap setupFilledRectangle(bool useLineColor);
     Bitmap setupInvert_NONE();
     Bitmap setupInvert_N50();
     Bitmap setupInvert_TrackFrame();
@@ -167,7 +167,7 @@ public:
     OutputDeviceTestPolygon() = default;
 
     Bitmap setupRectangle(bool bEnableAA);
-    Bitmap setupFilledRectangle();
+    Bitmap setupFilledRectangle(bool useLineColor);
     Bitmap setupDiamond();
     Bitmap setupLines();
     Bitmap setupAALines();
@@ -179,7 +179,7 @@ public:
     OutputDeviceTestPolyPolygon() = default;
 
     Bitmap setupRectangle(bool bEnableAA);
-    Bitmap setupFilledRectangle();
+    Bitmap setupFilledRectangle(bool useLineColor);
 };
 
 class VCL_DLLPUBLIC OutputDeviceTestPolyPolygonB2D : public OutputDeviceTestCommon
@@ -188,7 +188,7 @@ public:
     OutputDeviceTestPolyPolygonB2D() = default;
 
     Bitmap setupRectangle(bool bEnableAA);
-    Bitmap setupFilledRectangle();
+    Bitmap setupFilledRectangle(bool useLineColor);
 };
 
 class VCL_DLLPUBLIC OutputDeviceTestGradient : public OutputDeviceTestCommon
diff --git a/vcl/qa/cppunit/BackendTest.cxx b/vcl/qa/cppunit/BackendTest.cxx
index fd9bf10eaff5..01a81c0f17f3 100644
--- a/vcl/qa/cppunit/BackendTest.cxx
+++ b/vcl/qa/cppunit/BackendTest.cxx
@@ -237,9 +237,14 @@ public:
     void testDrawFilledRectWithRectangle()
     {
         vcl::test::OutputDeviceTestRect aOutDevTest;
-        Bitmap aBitmap = aOutDevTest.setupFilledRectangle();
-        auto eResult = vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap);
-        exportImage("03-01_filled_rectangle_test-rectangle.png", aBitmap);
+        Bitmap aBitmap = aOutDevTest.setupFilledRectangle(false);
+        auto eResult = vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap, false);
+        exportImage("03-01_filled_rectangle_test-rectangle_noline.png", aBitmap);
+        if (SHOULD_ASSERT)
+            CPPUNIT_ASSERT(eResult != vcl::test::TestResult::Failed);
+        aBitmap = aOutDevTest.setupFilledRectangle(true);
+        eResult = vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap, true);
+        exportImage("03-01_filled_rectangle_test-rectangle_line.png", aBitmap);
         if (SHOULD_ASSERT)
             CPPUNIT_ASSERT(eResult != vcl::test::TestResult::Failed);
     }
@@ -247,9 +252,14 @@ public:
     void testDrawFilledRectWithPolygon()
     {
         vcl::test::OutputDeviceTestPolygon aOutDevTest;
-        Bitmap aBitmap = aOutDevTest.setupFilledRectangle();
-        auto eResult = vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap);
-        exportImage("03-02_filled_rectangle_test-polygon.png", aBitmap);
+        Bitmap aBitmap = aOutDevTest.setupFilledRectangle(false);
+        auto eResult = vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap, false);
+        exportImage("03-02_filled_rectangle_test-polygon_noline.png", aBitmap);
+        if (SHOULD_ASSERT)
+            CPPUNIT_ASSERT(eResult != vcl::test::TestResult::Failed);
+        aBitmap = aOutDevTest.setupFilledRectangle(true);
+        eResult = vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap, true);
+        exportImage("03-02_filled_rectangle_test-polygon_line.png", aBitmap);
         if (SHOULD_ASSERT)
             CPPUNIT_ASSERT(eResult != vcl::test::TestResult::Failed);
     }
@@ -257,9 +267,14 @@ public:
     void testDrawFilledRectWithPolyPolygon()
     {
         vcl::test::OutputDeviceTestPolyPolygon aOutDevTest;
-        Bitmap aBitmap = aOutDevTest.setupFilledRectangle();
-        auto eResult = vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap);
-        exportImage("03-03_filled_rectangle_test-polypolygon.png", aBitmap);
+        Bitmap aBitmap = aOutDevTest.setupFilledRectangle(false);
+        auto eResult = vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap, false);
+        exportImage("03-03_filled_rectangle_test-polypolygon_noline.png", aBitmap);
+        if (SHOULD_ASSERT)
+            CPPUNIT_ASSERT(eResult != vcl::test::TestResult::Failed);
+        aBitmap = aOutDevTest.setupFilledRectangle(true);
+        eResult = vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap, true);
+        exportImage("03-03_filled_rectangle_test-polypolygon_line.png", aBitmap);
         if (SHOULD_ASSERT)
             CPPUNIT_ASSERT(eResult != vcl::test::TestResult::Failed);
     }
@@ -267,9 +282,14 @@ public:
     void testDrawFilledRectWithPolyPolygon2D()
     {
         vcl::test::OutputDeviceTestPolyPolygonB2D aOutDevTest;
-        Bitmap aBitmap = aOutDevTest.setupFilledRectangle();
-        auto eResult = vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap);
-        exportImage("03-04_filled_rectangle_test-polypolygon_b2d.png", aBitmap);
+        Bitmap aBitmap = aOutDevTest.setupFilledRectangle(false);
+        auto eResult = vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap, false);
+        exportImage("03-04_filled_rectangle_test-polypolygon_b2d_noline.png", aBitmap);
+        if (SHOULD_ASSERT)
+            CPPUNIT_ASSERT(eResult != vcl::test::TestResult::Failed);
+        aBitmap = aOutDevTest.setupFilledRectangle(true);
+        eResult = vcl::test::OutputDeviceTestCommon::checkFilledRectangle(aBitmap, true);
+        exportImage("03-04_filled_rectangle_test-polypolygon_b2d_line.png", aBitmap);
         if (SHOULD_ASSERT)
             CPPUNIT_ASSERT(eResult != vcl::test::TestResult::Failed);
     }
@@ -434,9 +454,9 @@ public:
     CPPUNIT_TEST(testDrawRectAAWithPolyPolygonB2D);
 
     CPPUNIT_TEST(testDrawFilledRectWithRectangle);
-    //    CPPUNIT_TEST(testDrawFilledRectWithPolygon); TODO SKIA
-    //    CPPUNIT_TEST(testDrawFilledRectWithPolyPolygon); TODO SKIA
-    //    CPPUNIT_TEST(testDrawFilledRectWithPolyPolygon2D); TODO SKIA
+    CPPUNIT_TEST(testDrawFilledRectWithPolygon);
+    CPPUNIT_TEST(testDrawFilledRectWithPolyPolygon);
+    CPPUNIT_TEST(testDrawFilledRectWithPolyPolygon2D);
 
     CPPUNIT_TEST(testDrawDiamondWithPolygon);
     CPPUNIT_TEST(testDrawDiamondWithLine);
commit 441c883162d765b290950cf6935612deb9236078
Author:     Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Thu Nov 7 12:24:18 2019 +0100
Commit:     Luboš Luňák <l.lunak at collabora.com>
CommitDate: Fri Nov 8 12:46:09 2019 +0100

    make Skia copyArea() and copyBits() actually copy, not draw
    
    Change-Id: Ifcf8d4d7814daf3631b159cc979f3b8a80052196

diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index 861c3bceb591..6215317b18ee 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -680,7 +680,9 @@ void SkiaSalGraphicsImpl::copyArea(long nDestX, long nDestY, long nSrcX, long nS
                                      << Size(nSrcWidth, nSrcHeight));
     sk_sp<SkImage> image
         = mSurface->makeImageSnapshot(SkIRect::MakeXYWH(nSrcX, nSrcY, nSrcWidth, nSrcHeight));
-    mSurface->getCanvas()->drawImage(image, nDestX, nDestY);
+    SkPaint paint;
+    paint.setBlendMode(SkBlendMode::kSrc); // copy as is, including alpha
+    mSurface->getCanvas()->drawImage(image, nDestX, nDestY, &paint);
     postDraw();
 }
 
@@ -704,11 +706,13 @@ void SkiaSalGraphicsImpl::copyBits(const SalTwoRect& rPosAry, SalGraphics* pSrcG
     SAL_INFO("vcl.skia", "copybits(" << this << "): (" << src << "):" << rPosAry);
     sk_sp<SkImage> image = src->mSurface->makeImageSnapshot(
         SkIRect::MakeXYWH(rPosAry.mnSrcX, rPosAry.mnSrcY, rPosAry.mnSrcWidth, rPosAry.mnSrcHeight));
+    SkPaint paint;
+    paint.setBlendMode(SkBlendMode::kSrc); // copy as is, including alpha
     mSurface->getCanvas()->drawImageRect(image,
                                          SkRect::MakeXYWH(rPosAry.mnDestX, rPosAry.mnDestY,
                                                           rPosAry.mnDestWidth,
                                                           rPosAry.mnDestHeight),
-                                         nullptr);
+                                         &paint);
     postDraw();
 }
 
commit 7bf54f76bb51b7f5642a1a6361eb159f5754cda9
Author:     Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Thu Nov 7 11:59:13 2019 +0100
Commit:     Luboš Luňák <l.lunak at collabora.com>
CommitDate: Fri Nov 8 12:46:04 2019 +0100

    use center of pixels when doing GPU drawing using Skia
    
    According to https://bugs.chromium.org/p/skia/issues/detail?id=9611
    rounding errors may cause off-by-one errors, so compensate when
    converting int->SkScalar in relevant cases.
    
    Change-Id: I72a579064206c216c9f99adc7d7c2c57bbe567d6

diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx
index f45b29abb07e..8225e76d27ff 100644
--- a/vcl/inc/skia/gdiimpl.hxx
+++ b/vcl/inc/skia/gdiimpl.hxx
@@ -215,8 +215,7 @@ protected:
     void setProvider(SalGeometryProvider* provider) { mProvider = provider; }
 
     bool isOffscreen() const { return mProvider == nullptr || mProvider->IsOffScreen(); }
-    // TODO mainly for debugging purposes
-    bool isGPU() const;
+    bool isGPU() const { return mIsGPU; }
 
     void invert(basegfx::B2DPolygon const& rPoly, SalInvert eFlags);
 
@@ -231,6 +230,13 @@ protected:
 
     void drawMask(const SalTwoRect& rPosAry, const SkBitmap& rBitmap, Color nMaskColor);
 
+    // When drawing using GPU, rounding errors may result in off-by-one errors,
+    // see https://bugs.chromium.org/p/skia/issues/detail?id=9611 . Compensate for
+    // it by using centers of pixels (Skia uses float coordinates). In raster case
+    // it seems better to not do this though.
+    SkScalar toSkX(long x) const { return mIsGPU ? x + 0.5 : x; }
+    SkScalar toSkY(long y) const { return mIsGPU ? y + 0.5 : y; }
+
     // Which Skia backend to use.
     enum RenderMethod
     {
@@ -256,6 +262,7 @@ protected:
     SalGeometryProvider* mProvider;
     // The Skia surface that is target of all the rendering.
     sk_sp<SkSurface> mSurface;
+    bool mIsGPU; // whether the surface is GPU-backed
     vcl::Region mClipRegion;
     Color mLineColor;
     Color mFillColor;
diff --git a/vcl/qa/cppunit/BackendTest.cxx b/vcl/qa/cppunit/BackendTest.cxx
index a36c0faa5568..fd9bf10eaff5 100644
--- a/vcl/qa/cppunit/BackendTest.cxx
+++ b/vcl/qa/cppunit/BackendTest.cxx
@@ -438,8 +438,8 @@ public:
     //    CPPUNIT_TEST(testDrawFilledRectWithPolyPolygon); TODO SKIA
     //    CPPUNIT_TEST(testDrawFilledRectWithPolyPolygon2D); TODO SKIA
 
-    //    CPPUNIT_TEST(testDrawDiamondWithPolygon); TODO SKIA
-    //    CPPUNIT_TEST(testDrawDiamondWithLine); TODO SKIA
+    CPPUNIT_TEST(testDrawDiamondWithPolygon);
+    CPPUNIT_TEST(testDrawDiamondWithLine);
     CPPUNIT_TEST(testDrawDiamondWithPolyline);
     CPPUNIT_TEST(testDrawDiamondWithPolylineB2D);
 
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index 612e097b6e12..861c3bceb591 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -40,7 +40,7 @@ namespace
 {
 // Create Skia Path from B2DPolygon
 // TODO - use this for all Polygon / PolyPolygon needs
-void lclPolygonToPath(const basegfx::B2DPolygon& rPolygon, SkPath& rPath)
+void addPolygonToPath(const basegfx::B2DPolygon& rPolygon, SkPath& rPath)
 {
     const sal_uInt32 nPointCount(rPolygon.count());
 
@@ -105,7 +105,7 @@ void lclPolygonToPath(const basegfx::B2DPolygon& rPolygon, SkPath& rPath)
     }
 }
 
-void lclPolyPolygonToPath(const basegfx::B2DPolyPolygon& rPolyPolygon, SkPath& rPath)
+void addPolyPolygonToPath(const basegfx::B2DPolyPolygon& rPolyPolygon, SkPath& rPath)
 {
     const sal_uInt32 nPolygonCount(rPolyPolygon.count());
 
@@ -114,7 +114,7 @@ void lclPolyPolygonToPath(const basegfx::B2DPolyPolygon& rPolyPolygon, SkPath& r
 
     for (const auto& rPolygon : rPolyPolygon)
     {
-        lclPolygonToPath(rPolygon, rPath);
+        addPolygonToPath(rPolygon, rPath);
     }
 }
 
@@ -183,6 +183,7 @@ SkiaSalGraphicsImpl::RenderMethod SkiaSalGraphicsImpl::renderMethodToUse()
 SkiaSalGraphicsImpl::SkiaSalGraphicsImpl(SalGraphics& rParent, SalGeometryProvider* pProvider)
     : mParent(rParent)
     , mProvider(pProvider)
+    , mIsGPU(false)
     , mLineColor(SALCOLOR_NONE)
     , mFillColor(SALCOLOR_NONE)
     , mFlush(new SkiaFlushIdle(this))
@@ -210,6 +211,7 @@ void SkiaSalGraphicsImpl::createSurface()
     // Create surface for offscreen graphics. Subclasses will create GPU-backed
     // surfaces as appropriate.
     mSurface = SkSurface::MakeRasterN32Premul(GetWidth(), GetHeight());
+    mIsGPU = false;
 #ifdef DBG_UTIL
     prefillSurface();
 #endif
@@ -234,6 +236,7 @@ void SkiaSalGraphicsImpl::destroySurface()
     if (mSurface)
         mSurface->flush();
     mSurface.reset();
+    mIsGPU = false;
 }
 
 void SkiaSalGraphicsImpl::DeInit() { destroySurface(); }
@@ -287,7 +290,7 @@ static SkRegion toSkRegion(const vcl::Region& region)
     if (region.HasPolyPolygonOrB2DPolyPolygon())
     {
         SkPath path;
-        lclPolyPolygonToPath(region.GetAsB2DPolyPolygon(), path);
+        addPolyPolygonToPath(region.GetAsB2DPolyPolygon(), path);
         path.setFillType(SkPath::kEvenOdd_FillType);
         SkRegion skRegion;
         skRegion.setPath(path, SkRegion(path.getBounds().roundOut()));
@@ -327,7 +330,7 @@ bool SkiaSalGraphicsImpl::setClipRegion(const vcl::Region& region)
     if (!region.IsEmpty() && !region.IsRectangle())
     {
         SkPath path;
-        lclPolyPolygonToPath(region.GetAsB2DPolyPolygon(), path);
+        addPolyPolygonToPath(region.GetAsB2DPolyPolygon(), path);
         path.setFillType(SkPath::kEvenOdd_FillType);
         canvas->clipPath(path);
     }
@@ -407,9 +410,7 @@ void SkiaSalGraphicsImpl::drawPixel(long nX, long nY, Color nColor)
     paint.setColor(toSkColor(nColor));
     // Apparently drawPixel() is actually expected to set the pixel and not draw it.
     paint.setBlendMode(SkBlendMode::kSrc); // set as is, including alpha
-    if (isGPU()) // TODO this may need caching?
-        ++nX; // https://bugs.chromium.org/p/skia/issues/detail?id=9611
-    canvas->drawPoint(nX, nY, paint);
+    canvas->drawPoint(toSkX(nX), toSkY(nY), paint);
     postDraw();
 }
 
@@ -424,7 +425,7 @@ void SkiaSalGraphicsImpl::drawLine(long nX1, long nY1, long nX2, long nY2)
     SkPaint paint;
     paint.setColor(toSkColor(mLineColor));
     paint.setAntiAlias(mParent.getAntiAliasB2DDraw());
-    canvas->drawLine(nX1, nY1, nX2, nY2, paint);
+    canvas->drawLine(toSkX(nX1), toSkY(nY1), toSkX(nX2), toSkY(nY2), paint);
     postDraw();
 }
 
@@ -521,7 +522,7 @@ bool SkiaSalGraphicsImpl::drawPolyPolygon(const basegfx::B2DHomMatrix& rObjectTo
     aPolyPolygon.transform(rObjectToDevice);
     SAL_INFO("vcl.skia", "drawpolypolygon(" << this << "): " << aPolyPolygon << ":" << mLineColor
                                             << ":" << mFillColor);
-    lclPolyPolygonToPath(aPolyPolygon, aPath);
+    addPolyPolygonToPath(aPolyPolygon, aPath);
     aPath.setFillType(SkPath::kEvenOdd_FillType);
 
     SkPaint aPaint;
@@ -534,6 +535,8 @@ bool SkiaSalGraphicsImpl::drawPolyPolygon(const basegfx::B2DHomMatrix& rObjectTo
     }
     if (mLineColor != SALCOLOR_NONE)
     {
+        if (isGPU()) // Apply the same adjustment as toSkX()/toSkY() do.
+            aPath.offset(0.5, 0.5, nullptr);
         aPaint.setColor(toSkColorWithTransparency(mLineColor, fTransparency));
         aPaint.setStyle(SkPaint::kStroke_Style);
         mSurface->getCanvas()->drawPath(aPath, aPaint);
@@ -626,14 +629,12 @@ bool SkiaSalGraphicsImpl::drawPolyLine(const basegfx::B2DHomMatrix& rObjectToDev
     aPaint.setAntiAlias(mParent.getAntiAliasB2DDraw());
 
     SkPath aPath;
-    lclPolygonToPath(rPolyLine, aPath);
+    addPolygonToPath(rPolyLine, aPath);
     aPath.setFillType(SkPath::kEvenOdd_FillType);
-    SkMatrix matrix = SkMatrix::MakeTrans(0.5, 0.5);
-    {
-        SkAutoCanvasRestore autoRestore(mSurface->getCanvas(), true);
-        mSurface->getCanvas()->concat(matrix);
-        mSurface->getCanvas()->drawPath(aPath, aPaint);
-    }
+    // Apply the same adjustment as toSkX()/toSkY() do. Do it here even in the non-GPU
+    // case as it seems to produce better results.
+    aPath.offset(0.5, 0.5, nullptr);
+    mSurface->getCanvas()->drawPath(aPath, aPaint);
     postDraw();
 
     return true;
@@ -838,7 +839,7 @@ void SkiaSalGraphicsImpl::invert(basegfx::B2DPolygon const& rPoly, SalInvert eFl
     if (eFlags == SalInvert::TrackFrame)
     {
         SkPath aPath;
-        lclPolygonToPath(rPoly, aPath);
+        addPolygonToPath(rPoly, aPath);
         aPath.setFillType(SkPath::kEvenOdd_FillType);
         SkPaint aPaint;
         aPaint.setStrokeWidth(2);
@@ -853,7 +854,7 @@ void SkiaSalGraphicsImpl::invert(basegfx::B2DPolygon const& rPoly, SalInvert eFl
     else
     {
         SkPath aPath;
-        lclPolygonToPath(rPoly, aPath);
+        addPolygonToPath(rPoly, aPath);
         aPath.setFillType(SkPath::kEvenOdd_FillType);
         SkPaint aPaint;
         aPaint.setColor(SkColorSetARGB(255, 255, 255, 255));
@@ -1057,13 +1058,6 @@ bool SkiaSalGraphicsImpl::supportsOperation(OutDevSupportType eType) const
     }
 }
 
-bool SkiaSalGraphicsImpl::isGPU() const
-{
-    return mSurface.get()
-           && mSurface->getBackendRenderTarget(SkSurface::kFlushWrite_BackendHandleAccess)
-                  .isValid();
-}
-
 #ifdef DBG_UTIL
 void SkiaSalGraphicsImpl::dump(const char* file) const
 {
diff --git a/vcl/skia/win/gdiimpl.cxx b/vcl/skia/win/gdiimpl.cxx
index f908ace261a3..e9db555c25c0 100644
--- a/vcl/skia/win/gdiimpl.cxx
+++ b/vcl/skia/win/gdiimpl.cxx
@@ -54,6 +54,7 @@ void WinSkiaSalGraphicsImpl::createSurface()
                 mSurface = SkSurface::MakeRenderTarget(
                     SkiaVulkanGrContext::getGrContext(), SkBudgeted::kNo,
                     SkImageInfo::MakeN32Premul(GetWidth(), GetHeight()));
+                mIsGPU = true;
                 assert(mSurface.get());
 #ifdef DBG_UTIL
                 prefillSurface();
@@ -74,10 +75,12 @@ void WinSkiaSalGraphicsImpl::createSurface()
         case RenderRaster:
             mWindowContext = sk_app::window_context_factory::MakeRasterForWin(mWinParent.gethWnd(),
                                                                               displayParams);
+            mIsGPU = false;
             break;
         case RenderVulkan:
             mWindowContext = sk_app::window_context_factory::MakeVulkanForWin(mWinParent.gethWnd(),
                                                                               displayParams);
+            mIsGPU = true;
             break;
     }
     assert(SkToBool(mWindowContext)); // TODO
diff --git a/vcl/skia/x11/gdiimpl.cxx b/vcl/skia/x11/gdiimpl.cxx
index af602d35bede..72fc311f7aaa 100644
--- a/vcl/skia/x11/gdiimpl.cxx
+++ b/vcl/skia/x11/gdiimpl.cxx
@@ -49,6 +49,7 @@ void X11SkiaSalGraphicsImpl::createSurface()
                 mSurface = SkSurface::MakeRenderTarget(
                     SkiaVulkanGrContext::getGrContext(), SkBudgeted::kNo,
                     SkImageInfo::MakeN32Premul(GetWidth(), GetHeight()));
+                mIsGPU = true;
                 assert(mSurface.get());
 #ifdef DBG_UTIL
                 prefillSurface();
@@ -83,10 +84,12 @@ void X11SkiaSalGraphicsImpl::createSurface()
         case RenderRaster:
             mWindowContext
                 = sk_app::window_context_factory::MakeRasterForXlib(winInfo, displayParams);
+            mIsGPU = false;
             break;
         case RenderVulkan:
             mWindowContext
                 = sk_app::window_context_factory::MakeVulkanForXlib(winInfo, displayParams);
+            mIsGPU = true;
             break;
     }
     assert(SkToBool(mWindowContext)); // TODO


More information about the Libreoffice-commits mailing list