From 744fb7313f31069e9eceeca4c0ff33dbe2fe1737 Mon Sep 17 00:00:00 2001 From: Mike Klein Date: Mon, 23 Jun 2014 15:13:26 -0400 Subject: [PATCH] Enable basic drawing with SkRecord-based pictures. I've tagged all the functions in SkPicture.cpp is // fRecord TODO or // fRecord OK, depending on whether or not they're totally broken when used from an SkRecord-based picture. Obviously next steps are to eliminate all the TODOs, then clean up the notes. I converted SkPicture over to smart pointers too. It's particularly helpful that the smart pointers initialize to NULL by default. For now I've got all the SkRecord-based code jammed in at the bottom of the file. I figure it'll help me keep things straight for a bit, then we can rearrange later. BUG=skia: R=robertphillips@google.com Review URL: https://codereview.chromium.org/333823007 --- debugger/QT/SkDebuggerGUI.cpp | 10 +-- include/core/SkPicture.h | 23 +++--- src/core/SkPicture.cpp | 129 +++++++++++++++++++++++---------- src/core/SkPictureRecorder.cpp | 2 +- src/gpu/GrPictureUtils.cpp | 4 +- src/gpu/SkGpuDevice.cpp | 8 +- tools/bench_playback.cpp | 33 +++------ 7 files changed, 121 insertions(+), 88 deletions(-) diff --git a/debugger/QT/SkDebuggerGUI.cpp b/debugger/QT/SkDebuggerGUI.cpp index 5cecba273c..a3783e9359 100644 --- a/debugger/QT/SkDebuggerGUI.cpp +++ b/debugger/QT/SkDebuggerGUI.cpp @@ -286,16 +286,16 @@ public: return NULL; } - void resetTimes() { ((SkTimedPicturePlayback*) fPlayback)->resetTimes(); } + void resetTimes() { ((SkTimedPicturePlayback*) fPlayback.get())->resetTimes(); } - int count() const { return ((SkTimedPicturePlayback*) fPlayback)->count(); } + int count() const { return ((SkTimedPicturePlayback*) fPlayback.get())->count(); } // return the fraction of the total time this command consumed - double time(int index) const { return ((SkTimedPicturePlayback*) fPlayback)->time(index); } + double time(int index) const { return ((SkTimedPicturePlayback*) fPlayback.get())->time(index); } - const SkTDArray* typeTimes() const { return ((SkTimedPicturePlayback*) fPlayback)->typeTimes(); } + const SkTDArray* typeTimes() const { return ((SkTimedPicturePlayback*) fPlayback.get())->typeTimes(); } - double totTime() const { return ((SkTimedPicturePlayback*) fPlayback)->totTime(); } + double totTime() const { return ((SkTimedPicturePlayback*) fPlayback.get())->totTime(); } private: // disallow default ctor b.c. we don't have a good way to setup the fPlayback ptr diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h index 7b38230186..7cba671d0a 100644 --- a/include/core/SkPicture.h +++ b/include/core/SkPicture.h @@ -30,6 +30,8 @@ class SkWStream; struct SkPictInfo; +class SkRecord; + /** \class SkPicture The SkPicture class records the drawing commands made to a canvas, to @@ -67,16 +69,10 @@ public: SkPicture(const SkPicture& src); /** PRIVATE / EXPERIMENTAL -- do not call */ - void EXPERIMENTAL_addAccelData(const AccelData* data) const { - SkRefCnt_SafeAssign(fAccelData, data); - } + void EXPERIMENTAL_addAccelData(const AccelData*) const; + /** PRIVATE / EXPERIMENTAL -- do not call */ - const AccelData* EXPERIMENTAL_getAccelData(AccelData::Key key) const { - if (NULL != fAccelData && fAccelData->getKey() == key) { - return fAccelData; - } - return NULL; - } + const AccelData* EXPERIMENTAL_getAccelData(AccelData::Key) const; /** * Function signature defining a function that sets up an SkBitmap from encoded data. On @@ -255,12 +251,12 @@ protected: mutable uint32_t fUniqueID; - // fPlayback, fRecord, fWidth & fHeight are protected to allow derived classes to + // fPlayback, fWidth & fHeight are protected to allow derived classes to // install their own SkPicturePlayback-derived players,SkPictureRecord-derived // recorders and set the picture size - SkPicturePlayback* fPlayback; + SkAutoTDelete fPlayback; int fWidth, fHeight; - mutable const AccelData* fAccelData; + mutable SkAutoTUnref fAccelData; void needsNewGenID() { fUniqueID = SK_InvalidGenID; } @@ -318,6 +314,9 @@ private: friend class SkDebugCanvas; typedef SkRefCnt INHERITED; + + SkPicture(int width, int height, SkRecord*); // Takes ownership. + SkAutoTDelete fRecord; }; /** diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp index 76c2b9def9..11afc34c4b 100644 --- a/src/core/SkPicture.cpp +++ b/src/core/SkPicture.cpp @@ -32,6 +32,9 @@ #include "GrContext.h" #endif +#include "SkRecord.h" +#include "SkRecordDraw.h" + template int SafeCount(const T* obj) { return obj ? obj->count() : 0; } @@ -123,60 +126,60 @@ static void validateMatrix(const SkMatrix* matrix) { /////////////////////////////////////////////////////////////////////////////// +// fRecord OK SkPicture::SkPicture() - : fAccelData(NULL) { + : fWidth(0) + , fHeight(0) { this->needsNewGenID(); - fPlayback = NULL; - fWidth = fHeight = 0; } +// fRecord OK SkPicture::SkPicture(int width, int height, const SkPictureRecord& record, bool deepCopyOps) : fWidth(width) - , fHeight(height) - , fAccelData(NULL) { + , fHeight(height) { this->needsNewGenID(); SkPictInfo info; this->createHeader(&info); - fPlayback = SkNEW_ARGS(SkPicturePlayback, (record, info, deepCopyOps)); + fPlayback.reset(SkNEW_ARGS(SkPicturePlayback, (record, info, deepCopyOps))); } -SkPicture::SkPicture(const SkPicture& src) - : INHERITED() - , fAccelData(NULL) { +// fRecord TODO +SkPicture::SkPicture(const SkPicture& src) : INHERITED() { this->needsNewGenID(); fWidth = src.fWidth; fHeight = src.fHeight; - if (src.fPlayback) { - fPlayback = SkNEW_ARGS(SkPicturePlayback, (*src.fPlayback)); + if (src.fPlayback.get()) { + fPlayback.reset(SkNEW_ARGS(SkPicturePlayback, (*src.fPlayback))); fUniqueID = src.uniqueID(); // need to call method to ensure != 0 - } else { - fPlayback = NULL; } } -SkPicture::~SkPicture() { - SkDELETE(fPlayback); - SkSafeUnref(fAccelData); -} +// fRecord OK +SkPicture::~SkPicture() {} +// fRecord OK void SkPicture::swap(SkPicture& other) { SkTSwap(fUniqueID, other.fUniqueID); - SkTSwap(fPlayback, other.fPlayback); - SkTSwap(fAccelData, other.fAccelData); SkTSwap(fWidth, other.fWidth); SkTSwap(fHeight, other.fHeight); + + fAccelData.swap(&other.fAccelData); + fPlayback.swap(&other.fPlayback); + fRecord.swap(&other.fRecord); } +// fRecord TODO SkPicture* SkPicture::clone() const { SkPicture* clonedPicture = SkNEW(SkPicture); this->clone(clonedPicture, 1); return clonedPicture; } +// fRecord TODO void SkPicture::clone(SkPicture* pictures, int count) const { SkPictCopyInfo copyInfo; @@ -186,13 +189,13 @@ void SkPicture::clone(SkPicture* pictures, int count) const { clone->needsNewGenID(); clone->fWidth = fWidth; clone->fHeight = fHeight; - SkDELETE(clone->fPlayback); + clone->fPlayback.reset(NULL); /* We want to copy the src's playback. However, if that hasn't been built yet, we need to fake a call to endRecording() without actually calling it (since it is destructive, and we don't want to change src). */ - if (fPlayback) { + if (fPlayback.get()) { if (!copyInfo.initialized) { int paintCount = SafeCount(fPlayback->fPaints); @@ -235,14 +238,27 @@ void SkPicture::clone(SkPicture* pictures, int count) const { copyInfo.initialized = true; } - clone->fPlayback = SkNEW_ARGS(SkPicturePlayback, (*fPlayback, ©Info)); + clone->fPlayback.reset(SkNEW_ARGS(SkPicturePlayback, (*fPlayback, ©Info))); clone->fUniqueID = this->uniqueID(); // need to call method to ensure != 0 - } else { - clone->fPlayback = NULL; } } } +// fRecord OK +void SkPicture::EXPERIMENTAL_addAccelData(const SkPicture::AccelData* data) const { + fAccelData.reset(SkRef(data)); +} + +// fRecord OK +const SkPicture::AccelData* SkPicture::EXPERIMENTAL_getAccelData( + SkPicture::AccelData::Key key) const { + if (NULL != fAccelData.get() && fAccelData->getKey() == key) { + return fAccelData.get(); + } + return NULL; +} + +// fRecord OK SkPicture::AccelData::Domain SkPicture::AccelData::GenerateDomain() { static int32_t gNextID = 0; @@ -256,30 +272,40 @@ SkPicture::AccelData::Domain SkPicture::AccelData::GenerateDomain() { /////////////////////////////////////////////////////////////////////////////// +// fRecord OK const SkPicture::OperationList& SkPicture::OperationList::InvalidList() { static OperationList gInvalid; return gInvalid; } +// fRecord TODO const SkPicture::OperationList& SkPicture::EXPERIMENTAL_getActiveOps(const SkIRect& queryRect) const { - SkASSERT(NULL != fPlayback); - if (NULL != fPlayback) { + SkASSERT(NULL != fPlayback.get()); + if (NULL != fPlayback.get()) { return fPlayback->getActiveOps(queryRect); } return OperationList::InvalidList(); } +// fRecord TODO size_t SkPicture::EXPERIMENTAL_curOpID() const { - if (NULL != fPlayback) { + if (NULL != fPlayback.get()) { return fPlayback->curOpID(); } return 0; } -void SkPicture::draw(SkCanvas* surface, SkDrawPictureCallback* callback) const { - SkASSERT(NULL != fPlayback); - if (NULL != fPlayback) { - fPlayback->draw(*surface, callback); +// fRecord OK +void SkPicture::draw(SkCanvas* canvas, SkDrawPictureCallback* callback) const { + SkASSERT(NULL != canvas); + SkASSERT(NULL != fPlayback.get() || NULL != fRecord.get()); + + if (NULL != fPlayback.get()) { + fPlayback->draw(*canvas, callback); + } + if (NULL != fRecord.get()) { + // TODO: support SkDrawPictureCallback + SkRecordDraw(*fRecord, canvas); } } @@ -289,6 +315,7 @@ void SkPicture::draw(SkCanvas* surface, SkDrawPictureCallback* callback) const { static const char kMagic[] = { 's', 'k', 'i', 'a', 'p', 'i', 'c', 't' }; +// fRecord OK bool SkPicture::IsValidPictInfo(const SkPictInfo& info) { if (0 != memcmp(info.fMagic, kMagic, sizeof(kMagic))) { return false; @@ -302,6 +329,7 @@ bool SkPicture::IsValidPictInfo(const SkPictInfo& info) { return true; } +// fRecord OK bool SkPicture::InternalOnly_StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) { if (NULL == stream) { return false; @@ -320,6 +348,7 @@ bool SkPicture::InternalOnly_StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) { return true; } +// fRecord OK bool SkPicture::InternalOnly_BufferIsSKP(SkReadBuffer& buffer, SkPictInfo* pInfo) { // Check magic bytes. SkPictInfo info; @@ -334,14 +363,15 @@ bool SkPicture::InternalOnly_BufferIsSKP(SkReadBuffer& buffer, SkPictInfo* pInfo return true; } +// fRecord OK SkPicture::SkPicture(SkPicturePlayback* playback, int width, int height) : fPlayback(playback) , fWidth(width) - , fHeight(height) - , fAccelData(NULL) { + , fHeight(height) { this->needsNewGenID(); } +// fRecord OK SkPicture* SkPicture::CreateFromStream(SkStream* stream, InstallPixelRefProc proc) { SkPictInfo info; @@ -362,6 +392,7 @@ SkPicture* SkPicture::CreateFromStream(SkStream* stream, InstallPixelRefProc pro return NULL; } +// fRecord OK SkPicture* SkPicture::CreateFromBuffer(SkReadBuffer& buffer) { SkPictInfo info; @@ -382,6 +413,7 @@ SkPicture* SkPicture::CreateFromBuffer(SkReadBuffer& buffer) { return NULL; } +// fRecord OK void SkPicture::createHeader(SkPictInfo* info) const { // Copy magic bytes at the beginning of the header SkASSERT(sizeof(kMagic) == 8); @@ -401,8 +433,9 @@ void SkPicture::createHeader(SkPictInfo* info) const { } } +// fRecord TODO void SkPicture::serialize(SkWStream* stream, EncodeBitmap encoder) const { - SkPicturePlayback* playback = fPlayback; + SkPicturePlayback* playback = fPlayback.get(); SkPictInfo info; this->createHeader(&info); @@ -411,7 +444,7 @@ void SkPicture::serialize(SkWStream* stream, EncodeBitmap encoder) const { stream->writeBool(true); playback->serialize(stream, encoder); // delete playback if it is a local version (i.e. cons'd up just now) - if (playback != fPlayback) { + if (playback != fPlayback.get()) { SkDELETE(playback); } } else { @@ -419,18 +452,21 @@ void SkPicture::serialize(SkWStream* stream, EncodeBitmap encoder) const { } } +// fRecord OK void SkPicture::WriteTagSize(SkWriteBuffer& buffer, uint32_t tag, size_t size) { buffer.writeUInt(tag); buffer.writeUInt(SkToU32(size)); } +// fRecord OK void SkPicture::WriteTagSize(SkWStream* stream, uint32_t tag, size_t size) { stream->write32(tag); stream->write32(SkToU32(size)); } +// fRecord TODO void SkPicture::flatten(SkWriteBuffer& buffer) const { - SkPicturePlayback* playback = fPlayback; + SkPicturePlayback* playback = fPlayback.get(); SkPictInfo info; this->createHeader(&info); @@ -439,7 +475,7 @@ void SkPicture::flatten(SkWriteBuffer& buffer) const { buffer.writeBool(true); playback->flatten(buffer); // delete playback if it is a local version (i.e. cons'd up just now) - if (playback != fPlayback) { + if (playback != fPlayback.get()) { SkDELETE(playback); } } else { @@ -448,8 +484,9 @@ void SkPicture::flatten(SkWriteBuffer& buffer) const { } #if SK_SUPPORT_GPU +// fRecord TODO bool SkPicture::suitableForGpuRasterization(GrContext* context, const char **reason) const { - if (NULL == fPlayback) { + if (NULL == fPlayback.get()) { if (NULL != reason) { *reason = "Missing playback object."; } @@ -460,22 +497,25 @@ bool SkPicture::suitableForGpuRasterization(GrContext* context, const char **rea } #endif +// fRecord TODO bool SkPicture::willPlayBackBitmaps() const { - if (!fPlayback) { + if (!fPlayback.get()) { return false; } return fPlayback->containsBitmaps(); } #ifdef SK_BUILD_FOR_ANDROID +// fRecord TODO void SkPicture::abortPlayback() { - if (NULL == fPlayback) { + if (NULL == fPlayback.get()) { return; } fPlayback->abort(); } #endif +// fRecord OK static int32_t next_picture_generation_id() { static int32_t gPictureGenerationID = 0; // do a loop in case our global wraps around, as we never want to @@ -487,9 +527,18 @@ static int32_t next_picture_generation_id() { return genID; } +// fRecord OK uint32_t SkPicture::uniqueID() const { if (SK_InvalidGenID == fUniqueID) { fUniqueID = next_picture_generation_id(); } return fUniqueID; } + +// fRecord OK +SkPicture::SkPicture(int width, int height, SkRecord* record) + : fWidth(width) + , fHeight(height) + , fRecord(record) { + this->needsNewGenID(); +} diff --git a/src/core/SkPictureRecorder.cpp b/src/core/SkPictureRecorder.cpp index d0ce0a1383..1da9653436 100644 --- a/src/core/SkPictureRecorder.cpp +++ b/src/core/SkPictureRecorder.cpp @@ -60,7 +60,7 @@ SkPicture* SkPictureRecorder::endRecording() { SkPicture* picture = NULL; if (NULL != fRecord.get()) { - // TODO: picture = SkNEW_ARGS(SkPicture, (fWidth, fHeight, fRecord.detach())); + picture = SkNEW_ARGS(SkPicture, (fWidth, fHeight, fRecord.detach())); } if (NULL != fPictureRecord.get()) { diff --git a/src/gpu/GrPictureUtils.cpp b/src/gpu/GrPictureUtils.cpp index 6fed2f6d9f..c677c64bd1 100644 --- a/src/gpu/GrPictureUtils.cpp +++ b/src/gpu/GrPictureUtils.cpp @@ -250,11 +250,11 @@ protected: virtual void onDrawPicture(const SkPicture* picture) SK_OVERRIDE { // BBH-based rendering doesn't re-issue many of the operations the gather // process cares about (e.g., saves and restores) so it must be disabled. - if (NULL != picture->fPlayback) { + if (NULL != picture->fPlayback.get()) { picture->fPlayback->setUseBBH(false); } picture->draw(this); - if (NULL != picture->fPlayback) { + if (NULL != picture->fPlayback.get()) { picture->fPlayback->setUseBBH(true); } } diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index d0c32eb77c..8312356d92 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -474,7 +474,7 @@ void SkGpuDevice::drawRect(const SkDraw& draw, const SkRect& rect, GrPaint grPaint; SkPaint2GrPaintShader(this->context(), paint, true, &grPaint); - + fContext->drawRect(grPaint, rect, &strokeInfo); } @@ -487,7 +487,7 @@ void SkGpuDevice::drawRRect(const SkDraw& draw, const SkRRect& rect, GrPaint grPaint; SkPaint2GrPaintShader(this->context(), paint, true, &grPaint); - + GrStrokeInfo strokeInfo(paint); if (paint.getMaskFilter()) { // try to hit the fast path for drawing filtered round rects @@ -536,7 +536,7 @@ void SkGpuDevice::drawRRect(const SkDraw& draw, const SkRRect& rect, this->drawPath(draw, path, paint, NULL, true); return; } - + fContext->drawRRect(grPaint, rect, strokeInfo); } @@ -1945,7 +1945,7 @@ bool SkGpuDevice::EXPERIMENTAL_drawPicture(SkCanvas* canvas, const SkPicture* pi const GPUAccelData::SaveLayerInfo& info = gpuData->saveLayerInfo(i); - if (NULL != picture->fPlayback) { + if (NULL != picture->fPlayback.get()) { SkPicturePlayback::PlaybackReplacements::ReplacementInfo* layerInfo = replacements.push(); layerInfo->fStart = info.fSaveLayerOpID; diff --git a/tools/bench_playback.cpp b/tools/bench_playback.cpp index 6ebe19d36f..5343191974 100644 --- a/tools/bench_playback.cpp +++ b/tools/bench_playback.cpp @@ -5,6 +5,7 @@ * found in the LICENSE file. */ +#include "SkCanvas.h" #include "SkCommandLineFlags.h" #include "SkForceLinking.h" #include "SkGraphics.h" @@ -14,8 +15,6 @@ #include "SkStream.h" #include "SkString.h" -#include "../include/record/SkRecording.h" - #include "Stats.h" #include "Timer.h" @@ -37,7 +36,7 @@ static double timescale() { return 1; } -static SkPicture* rerecord_with_tilegrid(SkPicture& src) { +static SkPicture* rerecord(const SkPicture& src, bool skr) { SkTileGridFactory::TileGridInfo info; info.fTileInterval.set(FLAGS_tile, FLAGS_tile); info.fMargin.setEmpty(); @@ -45,27 +44,13 @@ static SkPicture* rerecord_with_tilegrid(SkPicture& src) { SkTileGridFactory factory(info); SkPictureRecorder recorder; - src.draw(recorder.beginRecording(src.width(), src.height(), &factory)); + src.draw(skr ? recorder.EXPERIMENTAL_beginRecording(src.width(), src.height(), &factory) + : recorder. beginRecording(src.width(), src.height(), &factory)); return recorder.endRecording(); } -static EXPERIMENTAL::SkPlayback* rerecord_with_skr(SkPicture& src) { - EXPERIMENTAL::SkRecording recording(src.width(), src.height()); - src.draw(recording.canvas()); - return recording.releasePlayback(); -} - -static void draw(const EXPERIMENTAL::SkPlayback& skr, const SkPicture& skp, SkCanvas* canvas) { - if (FLAGS_skr) { - skr.draw(canvas); - } else { - skp.draw(canvas); - } -} - -static void bench(SkPMColor* scratch, SkPicture& src, const char* name) { - SkAutoTUnref picture(rerecord_with_tilegrid(src)); - SkAutoTDelete record(rerecord_with_skr(src)); +static void bench(SkPMColor* scratch, const SkPicture& src, const char* name) { + SkAutoTUnref picture(rerecord(src, FLAGS_skr)); SkAutoTDelete canvas(SkCanvas::NewRasterDirectN32(src.width(), src.height(), @@ -74,7 +59,7 @@ static void bench(SkPMColor* scratch, SkPicture& src, const char* name) { canvas->clipRect(SkRect::MakeWH(SkIntToScalar(FLAGS_tile), SkIntToScalar(FLAGS_tile))); // Draw once to warm any caches. The first sample otherwise can be very noisy. - draw(*record, *picture, canvas.get()); + picture->draw(canvas.get()); WallTimer timer; const double scale = timescale(); @@ -83,7 +68,7 @@ static void bench(SkPMColor* scratch, SkPicture& src, const char* name) { // We assume timer overhead (typically, ~30ns) is insignificant // compared to draw runtime (at least ~100us, usually several ms). timer.start(); - draw(*record, *picture, canvas.get()); + picture->draw(canvas.get()); timer.end(); samples[i] = timer.fWall * scale; } @@ -129,7 +114,7 @@ int tool_main(int argc, char** argv) { failed = true; continue; } - SkAutoTUnref src(SkPicture::CreateFromStream(stream)); + SkAutoTUnref src(SkPicture::CreateFromStream(stream)); if (!src) { SkDebugf("Could not read %s as an SkPicture.\n", path.c_str()); failed = true;