From 5ff17b13524eea03e4b672ee3b3a49b649dd09fb Mon Sep 17 00:00:00 2001 From: robertphillips Date: Mon, 28 Mar 2016 13:13:42 -0700 Subject: Swap SkPictureImageFilter's factories over to smart pointers A trial balloon before converting the rest This requires https://codereview.chromium.org/1836443003/ (add SK_SUPPORT_LEGACY_IMAGEFILTER_PTR flag for future skia CL) to land in Chromium first GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1831323003 Review URL: https://codereview.chromium.org/1831323003 --- gm/filterfastbounds.cpp | 4 +- gm/pictureimagefilter.cpp | 91 ++++++++++++++++++---------------- gm/recordopts.cpp | 2 +- include/effects/SkPictureImageFilter.h | 46 +++++++++++------ samplecode/SampleFilterFuzz.cpp | 2 +- src/effects/SkPictureImageFilter.cpp | 18 +++---- tests/ImageFilterTest.cpp | 14 +++--- tests/RecordOptsTest.cpp | 2 +- 8 files changed, 97 insertions(+), 82 deletions(-) diff --git a/gm/filterfastbounds.cpp b/gm/filterfastbounds.cpp index 1de30e5826..9e84024897 100644 --- a/gm/filterfastbounds.cpp +++ b/gm/filterfastbounds.cpp @@ -256,10 +256,10 @@ protected: pic = rec.finishRecordingAsPicture(); } - SkAutoTUnref pif(SkPictureImageFilter::Create(pic.get())); + sk_sp pif(SkPictureImageFilter::Make(pic)); SkTArray pifPaints; - create_paints(pif, &pifPaints); + create_paints(pif.get(), &pifPaints); //----------- // Paints with a SkImageSource as a source diff --git a/gm/pictureimagefilter.cpp b/gm/pictureimagefilter.cpp index f25c86823b..8e5f1ef2a6 100644 --- a/gm/pictureimagefilter.cpp +++ b/gm/pictureimagefilter.cpp @@ -12,43 +12,44 @@ // This GM exercises the SkPictureImageFilter ImageFilter class. +static void fill_rect_filtered(SkCanvas* canvas, + const SkRect& clipRect, + sk_sp filter) { + SkPaint paint; + paint.setImageFilter(filter); + canvas->save(); + canvas->clipRect(clipRect); + canvas->drawPaint(paint); + canvas->restore(); +} + +static sk_sp make_picture() { + SkPictureRecorder recorder; + SkCanvas* canvas = recorder.beginRecording(100, 100, nullptr, 0); + canvas->clear(SK_ColorBLACK); + SkPaint paint; + paint.setAntiAlias(true); + sk_tool_utils::set_portable_typeface(&paint); + paint.setColor(0xFFFFFFFF); + paint.setTextSize(SkIntToScalar(96)); + const char* str = "e"; + canvas->drawText(str, strlen(str), SkIntToScalar(20), SkIntToScalar(70), paint); + return recorder.finishRecordingAsPicture(); +} + class PictureImageFilterGM : public skiagm::GM { public: - PictureImageFilterGM() { - } + PictureImageFilterGM() { } protected: SkString onShortName() override { return SkString("pictureimagefilter"); } - void makePicture() { - SkPictureRecorder recorder; - SkCanvas* canvas = recorder.beginRecording(100, 100, nullptr, 0); - canvas->clear(SK_ColorBLACK); - SkPaint paint; - paint.setAntiAlias(true); - sk_tool_utils::set_portable_typeface(&paint); - paint.setColor(0xFFFFFFFF); - paint.setTextSize(SkIntToScalar(96)); - const char* str = "e"; - canvas->drawText(str, strlen(str), SkIntToScalar(20), SkIntToScalar(70), paint); - fPicture = recorder.finishRecordingAsPicture(); - } - SkISize onISize() override { return SkISize::Make(600, 300); } void onOnceBeforeDraw() override { - this->makePicture(); - } - - static void fillRectFiltered(SkCanvas* canvas, const SkRect& clipRect, SkImageFilter* filter) { - SkPaint paint; - paint.setImageFilter(filter); - canvas->save(); - canvas->clipRect(clipRect); - canvas->drawPaint(paint); - canvas->restore(); + fPicture = make_picture(); } void onDraw(SkCanvas* canvas) override { @@ -57,30 +58,31 @@ protected: SkRect srcRect = SkRect::MakeXYWH(20, 20, 30, 30); SkRect emptyRect = SkRect::MakeXYWH(20, 20, 0, 0); SkRect bounds = SkRect::MakeXYWH(0, 0, 100, 100); - SkAutoTUnref pictureSource( - SkPictureImageFilter::Create(fPicture.get())); - SkAutoTUnref pictureSourceSrcRect( - SkPictureImageFilter::Create(fPicture.get(), srcRect)); - SkAutoTUnref pictureSourceEmptyRect( - SkPictureImageFilter::Create(fPicture.get(), emptyRect)); - SkAutoTUnref pictureSourceResampled( - SkPictureImageFilter::CreateForLocalSpace(fPicture.get(), fPicture->cullRect(), - kLow_SkFilterQuality)); - SkAutoTUnref pictureSourcePixelated( - SkPictureImageFilter::CreateForLocalSpace(fPicture.get(), fPicture->cullRect(), - kNone_SkFilterQuality)); + sk_sp pictureSource(SkPictureImageFilter::Make(fPicture)); + sk_sp pictureSourceSrcRect(SkPictureImageFilter::Make(fPicture, + srcRect)); + sk_sp pictureSourceEmptyRect(SkPictureImageFilter::Make(fPicture, + emptyRect)); + sk_sp pictureSourceResampled(SkPictureImageFilter::MakeForLocalSpace( + fPicture, + fPicture->cullRect(), + kLow_SkFilterQuality)); + sk_sp pictureSourcePixelated(SkPictureImageFilter::MakeForLocalSpace( + fPicture, + fPicture->cullRect(), + kNone_SkFilterQuality)); canvas->save(); // Draw the picture unscaled. - fillRectFiltered(canvas, bounds, pictureSource); + fill_rect_filtered(canvas, bounds, pictureSource); canvas->translate(SkIntToScalar(100), 0); // Draw an unscaled subset of the source picture. - fillRectFiltered(canvas, bounds, pictureSourceSrcRect); + fill_rect_filtered(canvas, bounds, pictureSourceSrcRect); canvas->translate(SkIntToScalar(100), 0); // Draw the picture to an empty rect (should draw nothing). - fillRectFiltered(canvas, bounds, pictureSourceEmptyRect); + fill_rect_filtered(canvas, bounds, pictureSourceEmptyRect); canvas->translate(SkIntToScalar(100), 0); canvas->restore(); @@ -89,20 +91,21 @@ protected: canvas->translate(0, SkIntToScalar(100)); canvas->scale(200 / srcRect.width(), 200 / srcRect.height()); canvas->translate(-srcRect.fLeft, -srcRect.fTop); - fillRectFiltered(canvas, srcRect, pictureSource); + fill_rect_filtered(canvas, srcRect, pictureSource); // Draw the picture scaled, but rasterized at original resolution canvas->translate(srcRect.width(), 0); - fillRectFiltered(canvas, srcRect, pictureSourceResampled); + fill_rect_filtered(canvas, srcRect, pictureSourceResampled); // Draw the picture scaled, pixelated canvas->translate(srcRect.width(), 0); - fillRectFiltered(canvas, srcRect, pictureSourcePixelated); + fill_rect_filtered(canvas, srcRect, pictureSourcePixelated); } } private: sk_sp fPicture; + typedef GM INHERITED; }; diff --git a/gm/recordopts.cpp b/gm/recordopts.cpp index bac6d8858c..3c44f4ac27 100644 --- a/gm/recordopts.cpp +++ b/gm/recordopts.cpp @@ -117,7 +117,7 @@ static void draw_svg_opacity_and_filter_layer_sequence(SkCanvas* canvas, SkColor canvas->save(); canvas->clipRect(targetRect); SkPaint drawPaint; - drawPaint.setImageFilter(SkPictureImageFilter::Create(shape.get()))->unref(); + drawPaint.setImageFilter(SkPictureImageFilter::Make(shape)); installDetector(&drawPaint); canvas->saveLayer(&targetRect, &drawPaint); canvas->restore(); diff --git a/include/effects/SkPictureImageFilter.h b/include/effects/SkPictureImageFilter.h index dbc87f7768..59097ddf59 100644 --- a/include/effects/SkPictureImageFilter.h +++ b/include/effects/SkPictureImageFilter.h @@ -16,17 +16,19 @@ public: /** * Refs the passed-in picture. */ - static SkImageFilter* Create(const SkPicture* picture) { - return new SkPictureImageFilter(picture); + static sk_sp Make(sk_sp picture) { + return sk_sp(new SkPictureImageFilter(std::move(picture))); } /** * Refs the passed-in picture. cropRect can be used to crop or expand the destination rect when * the picture is drawn. (No scaling is implied by the dest rect; only the CTM is applied.) */ - static SkImageFilter* Create(const SkPicture* picture, const SkRect& cropRect) { - return new SkPictureImageFilter(picture, cropRect, kDeviceSpace_PictureResolution, - kLow_SkFilterQuality); + static sk_sp Make(sk_sp picture, const SkRect& cropRect) { + return sk_sp(new SkPictureImageFilter(std::move(picture), + cropRect, + kDeviceSpace_PictureResolution, + kLow_SkFilterQuality)); } /** @@ -36,12 +38,30 @@ public: * expand the destination rect when the picture is drawn. (No scaling is implied by the * dest rect; only the CTM is applied.) */ + static sk_sp MakeForLocalSpace(sk_sp picture, + const SkRect& cropRect, + SkFilterQuality filterQuality) { + return sk_sp(new SkPictureImageFilter(std::move(picture), + cropRect, + kLocalSpace_PictureResolution, + filterQuality)); + } + +#ifdef SK_SUPPORT_LEGACY_IMAGEFILTER_PTR + static SkImageFilter* Create(const SkPicture* picture) { + return Make(sk_ref_sp(const_cast(picture))).release(); + } + static SkImageFilter* Create(const SkPicture* picture, const SkRect& cropRect) { + return Make(sk_ref_sp(const_cast(picture)), cropRect).release(); + } static SkImageFilter* CreateForLocalSpace(const SkPicture* picture, - const SkRect& cropRect, - SkFilterQuality filterQuality) { - return new SkPictureImageFilter(picture, cropRect, kLocalSpace_PictureResolution, - filterQuality); + const SkRect& cropRect, + SkFilterQuality filterQuality) { + return MakeForLocalSpace(sk_ref_sp(const_cast(picture)), + cropRect, + filterQuality).release(); } +#endif SK_TO_STRING_OVERRIDE() SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkPictureImageFilter) @@ -52,8 +72,6 @@ protected: kLocalSpace_PictureResolution }; - virtual ~SkPictureImageFilter(); - /* Constructs an SkPictureImageFilter object from an SkReadBuffer. * Note: If the SkPictureImageFilter object construction requires bitmap * decoding, the decoder must be set on the SkReadBuffer parameter by calling @@ -65,8 +83,8 @@ protected: SkIPoint* offset) const override; private: - explicit SkPictureImageFilter(const SkPicture* picture); - SkPictureImageFilter(const SkPicture* picture, const SkRect& cropRect, + explicit SkPictureImageFilter(sk_sp picture); + SkPictureImageFilter(sk_sp picture, const SkRect& cropRect, PictureResolution, SkFilterQuality); void drawPictureAtDeviceResolution(SkBaseDevice*, const SkIRect& deviceBounds, @@ -74,7 +92,7 @@ private: void drawPictureAtLocalResolution(Proxy*, SkBaseDevice*, const SkIRect& deviceBounds, const Context&) const; - const SkPicture* fPicture; + sk_sp fPicture; SkRect fCropRect; PictureResolution fPictureResolution; SkFilterQuality fFilterQuality; diff --git a/samplecode/SampleFilterFuzz.cpp b/samplecode/SampleFilterFuzz.cpp index dd72d1237c..0602e2bcbb 100644 --- a/samplecode/SampleFilterFuzz.cpp +++ b/samplecode/SampleFilterFuzz.cpp @@ -701,7 +701,7 @@ static SkImageFilter* make_image_filter(bool canBeNull) { &factory, 0); drawSomething(recordingCanvas); sk_sp pict(recorder.finishRecordingAsPicture()); - filter = SkPictureImageFilter::Create(pict.get(), make_rect()); + filter = SkPictureImageFilter::Make(pict, make_rect()).release(); } break; case PAINT: diff --git a/src/effects/SkPictureImageFilter.cpp b/src/effects/SkPictureImageFilter.cpp index 14c14bb176..8f8853ec36 100644 --- a/src/effects/SkPictureImageFilter.cpp +++ b/src/effects/SkPictureImageFilter.cpp @@ -13,28 +13,24 @@ #include "SkWriteBuffer.h" #include "SkValidationUtils.h" -SkPictureImageFilter::SkPictureImageFilter(const SkPicture* picture) +SkPictureImageFilter::SkPictureImageFilter(sk_sp picture) : INHERITED(0, 0, nullptr) - , fPicture(SkSafeRef(picture)) - , fCropRect(picture ? picture->cullRect() : SkRect::MakeEmpty()) + , fPicture(std::move(picture)) + , fCropRect(fPicture ? fPicture->cullRect() : SkRect::MakeEmpty()) , fPictureResolution(kDeviceSpace_PictureResolution) , fFilterQuality(kLow_SkFilterQuality) { } -SkPictureImageFilter::SkPictureImageFilter(const SkPicture* picture, const SkRect& cropRect, +SkPictureImageFilter::SkPictureImageFilter(sk_sp picture, const SkRect& cropRect, PictureResolution pictureResolution, SkFilterQuality filterQuality) : INHERITED(0, 0, nullptr) - , fPicture(SkSafeRef(picture)) + , fPicture(std::move(picture)) , fCropRect(cropRect) , fPictureResolution(pictureResolution) , fFilterQuality(filterQuality) { } -SkPictureImageFilter::~SkPictureImageFilter() { - SkSafeUnref(fPicture); -} - SkFlattenable* SkPictureImageFilter::CreateProc(SkReadBuffer& buffer) { sk_sp picture; SkRect cropRect; @@ -62,9 +58,9 @@ SkFlattenable* SkPictureImageFilter::CreateProc(SkReadBuffer& buffer) { } else { filterQuality = (SkFilterQuality)buffer.readInt(); } - return CreateForLocalSpace(picture.get(), cropRect, filterQuality); + return MakeForLocalSpace(picture, cropRect, filterQuality).release(); } - return Create(picture.get(), cropRect); + return Make(picture, cropRect).release(); } void SkPictureImageFilter::flatten(SkWriteBuffer& buffer) const { diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp index 4375aac8cd..8268f959b0 100644 --- a/tests/ImageFilterTest.cpp +++ b/tests/ImageFilterTest.cpp @@ -134,7 +134,7 @@ public: greenPaint.setColor(SK_ColorGREEN); recordingCanvas->drawRect(SkRect::Make(SkIRect::MakeXYWH(10, 10, 30, 20)), greenPaint); sk_sp picture(recorder.finishRecordingAsPicture()); - SkAutoTUnref pictureFilter(SkPictureImageFilter::Create(picture.get())); + sk_sp pictureFilter(SkPictureImageFilter::Make(picture)); sk_sp shader(SkPerlinNoiseShader::MakeTurbulence(SK_Scalar1, SK_Scalar1, 1, 0)); SkPaint paint; @@ -1024,18 +1024,17 @@ DEF_TEST(ImageFilterCrossProcessPictureImageFilter, reporter) { sk_sp picture(recorder.finishRecordingAsPicture()); // Wrap that SkPicture in an SkPictureImageFilter. - SkAutoTUnref imageFilter( - SkPictureImageFilter::Create(picture.get())); + sk_sp imageFilter(SkPictureImageFilter::Make(picture)); // Check that SkPictureImageFilter successfully serializes its contained // SkPicture when not in cross-process mode. SkPaint paint; - paint.setImageFilter(imageFilter.get()); + paint.setImageFilter(imageFilter); SkPictureRecorder outerRecorder; SkCanvas* outerCanvas = outerRecorder.beginRecording(1, 1, &factory, 0); SkPaint redPaintWithFilter; redPaintWithFilter.setColor(SK_ColorRED); - redPaintWithFilter.setImageFilter(imageFilter.get()); + redPaintWithFilter.setImageFilter(imageFilter); outerCanvas->drawRect(SkRect::Make(SkIRect::MakeWH(1, 1)), redPaintWithFilter); sk_sp outerPicture(outerRecorder.finishRecordingAsPicture()); @@ -1091,7 +1090,7 @@ static void test_clipped_picture_imagefilter(SkImageFilter::Proxy* proxy, sk_sp srcImg(create_empty_special_image(context, proxy, 2)); - SkAutoTUnref imageFilter(SkPictureImageFilter::Create(picture.get())); + sk_sp imageFilter(SkPictureImageFilter::Make(picture)); SkIPoint offset; SkImageFilter::Context ctx(SkMatrix::I(), SkIRect::MakeXYWH(1, 1, 1, 1), nullptr); @@ -1379,8 +1378,7 @@ static void test_composed_imagefilter_bounds(SkImageFilter::Proxy* proxy, recordingCanvas->clipRect(SkRect::MakeXYWH(100, 0, 100, 100)); recordingCanvas->clear(SK_ColorGREEN); sk_sp picture = recorder.finishRecordingAsPicture(); - sk_sp pictureFilter( - SkPictureImageFilter::Create(picture.get())); + sk_sp pictureFilter(SkPictureImageFilter::Make(picture)); SkImageFilter::CropRect cropRect(SkRect::MakeWH(100, 100)); sk_sp offsetFilter(SkOffsetImageFilter::Create(-100, 0, nullptr, &cropRect)); sk_sp composedFilter( diff --git a/tests/RecordOptsTest.cpp b/tests/RecordOptsTest.cpp index 77eb76bc02..242712da3b 100644 --- a/tests/RecordOptsTest.cpp +++ b/tests/RecordOptsTest.cpp @@ -239,7 +239,7 @@ DEF_TEST(RecordOpts_MergeSvgOpacityAndFilterLayers, r) { canvas->drawRect(SkRect::MakeWH(SkIntToScalar(50), SkIntToScalar(50)), shapePaint); shape = recorder.finishRecordingAsPicture(); } - translucentFilterLayerPaint.setImageFilter(SkPictureImageFilter::Create(shape.get()))->unref(); + translucentFilterLayerPaint.setImageFilter(SkPictureImageFilter::Make(shape)); int index = 0; -- cgit v1.2.3