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>

include/core/SkRect.h

@@ -411,6 +411,14 @@ struct SK_API SkRect {
         return r;
     }
 
+    // The largest SkRect for which round() is well-defined.
+    static SkRect SK_WARN_UNUSED_RESULT MakeLargestS32() {
+        const SkRect r = MakeLTRB(SK_MinS32FitsInFloat, SK_MinS32FitsInFloat,
+                                  SK_MaxS32FitsInFloat, SK_MaxS32FitsInFloat);
+        SkASSERT(r == Make(r.roundOut()));
+        return r;
+    }
+
     static constexpr SkRect SK_WARN_UNUSED_RESULT MakeWH(SkScalar w, SkScalar h) {
         return SkRect{0, 0, w, h};
     }

infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-UBSAN_float_cast_overflow.json

@@ -457,54 +457,20 @@
       "565",
       "gm",
       "_",
-      "bigrect",
-      "565",
-      "gm",
-      "_",
-      "clippedcubic2",
-      "565",
-      "gm",
-      "_",
-      "conicpaths",
-      "8888",
-      "gm",
-      "_",
-      "bigrect",
-      "8888",
-      "gm",
-      "_",
       "clippedcubic2",
       "8888",
       "gm",
       "_",
-      "conicpaths",
-      "f16",
-      "gm",
-      "_",
-      "bigrect",
-      "f16",
-      "gm",
-      "_",
       "clippedcubic2",
       "f16",
       "gm",
       "_",
-      "conicpaths",
-      "srgb",
-      "gm",
-      "_",
-      "bigrect",
-      "srgb",
-      "gm",
-      "_",
       "clippedcubic2",
       "srgb",
       "gm",
       "_",
-      "conicpaths",
+      "clippedcubic2",
       "--match",
-      "~^DashPathEffectTest_asPoints_limit$",
-      "~^PathBigCubic$",
       "~^PathOpsCubicIntersection$",
       "~^PathOpsCubicLineIntersection$",
       "~^PathOpsOpCubicsThreaded$",

infra/bots/recipes/test.py

@@ -531,11 +531,7 @@ def dm_flags(api, bot):
   if 'float_cast_overflow' in bot and 'CPU' in bot:
     # skia:4632
     for config in ['565', '8888', 'f16', 'srgb']:
-      blacklist([config, 'gm', '_', 'bigrect'])
       blacklist([config, 'gm', '_', 'clippedcubic2'])
-      blacklist([config, 'gm', '_', 'conicpaths'])
-    match.append('~^DashPathEffectTest_asPoints_limit$')
-    match.append('~^PathBigCubic$')
     match.append('~^PathOpsCubicIntersection$')
     match.append('~^PathOpsCubicLineIntersection$')
     match.append('~^PathOpsOpCubicsThreaded$')

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);
+        draw_rect_as_path(*this, prePaintRect, paint, matrix);
-        if (paintMatrix) {
-            draw.fMatrix = matrix;
-        }
-        SkPath  tmp;
-        tmp.addRect(prePaintRect);
-        tmp.setFillType(SkPath::kWinding_FillType);
-        draw.drawPath(tmp, paint, nullptr, true);
         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;

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; }

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;
+    SkBlitterClipper clipper;
-    SkIRect             r;
+    const SkIRect r = SkIRect::MakeLTRB(SkScalarFloorToInt(rect.fLeft),
-
+                                        SkScalarFloorToInt(rect.fTop),
-    r.set(SkScalarToFixed(rect.fLeft) >> 16,
+                                        SkScalarFloorToInt(rect.fRight) + 1,
-          SkScalarToFixed(rect.fTop) >> 16,
+                                        SkScalarFloorToInt(rect.fBottom) + 1);
-          (SkScalarToFixed(rect.fRight) >> 16) + 1,
-          (SkScalarToFixed(rect.fBottom) >> 16) + 1);
 
     if (clip.quickReject(r)) {
         return;

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) {