vcl/skia/gdiimpl.cxx | 63 +++++++++++++++------------------------------------ 1 file changed, 19 insertions(+), 44 deletions(-)
New commits: commit dcd3b5a52b343aa82933ba27be3396f46ee465ee Author: Luboš Luňák <[email protected]> AuthorDate: Fri Dec 13 12:20:43 2019 +0100 Commit: Luboš Luňák <[email protected]> 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 <[email protected]> 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 <[email protected]> AuthorDate: Thu Dec 12 16:33:04 2019 +0100 Commit: Luboš Luňák <[email protected]> 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 <[email protected]> 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); _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
