diff options
author | 2017-10-09 22:58:53 +0000 | |
---|---|---|
committer | 2017-10-09 22:59:06 +0000 | |
commit | 1c35e571c3f041069e2b16979a733d784c7f0f4c (patch) | |
tree | 8951628c9eb41658925994122f7760fcfc1b9491 /src/core | |
parent | 3f9855293350bd1949855a6002943d3cef97a32f (diff) |
Revert "Revert "Fix a couple float-cast-overflow in SkScan*.""
This reverts commit b6abb9b4e088abee8b8dfcce9c9f7eb759518198.
Reason for revert: It doesn't make sense that this CL would affect the tests implicated in the perf regression in skia:7143, and the revert had no effect on the perf of those tests. Seems like the perf alert was either noise or due to a different CL.
Original change's description:
> Revert "Fix a couple float-cast-overflow in SkScan*."
>
> This reverts commit 3cd0bef0fd9d062bbcc313c329b4f31925e8ded7.
>
> Reason for revert: https://bugs.chromium.org/p/skia/issues/detail?id=7143
>
> Original change's description:
> > Fix a couple float-cast-overflow in SkScan*.
> >
> > Bug: skia:5060
> > Change-Id: I60a48993c77631aaad9354bb86b13204dc618bf4
> > Reviewed-on: https://skia-review.googlesource.com/47422
> > Commit-Queue: Ben Wagner <benjaminwagner@google.com>
> > Reviewed-by: Mike Reed <reed@google.com>
>
> TBR=benjaminwagner@google.com,reed@google.com
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: skia:7143
> Change-Id: I0f19720a7d8344789a375bbb6b9e28bf4f4e55ae
> Reviewed-on: https://skia-review.googlesource.com/57240
> Commit-Queue: Ben Wagner <benjaminwagner@google.com>
> Reviewed-by: Ben Wagner <benjaminwagner@google.com>
TBR=benjaminwagner@google.com,reed@google.com
Change-Id: I29ac47d6665e2e52ee2a6500488dc407c8d2af1c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:7143
Reviewed-on: https://skia-review.googlesource.com/57440
Reviewed-by: Ben Wagner <benjaminwagner@google.com>
Commit-Queue: Ben Wagner <benjaminwagner@google.com>
Diffstat (limited to 'src/core')
-rw-r--r-- | src/core/SkDraw.cpp | 25 | ||||
-rw-r--r-- | src/core/SkScanPriv.h | 2 | ||||
-rw-r--r-- | src/core/SkScan_Hairline.cpp | 13 | ||||
-rw-r--r-- | src/core/SkScan_Path.cpp | 20 |
4 files changed, 38 insertions, 22 deletions
diff --git a/src/core/SkDraw.cpp b/src/core/SkDraw.cpp index bf91aa15b5..39b684356a 100644 --- a/src/core/SkDraw.cpp +++ b/src/core/SkDraw.cpp @@ -722,6 +722,16 @@ static SkPoint* rect_points(SkRect& r) { return SkTCast<SkPoint*>(&r); } +static void draw_rect_as_path(const SkDraw& orig, const SkRect& prePaintRect, + const SkPaint& paint, const SkMatrix* matrix) { + SkDraw draw(orig); + draw.fMatrix = matrix; + SkPath tmp; + tmp.addRect(prePaintRect); + tmp.setFillType(SkPath::kWinding_FillType); + draw.drawPath(tmp, paint, nullptr, true); +} + void SkDraw::drawRect(const SkRect& prePaintRect, const SkPaint& paint, const SkMatrix* paintMatrix, const SkRect* postPaintRect) const { SkDEBUGCODE(this->validate();) @@ -746,14 +756,7 @@ void SkDraw::drawRect(const SkRect& prePaintRect, const SkPaint& paint, RectType rtype = ComputeRectType(paint, *fMatrix, &strokeSize); if (kPath_RectType == rtype) { - SkDraw draw(*this); - if (paintMatrix) { - draw.fMatrix = matrix; - } - SkPath tmp; - tmp.addRect(prePaintRect); - tmp.setFillType(SkPath::kWinding_FillType); - draw.drawPath(tmp, paint, nullptr, true); + draw_rect_as_path(*this, prePaintRect, paint, matrix); return; } @@ -778,6 +781,12 @@ void SkDraw::drawRect(const SkRect& prePaintRect, const SkPaint& paint, } } + if (!SkRect::MakeLargestS32().contains(bbox)) { + // bbox.roundOut() is undefined; use slow path. + draw_rect_as_path(*this, prePaintRect, paint, matrix); + return; + } + SkIRect ir = bbox.roundOut(); if (fRC->quickReject(ir)) { return; diff --git a/src/core/SkScanPriv.h b/src/core/SkScanPriv.h index 03b09ceee5..96ee695165 100644 --- a/src/core/SkScanPriv.h +++ b/src/core/SkScanPriv.h @@ -16,7 +16,7 @@ class SkScanClipper { public: SkScanClipper(SkBlitter* blitter, const SkRegion* clip, const SkIRect& bounds, - bool skipRejectTest = false); + bool skipRejectTest = false, bool boundsPreClipped = false); SkBlitter* getBlitter() const { return fBlitter; } const SkIRect* getClipRect() const { return fClipRect; } diff --git a/src/core/SkScan_Hairline.cpp b/src/core/SkScan_Hairline.cpp index ec6acb9b1f..37be03016c 100644 --- a/src/core/SkScan_Hairline.cpp +++ b/src/core/SkScan_Hairline.cpp @@ -143,17 +143,14 @@ void SkScan::HairLineRgn(const SkPoint array[], int arrayCount, const SkRegion* // we don't just draw 4 lines, 'cause that can leave a gap in the bottom-right // and double-hit the top-left. -// TODO: handle huge coordinates on rect (before calling SkScalarToFixed) void SkScan::HairRect(const SkRect& rect, const SkRasterClip& clip, SkBlitter* blitter) { SkAAClipBlitterWrapper wrapper; - SkBlitterClipper clipper; - SkIRect r; - - r.set(SkScalarToFixed(rect.fLeft) >> 16, - SkScalarToFixed(rect.fTop) >> 16, - (SkScalarToFixed(rect.fRight) >> 16) + 1, - (SkScalarToFixed(rect.fBottom) >> 16) + 1); + SkBlitterClipper clipper; + const SkIRect r = SkIRect::MakeLTRB(SkScalarFloorToInt(rect.fLeft), + SkScalarFloorToInt(rect.fTop), + SkScalarFloorToInt(rect.fRight) + 1, + SkScalarFloorToInt(rect.fBottom) + 1); if (clip.quickReject(r)) { return; diff --git a/src/core/SkScan_Path.cpp b/src/core/SkScan_Path.cpp index 8af0ea6b86..a8a43212c7 100644 --- a/src/core/SkScan_Path.cpp +++ b/src/core/SkScan_Path.cpp @@ -503,7 +503,7 @@ void sk_blit_below(SkBlitter* blitter, const SkIRect& ir, const SkRegion& clip) * is outside of the clip. */ SkScanClipper::SkScanClipper(SkBlitter* blitter, const SkRegion* clip, - const SkIRect& ir, bool skipRejectTest) { + const SkIRect& ir, bool skipRejectTest, bool irPreClipped) { fBlitter = nullptr; // null means blit nothing fClipRect = nullptr; @@ -514,7 +514,7 @@ SkScanClipper::SkScanClipper(SkBlitter* blitter, const SkRegion* clip, } if (clip->isRect()) { - if (fClipRect->contains(ir)) { + if (!irPreClipped && fClipRect->contains(ir)) { #ifdef SK_DEBUG fRectClipCheckBlitter.init(blitter, *fClipRect); blitter = &fRectClipCheckBlitter; @@ -522,7 +522,8 @@ SkScanClipper::SkScanClipper(SkBlitter* blitter, const SkRegion* clip, fClipRect = nullptr; } else { // only need a wrapper blitter if we're horizontally clipped - if (fClipRect->fLeft > ir.fLeft || fClipRect->fRight < ir.fRight) { + if (irPreClipped || + fClipRect->fLeft > ir.fLeft || fClipRect->fRight < ir.fRight) { fRectBlitter.init(blitter, *fClipRect); blitter = &fRectBlitter; } else { @@ -639,12 +640,21 @@ void SkScan::FillPath(const SkPath& path, const SkRegion& origClip, } // don't reference "origClip" any more, just use clipPtr + + SkRect bounds = path.getBounds(); + bool irPreClipped = false; + if (!SkRect::MakeLargestS32().contains(bounds)) { + if (!bounds.intersect(SkRect::MakeLargestS32())) { + bounds.setEmpty(); + } + irPreClipped = true; + } SkIRect ir; // We deliberately call round_asymmetric_to_int() instead of round(), since we can't afford // to generate a bounds that is tighter than the corresponding SkEdges. The edge code basically // converts the floats to fixed, and then "rounds". If we called round() instead of // round_asymmetric_to_int() here, we could generate the wrong ir for values like 0.4999997. - round_asymmetric_to_int(path.getBounds(), &ir); + round_asymmetric_to_int(bounds, &ir); if (ir.isEmpty()) { if (path.isInverseFillType()) { blitter->blitRegion(*clipPtr); @@ -652,7 +662,7 @@ void SkScan::FillPath(const SkPath& path, const SkRegion& origClip, return; } - SkScanClipper clipper(blitter, clipPtr, ir, path.isInverseFillType()); + SkScanClipper clipper(blitter, clipPtr, ir, path.isInverseFillType(), irPreClipped); blitter = clipper.getBlitter(); if (blitter) { |