From 23e7af0e8ab8377b28e1399b4950def672284724 Mon Sep 17 00:00:00 2001 From: ethannicholas Date: Mon, 22 Feb 2016 07:42:18 -0800 Subject: [PATCH] Revert of fix misc asserts and checks found by fuzzer (patchset #1 id:1 of https://codereview.chromium.org/1719913002/ ) 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 | 16 ++------- include/effects/Sk1DPathEffect.h | 6 +++- include/effects/SkDashPathEffect.h | 7 ++-- samplecode/SampleFilterFuzz.cpp | 4 +-- src/effects/Sk1DPathEffect.cpp | 60 ++++++++++++++++------------------ src/effects/SkAlphaThresholdFilter.cpp | 9 +---- src/effects/SkDashPathEffect.cpp | 14 -------- 7 files changed, 41 insertions(+), 75 deletions(-) diff --git a/include/core/SkPathEffect.h b/include/core/SkPathEffect.h index a77b946..bd68c8f 100644 --- a/include/core/SkPathEffect.h +++ b/include/core/SkPathEffect.h @@ -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); } diff --git a/include/effects/Sk1DPathEffect.h b/include/effects/Sk1DPathEffect.h index c832888..3419dc2 100644 --- a/include/effects/Sk1DPathEffect.h +++ b/include/effects/Sk1DPathEffect.h @@ -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; diff --git a/include/effects/SkDashPathEffect.h b/include/effects/SkDashPathEffect.h index 08b0a46..3c1407b 100644 --- a/include/effects/SkDashPathEffect.h +++ b/include/effects/SkDashPathEffect.h @@ -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; diff --git a/samplecode/SampleFilterFuzz.cpp b/samplecode/SampleFilterFuzz.cpp index 45b43df..f8ceed4 100644 --- a/samplecode/SampleFilterFuzz.cpp +++ b/samplecode/SampleFilterFuzz.cpp @@ -204,7 +204,7 @@ static SkTypeface::Style make_typeface_style() { } static SkPath1DPathEffect::Style make_path_1d_path_effect_style() { - return static_cast(R((int)SkPath1DPathEffect::kMorph_Style + 1)); + return static_cast(R(SkPath1DPathEffect::kStyleCount)); } static SkColor make_color() { @@ -529,9 +529,7 @@ static SkPaint make_paint() { paint.setMaskFilter(make_mask_filter()); SkAutoTUnref 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) { diff --git a/src/effects/Sk1DPathEffect.cpp b/src/effects/Sk1DPathEffect.cpp index 4be6f97..041886e 100644 --- a/src/effects/Sk1DPathEffect.cpp +++ b/src/effects/Sk1DPathEffect.cpp @@ -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); -} diff --git a/src/effects/SkAlphaThresholdFilter.cpp b/src/effects/SkAlphaThresholdFilter.cpp index 4067970..7952060 100644 --- a/src/effects/SkAlphaThresholdFilter.cpp +++ b/src/effects/SkAlphaThresholdFilter.cpp @@ -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; diff --git a/src/effects/SkDashPathEffect.cpp b/src/effects/SkDashPathEffect.cpp index ced0aab..6e10e54 100644 --- a/src/effects/SkDashPathEffect.cpp +++ b/src/effects/SkDashPathEffect.cpp @@ -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); -} -- 2.7.4