add sanity checks to handles extremely large coordinates or filter margins.
authorreed@android.com <reed@android.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Mon, 16 Mar 2009 18:46:55 +0000 (18:46 +0000)
committerreed@android.com <reed@android.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Mon, 16 Mar 2009 18:46:55 +0000 (18:46 +0000)
Should not hurt features (we hope), but is an easy way to survive malicious
values on a small-memory machine like a handset.

git-svn-id: http://skia.googlecode.com/svn/trunk@123 2bbb7eff-a529-9590-31e7-b0007b416f81

samplecode/SampleFontScalerTest.cpp
src/core/SkDraw.cpp
src/core/SkScan_AntiPath.cpp
src/core/SkScan_Antihair.cpp
src/effects/SkBlurMaskFilter.cpp

index 81dd60d..256d2f4 100644 (file)
@@ -71,10 +71,24 @@ protected:
     virtual void onDraw(SkCanvas* canvas) {
         this->drawBG(canvas);
         
+        SkPaint paint;
+
+        {
+            SkPoint pts[4];
+            pts[0].set(1.61061274e+09, 6291456);
+            pts[1].set(-7.18397061e+15, -1.53091184e+13);
+            pts[2].set(-1.30077315e+16, -2.77196141e+13);
+            pts[3].set(-1.30077315e+16, -2.77196162e+13);
+            
+            SkPath path;
+            path.moveTo(pts[0]);
+            path.cubicTo(pts[1], pts[2], pts[3]);
+            canvas->drawPath(path, paint);
+        }
+        
         canvas->translate(200, 20);
         canvas->rotate(30);
 
-        SkPaint paint;
         paint.setAntiAlias(true);
         paint.setTypeface(SkTypeface::CreateFromName("Times Roman", SkTypeface::kNormal))->safeUnref();
         
index 1d3263f..bc981f9 100644 (file)
@@ -2305,7 +2305,15 @@ static bool compute_bounds(const SkPath& devPath, const SkIRect* clipBounds,
     if (clipBounds && !clipBounds->contains(*bounds)) {
         SkIRect tmp = *bounds;
         (void)tmp.intersect(*clipBounds);
-        tmp.inset(-margin.fX, -margin.fY);
+        // Ugh. Guard against gigantic margins from wacky filters. Without this
+        // check we can request arbitrary amounts of slop beyond our visible
+        // clip, and bring down the renderer (at least on finite RAM machines
+        // like handsets, etc.). Need to balance this invented value between
+        // quality of large filters like blurs, and the corresponding memory
+        // requests.
+        static const int MAX_MARGIN = 128;
+        tmp.inset(-SkMin32(margin.fX, MAX_MARGIN),
+                  -SkMin32(margin.fY, MAX_MARGIN));
         (void)bounds->intersect(tmp);
     }
 
index 9cdeeaa..422d060 100644 (file)
@@ -335,8 +335,13 @@ void MaskSuperBlitter::blitH(int x, int y, int width)
 
 ///////////////////////////////////////////////////////////////////////////////
 
-static int overflows_short(int value) {
-    return value - (short)value;
+/*  Returns non-zero if (value << shift) overflows a short, which would mean
+    we could not shift it up and then convert to SkFixed.
+    i.e. is x expressible as signed (16-shift) bits?
+ */
+static int overflows_short_shift(int value, int shift) {
+    const int s = 16 + shift;
+    return (value << s >> s) - value;
 }
 
 void SkScan::AntiFillPath(const SkPath& path, const SkRegion& clip,
@@ -354,13 +359,13 @@ void SkScan::AntiFillPath(const SkPath& path, const SkRegion& clip,
         return;
     }
 
-    if (overflows_short(ir.fLeft << SHIFT) ||
-            overflows_short(ir.fRight << SHIFT) ||
-            overflows_short(ir.width() << SHIFT) ||
-            overflows_short(ir.fTop << SHIFT) ||
-            overflows_short(ir.fBottom << SHIFT) ||
-            overflows_short(ir.height() << SHIFT)) {
-        // can't supersample, try drawing w/o antialiasing
+    // use bit-or since we expect all to pass, so no need to go slower with
+    // a short-circuiting logical-or
+    if (overflows_short_shift(ir.fLeft, SHIFT) |
+            overflows_short_shift(ir.fRight, SHIFT) |
+            overflows_short_shift(ir.fTop, SHIFT) |
+            overflows_short_shift(ir.fBottom, SHIFT)) {
+        // can't supersample, so draw w/o antialiasing
         SkScan::FillPath(path, clip, blitter);
         return;
     }
index 04e4690..3344698 100644 (file)
@@ -212,8 +212,14 @@ static void do_anti_hairline(SkFDot6 x0, SkFDot6 y0, SkFDot6 x1, SkFDot6 y1,
 
     if (SkAbs32(x1 - x0) > SkIntToFDot6(511) || SkAbs32(y1 - y0) > SkIntToFDot6(511))
     {
-        int hx = (x0 + x1) >> 1;
-        int hy = (y0 + y1) >> 1;
+        /*  instead of (x0 + x1) >> 1, we shift each separately. This is less
+            precise, but avoids overflowing the intermediate result if the
+            values are huge. A better fix might be to clip the original pts
+            directly (i.e. do the divide), so we don't spend time subdividing
+            huge lines at all.
+         */
+        int hx = (x0 >> 1) + (x1 >> 1);
+        int hy = (y0 >> 1) + (y1 >> 1);
         do_anti_hairline(x0, y0, hx, hy, clip, blitter);
         do_anti_hairline(hx, hy, x1, y1, clip, blitter);
         return;
index 2db60a5..b74cd99 100644 (file)
@@ -81,11 +81,19 @@ SkMask::Format SkBlurMaskFilterImpl::getFormat()
 bool SkBlurMaskFilterImpl::filterMask(SkMask* dst, const SkMask& src, const SkMatrix& matrix, SkIPoint* margin)
 {
     SkScalar radius = matrix.mapRadius(fRadius);
+    // To avoid unseemly allocation requests (esp. for finite platforms like
+    // handset) we limit the radius so something manageable. (as opposed to
+    // a request like 10,000)
+    static const SkScalar MAX_RADIUS = SkIntToScalar(128);
+    radius = SkMinScalar(radius, MAX_RADIUS);
 
     if (SkBlurMask::Blur(dst, src, radius, (SkBlurMask::Style)fBlurStyle))
     {
-        if (margin)
+        if (margin) {
+            // we need to integralize radius for our margin, so take the ceil
+            // just to be safe.
             margin->set(SkScalarCeil(radius), SkScalarCeil(radius));
+        }
         return true;
     }
     return false;