vcl/qa/cppunit/skia/skia.cxx | 23 +++++++++++++++++
vcl/skia/gdiimpl.cxx | 57 ++++++++++++++++++++++++++++++-------------
2 files changed, 64 insertions(+), 16 deletions(-)
New commits:
commit 5b292e878703ebc32e875406f4116cba145a9042
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Wed Nov 18 16:30:52 2020 +0100
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Thu Nov 19 21:45:14 2020 +0100
avoid Skia floating point position fixups for rectangles (tdf#137329)
Avoid to toSkX()/toSkY() tweaks for rectangular areas, so with AA
enabled it leads to fuzzy edges, and rectangles should line up
perfectly with all pixels even without tweaks.
Change-Id: I45bf5a57a9f2d941eb7ec224992fc452481a2f98
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106060
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lu...@collabora.com>
diff --git a/vcl/qa/cppunit/skia/skia.cxx b/vcl/qa/cppunit/skia/skia.cxx
index d13e1530f95e..1f04c5825584 100644
--- a/vcl/qa/cppunit/skia/skia.cxx
+++ b/vcl/qa/cppunit/skia/skia.cxx
@@ -35,6 +35,7 @@ public:
void testInterpretAs8Bit();
void testAlphaBlendWith();
void testBitmapCopyOnWrite();
+ void testTdf137329();
CPPUNIT_TEST_SUITE(SkiaTest);
CPPUNIT_TEST(testBitmapErase);
@@ -42,6 +43,7 @@ public:
CPPUNIT_TEST(testInterpretAs8Bit);
CPPUNIT_TEST(testAlphaBlendWith);
CPPUNIT_TEST(testBitmapCopyOnWrite);
+ CPPUNIT_TEST(testTdf137329);
CPPUNIT_TEST_SUITE_END();
private:
@@ -303,6 +305,27 @@ void SkiaTest::testBitmapCopyOnWrite()
CPPUNIT_ASSERT(bitmap.unittestGetAlphaImage() != oldAlphaImage);
}
+void SkiaTest::testTdf137329()
+{
+ // Draw a filled polygon in the entire device, with AA enabled.
+ // All pixels in the device should be black, even those at edges (i.e. not
affected by AA).
+ ScopedVclPtr<VirtualDevice> device =
VclPtr<VirtualDevice>::Create(DeviceFormat::DEFAULT);
+ device->SetOutputSizePixel(Size(10, 10));
+ device->SetBackground(Wallpaper(COL_WHITE));
+ device->SetAntialiasing(AntialiasingFlags::Enable);
+ device->Erase();
+ device->SetLineColor();
+ device->SetFillColor(COL_BLACK);
+ device->DrawPolyPolygon(
+ basegfx::B2DPolyPolygon(basegfx::B2DPolygon{ { 0, 0 }, { 10, 0 }, {
10, 10 }, { 0, 10 } }));
+ // savePNG("/tmp/tdf137329.png", device);
+ CPPUNIT_ASSERT_EQUAL(COL_BLACK, device->GetPixel(Point(0, 0)));
+ CPPUNIT_ASSERT_EQUAL(COL_BLACK, device->GetPixel(Point(9, 0)));
+ CPPUNIT_ASSERT_EQUAL(COL_BLACK, device->GetPixel(Point(9, 9)));
+ CPPUNIT_ASSERT_EQUAL(COL_BLACK, device->GetPixel(Point(0, 9)));
+ CPPUNIT_ASSERT_EQUAL(COL_BLACK, device->GetPixel(Point(4, 4)));
+}
+
} // namespace
CPPUNIT_TEST_SUITE_REGISTRATION(SkiaTest);
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index 1f1c4002f94d..c7035e7d3de7 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -53,7 +53,8 @@ namespace
// bottom-most line of pixels of the bounding rectangle (see
//
https://lists.freedesktop.org/archives/libreoffice/2019-November/083709.html).
// So be careful with rectangle->polygon conversions (generally avoid them).
-void addPolygonToPath(const basegfx::B2DPolygon& rPolygon, SkPath& rPath)
+void addPolygonToPath(const basegfx::B2DPolygon& rPolygon, SkPath& rPath,
+ bool* hasOnlyOrthogonal = nullptr)
{
const sal_uInt32 nPointCount(rPolygon.count());
@@ -88,6 +89,11 @@ void addPolygonToPath(const basegfx::B2DPolygon& rPolygon,
SkPath& rPath)
else if (!bHasCurves)
{
rPath.lineTo(aCurrentPoint.getX(), aCurrentPoint.getY());
+ // If asked for, check whether the polygon has a line that is not
+ // strictly horizontal or vertical.
+ if (hasOnlyOrthogonal != nullptr && aCurrentPoint.getX() !=
aPreviousPoint.getX()
+ && aCurrentPoint.getY() != aPreviousPoint.getY())
+ *hasOnlyOrthogonal = false;
}
else
{
@@ -96,7 +102,12 @@ void addPolygonToPath(const basegfx::B2DPolygon& rPolygon,
SkPath& rPath)
if (aPreviousControlPoint.equal(aPreviousPoint)
&& aCurrentControlPoint.equal(aCurrentPoint))
+ {
rPath.lineTo(aCurrentPoint.getX(), aCurrentPoint.getY()); // a
straight line
+ if (hasOnlyOrthogonal != nullptr && aCurrentPoint.getX() !=
aPreviousPoint.getX()
+ && aCurrentPoint.getY() != aPreviousPoint.getY())
+ *hasOnlyOrthogonal = false;
+ }
else
{
if (aPreviousControlPoint.equal(aPreviousPoint))
@@ -112,6 +123,8 @@ void addPolygonToPath(const basegfx::B2DPolygon& rPolygon,
SkPath& rPath)
rPath.cubicTo(aPreviousControlPoint.getX(),
aPreviousControlPoint.getY(),
aCurrentControlPoint.getX(),
aCurrentControlPoint.getY(),
aCurrentPoint.getX(), aCurrentPoint.getY());
+ if (hasOnlyOrthogonal != nullptr)
+ *hasOnlyOrthogonal = false;
}
}
aPreviousPoint = aCurrentPoint;
@@ -123,7 +136,8 @@ void addPolygonToPath(const basegfx::B2DPolygon& rPolygon,
SkPath& rPath)
}
}
-void addPolyPolygonToPath(const basegfx::B2DPolyPolygon& rPolyPolygon, SkPath&
rPath)
+void addPolyPolygonToPath(const basegfx::B2DPolyPolygon& rPolyPolygon, SkPath&
rPath,
+ bool* hasOnlyOrthogonal = nullptr)
{
const sal_uInt32 nPolygonCount(rPolyPolygon.count());
@@ -132,7 +146,7 @@ void addPolyPolygonToPath(const basegfx::B2DPolyPolygon&
rPolyPolygon, SkPath& r
for (const auto& rPolygon : rPolyPolygon)
{
- addPolygonToPath(rPolygon, rPath);
+ addPolygonToPath(rPolygon, rPath, hasOnlyOrthogonal);
}
}
@@ -852,36 +866,47 @@ void SkiaSalGraphicsImpl::performDrawPolyPolygon(const
basegfx::B2DPolyPolygon&
preDraw();
SkPath polygonPath;
- addPolyPolygonToPath(aPolyPolygon, polygonPath);
+ bool hasOnlyOrthogonal = true;
+ addPolyPolygonToPath(aPolyPolygon, polygonPath, &hasOnlyOrthogonal);
polygonPath.setFillType(SkPathFillType::kEvenOdd);
addUpdateRegion(polygonPath.getBounds());
SkPaint aPaint;
aPaint.setAntiAlias(useAA);
- // We normally use pixel at their center positions, but slightly off (see
toSkX/Y()).
- // With AA lines that "slightly off" causes tiny changes of color, making
some tests
- // fail. Since moving AA-ed line slightly to a side doesn't cause any real
visual
- // difference, just place exactly at the center. tdf#134346
- const SkScalar posFix = useAA ? toSkXYFix : 0;
+
+ // For lines we use toSkX()/toSkY() in order to pass centers of pixels to
Skia,
+ // as that leads to better results with floating-point coordinates
+ // (e.g. https://bugs.chromium.org/p/skia/issues/detail?id=9611).
+ // But that means that we generally need to use it also for areas, so that
they
+ // line up properly if used together (tdf#134346).
+ // On the other hand, with AA enabled and rectangular areas, this leads to
fuzzy
+ // edges (tdf#137329). But since rectangular areas line up perfectly to
pixels
+ // everywhere, it shouldn't be necessary to do this for them.
+ // So if AA is enabled, avoid this fixup for rectangular areas.
+ if (!useAA || !hasOnlyOrthogonal)
+ {
+ // We normally use pixel at their center positions, but slightly off
(see toSkX/Y()).
+ // With AA lines that "slightly off" causes tiny changes of color,
making some tests
+ // fail. Since moving AA-ed line slightly to a side doesn't cause any
real visual
+ // difference, just place exactly at the center. tdf#134346
+ const SkScalar posFix = useAA ? toSkXYFix : 0;
+ polygonPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, nullptr);
+ }
if (mFillColor != SALCOLOR_NONE)
{
- SkPath path;
- polygonPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, &path);
aPaint.setColor(toSkColorWithTransparency(mFillColor, fTransparency));
aPaint.setStyle(SkPaint::kFill_Style);
// HACK: If the polygon is just a line, it still should be drawn. But
when filling
// Skia doesn't draw empty polygons, so in that case ensure the line
is drawn.
- if (mLineColor == SALCOLOR_NONE && path.getBounds().isEmpty())
+ if (mLineColor == SALCOLOR_NONE && polygonPath.getBounds().isEmpty())
aPaint.setStyle(SkPaint::kStroke_Style);
- getDrawCanvas()->drawPath(path, aPaint);
+ getDrawCanvas()->drawPath(polygonPath, aPaint);
}
if (mLineColor != SALCOLOR_NONE)
{
- SkPath path;
- polygonPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, &path);
aPaint.setColor(toSkColorWithTransparency(mLineColor, fTransparency));
aPaint.setStyle(SkPaint::kStroke_Style);
- getDrawCanvas()->drawPath(path, aPaint);
+ getDrawCanvas()->drawPath(polygonPath, aPaint);
}
postDraw();
#if defined LINUX
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
