ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

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

Browse Source 
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>
This commit is contained in:
Luboš Luňák
2021-11-30 22:06:15 +01:00
parent a49cd1cebd
commit cdebd76284
3 changed files with 124 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
vcl/CppunitTest_vcl_backend_test.mk
+1
View File
@@ -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,\
vcl/qa/cppunit/BackendTest.cxx
+96
View File
@@ -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();
};
vcl/skia/gdiimpl.cxx
+27 -3
View File
@@ -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,