Revert[2] "Change SkCanvas to *not* inherit from SkRefCnt"
Changes over original:
- conditionalize ownership in SkPictureRecorder
- conditionalize ownership in SkCanvasStateUtils
This reverts commit b613c266df48cf45296ecc23d1bd7098c84bb7ba.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4742
Change-Id: Ib25514d6f546c69b6650b5c957403b04f7380dc2
Reviewed-on: https://skia-review.googlesource.com/4742
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
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 @@
return true;
}
-SkCanvas* SkCanvas::NewRasterDirect(const SkImageInfo& info, void* pixels, size_t rowBytes) {
+std::unique_ptr<SkCanvas> SkCanvas::MakeRasterDirect(const SkImageInfo& info, void* pixels,
+ size_t rowBytes) {
if (!supported_for_raster_canvas(info)) {
return nullptr;
}
@@ -3332,7 +3334,7 @@
if (!bitmap.installPixels(info, pixels, rowBytes)) {
return nullptr;
}
- return new SkCanvas(bitmap);
+ return skstd::make_unique<SkCanvas>(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 @@
if (!bm.installPixels(srcInfo, const_cast<void*>(srcPixels), srcRB, ctable, nullptr, nullptr)) {
return false;
}
- sk_sp<SkCanvas> canvas(SkCanvas::NewRasterDirect(dstInfo, dstPixels, dstRB));
- if (nullptr == canvas.get()) {
+ std::unique_ptr<SkCanvas> 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::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::Reset(SkTDArray<DrawData>& 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 @@
virtual sk_sp<SkSpecialImage> onMakeImageSnapshot() = 0;
protected:
- sk_sp<SkCanvas> fCanvas; // initialized by derived classes in ctors
+ std::unique_ptr<SkCanvas> 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 @@
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 @@
}
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 @@
uint32_t newGenerationID();
private:
- SkCanvas* fCachedCanvas;
- SkImage* fCachedImage;
+ std::unique_ptr<SkCanvas> fCachedCanvas;
+ SkImage* fCachedImage;
void aboutToDraw(ContentChangeMode mode);
@@ -106,12 +106,12 @@
SkCanvas* SkSurface_Base::getCachedCanvas() {
if (nullptr == fCachedCanvas) {
- fCachedCanvas = this->onNewCanvas();
+ fCachedCanvas = std::unique_ptr<SkCanvas>(this->onNewCanvas());
if (fCachedCanvas) {
fCachedCanvas->setSurfaceBase(this);
}
}
- return fCachedCanvas;
+ return fCachedCanvas.get();
}
sk_sp<SkImage> 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 @@
SkScalarRoundToInt(width), SkScalarRoundToInt(height));
fPageDevice.reset(
SkPDFDevice::Create(pageSize, fRasterDpi, this));
- fCanvas = sk_make_sp<SkPDFCanvas>(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 @@
SkTHashSet<SkPDFFont*> fFonts;
sk_sp<SkPDFDict> fDests;
sk_sp<SkPDFDevice> fPageDevice;
- sk_sp<SkCanvas> fCanvas;
+ std::unique_ptr<SkCanvas> fCanvas;
sk_sp<SkPDFObject> fID;
sk_sp<SkPDFObject> 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<SkCanvas> SkSVGCanvas::Make(const SkRect& bounds, SkXMLWriter* writer) {
// TODO: pass full bounds to the device
SkISize size = bounds.roundOut().size();
sk_sp<SkBaseDevice> device(SkSVGDevice::Create(size, writer));
- return new SkCanvas(device.get());
+ return skstd::make_unique<SkCanvas>(device.get());
}
diff --git a/src/utils/SkCanvasStateUtils.cpp b/src/utils/SkCanvasStateUtils.cpp
index 062dc13..6ee1c33 100644
--- a/src/utils/SkCanvasStateUtils.cpp
+++ b/src/utils/SkCanvasStateUtils.cpp
@@ -100,14 +100,16 @@
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;
+#ifdef SK_SUPPORT_LEGACY_CANVAS_IS_REFCNT
originalCanvas = SkRef(canvas);
+#else
+ originalCanvas = canvas;
+#endif
}
~SkCanvasState_v1() {
@@ -119,9 +121,9 @@
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)
+#ifdef SK_SUPPORT_LEGACY_CANVAS_IS_REFCNT
originalCanvas->unref();
+#endif
}
SkMCState mcState;
@@ -284,7 +286,8 @@
canvas->clipRegion(clip, SkCanvas::kReplace_Op);
}
-static SkCanvas* create_canvas_from_canvas_layer(const SkCanvasLayerState& layerState) {
+static std::unique_ptr<SkCanvas>
+make_canvas_from_canvas_layer(const SkCanvasLayerState& layerState) {
SkASSERT(kRaster_CanvasBackend == layerState.type);
SkBitmap bitmap;
@@ -304,15 +307,15 @@
SkASSERT(!bitmap.empty());
SkASSERT(!bitmap.isNull());
- sk_sp<SkCanvas> canvas(new SkCanvas(bitmap));
+ std::unique_ptr<SkCanvas> 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<SkCanvas> SkCanvasStateUtils::MakeFromCanvasState(const SkCanvasState* state) {
SkASSERT(state);
// Currently there is only one possible version.
SkASSERT(SkCanvasState_v1::kVersion == state->version);
@@ -323,14 +326,14 @@
return nullptr;
}
- sk_sp<SkCanvasStack> canvas(new SkCanvasStack(state->width, state->height));
+ std::unique_ptr<SkCanvasStack> 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<SkCanvas> canvasLayer(create_canvas_from_canvas_layer(state_v1->layers[i]));
+ std::unique_ptr<SkCanvas> canvasLayer = make_canvas_from_canvas_layer(state_v1->layers[i]);
if (!canvasLayer.get()) {
return nullptr;
}
@@ -338,7 +341,7 @@
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 @@
lua_setmetatable(L, -2);
}
+template <typename T> T* push_ptr(lua_State* L, T* ptr) {
+ *(T**)lua_newuserdata(L, sizeof(T*)) = ptr;
+ luaL_getmetatable(L, get_mtname<T>());
+ lua_setmetatable(L, -2);
+ return ptr;
+}
+
template <typename T> T* push_ref(lua_State* L, T* ref) {
*(T**)lua_newuserdata(L, sizeof(T*)) = SkSafeRef(ref);
luaL_getmetatable(L, get_mtname<T>());
@@ -333,7 +340,7 @@
}
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_gc(lua_State* L) {
- get_ref<SkCanvas>(L, 1)->unref();
+ // don't know how to track a ptr...
return 0;
}
@@ -757,7 +764,7 @@
static int ldocument_beginPage(lua_State* L) {
const SkRect* contentPtr = nullptr;
- push_ref(L, get_ref<SkDocument>(L, 1)->beginPage(lua2scalar(L, 2),
+ push_ptr(L, get_ref<SkDocument>(L, 1)->beginPage(lua2scalar(L, 2),
lua2scalar(L, 3),
contentPtr));
return 1;
@@ -1750,7 +1757,7 @@
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 @@
return 1;
}
- push_ref(L, canvas);
+ push_ptr(L, canvas);
return 1;
}
@@ -1824,7 +1831,7 @@
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 @@
void SkNWayCanvas::addCanvas(SkCanvas* canvas) {
if (canvas) {
- canvas->ref();
*fList.append() = canvas;
}
}
@@ -23,13 +22,11 @@
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<SkCanvas> 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<SkCanvas>(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 @@
private:
SkXPSDevice fDevice;
- sk_sp<SkCanvas> fCanvas;
+ std::unique_ptr<SkCanvas> fCanvas;
SkVector fUnitsPerMeter;
SkVector fPixelsPerMeter;
};