vcl/CppunitTest_vcl_backend_test.mk | 1
vcl/qa/cppunit/BackendTest.cxx | 96 ++++++++++++++++++++++++++++++++++++
vcl/skia/gdiimpl.cxx | 30 ++++++++++-
3 files changed, 124 insertions(+), 3 deletions(-)
New commits:
commit cdebd76284204f6a34df2a01d4eaedbd540c5fe6
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Tue Nov 30 18:05:20 2021 +0100
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Tue Nov 30 22:06:15 2021 +0100
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.lu...@collabora.com>
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 f1670e8c6db5..5c014b918ded 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -1220,14 +1220,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,
@@ -1236,6 +1257,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,
