From 824075071885b6b741c141cbe2134d8345d34589 Mon Sep 17 00:00:00 2001 From: Mike Reed Date: Wed, 9 Nov 2016 13:59:58 -0500 Subject: [PATCH] Change SkCanvas to *not* inherit from SkRefCnt Definitely tricky for classes like SkNWayCanvas, where the caller (today) need not pay attention to ownership of the canvases it gave the NWay (after this CL, the caller *must* managed ownership) BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4441 DOCS_PREVIEW= https://skia.org/?cl=4441 Change-Id: Ib1ac07a3cdf0686d78e7aaa4735d45cc90bea081 Reviewed-on: https://skia-review.googlesource.com/4441 Commit-Queue: Mike Reed Reviewed-by: Florin Malita Reviewed-by: Robert Phillips --- dm/DMSrcSink.cpp | 12 +++++------- gm/aaclip.cpp | 14 +++++++------- gn/android_framework_defines.gni | 1 + include/core/SkCanvas.h | 23 ++++++++++++++++++++--- include/core/SkMultiPictureDraw.h | 2 +- include/core/SkPictureRecorder.h | 14 +++++++------- include/svg/SkSVGCanvas.h | 8 +++++++- include/utils/SkCanvasStateUtils.h | 7 ++++++- include/utils/SkNullCanvas.h | 8 +++++++- public.bzl | 1 + samplecode/SamplePathFuzz.cpp | 4 ++-- src/core/SkCanvas.cpp | 6 ++++-- src/core/SkConfig8888.cpp | 4 ++-- src/core/SkMultiPictureDraw.cpp | 3 +-- src/core/SkSpecialSurface.cpp | 2 +- src/image/SkSurface.cpp | 3 --- src/image/SkSurface_Base.h | 8 ++++---- src/pdf/SkPDFDocument.cpp | 2 +- src/pdf/SkPDFDocument.h | 2 +- src/svg/SkSVGCanvas.cpp | 5 +++-- src/utils/SkCanvasStateUtils.cpp | 25 ++++++++++--------------- src/utils/SkLua.cpp | 19 +++++++++++++------ src/utils/SkNWayCanvas.cpp | 3 --- src/utils/SkNullCanvas.cpp | 6 +++--- src/xps/SkDocument_XPS.cpp | 2 +- tests/CanvasStateHelpers.cpp | 10 ++++------ tests/CanvasStateTest.cpp | 3 +-- tests/CanvasTest.cpp | 12 +++++------- tests/PipeTest.cpp | 2 +- tests/SVGDeviceTest.cpp | 6 +++--- tools/lua/lua_pictures.cpp | 2 +- tools/skiaserve/Request.h | 2 +- tools/skp_parser.cpp | 2 +- 33 files changed, 125 insertions(+), 98 deletions(-) diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp index 3bef299..b495520 100644 --- a/dm/DMSrcSink.cpp +++ b/dm/DMSrcSink.cpp @@ -1192,8 +1192,7 @@ Name MSKPSrc::name() const { return SkOSPath::Basename(fPath.c_str()); } /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/ Error NullSink::draw(const Src& src, SkBitmap*, SkWStream*, SkString*) const { - std::unique_ptr canvas(SkCreateNullCanvas()); - return src.draw(canvas.get()); + return src.draw(SkMakeNullCanvas().get()); } /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/ @@ -1356,7 +1355,7 @@ Error DebugSink::draw(const Src& src, SkBitmap*, SkWStream* dst, SkString*) cons if (!err.isEmpty()) { return err; } - sk_sp nullCanvas(SkCreateNullCanvas()); + std::unique_ptr nullCanvas = SkMakeNullCanvas(); UrlDataManager dataManager(SkString("data")); Json::Value json = debugCanvas.toJSON( dataManager, debugCanvas.getSize(), nullCanvas.get()); @@ -1371,10 +1370,9 @@ SVGSink::SVGSink() {} Error SVGSink::draw(const Src& src, SkBitmap*, SkWStream* dst, SkString*) const { #if defined(SK_XML) std::unique_ptr xmlWriter(new SkXMLStreamWriter(dst)); - sk_sp canvas(SkSVGCanvas::Create( - SkRect::MakeWH(SkIntToScalar(src.size().width()), SkIntToScalar(src.size().height())), - xmlWriter.get())); - return src.draw(canvas.get()); + return src.draw(SkSVGCanvas::Make(SkRect::MakeWH(SkIntToScalar(src.size().width()), + SkIntToScalar(src.size().height())), + xmlWriter.get()).get()); #else return Error("SVG sink is disabled."); #endif // SK_XML diff --git a/gm/aaclip.cpp b/gm/aaclip.cpp index dd83b7c..683f5ff 100644 --- a/gm/aaclip.cpp +++ b/gm/aaclip.cpp @@ -8,6 +8,7 @@ #include "gm.h" #include "SkCanvas.h" #include "SkPath.h" +#include "SkMakeUnique.h" static void do_draw(SkCanvas* canvas, const SkRect& r) { SkPaint paint; @@ -166,14 +167,14 @@ DEF_GM(return new AAClipGM;) #ifdef SK_BUILD_FOR_MAC -static SkCanvas* make_canvas(const SkBitmap& bm) { +static std::unique_ptr make_canvas(const SkBitmap& bm) { const SkImageInfo& info = bm.info(); if (info.bytesPerPixel() == 4) { - return SkCanvas::NewRasterDirectN32(info.width(), info.height(), - (SkPMColor*)bm.getPixels(), - bm.rowBytes()); + return SkCanvas::MakeRasterDirectN32(info.width(), info.height(), + (SkPMColor*)bm.getPixels(), + bm.rowBytes()); } else { - return new SkCanvas(bm); + return skstd::make_unique(bm); } } @@ -182,7 +183,6 @@ static void test_image(SkCanvas* canvas, const SkImageInfo& info) { SkBitmap bm; bm.allocPixels(info); - sk_sp newc(make_canvas(bm)); if (info.isOpaque()) { bm.eraseColor(SK_ColorGREEN); } else { @@ -192,7 +192,7 @@ static void test_image(SkCanvas* canvas, const SkImageInfo& info) { SkPaint paint; paint.setAntiAlias(true); paint.setColor(SK_ColorBLUE); - newc->drawCircle(50, 50, 49, paint); + make_canvas(bm)->drawCircle(50, 50, 49, paint); canvas->drawBitmap(bm, 10, 10); CGImageRef image = SkCreateCGImageRefWithColorspace(bm, nullptr); diff --git a/gn/android_framework_defines.gni b/gn/android_framework_defines.gni index 12e3b11..90fea2f 100644 --- a/gn/android_framework_defines.gni +++ b/gn/android_framework_defines.gni @@ -12,6 +12,7 @@ android_framework_defines = [ "SK_SUPPORT_LEGACY_GRADIENT_DITHERING", "SK_SUPPORT_LEGACY_DRAWFILTER", "SK_IGNORE_GPU_DITHER", + "SK_SUPPORT_LEGACY_CANVAS_IS_REFCNT", "SK_SUPPORT_LEGACY_CLIP_REGIONOPS", "SK_SUPPORT_LEGACY_SHADER_ISABITMAP", ] diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h index ab47edd..ba8a383 100644 --- a/include/core/SkCanvas.h +++ b/include/core/SkCanvas.h @@ -60,7 +60,13 @@ class SkTextBlob; color, typeface, textSize, strokeWidth, shader (e.g. gradients, patterns), etc. */ -class SK_API SkCanvas : public SkRefCnt { +class SK_API SkCanvas +#ifdef SK_SUPPORT_LEGACY_CANVAS_IS_REFCNT +: public SkRefCnt +#else +: SkNoncopyable +#endif +{ enum PrivateSaveLayerFlags { kDontClipToLayer_PrivateSaveLayerFlag = 1U << 31, }; @@ -100,11 +106,22 @@ public: * Note: it is valid to request a supported ImageInfo, but with zero * dimensions. */ - static SkCanvas* NewRasterDirect(const SkImageInfo&, void*, size_t); + static std::unique_ptr MakeRasterDirect(const SkImageInfo&, void*, size_t); + + static std::unique_ptr MakeRasterDirectN32(int width, int height, SkPMColor* pixels, + size_t rowBytes) { + return MakeRasterDirect(SkImageInfo::MakeN32Premul(width, height), pixels, rowBytes); + } + +#ifdef SK_SUPPORT_LEGACY_CANVAS_IS_REFCNT + static SkCanvas* NewRasterDirect(const SkImageInfo& info, void* pixels, size_t rowBytes) { + return MakeRasterDirect(info, pixels, rowBytes).release(); + } static SkCanvas* NewRasterDirectN32(int width, int height, SkPMColor* pixels, size_t rowBytes) { - return NewRasterDirect(SkImageInfo::MakeN32Premul(width, height), pixels, rowBytes); + return MakeRasterDirectN32(width, height, pixels, rowBytes).release(); } +#endif /** * Creates an empty canvas with no backing device/pixels, and zero diff --git a/include/core/SkMultiPictureDraw.h b/include/core/SkMultiPictureDraw.h index cd46a30..9995721 100644 --- a/include/core/SkMultiPictureDraw.h +++ b/include/core/SkMultiPictureDraw.h @@ -57,7 +57,7 @@ public: private: struct DrawData { - SkCanvas* fCanvas; // reffed + SkCanvas* fCanvas; const SkPicture* fPicture; // reffed SkMatrix fMatrix; SkPaint* fPaint; // owned diff --git a/include/core/SkPictureRecorder.h b/include/core/SkPictureRecorder.h index 59e8f14..a440790 100644 --- a/include/core/SkPictureRecorder.h +++ b/include/core/SkPictureRecorder.h @@ -111,13 +111,13 @@ private: friend class SkPictureRecorderReplayTester; // for unit testing void partialReplay(SkCanvas* canvas) const; - bool fActivelyRecording; - uint32_t fFlags; - SkRect fCullRect; - sk_sp fBBH; - sk_sp fRecorder; - sk_sp fRecord; - SkMiniRecorder fMiniRecorder; + bool fActivelyRecording; + uint32_t fFlags; + SkRect fCullRect; + sk_sp fBBH; + std::unique_ptr fRecorder; + sk_sp fRecord; + SkMiniRecorder fMiniRecorder; typedef SkNoncopyable INHERITED; }; diff --git a/include/svg/SkSVGCanvas.h b/include/svg/SkSVGCanvas.h index e285faa..b72f273 100644 --- a/include/svg/SkSVGCanvas.h +++ b/include/svg/SkSVGCanvas.h @@ -25,7 +25,13 @@ public: * The 'bounds' parameter defines an initial SVG viewport (viewBox attribute on the root * SVG element). */ - static SkCanvas* Create(const SkRect& bounds, SkXMLWriter*); + static std::unique_ptr Make(const SkRect& bounds, SkXMLWriter*); + +#ifdef SK_SUPPORT_LEGACY_CANVAS_IS_REFCNT + static SkCanvas* Create(const SkRect& bounds, SkXMLWriter* writer) { + return Make(bounds, writer).release(); + } +#endif }; #endif diff --git a/include/utils/SkCanvasStateUtils.h b/include/utils/SkCanvasStateUtils.h index 3071c75..fbc3a6f 100644 --- a/include/utils/SkCanvasStateUtils.h +++ b/include/utils/SkCanvasStateUtils.h @@ -62,7 +62,12 @@ public: * identical to the captured canvas. The caller is responsible for * calling unref on the SkCanvas. */ - static SkCanvas* CreateFromCanvasState(const SkCanvasState* state); + static std::unique_ptr MakeFromCanvasState(const SkCanvasState* state); +#ifdef SK_SUPPORT_LEGACY_CANVAS_IS_REFCNT + static SkCanvas* CreateFromCanvasState(const SkCanvasState* state) { + return MakeFromCanvasState(state).release(); + } +#endif /** * Free the memory associated with the captured canvas state. The state diff --git a/include/utils/SkNullCanvas.h b/include/utils/SkNullCanvas.h index 99a26da..884b68b 100644 --- a/include/utils/SkNullCanvas.h +++ b/include/utils/SkNullCanvas.h @@ -15,6 +15,12 @@ class SkCanvas; /** * Creates a canvas that draws nothing. This is useful for performance testing. */ -SK_API SkCanvas* SkCreateNullCanvas(); +SK_API std::unique_ptr SkMakeNullCanvas(); + +#ifdef SK_SUPPORT_LEGACY_CANVAS_IS_REFCNT +static inline SkCanvas* SkCreateNullCanvas() { + return SkMakeNullCanvas().release(); +} +#endif #endif diff --git a/public.bzl b/public.bzl index de3d6f2..734aaaf 100644 --- a/public.bzl +++ b/public.bzl @@ -600,6 +600,7 @@ DEFINES_ALL = [ "GOOGLE3", # Staging flags for API changes "SK_SUPPORT_LEGACY_ACCESSBITMAP", + "SK_SUPPORT_LEGACY_CANVAS_IS_REFCNT", "SK_SUPPORT_LEGACY_CLIP_REGIONOPS", ] diff --git a/samplecode/SamplePathFuzz.cpp b/samplecode/SamplePathFuzz.cpp index 05eb76f..b66b881 100644 --- a/samplecode/SamplePathFuzz.cpp +++ b/samplecode/SamplePathFuzz.cpp @@ -629,8 +629,8 @@ static void path_fuzz_stroker(SkBitmap* bitmap, int seed) { const SkPath& path = fuzzPath.getPath(); const SkPaint& paint = fuzzPath.getPaint(); const SkImageInfo& info = bitmap->info(); - SkCanvas* canvas( - SkCanvas::NewRasterDirect(info, bitmap->getPixels(), bitmap->rowBytes())); + std::unique_ptr canvas( + SkCanvas::MakeRasterDirect(info, bitmap->getPixels(), bitmap->rowBytes())); int w = info.width() / 4; int h = info.height() / 4; int x = localSeed / 4 % 4; diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp index cd4dcbc..a8d73a9 100644 --- a/src/core/SkCanvas.cpp +++ b/src/core/SkCanvas.cpp @@ -19,6 +19,7 @@ #include "SkImageFilter.h" #include "SkImageFilterCache.h" #include "SkLatticeIter.h" +#include "SkMakeUnique.h" #include "SkMatrixUtils.h" #include "SkMetaData.h" #include "SkNx.h" @@ -3323,7 +3324,8 @@ static bool supported_for_raster_canvas(const SkImageInfo& info) { return true; } -SkCanvas* SkCanvas::NewRasterDirect(const SkImageInfo& info, void* pixels, size_t rowBytes) { +std::unique_ptr SkCanvas::MakeRasterDirect(const SkImageInfo& info, void* pixels, + size_t rowBytes) { if (!supported_for_raster_canvas(info)) { return nullptr; } @@ -3332,7 +3334,7 @@ SkCanvas* SkCanvas::NewRasterDirect(const SkImageInfo& info, void* pixels, size_ if (!bitmap.installPixels(info, pixels, rowBytes)) { return nullptr; } - return new SkCanvas(bitmap); + return skstd::make_unique(bitmap); } /////////////////////////////////////////////////////////////////////////////// diff --git a/src/core/SkConfig8888.cpp b/src/core/SkConfig8888.cpp index 639a444..fd945cf 100644 --- a/src/core/SkConfig8888.cpp +++ b/src/core/SkConfig8888.cpp @@ -354,8 +354,8 @@ bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t if (!bm.installPixels(srcInfo, const_cast(srcPixels), srcRB, ctable, nullptr, nullptr)) { return false; } - sk_sp canvas(SkCanvas::NewRasterDirect(dstInfo, dstPixels, dstRB)); - if (nullptr == canvas.get()) { + std::unique_ptr canvas = SkCanvas::MakeRasterDirect(dstInfo, dstPixels, dstRB); + if (!canvas) { return false; } diff --git a/src/core/SkMultiPictureDraw.cpp b/src/core/SkMultiPictureDraw.cpp index b3c6368..1df27ff 100644 --- a/src/core/SkMultiPictureDraw.cpp +++ b/src/core/SkMultiPictureDraw.cpp @@ -18,7 +18,7 @@ void SkMultiPictureDraw::DrawData::draw() { void SkMultiPictureDraw::DrawData::init(SkCanvas* canvas, const SkPicture* picture, const SkMatrix* matrix, const SkPaint* paint) { fPicture = SkRef(picture); - fCanvas = SkRef(canvas); + fCanvas = canvas; if (matrix) { fMatrix = *matrix; } else { @@ -34,7 +34,6 @@ void SkMultiPictureDraw::DrawData::init(SkCanvas* canvas, const SkPicture* pictu void SkMultiPictureDraw::DrawData::Reset(SkTDArray& data) { for (int i = 0; i < data.count(); ++i) { data[i].fPicture->unref(); - data[i].fCanvas->unref(); delete data[i].fPaint; } data.rewind(); diff --git a/src/core/SkSpecialSurface.cpp b/src/core/SkSpecialSurface.cpp index beba915..b490421 100644 --- a/src/core/SkSpecialSurface.cpp +++ b/src/core/SkSpecialSurface.cpp @@ -29,7 +29,7 @@ public: virtual sk_sp onMakeImageSnapshot() = 0; protected: - sk_sp fCanvas; // initialized by derived classes in ctors + std::unique_ptr fCanvas; // initialized by derived classes in ctors private: typedef SkSpecialSurface INHERITED; diff --git a/src/image/SkSurface.cpp b/src/image/SkSurface.cpp index 38bab9e..3d6670f 100644 --- a/src/image/SkSurface.cpp +++ b/src/image/SkSurface.cpp @@ -58,14 +58,12 @@ SkSurfaceProps::SkSurfaceProps(const SkSurfaceProps& other) SkSurface_Base::SkSurface_Base(int width, int height, const SkSurfaceProps* props) : INHERITED(width, height, props) { - fCachedCanvas = nullptr; fCachedImage = nullptr; } SkSurface_Base::SkSurface_Base(const SkImageInfo& info, const SkSurfaceProps* props) : INHERITED(info, props) { - fCachedCanvas = nullptr; fCachedImage = nullptr; } @@ -76,7 +74,6 @@ SkSurface_Base::~SkSurface_Base() { } SkSafeUnref(fCachedImage); - SkSafeUnref(fCachedCanvas); } void SkSurface_Base::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y, const SkPaint* paint) { diff --git a/src/image/SkSurface_Base.h b/src/image/SkSurface_Base.h index 8351bb8..a8c1d8f 100644 --- a/src/image/SkSurface_Base.h +++ b/src/image/SkSurface_Base.h @@ -89,8 +89,8 @@ public: uint32_t newGenerationID(); private: - SkCanvas* fCachedCanvas; - SkImage* fCachedImage; + std::unique_ptr fCachedCanvas; + SkImage* fCachedImage; void aboutToDraw(ContentChangeMode mode); @@ -106,12 +106,12 @@ private: SkCanvas* SkSurface_Base::getCachedCanvas() { if (nullptr == fCachedCanvas) { - fCachedCanvas = this->onNewCanvas(); + fCachedCanvas = std::unique_ptr(this->onNewCanvas()); if (fCachedCanvas) { fCachedCanvas->setSurfaceBase(this); } } - return fCachedCanvas; + return fCachedCanvas.get(); } sk_sp SkSurface_Base::refCachedImage(SkBudgeted budgeted, ForceUnique unique) { diff --git a/src/pdf/SkPDFDocument.cpp b/src/pdf/SkPDFDocument.cpp index ab5f465..92e82fc 100644 --- a/src/pdf/SkPDFDocument.cpp +++ b/src/pdf/SkPDFDocument.cpp @@ -217,7 +217,7 @@ SkCanvas* SkPDFDocument::onBeginPage(SkScalar width, SkScalar height, SkScalarRoundToInt(width), SkScalarRoundToInt(height)); fPageDevice.reset( SkPDFDevice::Create(pageSize, fRasterDpi, this)); - fCanvas = sk_make_sp(fPageDevice); + fCanvas.reset(new SkPDFCanvas(fPageDevice)); fCanvas->clipRect(trimBox); fCanvas->translate(trimBox.x(), trimBox.y()); return fCanvas.get(); diff --git a/src/pdf/SkPDFDocument.h b/src/pdf/SkPDFDocument.h index b62a7a5..15e1479 100644 --- a/src/pdf/SkPDFDocument.h +++ b/src/pdf/SkPDFDocument.h @@ -76,7 +76,7 @@ private: SkTHashSet fFonts; sk_sp fDests; sk_sp fPageDevice; - sk_sp fCanvas; + std::unique_ptr fCanvas; sk_sp fID; sk_sp fXMP; SkScalar fRasterDpi; diff --git a/src/svg/SkSVGCanvas.cpp b/src/svg/SkSVGCanvas.cpp index d3511c0..95a4625 100644 --- a/src/svg/SkSVGCanvas.cpp +++ b/src/svg/SkSVGCanvas.cpp @@ -7,11 +7,12 @@ #include "SkSVGCanvas.h" #include "SkSVGDevice.h" +#include "SkMakeUnique.h" -SkCanvas* SkSVGCanvas::Create(const SkRect& bounds, SkXMLWriter* writer) { +std::unique_ptr SkSVGCanvas::Make(const SkRect& bounds, SkXMLWriter* writer) { // TODO: pass full bounds to the device SkISize size = bounds.roundOut().size(); sk_sp device(SkSVGDevice::Create(size, writer)); - return new SkCanvas(device.get()); + return skstd::make_unique(device.get()); } diff --git a/src/utils/SkCanvasStateUtils.cpp b/src/utils/SkCanvasStateUtils.cpp index 062dc13..58f26b6 100644 --- a/src/utils/SkCanvasStateUtils.cpp +++ b/src/utils/SkCanvasStateUtils.cpp @@ -100,14 +100,12 @@ class SkCanvasState_v1 : public SkCanvasState { public: static const int32_t kVersion = 1; - SkCanvasState_v1(SkCanvas* canvas) - : INHERITED(kVersion, canvas) - { + SkCanvasState_v1(SkCanvas* canvas) : INHERITED(kVersion, canvas) { layerCount = 0; layers = nullptr; mcState.clipRectCount = 0; mcState.clipRects = nullptr; - originalCanvas = SkRef(canvas); + originalCanvas = canvas; } ~SkCanvasState_v1() { @@ -118,10 +116,6 @@ public: sk_free(mcState.clipRects); sk_free(layers); - - // it is now safe to free the canvas since there should be no remaining - // references to the content that is referenced by this canvas (e.g. pixels) - originalCanvas->unref(); } SkMCState mcState; @@ -284,7 +278,8 @@ static void setup_canvas_from_MC_state(const SkMCState& state, SkCanvas* canvas) canvas->clipRegion(clip, SkCanvas::kReplace_Op); } -static SkCanvas* create_canvas_from_canvas_layer(const SkCanvasLayerState& layerState) { +static std::unique_ptr +make_canvas_from_canvas_layer(const SkCanvasLayerState& layerState) { SkASSERT(kRaster_CanvasBackend == layerState.type); SkBitmap bitmap; @@ -304,15 +299,15 @@ static SkCanvas* create_canvas_from_canvas_layer(const SkCanvasLayerState& layer SkASSERT(!bitmap.empty()); SkASSERT(!bitmap.isNull()); - sk_sp canvas(new SkCanvas(bitmap)); + std::unique_ptr canvas(new SkCanvas(bitmap)); // setup the matrix and clip setup_canvas_from_MC_state(layerState.mcState, canvas.get()); - return canvas.release(); + return canvas; } -SkCanvas* SkCanvasStateUtils::CreateFromCanvasState(const SkCanvasState* state) { +std::unique_ptr SkCanvasStateUtils::MakeFromCanvasState(const SkCanvasState* state) { SkASSERT(state); // Currently there is only one possible version. SkASSERT(SkCanvasState_v1::kVersion == state->version); @@ -323,14 +318,14 @@ SkCanvas* SkCanvasStateUtils::CreateFromCanvasState(const SkCanvasState* state) return nullptr; } - sk_sp canvas(new SkCanvasStack(state->width, state->height)); + std::unique_ptr canvas(new SkCanvasStack(state->width, state->height)); // setup the matrix and clip on the n-way canvas setup_canvas_from_MC_state(state_v1->mcState, canvas.get()); // Iterate over the layers and add them to the n-way canvas for (int i = state_v1->layerCount - 1; i >= 0; --i) { - sk_sp canvasLayer(create_canvas_from_canvas_layer(state_v1->layers[i])); + std::unique_ptr canvasLayer = make_canvas_from_canvas_layer(state_v1->layers[i]); if (!canvasLayer.get()) { return nullptr; } @@ -338,7 +333,7 @@ SkCanvas* SkCanvasStateUtils::CreateFromCanvasState(const SkCanvasState* state) state_v1->layers[i].y)); } - return canvas.release(); + return std::move(canvas); } //////////////////////////////////////////////////////////////////////////////// diff --git a/src/utils/SkLua.cpp b/src/utils/SkLua.cpp index a223456..27127df 100644 --- a/src/utils/SkLua.cpp +++ b/src/utils/SkLua.cpp @@ -73,6 +73,13 @@ template void push_obj(lua_State* L, const T& obj) { lua_setmetatable(L, -2); } +template T* push_ptr(lua_State* L, T* ptr) { + *(T**)lua_newuserdata(L, sizeof(T*)) = ptr; + luaL_getmetatable(L, get_mtname()); + lua_setmetatable(L, -2); + return ptr; +} + template T* push_ref(lua_State* L, T* ref) { *(T**)lua_newuserdata(L, sizeof(T*)) = SkSafeRef(ref); luaL_getmetatable(L, get_mtname()); @@ -333,7 +340,7 @@ void SkLua::pushPath(const SkPath& path, const char key[]) { } void SkLua::pushCanvas(SkCanvas* canvas, const char key[]) { - push_ref(fL, canvas); + push_ptr(fL, canvas); CHECK_SETFIELD(key); } @@ -715,7 +722,7 @@ static int lcanvas_newSurface(lua_State* L) { } static int lcanvas_gc(lua_State* L) { - get_ref(L, 1)->unref(); + // don't know how to track a ptr... return 0; } @@ -757,7 +764,7 @@ const struct luaL_Reg gSkCanvas_Methods[] = { static int ldocument_beginPage(lua_State* L) { const SkRect* contentPtr = nullptr; - push_ref(L, get_ref(L, 1)->beginPage(lua2scalar(L, 2), + push_ptr(L, get_ref(L, 1)->beginPage(lua2scalar(L, 2), lua2scalar(L, 3), contentPtr)); return 1; @@ -1750,7 +1757,7 @@ static int lsurface_getCanvas(lua_State* L) { if (nullptr == canvas) { lua_pushnil(L); } else { - push_ref(L, canvas); + push_ptr(L, canvas); // note: we don't unref canvas, since getCanvas did not ref it. // warning: this is weird: now Lua owns a ref on this canvas, but what if they let // the real owner (the surface) go away, but still hold onto the canvas? @@ -1814,7 +1821,7 @@ static int lpicturerecorder_beginRecording(lua_State* L) { return 1; } - push_ref(L, canvas); + push_ptr(L, canvas); return 1; } @@ -1824,7 +1831,7 @@ static int lpicturerecorder_getCanvas(lua_State* L) { lua_pushnil(L); return 1; } - push_ref(L, canvas); + push_ptr(L, canvas); return 1; } diff --git a/src/utils/SkNWayCanvas.cpp b/src/utils/SkNWayCanvas.cpp index b2d71d2..a910284 100644 --- a/src/utils/SkNWayCanvas.cpp +++ b/src/utils/SkNWayCanvas.cpp @@ -15,7 +15,6 @@ SkNWayCanvas::~SkNWayCanvas() { void SkNWayCanvas::addCanvas(SkCanvas* canvas) { if (canvas) { - canvas->ref(); *fList.append() = canvas; } } @@ -23,13 +22,11 @@ void SkNWayCanvas::addCanvas(SkCanvas* canvas) { void SkNWayCanvas::removeCanvas(SkCanvas* canvas) { int index = fList.find(canvas); if (index >= 0) { - canvas->unref(); fList.removeShuffle(index); } } void SkNWayCanvas::removeAll() { - fList.unrefAll(); fList.reset(); } diff --git a/src/utils/SkNullCanvas.cpp b/src/utils/SkNullCanvas.cpp index b5ee8d3..1f28706 100644 --- a/src/utils/SkNullCanvas.cpp +++ b/src/utils/SkNullCanvas.cpp @@ -9,10 +9,10 @@ #include "SkCanvas.h" #include "SkNWayCanvas.h" +#include "SkMakeUnique.h" - -SkCanvas* SkCreateNullCanvas() { +std::unique_ptr SkMakeNullCanvas() { // An N-Way canvas forwards calls to N canvas's. When N == 0 it's // effectively a null canvas. - return new SkNWayCanvas(0, 0); + return std::unique_ptr(new SkNWayCanvas(0, 0)); } diff --git a/src/xps/SkDocument_XPS.cpp b/src/xps/SkDocument_XPS.cpp index e333b5a..d05764a 100644 --- a/src/xps/SkDocument_XPS.cpp +++ b/src/xps/SkDocument_XPS.cpp @@ -58,7 +58,7 @@ protected: private: SkXPSDevice fDevice; - sk_sp fCanvas; + std::unique_ptr fCanvas; SkVector fUnitsPerMeter; SkVector fPixelsPerMeter; }; diff --git a/tests/CanvasStateHelpers.cpp b/tests/CanvasStateHelpers.cpp index 801eaea..98b5d23 100644 --- a/tests/CanvasStateHelpers.cpp +++ b/tests/CanvasStateHelpers.cpp @@ -27,12 +27,11 @@ void complex_layers_draw(SkCanvas* canvas, float left, float top, extern "C" bool complex_layers_draw_from_canvas_state(SkCanvasState* state, float left, float top, float right, float bottom, int32_t spacer) { - SkCanvas* canvas = SkCanvasStateUtils::CreateFromCanvasState(state); + std::unique_ptr canvas = SkCanvasStateUtils::MakeFromCanvasState(state); if (!canvas) { return false; } - complex_layers_draw(canvas, left, top, right, bottom, spacer); - canvas->unref(); + complex_layers_draw(canvas.get(), left, top, right, bottom, spacer); return true; } @@ -52,7 +51,7 @@ void complex_clips_draw(SkCanvas* canvas, int32_t left, int32_t top, extern "C" bool complex_clips_draw_from_canvas_state(SkCanvasState* state, int32_t left, int32_t top, int32_t right, int32_t bottom, int32_t clipOp, int32_t regionRects, int32_t* rectCoords) { - SkCanvas* canvas = SkCanvasStateUtils::CreateFromCanvasState(state); + std::unique_ptr canvas = SkCanvasStateUtils::MakeFromCanvasState(state); if (!canvas) { return false; } @@ -64,8 +63,7 @@ extern "C" bool complex_clips_draw_from_canvas_state(SkCanvasState* state, rectCoords += 4; } - complex_clips_draw(canvas, left, top, right, bottom, clipOp, localRegion); - canvas->unref(); + complex_clips_draw(canvas.get(), left, top, right, bottom, clipOp, localRegion); return true; } #endif // SK_SUPPORT_LEGACY_CLIPTOLAYERFLAG diff --git a/tests/CanvasStateTest.cpp b/tests/CanvasStateTest.cpp index 890e85d..2c7c590 100644 --- a/tests/CanvasStateTest.cpp +++ b/tests/CanvasStateTest.cpp @@ -278,13 +278,12 @@ DEF_TEST(CanvasState_test_draw_filters, reporter) { SkCanvasState* state = SkCanvasStateUtils::CaptureCanvasState(&canvas); REPORTER_ASSERT(reporter, state); - SkCanvas* tmpCanvas = SkCanvasStateUtils::CreateFromCanvasState(state); + std::unique_ptr tmpCanvas = SkCanvasStateUtils::MakeFromCanvasState(state); REPORTER_ASSERT(reporter, tmpCanvas); REPORTER_ASSERT(reporter, canvas.getDrawFilter()); REPORTER_ASSERT(reporter, nullptr == tmpCanvas->getDrawFilter()); - tmpCanvas->unref(); SkCanvasStateUtils::ReleaseCanvasState(state); } diff --git a/tests/CanvasTest.cpp b/tests/CanvasTest.cpp index c062e6c..3f418db 100644 --- a/tests/CanvasTest.cpp +++ b/tests/CanvasTest.cpp @@ -542,7 +542,7 @@ static void test_newraster(skiatest::Reporter* reporter) { SkPMColor* baseAddr = storage.get(); sk_bzero(baseAddr, size); - SkCanvas* canvas = SkCanvas::NewRasterDirect(info, baseAddr, minRowBytes); + std::unique_ptr canvas = SkCanvas::MakeRasterDirect(info, baseAddr, minRowBytes); REPORTER_ASSERT(reporter, canvas); SkPixmap pmap; @@ -556,25 +556,23 @@ static void test_newraster(skiatest::Reporter* reporter) { } addr = (const SkPMColor*)((const char*)addr + pmap.rowBytes()); } - delete canvas; // now try a deliberately bad info info = info.makeWH(-1, info.height()); - REPORTER_ASSERT(reporter, nullptr == SkCanvas::NewRasterDirect(info, baseAddr, minRowBytes)); + REPORTER_ASSERT(reporter, nullptr == SkCanvas::MakeRasterDirect(info, baseAddr, minRowBytes)); // too big info = info.makeWH(1 << 30, 1 << 30); - REPORTER_ASSERT(reporter, nullptr == SkCanvas::NewRasterDirect(info, baseAddr, minRowBytes)); + REPORTER_ASSERT(reporter, nullptr == SkCanvas::MakeRasterDirect(info, baseAddr, minRowBytes)); // not a valid pixel type info = SkImageInfo::Make(10, 10, kUnknown_SkColorType, info.alphaType()); - REPORTER_ASSERT(reporter, nullptr == SkCanvas::NewRasterDirect(info, baseAddr, minRowBytes)); + REPORTER_ASSERT(reporter, nullptr == SkCanvas::MakeRasterDirect(info, baseAddr, minRowBytes)); // We should succeed with a zero-sized valid info info = SkImageInfo::MakeN32Premul(0, 0); - canvas = SkCanvas::NewRasterDirect(info, baseAddr, minRowBytes); + canvas = SkCanvas::MakeRasterDirect(info, baseAddr, minRowBytes); REPORTER_ASSERT(reporter, canvas); - delete canvas; } DEF_TEST(Canvas, reporter) { diff --git a/tests/PipeTest.cpp b/tests/PipeTest.cpp index 3b89441..e5d2f09 100644 --- a/tests/PipeTest.cpp +++ b/tests/PipeTest.cpp @@ -18,7 +18,7 @@ #include "SkPictureRecorder.h" static void drain(SkPipeDeserializer* deserial, SkDynamicMemoryWStream* stream) { - std::unique_ptr canvas(SkCreateNullCanvas()); + std::unique_ptr canvas = SkMakeNullCanvas(); sk_sp data = stream->detachAsData(); deserial->playback(data->data(), data->size(), canvas.get()); } diff --git a/tests/SVGDeviceTest.cpp b/tests/SVGDeviceTest.cpp index b010e57..715dbc7 100644 --- a/tests/SVGDeviceTest.cpp +++ b/tests/SVGDeviceTest.cpp @@ -91,7 +91,7 @@ void test_whitespace_pos(skiatest::Reporter* reporter, { SkXMLParserWriter writer(dom.beginParsing()); - sk_sp svgCanvas(SkSVGCanvas::Create(SkRect::MakeWH(100, 100), &writer)); + std::unique_ptr svgCanvas = SkSVGCanvas::Make(SkRect::MakeWH(100, 100), &writer); svgCanvas->drawText(txt, len, offset.x(), offset.y(), paint); } check_text_node(reporter, dom, dom.finishParsing(), offset, 0, expected); @@ -103,7 +103,7 @@ void test_whitespace_pos(skiatest::Reporter* reporter, } SkXMLParserWriter writer(dom.beginParsing()); - sk_sp svgCanvas(SkSVGCanvas::Create(SkRect::MakeWH(100, 100), &writer)); + std::unique_ptr svgCanvas = SkSVGCanvas::Make(SkRect::MakeWH(100, 100), &writer); svgCanvas->drawPosTextH(txt, len, xpos, offset.y(), paint); } check_text_node(reporter, dom, dom.finishParsing(), offset, 1, expected); @@ -115,7 +115,7 @@ void test_whitespace_pos(skiatest::Reporter* reporter, } SkXMLParserWriter writer(dom.beginParsing()); - sk_sp svgCanvas(SkSVGCanvas::Create(SkRect::MakeWH(100, 100), &writer)); + std::unique_ptr svgCanvas = SkSVGCanvas::Make(SkRect::MakeWH(100, 100), &writer); svgCanvas->drawPosText(txt, len, pos, paint); } check_text_node(reporter, dom, dom.finishParsing(), offset, 2, expected); diff --git a/tools/lua/lua_pictures.cpp b/tools/lua/lua_pictures.cpp index bd8fda1..639a522 100644 --- a/tools/lua/lua_pictures.cpp +++ b/tools/lua/lua_pictures.cpp @@ -145,7 +145,7 @@ int tool_main(int argc, char** argv) { auto pic(load_picture(path)); if (pic.get()) { - sk_sp canvas( + std::unique_ptr canvas( new SkLuaCanvas(SkScalarCeilToInt(pic->cullRect().width()), SkScalarCeilToInt(pic->cullRect().height()), L.get(), gAccumulateFunc)); diff --git a/tools/skiaserve/Request.h b/tools/skiaserve/Request.h index 6b065a2..4058d7c 100644 --- a/tools/skiaserve/Request.h +++ b/tools/skiaserve/Request.h @@ -63,7 +63,7 @@ struct Request { SkColor getPixel(int x, int y); UploadContext* fUploadContext; - sk_sp fDebugCanvas; + std::unique_ptr fDebugCanvas; UrlDataManager fUrlDataManager; private: diff --git a/tools/skp_parser.cpp b/tools/skp_parser.cpp index d242524..887c50d 100644 --- a/tools/skp_parser.cpp +++ b/tools/skp_parser.cpp @@ -34,7 +34,7 @@ int main(int argc, char** argv) { SkISize size = pic->cullRect().roundOut().size(); SkDebugCanvas debugCanvas(size.width(), size.height()); pic->playback(&debugCanvas); - sk_sp nullCanvas(SkCreateNullCanvas()); + std::unique_ptr nullCanvas = SkMakeNullCanvas(); UrlDataManager dataManager(SkString("data")); Json::Value json = debugCanvas.toJSON( dataManager, debugCanvas.getSize(), nullCanvas.get()); -- 2.7.4