diff options
author | Luboš Luňák <l.lunak@collabora.com> | 2019-12-12 16:33:04 +0100 |
---|---|---|
committer | Luboš Luňák <l.lunak@collabora.com> | 2019-12-13 14:13:29 +0100 |
commit | fe8ca52b1265e5da0e1ef645f364296cf9ee8b12 (patch) | |
tree | e30fcf2a91462a779f089322e280f1e7fd470193 | |
parent | 4a6041235d923755eda3b3f5e54f6ae5a5436072 (diff) |
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@collabora.com>
-rw-r--r-- | vcl/skia/gdiimpl.cxx | 42 |
1 files changed, 31 insertions, 11 deletions
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); |