From e80eb928ba0248a5a5dea6e1f0005aa08ecf8740 Mon Sep 17 00:00:00 2001 From: robertphillips Date: Thu, 17 Dec 2015 11:33:12 -0800 Subject: [PATCH] Add default ctor to SkMask The minimal fix here seems to be handling BoxBlur's return value in SkBlurMaskFilter.cpp::GrRRectBlurEffect::Create. We seem to do enough special handling of the fImage field though that always initialializing it may not be a bad idea. BUG=570232 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1539553002 Review URL: https://codereview.chromium.org/1539553002 --- bench/BlurRectBench.cpp | 26 ++++++++++++++------------ gm/blurrect.cpp | 5 ++++- include/core/SkMask.h | 2 ++ include/core/SkMaskFilter.h | 4 +--- include/effects/SkEmbossMaskFilter.h | 4 ++-- src/core/SkAAClip.cpp | 1 - src/core/SkBitmap.cpp | 1 - src/core/SkDraw.cpp | 5 +---- src/core/SkMaskFilter.cpp | 1 - src/core/SkRasterizer.cpp | 1 - src/device/xps/SkXPSDevice.cpp | 13 ++----------- src/effects/SkBlurMask.h | 28 +++++++++++++++------------- src/effects/SkBlurMaskFilter.cpp | 8 ++++---- tests/AAClipTest.cpp | 1 - tests/BlurTest.cpp | 5 +++-- 15 files changed, 48 insertions(+), 57 deletions(-) diff --git a/bench/BlurRectBench.cpp b/bench/BlurRectBench.cpp index b8a7211815..53cf2da1b5 100644 --- a/bench/BlurRectBench.cpp +++ b/bench/BlurRectBench.cpp @@ -63,7 +63,7 @@ protected: preBenchSetup(r); for (int i = 0; i < loops; i++) { - makeBlurryRect(r); + this->makeBlurryRect(r); } } @@ -90,8 +90,10 @@ class BlurRectDirectBench: public BlurRectBench { protected: void makeBlurryRect(const SkRect& r) override { SkMask mask; - SkBlurMask::BlurRect(SkBlurMask::ConvertRadiusToSigma(this->radius()), - &mask, r, kNormal_SkBlurStyle); + if (!SkBlurMask::BlurRect(SkBlurMask::ConvertRadiusToSigma(this->radius()), + &mask, r, kNormal_SkBlurStyle)) { + return; + } SkMask::FreeImage(mask.fImage); } private: @@ -101,9 +103,7 @@ private: class BlurRectSeparableBench: public BlurRectBench { public: - BlurRectSeparableBench(SkScalar rad) : INHERITED(rad) { - fSrcMask.fImage = nullptr; - } + BlurRectSeparableBench(SkScalar rad) : INHERITED(rad) { } ~BlurRectSeparableBench() { SkMask::FreeImage(fSrcMask.fImage); @@ -144,9 +144,10 @@ protected: void makeBlurryRect(const SkRect&) override { SkMask mask; - mask.fImage = nullptr; - SkBlurMask::BoxBlur(&mask, fSrcMask, SkBlurMask::ConvertRadiusToSigma(this->radius()), - kNormal_SkBlurStyle, kHigh_SkBlurQuality); + if (!SkBlurMask::BoxBlur(&mask, fSrcMask, SkBlurMask::ConvertRadiusToSigma(this->radius()), + kNormal_SkBlurStyle, kHigh_SkBlurQuality)) { + return; + } SkMask::FreeImage(mask.fImage); } private: @@ -171,9 +172,10 @@ protected: void makeBlurryRect(const SkRect&) override { SkMask mask; - mask.fImage = nullptr; - SkBlurMask::BlurGroundTruth(SkBlurMask::ConvertRadiusToSigma(this->radius()), - &mask, fSrcMask, kNormal_SkBlurStyle); + if (!SkBlurMask::BlurGroundTruth(SkBlurMask::ConvertRadiusToSigma(this->radius()), + &mask, fSrcMask, kNormal_SkBlurStyle)) { + return; + } SkMask::FreeImage(mask.fImage); } private: diff --git a/gm/blurrect.cpp b/gm/blurrect.cpp index 4f15815748..55c592b627 100644 --- a/gm/blurrect.cpp +++ b/gm/blurrect.cpp @@ -199,7 +199,10 @@ DEF_SIMPLE_GM(blurrect_gallery, canvas, 1200, 1024) { SkBlurStyle style = styles[k]; SkMask mask; - SkBlurMask::BlurRect(SkBlurMask::ConvertRadiusToSigma(radius), &mask, r, style); + if (!SkBlurMask::BlurRect(SkBlurMask::ConvertRadiusToSigma(radius), + &mask, r, style)) { + continue; + } SkAutoMaskFreeImage amfi(mask.fImage); diff --git a/include/core/SkMask.h b/include/core/SkMask.h index 7be6aff614..a6d5606478 100644 --- a/include/core/SkMask.h +++ b/include/core/SkMask.h @@ -17,6 +17,8 @@ the 3-channel 3D format. These are passed to SkMaskFilter objects. */ struct SkMask { + SkMask() : fImage(nullptr) {} + enum Format { kBW_Format, //!< 1bit per pixel mask (e.g. monochrome) kA8_Format, //!< 8bits per pixel mask (e.g. antialiasing) diff --git a/include/core/SkMaskFilter.h b/include/core/SkMaskFilter.h index 1fe1d9adaf..8d3d1caa67 100644 --- a/include/core/SkMaskFilter.h +++ b/include/core/SkMaskFilter.h @@ -184,9 +184,7 @@ protected: class NinePatch : ::SkNoncopyable { public: - NinePatch() : fCache(NULL) { - fMask.fImage = NULL; - } + NinePatch() : fCache(nullptr) { } ~NinePatch(); SkMask fMask; // fBounds must have [0,0] in its top-left diff --git a/include/effects/SkEmbossMaskFilter.h b/include/effects/SkEmbossMaskFilter.h index 41dfa2a66c..b6bd1a1642 100644 --- a/include/effects/SkEmbossMaskFilter.h +++ b/include/effects/SkEmbossMaskFilter.h @@ -29,8 +29,8 @@ public: // This method is not exported to java. SkMask::Format getFormat() const override; // This method is not exported to java. - virtual bool filterMask(SkMask* dst, const SkMask& src, const SkMatrix&, - SkIPoint* margin) const override; + bool filterMask(SkMask* dst, const SkMask& src, const SkMatrix&, + SkIPoint* margin) const override; SK_TO_STRING_OVERRIDE() SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkEmbossMaskFilter) diff --git a/src/core/SkAAClip.cpp b/src/core/SkAAClip.cpp index 0de6d350eb..71d212a292 100644 --- a/src/core/SkAAClip.cpp +++ b/src/core/SkAAClip.cpp @@ -2165,7 +2165,6 @@ void SkAAClipBlitter::blitMask(const SkMask& origMask, const SkIRect& clip) { // if we're BW, we need to upscale to A8 (ugh) SkMask grayMask; - grayMask.fImage = nullptr; if (SkMask::kBW_Format == origMask.fFormat) { grayMask.fFormat = SkMask::kA8_Format; grayMask.fBounds = origMask.fBounds; diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp index 661dca3ea5..c66a5b2fd5 100644 --- a/src/core/SkBitmap.cpp +++ b/src/core/SkBitmap.cpp @@ -1033,7 +1033,6 @@ bool SkBitmap::extractAlpha(SkBitmap* dst, const SkPaint* paint, // compute our (larger?) dst bounds if we have a filter if (filter) { identity.reset(); - srcM.fImage = nullptr; if (!filter->filterMask(&dstM, srcM, identity, nullptr)) { goto NO_FILTER_CASE; } diff --git a/src/core/SkDraw.cpp b/src/core/SkDraw.cpp index 7ebad61dde..883359ca59 100644 --- a/src/core/SkDraw.cpp +++ b/src/core/SkDraw.cpp @@ -908,10 +908,8 @@ void SkDraw::drawDevMask(const SkMask& srcM, const SkPaint& paint) const { SkMask dstM; if (paint.getMaskFilter() && - paint.getMaskFilter()->filterMask(&dstM, srcM, *fMatrix, nullptr)) { + paint.getMaskFilter()->filterMask(&dstM, srcM, *fMatrix, nullptr)) { mask = &dstM; - } else { - dstM.fImage = nullptr; } SkAutoMaskFreeImage ami(dstM.fImage); @@ -2013,7 +2011,6 @@ static bool compute_bounds(const SkPath& devPath, const SkIRect* clipBounds, srcM.fBounds = *bounds; srcM.fFormat = SkMask::kA8_Format; - srcM.fImage = nullptr; if (!filter->filterMask(&dstM, srcM, *filterMatrix, &margin)) { return false; } diff --git a/src/core/SkMaskFilter.cpp b/src/core/SkMaskFilter.cpp index 0753944a3b..8749f50c4d 100644 --- a/src/core/SkMaskFilter.cpp +++ b/src/core/SkMaskFilter.cpp @@ -349,7 +349,6 @@ bool SkMaskFilter::filterMaskGPU(GrTexture* src, void SkMaskFilter::computeFastBounds(const SkRect& src, SkRect* dst) const { SkMask srcM, dstM; - srcM.fImage = nullptr; srcM.fBounds = src.roundOut(); srcM.fRowBytes = 0; srcM.fFormat = SkMask::kA8_Format; diff --git a/src/core/SkRasterizer.cpp b/src/core/SkRasterizer.cpp index 87b9caa242..6a3ea10678 100644 --- a/src/core/SkRasterizer.cpp +++ b/src/core/SkRasterizer.cpp @@ -23,7 +23,6 @@ bool SkRasterizer::rasterize(const SkPath& fillPath, const SkMatrix& matrix, srcM.fFormat = SkMask::kA8_Format; srcM.fBounds.set(0, 0, 1, 1); - srcM.fImage = nullptr; if (!filter->filterMask(&dstM, srcM, matrix, &margin)) { return false; } diff --git a/src/device/xps/SkXPSDevice.cpp b/src/device/xps/SkXPSDevice.cpp index 86c729c9e4..48233bb554 100644 --- a/src/device/xps/SkXPSDevice.cpp +++ b/src/device/xps/SkXPSDevice.cpp @@ -1628,12 +1628,8 @@ void SkXPSDevice::drawPath(const SkDraw& d, //[Mask -> Mask] SkMask filteredMask; - if (filter && - filter->filterMask(&filteredMask, *mask, *d.fMatrix, nullptr)) { - + if (filter && filter->filterMask(&filteredMask, *mask, *d.fMatrix, nullptr)) { mask = &filteredMask; - } else { - filteredMask.fImage = nullptr; } SkAutoMaskFreeImage filteredAmi(filteredMask.fImage); @@ -1675,13 +1671,8 @@ void SkXPSDevice::drawPath(const SkDraw& d, //[Mask -> Mask] SkMask filteredMask; - if (filter->filterMask(&filteredMask, - rasteredMask, - matrix, - nullptr)) { + if (filter->filterMask(&filteredMask, rasteredMask, matrix, nullptr)) { mask = &filteredMask; - } else { - filteredMask.fImage = nullptr; } SkAutoMaskFreeImage filteredAmi(filteredMask.fImage); diff --git a/src/effects/SkBlurMask.h b/src/effects/SkBlurMask.h index b6c37fb461..25f890e263 100644 --- a/src/effects/SkBlurMask.h +++ b/src/effects/SkBlurMask.h @@ -15,14 +15,14 @@ class SkBlurMask { public: - static bool BlurRect(SkScalar sigma, SkMask *dst, const SkRect &src, SkBlurStyle, - SkIPoint *margin = nullptr, - SkMask::CreateMode createMode = - SkMask::kComputeBoundsAndRenderImage_CreateMode); - static bool BlurRRect(SkScalar sigma, SkMask *dst, const SkRRect &src, SkBlurStyle, - SkIPoint *margin = nullptr, - SkMask::CreateMode createMode = - SkMask::kComputeBoundsAndRenderImage_CreateMode); + static bool SK_WARN_UNUSED_RESULT BlurRect(SkScalar sigma, SkMask *dst, const SkRect &src, + SkBlurStyle, SkIPoint *margin = nullptr, + SkMask::CreateMode createMode = + SkMask::kComputeBoundsAndRenderImage_CreateMode); + static bool SK_WARN_UNUSED_RESULT BlurRRect(SkScalar sigma, SkMask *dst, const SkRRect &src, + SkBlurStyle, SkIPoint *margin = nullptr, + SkMask::CreateMode createMode = + SkMask::kComputeBoundsAndRenderImage_CreateMode); // forceQuality will prevent BoxBlur from falling back to the low quality approach when sigma // is very small -- this can be used predict the margin bump ahead of time without completely @@ -30,14 +30,16 @@ public: // but also being able to predict precisely at what pixels the blurred profile of e.g. a // rectangle will lie. - static bool BoxBlur(SkMask* dst, const SkMask& src, - SkScalar sigma, SkBlurStyle style, SkBlurQuality quality, - SkIPoint* margin = nullptr, bool force_quality=false); + static bool SK_WARN_UNUSED_RESULT BoxBlur(SkMask* dst, const SkMask& src, + SkScalar sigma, SkBlurStyle style, SkBlurQuality, + SkIPoint* margin = nullptr, + bool forceQuality = false); // the "ground truth" blur does a gaussian convolution; it's slow // but useful for comparison purposes. - static bool BlurGroundTruth(SkScalar sigma, SkMask* dst, const SkMask& src, SkBlurStyle, - SkIPoint* margin = nullptr); + static bool SK_WARN_UNUSED_RESULT BlurGroundTruth(SkScalar sigma, SkMask* dst, + const SkMask& src, + SkBlurStyle, SkIPoint* margin = nullptr); // If radius > 0, return the corresponding sigma, else return 0 static SkScalar ConvertRadiusToSigma(SkScalar radius); diff --git a/src/effects/SkBlurMaskFilter.cpp b/src/effects/SkBlurMaskFilter.cpp index ca0b18042b..a4dd70270b 100644 --- a/src/effects/SkBlurMaskFilter.cpp +++ b/src/effects/SkBlurMaskFilter.cpp @@ -353,7 +353,6 @@ SkBlurMaskFilterImpl::filterRRectToNine(const SkRRect& rrect, const SkMatrix& ma SkIPoint margin; SkMask srcM, dstM; srcM.fBounds = rrect.rect().roundOut(); - srcM.fImage = nullptr; srcM.fFormat = SkMask::kA8_Format; srcM.fRowBytes = 0; @@ -473,7 +472,6 @@ SkBlurMaskFilterImpl::filterRectsToNine(const SkRect rects[], int count, SkIPoint margin; SkMask srcM, dstM; srcM.fBounds = rects[0].roundOut(); - srcM.fImage = nullptr; srcM.fFormat = SkMask::kA8_Format; srcM.fRowBytes = 0; @@ -989,8 +987,10 @@ const GrFragmentProcessor* GrRRectBlurEffect::Create(GrTextureProvider* texProvi SkMask::kJustRenderImage_CreateMode, SkPaint::kFill_Style); SkMask blurredMask; - SkBlurMask::BoxBlur(&blurredMask, mask, sigma, kNormal_SkBlurStyle, kHigh_SkBlurQuality, - nullptr, true); + if (!SkBlurMask::BoxBlur(&blurredMask, mask, sigma, kNormal_SkBlurStyle, + kHigh_SkBlurQuality, nullptr, true)) { + return nullptr; + } unsigned int texSide = smallRectSide + 2*blurRadius; GrSurfaceDesc texDesc; diff --git a/tests/AAClipTest.cpp b/tests/AAClipTest.cpp index 42fc7e7f96..1ee6358a6e 100644 --- a/tests/AAClipTest.cpp +++ b/tests/AAClipTest.cpp @@ -228,7 +228,6 @@ static void test_empty(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, clip1 == clip0); SkMask mask; - mask.fImage = nullptr; clip0.copyToMask(&mask); REPORTER_ASSERT(reporter, nullptr == mask.fImage); REPORTER_ASSERT(reporter, mask.fBounds.isEmpty()); diff --git a/tests/BlurTest.cpp b/tests/BlurTest.cpp index f5bb215065..556930e6e9 100644 --- a/tests/BlurTest.cpp +++ b/tests/BlurTest.cpp @@ -170,8 +170,9 @@ static void ground_truth_2d(int width, int height, memset(src.fImage, 0xff, src.computeTotalImageSize()); - dst.fImage = nullptr; - SkBlurMask::BlurGroundTruth(sigma, &dst, src, kNormal_SkBlurStyle); + if (!SkBlurMask::BlurGroundTruth(sigma, &dst, src, kNormal_SkBlurStyle)) { + return; + } int midX = dst.fBounds.centerX(); int midY = dst.fBounds.centerY();