Revert of fix misc asserts and checks found by fuzzer (patchset #1 id:1 of https...
authorethannicholas <ethannicholas@google.com>
Mon, 22 Feb 2016 15:42:18 +0000 (07:42 -0800)
committerCommit bot <commit-bot@chromium.org>
Mon, 22 Feb 2016 15:42:18 +0000 (07:42 -0800)
Reason for revert:
Looks to be causing failures in LayerTreeHostFilters* tests (https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/177297/steps/cc_unittests%20%28with%20patch%29/logs/stdio).

Original issue's description:
> fix misc asserts and checks found by fuzzer
>
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1713383002
>
> TBR=
>
> Committed: https://skia.googlesource.com/skia/+/653db51b440491b0fb1908bf5a43dcc89c90044d

TBR=reed@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/1721743002

include/core/SkPathEffect.h
include/effects/Sk1DPathEffect.h
include/effects/SkDashPathEffect.h
samplecode/SampleFilterFuzz.cpp
src/effects/Sk1DPathEffect.cpp
src/effects/SkAlphaThresholdFilter.cpp
src/effects/SkDashPathEffect.cpp

index a77b946..bd68c8f 100644 (file)
@@ -183,13 +183,7 @@ public:
         The reference counts for outer and inner are both incremented in the constructor,
         and decremented in the destructor.
     */
-    static SkPathEffect* Create(SkPathEffect* outer, SkPathEffect* inner) {
-        if (!outer) {
-            return SkSafeRef(inner);
-        }
-        if (!inner) {
-            return SkSafeRef(outer);
-        }
+    static SkComposePathEffect* Create(SkPathEffect* outer, SkPathEffect* inner) {
         return new SkComposePathEffect(outer, inner);
     }
 
@@ -226,13 +220,7 @@ public:
         The reference counts for first and second are both incremented in the constructor,
         and decremented in the destructor.
     */
-    static SkPathEffect* Create(SkPathEffect* first, SkPathEffect* second) {
-        if (!first) {
-            return SkSafeRef(second);
-        }
-        if (!second) {
-            return SkSafeRef(first);
-        }
+    static SkSumPathEffect* Create(SkPathEffect* first, SkPathEffect* second) {
         return new SkSumPathEffect(first, second);
     }
 
index c832888..3419dc2 100644 (file)
@@ -45,6 +45,8 @@ public:
         kTranslate_Style,   // translate the shape to each position
         kRotate_Style,      // rotate the shape about its center
         kMorph_Style,       // transform each point, and turn lines into curves
+
+        kStyleCount
     };
 
     /** Dash by replicating the specified path.
@@ -54,7 +56,9 @@ public:
         @param style how to transform path at each point (based on the current
                      position and tangent)
     */
-    static SkPathEffect* Create(const SkPath& path, SkScalar advance, SkScalar phase, Style);
+    static SkPathEffect* Create(const SkPath& path, SkScalar advance, SkScalar phase, Style style) {
+        return new SkPath1DPathEffect(path, advance, phase, style);
+    }
 
     virtual bool filterPath(SkPath*, const SkPath&,
                             SkStrokeRec*, const SkRect*) const override;
index 08b0a46..3c1407b 100644 (file)
@@ -36,7 +36,10 @@ public:
 
         Note: only affects stroked paths.
     */
-    static SkPathEffect* Create(const SkScalar intervals[], int count, SkScalar phase);
+    static SkPathEffect* Create(const SkScalar intervals[], int count, SkScalar phase) {
+        return new SkDashPathEffect(intervals, count, phase);
+    }
+    virtual ~SkDashPathEffect();
 
     virtual bool filterPath(SkPath* dst, const SkPath& src,
                             SkStrokeRec*, const SkRect*) const override;
@@ -55,7 +58,6 @@ public:
 #endif
 
 protected:
-    virtual ~SkDashPathEffect();
     SkDashPathEffect(const SkScalar intervals[], int count, SkScalar phase);
     void flatten(SkWriteBuffer&) const override;
 
@@ -64,7 +66,6 @@ private:
     int32_t     fCount;
     SkScalar    fPhase;
     // computed from phase
-
     SkScalar    fInitialDashLength;
     int32_t     fInitialDashIndex;
     SkScalar    fIntervalLength;
index 45b43df..f8ceed4 100644 (file)
@@ -204,7 +204,7 @@ static SkTypeface::Style make_typeface_style() {
 }
 
 static SkPath1DPathEffect::Style make_path_1d_path_effect_style() {
-    return static_cast<SkPath1DPathEffect::Style>(R((int)SkPath1DPathEffect::kMorph_Style + 1));
+    return static_cast<SkPath1DPathEffect::Style>(R(SkPath1DPathEffect::kStyleCount));
 }
 
 static SkColor make_color() {
@@ -529,9 +529,7 @@ static SkPaint make_paint() {
     paint.setMaskFilter(make_mask_filter());
     SkAutoTUnref<SkTypeface> typeface(
         SkTypeface::CreateFromName(make_font_name().c_str(), make_typeface_style()));
-#if 0
     paint.setTypeface(typeface);
-#endif
     SkLayerRasterizer::Builder rasterizerBuilder;
     SkPaint paintForRasterizer;
     if (R(2) == 1) {
index 4be6f97..041886e 100644 (file)
@@ -35,33 +35,39 @@ bool Sk1DPathEffect::filterPath(SkPath* dst, const SkPath& src,
 SkPath1DPathEffect::SkPath1DPathEffect(const SkPath& path, SkScalar advance,
     SkScalar phase, Style style) : fPath(path)
 {
-    SkASSERT(advance > 0 && !path.isEmpty());
-    // cleanup their phase parameter, inverting it so that it becomes an
-    // offset along the path (to match the interpretation in PostScript)
-    if (phase < 0) {
-        phase = -phase;
-        if (phase > advance) {
-            phase = SkScalarMod(phase, advance);
-        }
+    if (advance <= 0 || path.isEmpty()) {
+        SkDEBUGF(("SkPath1DPathEffect can't use advance <= 0\n"));
+        fAdvance = 0;   // signals we can't draw anything
+        fInitialOffset = 0;
+        fStyle = kStyleCount;
     } else {
-        if (phase > advance) {
-            phase = SkScalarMod(phase, advance);
+        // cleanup their phase parameter, inverting it so that it becomes an
+        // offset along the path (to match the interpretation in PostScript)
+        if (phase < 0) {
+            phase = -phase;
+            if (phase > advance) {
+                phase = SkScalarMod(phase, advance);
+            }
+        } else {
+            if (phase > advance) {
+                phase = SkScalarMod(phase, advance);
+            }
+            phase = advance - phase;
         }
-        phase = advance - phase;
-    }
-    // now catch the edge case where phase == advance (within epsilon)
-    if (phase >= advance) {
-        phase = 0;
-    }
-    SkASSERT(phase >= 0);
+        // now catch the edge case where phase == advance (within epsilon)
+        if (phase >= advance) {
+            phase = 0;
+        }
+        SkASSERT(phase >= 0);
 
-    fAdvance = advance;
-    fInitialOffset = phase;
+        fAdvance = advance;
+        fInitialOffset = phase;
 
-    if ((unsigned)style > kMorph_Style) {
-        SkDEBUGF(("SkPath1DPathEffect style enum out of range %d\n", style));
+        if ((unsigned)style >= kStyleCount) {
+            SkDEBUGF(("SkPath1DPathEffect style enum out of range %d\n", style));
+        }
+        fStyle = style;
     }
-    fStyle = style;
 }
 
 bool SkPath1DPathEffect::filterPath(SkPath* dst, const SkPath& src,
@@ -201,13 +207,3 @@ void SkPath1DPathEffect::toString(SkString* str) const {
     str->appendf(")");
 }
 #endif
-
-///////////////////////////////////////////////////////////////////////////////////////////////////
-
-SkPathEffect* SkPath1DPathEffect::Create(const SkPath& path, SkScalar advance, SkScalar phase,
-                                         Style style) {
-    if (advance <= 0 || path.isEmpty()) {
-        return nullptr;
-    }
-    return new SkPath1DPathEffect(path, advance, phase, style);
-}
index 4067970..7952060 100644 (file)
@@ -45,19 +45,11 @@ SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_START(SkAlphaThresholdFilter)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkAlphaThresholdFilterImpl)
 SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_END
 
-static bool outside_unit(SkScalar x) {
-    return x < 0 || x > 1;
-}
 
 SkImageFilter* SkAlphaThresholdFilter::Create(const SkRegion& region,
                                               SkScalar innerThreshold,
                                               SkScalar outerThreshold,
                                               SkImageFilter* input) {
-    if (outside_unit(innerThreshold) || outside_unit(outerThreshold) ||
-        innerThreshold > outerThreshold)
-    {
-        return nullptr;
-    }
     return new SkAlphaThresholdFilterImpl(region, innerThreshold, outerThreshold, input);
 }
 
@@ -342,6 +334,7 @@ void SkAlphaThresholdFilterImpl::flatten(SkWriteBuffer& buffer) const {
 bool SkAlphaThresholdFilterImpl::onFilterImageDeprecated(Proxy* proxy, const SkBitmap& src,
                                                          const Context& ctx, SkBitmap* dst,
                                                          SkIPoint* offset) const {
+    SkASSERT(src.colorType() == kN32_SkColorType);
 
     if (src.colorType() != kN32_SkColorType) {
         return false;
index ced0aab..6e10e54 100644 (file)
@@ -384,17 +384,3 @@ void SkDashPathEffect::toString(SkString* str) const {
     str->appendf("))");
 }
 #endif
-
-//////////////////////////////////////////////////////////////////////////////////////////////////
-
-SkPathEffect* SkDashPathEffect::Create(const SkScalar intervals[], int count, SkScalar phase) {
-    if ((count < 2) || !SkIsAlign2(count)) {
-        return nullptr;
-    }
-    for (int i = 0; i < count; i++) {
-        if (intervals[i] < 0) {
-            return nullptr;
-        }
-    }
-    return new SkDashPathEffect(intervals, count, phase);
-}