[Libreoffice-commits] core.git: 2 commits - vcl/skia
Luboš Luňák (via logerrit)
logerrit at kemper.freedesktop.org
Fri Dec 13 13:14:17 UTC 2019
vcl/skia/gdiimpl.cxx | 63 +++++++++++++++------------------------------------
1 file changed, 19 insertions(+), 44 deletions(-)
New commits:
commit dcd3b5a52b343aa82933ba27be3396f46ee465ee
Author: Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Fri Dec 13 12:20:43 2019 +0100
Commit: Luboš Luňák <l.lunak at collabora.com>
CommitDate: Fri Dec 13 14:13:42 2019 +0100
use SkCanvas::clipPath() as the real solution
SkCanvas::clipRegion() is buggy and may be removed in future
(https://bugs.chromium.org/p/skia/issues/detail?id=9580).
Change-Id: I7070d3616e579ec8ce795f6a4bdef66b1ca1c493
Reviewed-on: https://gerrit.libreoffice.org/85102
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak at collabora.com>
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index 532f819080a4..25980de0cc60 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -343,40 +343,6 @@ void SkiaSalGraphicsImpl::checkSurface()
}
}
-static SkIRect toSkIRect(const tools::Rectangle& rectangle)
-{
- return SkIRect::MakeXYWH(rectangle.Left(), rectangle.Top(), rectangle.GetWidth(),
- rectangle.GetHeight());
-}
-
-static SkRegion toSkRegion(const vcl::Region& region)
-{
- if (region.IsEmpty())
- return SkRegion();
- if (region.IsRectangle())
- return SkRegion(toSkIRect(region.GetBoundRect()));
- // Prefer rectangles to polygons (simpler and also see the addPolygonToPath() comment).
- if (region.getRegionBand())
- {
- SkRegion skRegion;
- RectangleVector rectangles;
- region.GetRegionRectangles(rectangles);
- for (const tools::Rectangle& rect : rectangles)
- skRegion.op(toSkIRect(rect), SkRegion::kUnion_Op);
- return skRegion;
- }
- else
- {
- assert(region.HasPolyPolygonOrB2DPolyPolygon());
- SkPath path;
- addPolyPolygonToPath(region.GetAsB2DPolyPolygon(), path);
- path.setFillType(SkPathFillType::kEvenOdd);
- SkRegion skRegion;
- skRegion.setPath(path, SkRegion(path.getBounds().roundOut()));
- return skRegion;
- }
-}
-
bool SkiaSalGraphicsImpl::setClipRegion(const vcl::Region& region)
{
if (mClipRegion == region)
@@ -393,34 +359,23 @@ bool SkiaSalGraphicsImpl::setClipRegion(const vcl::Region& region)
assert(canvas->getSaveCount() == 2); // = there is just one save()
canvas->restore();
canvas->save();
-#if 1
- // TODO
- // SkCanvas::clipRegion() is buggy with Vulkan, use SkCanvas::clipPath().
- // https://bugs.chromium.org/p/skia/issues/detail?id=9580
- // This is further complicated by rectangle->polygon area conversions
- // being problematic (see addPolygonToPath() comment), so handle rectangles
- // first before resorting to polygons.
+ SkPath path;
+ // Handle polygons last, since rectangle->polygon area conversions
+ // are problematic (see addPolygonToPath() comment).
if (region.getRegionBand())
{
RectangleVector rectangles;
region.GetRegionRectangles(rectangles);
- SkPath path;
for (const tools::Rectangle& rectangle : rectangles)
path.addRect(SkRect::MakeXYWH(rectangle.getX(), rectangle.getY(), rectangle.GetWidth(),
rectangle.GetHeight()));
- path.setFillType(SkPathFillType::kEvenOdd);
- canvas->clipPath(path);
}
- else if (!region.IsEmpty() && !region.IsRectangle())
+ else if (!region.IsEmpty())
{
- SkPath path;
addPolyPolygonToPath(region.GetAsB2DPolyPolygon(), path);
- path.setFillType(SkPathFillType::kEvenOdd);
- canvas->clipPath(path);
}
- else
-#endif
- canvas->clipRegion(toSkRegion(region));
+ path.setFillType(SkPathFillType::kEvenOdd);
+ canvas->clipPath(path);
return true;
}
commit fe8ca52b1265e5da0e1ef645f364296cf9ee8b12
Author: Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Thu Dec 12 16:33:04 2019 +0100
Commit: Luboš Luňák <l.lunak at collabora.com>
CommitDate: Fri Dec 13 14:13:29 2019 +0100
fix off-by-one with rectangle->polygon Skia clipping (tdf#129211)
This appears to be yet another case of
https://lists.freedesktop.org/archives/libreoffice/2019-November/083709.html,
where converting rectangles to polygons for areas has unexpected results
for the right and bottom edge pixels.
Change-Id: I819f3eb1a739ac8fd18d792b7031b82fe52e4b4c
Reviewed-on: https://gerrit.libreoffice.org/85061
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak at collabora.com>
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index ff700c5f0362..532f819080a4 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -38,7 +38,11 @@
namespace
{
// Create Skia Path from B2DPolygon
-// TODO - use this for all Polygon / PolyPolygon needs
+// Note that polygons generally have the complication that when used
+// for area (fill) operations they usually miss the right-most and
+// 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)
{
const sal_uInt32 nPointCount(rPolygon.count());
@@ -351,22 +355,24 @@ static SkRegion toSkRegion(const vcl::Region& region)
return SkRegion();
if (region.IsRectangle())
return SkRegion(toSkIRect(region.GetBoundRect()));
- if (region.HasPolyPolygonOrB2DPolyPolygon())
+ // Prefer rectangles to polygons (simpler and also see the addPolygonToPath() comment).
+ if (region.getRegionBand())
{
- SkPath path;
- addPolyPolygonToPath(region.GetAsB2DPolyPolygon(), path);
- path.setFillType(SkPathFillType::kEvenOdd);
SkRegion skRegion;
- skRegion.setPath(path, SkRegion(path.getBounds().roundOut()));
+ RectangleVector rectangles;
+ region.GetRegionRectangles(rectangles);
+ for (const tools::Rectangle& rect : rectangles)
+ skRegion.op(toSkIRect(rect), SkRegion::kUnion_Op);
return skRegion;
}
else
{
+ assert(region.HasPolyPolygonOrB2DPolyPolygon());
+ SkPath path;
+ addPolyPolygonToPath(region.GetAsB2DPolyPolygon(), path);
+ path.setFillType(SkPathFillType::kEvenOdd);
SkRegion skRegion;
- RectangleVector rectangles;
- region.GetRegionRectangles(rectangles);
- for (const tools::Rectangle& rect : rectangles)
- skRegion.op(toSkIRect(rect), SkRegion::kUnion_Op);
+ skRegion.setPath(path, SkRegion(path.getBounds().roundOut()));
return skRegion;
}
}
@@ -391,7 +397,21 @@ bool SkiaSalGraphicsImpl::setClipRegion(const vcl::Region& region)
// TODO
// SkCanvas::clipRegion() is buggy with Vulkan, use SkCanvas::clipPath().
// https://bugs.chromium.org/p/skia/issues/detail?id=9580
- if (!region.IsEmpty() && !region.IsRectangle())
+ // This is further complicated by rectangle->polygon area conversions
+ // being problematic (see addPolygonToPath() comment), so handle rectangles
+ // first before resorting to polygons.
+ if (region.getRegionBand())
+ {
+ RectangleVector rectangles;
+ region.GetRegionRectangles(rectangles);
+ SkPath path;
+ for (const tools::Rectangle& rectangle : rectangles)
+ path.addRect(SkRect::MakeXYWH(rectangle.getX(), rectangle.getY(), rectangle.GetWidth(),
+ rectangle.GetHeight()));
+ path.setFillType(SkPathFillType::kEvenOdd);
+ canvas->clipPath(path);
+ }
+ else if (!region.IsEmpty() && !region.IsRectangle())
{
SkPath path;
addPolyPolygonToPath(region.GetAsB2DPolyPolygon(), path);
More information about the Libreoffice-commits
mailing list