fix Skia copyArea() not coping with coordinates outside (tdf#145811)

Apparently the call is expected to handle even copies that are
partially outside of the area, e.g. window scrolling seems to do
this occassionally.

Change-Id: I9a06c047f00d6b5b920d61f577baa9181bdc5a2b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126147
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>
(cherry picked from commit cdebd76284204f6a34df2a01d4eaedbd540c5fe6)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126130

3 files changed, 124 insertions, 3 deletions

diff --git a/vcl/CppunitTest_vcl_backend_test.mk b/vcl/CppunitTest_vcl_backend_test.mk
index 5a886224034a..53338c3490a6 100644
--- a/vcl/CppunitTest_vcl_backend_test.mk
+++ b/vcl/CppunitTest_vcl_backend_test.mk
@@ -32,6 +32,7 @@ $(eval $(call gb_CppunitTest_use_vcl,vcl_backend_test))
 
 $(eval $(call gb_CppunitTest_use_externals,vcl_backend_test,\
     boost_headers\
+    harfbuzz \
 ))
 
 $(eval $(call gb_CppunitTest_set_include,vcl_backend_test,\
diff --git a/vcl/qa/cppunit/BackendTest.cxx b/vcl/qa/cppunit/BackendTest.cxx
index 7badb40464be..619e377194c9 100644
--- a/vcl/qa/cppunit/BackendTest.cxx
+++ b/vcl/qa/cppunit/BackendTest.cxx
@@ -18,6 +18,7 @@
 
 #include <svdata.hxx>
 #include <salinst.hxx>
+#include <salgdi.hxx>
 
 #include <test/outputdevice.hxx>
 
@@ -1395,6 +1396,100 @@ public:
 #endif
     }
 
+    void testTdf145811()
+    {
+// TODO: This unit test is not executed for macOS unless bitmap scaling is implemented
+#ifndef MACOSX
+        // VCL may call copyArea()/copyBits() of backends even with coordinates partially
+        // outside of the device, so try various copying like that.
+        ScopedVclPtr<VirtualDevice> device1 = VclPtr<VirtualDevice>::Create(DeviceFormat::DEFAULT);
+        device1->SetOutputSizePixel(Size(100, 100));
+        device1->SetBackground(Wallpaper(COL_YELLOW));
+        device1->Erase();
+        device1->SetLineColor(COL_BLUE);
+        device1->DrawPixel(Point(0, 0), COL_BLUE);
+        device1->DrawPixel(Point(99, 99), COL_BLUE);
+
+        // Plain 1:1 copy device1->device2.
+        ScopedVclPtr<VirtualDevice> device2 = VclPtr<VirtualDevice>::Create(DeviceFormat::DEFAULT);
+        device2->SetOutputSizePixel(Size(100, 100));
+        device2->DrawOutDev(Point(0, 0), Size(100, 100), Point(0, 0), Size(100, 100), *device1);
+        exportDevice("tdf145811-1.png", device2);
+        CPPUNIT_ASSERT_EQUAL(COL_BLUE, device2->GetPixel(Point(0, 0)));
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(1, 1)));
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(98, 98)));
+        CPPUNIT_ASSERT_EQUAL(COL_BLUE, device2->GetPixel(Point(99, 99)));
+
+        // For the rest call directly SalGraphics, because OutputDevice does range checking,
+        // but other code may call copyArea()/copyBits() of SalGraphics directly without range checking.
+        SalGraphics* graphics1 = device1->GetGraphics();
+        SalGraphics* graphics2 = device2->GetGraphics();
+
+        device2->DrawOutDev(Point(0, 0), Size(100, 100), Point(0, 0), Size(100, 100), *device1);
+        // Copy device1->device2 offset by 10,10.
+        graphics2->CopyBits(SalTwoRect(0, 0, 100, 100, 10, 10, 100, 100), *graphics1, *device2,
+                            *device1);
+        exportDevice("tdf145811-2.png", device2);
+        CPPUNIT_ASSERT_EQUAL(COL_BLUE, device2->GetPixel(Point(0, 0))); // unmodified
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(9, 9)));
+        CPPUNIT_ASSERT_EQUAL(COL_BLUE, device2->GetPixel(Point(10, 10)));
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(11, 11)));
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(99, 99)));
+
+        device2->DrawOutDev(Point(0, 0), Size(100, 100), Point(0, 0), Size(100, 100), *device1);
+        // Copy area of device2 offset by 10,10.
+        graphics2->CopyArea(10, 10, 0, 0, 100, 100, *device1);
+        exportDevice("tdf145811-3.png", device2);
+        CPPUNIT_ASSERT_EQUAL(COL_BLUE, device2->GetPixel(Point(0, 0))); // unmodified
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(9, 9)));
+        CPPUNIT_ASSERT_EQUAL(COL_BLUE, device2->GetPixel(Point(10, 10)));
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(11, 11)));
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(99, 99)));
+
+        device2->DrawOutDev(Point(0, 0), Size(100, 100), Point(0, 0), Size(100, 100), *device1);
+        // Copy device1->device2 offset by -20,-20.
+        graphics2->CopyBits(SalTwoRect(0, 0, 100, 100, -20, -20, 100, 100), *graphics1, *device2,
+                            *device1);
+        exportDevice("tdf145811-4.png", device2);
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(0, 0)));
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(78, 78)));
+        CPPUNIT_ASSERT_EQUAL(COL_BLUE, device2->GetPixel(Point(79, 79)));
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(80, 80)));
+        CPPUNIT_ASSERT_EQUAL(COL_BLUE, device2->GetPixel(Point(99, 99))); // unmodified
+
+        device2->DrawOutDev(Point(0, 0), Size(100, 100), Point(0, 0), Size(100, 100), *device1);
+        // Copy area of device2 offset by -20,-20.
+        graphics2->CopyArea(-20, -20, 0, 0, 100, 100, *device1);
+        exportDevice("tdf145811-5.png", device2);
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(0, 0)));
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(78, 78)));
+        CPPUNIT_ASSERT_EQUAL(COL_BLUE, device2->GetPixel(Point(79, 79)));
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(80, 80)));
+        CPPUNIT_ASSERT_EQUAL(COL_BLUE, device2->GetPixel(Point(99, 99))); // unmodified
+
+        device2->DrawOutDev(Point(0, 0), Size(100, 100), Point(0, 0), Size(100, 100), *device1);
+        // Copy device1->device2 offset by -10,-10 starting from -20,-20 at 150x150 size
+        // (i.e. outside in all directions).
+        graphics2->CopyBits(SalTwoRect(-20, -20, 150, 150, -30, -30, 150, 150), *graphics1,
+                            *device2, *device1);
+        exportDevice("tdf145811-6.png", device2);
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(0, 0)));
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(88, 88)));
+        CPPUNIT_ASSERT_EQUAL(COL_BLUE, device2->GetPixel(Point(89, 89)));
+        // (90,90) and further originate from outside and may be garbage.
+
+        device2->DrawOutDev(Point(0, 0), Size(100, 100), Point(0, 0), Size(100, 100), *device1);
+        // Copy area of device2 offset by -10,-10 starting from -20,-20 at 150x150 size
+        // (i.e. outside in all directions).
+        graphics2->CopyArea(-30, -30, -20, -20, 150, 150, *device1);
+        exportDevice("tdf145811-7.png", device2);
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(0, 0)));
+        CPPUNIT_ASSERT_EQUAL(COL_YELLOW, device2->GetPixel(Point(88, 88)));
+        CPPUNIT_ASSERT_EQUAL(COL_BLUE, device2->GetPixel(Point(89, 89)));
+        // (90,90) and further originate from outside and may be garbage.
+#endif
+    }
+
     CPPUNIT_TEST_SUITE(BackendTest);
     CPPUNIT_TEST(testDrawRectWithRectangle);
     CPPUNIT_TEST(testDrawRectWithPixel);
@@ -1516,6 +1611,7 @@ public:
 
     CPPUNIT_TEST(testTdf124848);
     CPPUNIT_TEST(testTdf136171);
+    CPPUNIT_TEST(testTdf145811);
 
     CPPUNIT_TEST_SUITE_END();
 };
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index 3d7697c4dcfa..bab66a93c63e 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -1318,14 +1318,35 @@ void SkiaSalGraphicsImpl::privateCopyBits(const SalTwoRect& rPosAry, SkiaSalGrap
                                         rPosAry.mnSrcHeight);
     SkRect destRect = SkRect::MakeXYWH(rPosAry.mnDestX, rPosAry.mnDestY, rPosAry.mnDestWidth,
                                        rPosAry.mnDestHeight);
-    // Scaling for source coordinates must be done manually.
-    if (src->mScaling != 1)
-        srcRect = scaleRect(srcRect, src->mScaling);
+
+    if (!SkIRect::Intersects(srcRect, SkIRect::MakeWH(src->GetWidth(), src->GetHeight()))
+        || !SkRect::Intersects(destRect, SkRect::MakeWH(GetWidth(), GetHeight())))
+        return;
+
     if (src == this)
     {
         // Copy-to-self means that we'd take a snapshot, which would refcount the data,
         // and then drawing would result in copy in write, copying the entire surface.
         // Try to copy less by making a snapshot of only what is needed.
+        // A complication here is that drawImageRect() can handle coordinates outside
+        // of surface fine, but makeImageSnapshot() will crop to the surface area,
+        // so do that manually here in order to adjust also destination rectangle.
+        if (srcRect.x() < 0 || srcRect.y() < 0)
+        {
+            destRect.fLeft += -srcRect.x();
+            destRect.fTop += -srcRect.y();
+            srcRect.adjust(-srcRect.x(), -srcRect.y(), 0, 0);
+        }
+        // Note that right() and bottom() are not inclusive (are outside of the rect).
+        if (srcRect.right() - 1 > GetWidth() || srcRect.bottom() - 1 > GetHeight())
+        {
+            destRect.fRight += GetWidth() - srcRect.right();
+            destRect.fBottom += GetHeight() - srcRect.bottom();
+            srcRect.adjust(0, 0, GetWidth() - srcRect.right(), GetHeight() - srcRect.bottom());
+        }
+        // Scaling for source coordinates must be done manually.
+        if (src->mScaling != 1)
+            srcRect = scaleRect(srcRect, src->mScaling);
         sk_sp<SkImage> image = makeCheckedImageSnapshot(src->mSurface, srcRect);
         srcRect.offset(-srcRect.x(), -srcRect.y());
         getDrawCanvas()->drawImageRect(image, SkRect::Make(srcRect), destRect,
@@ -1334,6 +1355,9 @@ void SkiaSalGraphicsImpl::privateCopyBits(const SalTwoRect& rPosAry, SkiaSalGrap
     }
     else
     {
+        // Scaling for source coordinates must be done manually.
+        if (src->mScaling != 1)
+            srcRect = scaleRect(srcRect, src->mScaling);
         // Do not use makeImageSnapshot(rect), as that one may make a needless data copy.
         getDrawCanvas()->drawImageRect(makeCheckedImageSnapshot(src->mSurface),
                                        SkRect::Make(srcRect), destRect,