From 887f3979f0c717e69a8b7d169169bc27eb46d3b5 Mon Sep 17 00:00:00 2001 From: mtklein Date: Tue, 17 Jun 2014 12:08:15 -0700 Subject: [PATCH] Add EXPERIMENTAL_beginRecording() for SkRecord-based recording. The interesting stuff is in SkPictureRecorder.{h,cpp}. The rest is mostly moving SkRecord from its own directories into core to avoid circular dependencies in GYP. After plumbing SkRecord all the way through in Picture, I'll delete its old entry point include/record/SkRecording.h. For now it and record.gypi need to stay where they are to keep Chrome building. BUG=skia: R=reed@google.com, mtklein@google.com Author: mtklein@chromium.org Review URL: https://codereview.chromium.org/331573004 --- gyp/core.gyp | 3 +- gyp/core.gypi | 4 ++ gyp/dm.gyp | 1 - gyp/record.gyp | 19 ------ gyp/record.gypi | 11 +--- gyp/tests.gypi | 2 - gyp/tools.gyp | 5 -- include/core/SkPictureRecorder.h | 18 +++++- src/core/SkPictureRecorder.cpp | 82 ++++++++++++++++++-------- src/{record => core}/SkRecord.h | 0 src/{record => core}/SkRecordDraw.cpp | 0 src/{record => core}/SkRecordDraw.h | 0 src/{record => core}/SkRecordOpts.cpp | 0 src/{record => core}/SkRecordOpts.h | 0 src/{record => core}/SkRecordPattern.h | 0 src/{record => core}/SkRecorder.cpp | 0 src/{record => core}/SkRecorder.h | 0 src/{record => core}/SkRecording.cpp | 2 +- src/{record => core}/SkRecords.h | 0 tests/RecordingTest.cpp | 2 +- tools/bench_playback.cpp | 3 +- tools/bench_record.cpp | 10 +--- 22 files changed, 86 insertions(+), 76 deletions(-) delete mode 100644 gyp/record.gyp rename src/{record => core}/SkRecord.h (100%) rename src/{record => core}/SkRecordDraw.cpp (100%) rename src/{record => core}/SkRecordDraw.h (100%) rename src/{record => core}/SkRecordOpts.cpp (100%) rename src/{record => core}/SkRecordOpts.h (100%) rename src/{record => core}/SkRecordPattern.h (100%) rename src/{record => core}/SkRecorder.cpp (100%) rename src/{record => core}/SkRecorder.h (100%) rename src/{record => core}/SkRecording.cpp (95%) rename src/{record => core}/SkRecords.h (100%) diff --git a/gyp/core.gyp b/gyp/core.gyp index 8c5270e691..b4f05796bd 100644 --- a/gyp/core.gyp +++ b/gyp/core.gyp @@ -21,8 +21,9 @@ '../include/utils', '../include/xml', '../src/core', - '../src/opts', '../src/image', + '../src/opts', + '../src/utils', ], 'sources': [ 'core.gypi', # Makes the gypi appear in IDEs (but does not modify the build). diff --git a/gyp/core.gypi b/gyp/core.gypi index e14fba742d..e1bdbd5289 100644 --- a/gyp/core.gypi +++ b/gyp/core.gypi @@ -151,6 +151,10 @@ '<(skia_src_path)/core/SkRasterClip.cpp', '<(skia_src_path)/core/SkRasterizer.cpp', '<(skia_src_path)/core/SkReadBuffer.cpp', + '<(skia_src_path)/core/SkRecordDraw.cpp', + '<(skia_src_path)/core/SkRecordOpts.cpp', + '<(skia_src_path)/core/SkRecorder.cpp', + '<(skia_src_path)/core/SkRecording.cpp', '<(skia_src_path)/core/SkRect.cpp', '<(skia_src_path)/core/SkRefDict.cpp', '<(skia_src_path)/core/SkRegion.cpp', diff --git a/gyp/dm.gyp b/gyp/dm.gyp index 26d14db1ac..a849e5433c 100644 --- a/gyp/dm.gyp +++ b/gyp/dm.gyp @@ -59,7 +59,6 @@ 'flags.gyp:flags', 'jsoncpp.gyp:jsoncpp', 'gputest.gyp:skgputest', - 'record.gyp:*', 'etc1.gyp:libetc1', ], 'conditions': [ diff --git a/gyp/record.gyp b/gyp/record.gyp deleted file mode 100644 index 5fa7f818cd..0000000000 --- a/gyp/record.gyp +++ /dev/null @@ -1,19 +0,0 @@ -# An experimental library for faster recording of SkCanvas commands. -{ - 'targets': [{ - 'target_name': 'record', - 'type': 'static_library', - 'includes': [ 'record.gypi' ], - 'include_dirs': [ - '../include/config', - '../include/core', - '../include/record', - '../src/utils', - ], - 'direct_dependent_settings': { - 'include_dirs': [ - '../include/record', # Public headers. - ], - }, - }] -} diff --git a/gyp/record.gypi b/gyp/record.gypi index adfd4623ae..9415a57547 100644 --- a/gyp/record.gypi +++ b/gyp/record.gypi @@ -1,12 +1,3 @@ -# Source list for SkRecord -# The parent gyp/gypi file must define -# 'skia_src_path' e.g. skia/trunk/src -# The Skia build defines this in common_variables.gypi. +# TODO: cleanup references in Chrome build and remove { - 'sources': [ - '<(skia_src_path)/record/SkRecordDraw.cpp', - '<(skia_src_path)/record/SkRecordOpts.cpp', - '<(skia_src_path)/record/SkRecorder.cpp', - '<(skia_src_path)/record/SkRecording.cpp', - ] } diff --git a/gyp/tests.gypi b/gyp/tests.gypi index cf56d53d6e..556143f279 100644 --- a/gyp/tests.gypi +++ b/gyp/tests.gypi @@ -9,7 +9,6 @@ '../src/pathops', '../src/pdf', '../src/pipe/utils', - '../src/record', '../src/utils', '../src/utils/debugger', '../tools/', @@ -24,7 +23,6 @@ 'flags.gyp:flags', 'pdf.gyp:pdf', 'tools.gyp:picture_utils', - 'record.gyp:record', ], 'sources': [ '../tests/Test.cpp', diff --git a/gyp/tools.gyp b/gyp/tools.gyp index b72b2099f4..9e1cf05558 100644 --- a/gyp/tools.gyp +++ b/gyp/tools.gyp @@ -324,7 +324,6 @@ 'bench.gyp:bench_timer', 'flags.gyp:flags', 'skia_lib.gyp:skia_lib', - 'record.gyp:*', ], }, { @@ -336,13 +335,11 @@ 'include_dirs': [ '../src/core/', '../src/images', - '../src/record', ], 'dependencies': [ 'bench.gyp:bench_timer', 'flags.gyp:flags', 'skia_lib.gyp:skia_lib', - 'record.gyp:*', ], }, { @@ -357,12 +354,10 @@ '../src/core/', '../src/images', '../src/lazy', - '../src/record', ], 'dependencies': [ 'bench.gyp:bench_timer', 'flags.gyp:flags', - 'record.gyp:*', 'skia_lib.gyp:skia_lib', ], }, diff --git a/include/core/SkPictureRecorder.h b/include/core/SkPictureRecorder.h index 8d29ff36da..b13b8a809b 100644 --- a/include/core/SkPictureRecorder.h +++ b/include/core/SkPictureRecorder.h @@ -14,10 +14,12 @@ class SkCanvas; class SkPictureRecord; +class SkRecord; +class SkRecorder; class SK_API SkPictureRecorder : SkNoncopyable { public: - SkPictureRecorder() : fCanvas(NULL) { } + SkPictureRecorder() : fPictureRecord(NULL), fRecorder(NULL), fRecord(NULL) { } ~SkPictureRecorder(); /** Returns the canvas that records the drawing commands. @@ -33,6 +35,10 @@ public: SkBBHFactory* bbhFactory = NULL, uint32_t recordFlags = 0); + /** Same as beginRecording(), using a new faster backend. */ + SkCanvas* EXPERIMENTAL_beginRecording(int width, int height, + SkBBHFactory* bbhFactory = NULL); + /** Returns the recording canvas if one is active, or NULL if recording is not active. This does not alter the refcnt on the canvas (if present). */ @@ -54,6 +60,8 @@ public: void internalOnly_EnableOpts(bool enableOpts); private: + void reset(); + /** Replay the current (partially recorded) operation stream into canvas. This call doesn't close the current recording. */ @@ -63,7 +71,13 @@ private: int fWidth; int fHeight; - SkPictureRecord* fCanvas; // ref counted + + // Both ref counted. One of these two will be non-null: + SkPictureRecord* fPictureRecord; // beginRecording() + SkRecorder* fRecorder; // EXPERIMENTAL_beginRecording() + + // Not refcounted. Used by EXPERIMENTAL_beginRecording(). + SkRecord* fRecord; typedef SkNoncopyable INHERITED; }; diff --git a/src/core/SkPictureRecorder.cpp b/src/core/SkPictureRecorder.cpp index 3abd08e1b0..f1423e3992 100644 --- a/src/core/SkPictureRecorder.cpp +++ b/src/core/SkPictureRecorder.cpp @@ -9,17 +9,26 @@ #include "SkPicturePlayback.h" #include "SkPictureRecord.h" #include "SkPictureRecorder.h" -#include "SkTypes.h" +#include "SkRecord.h" +#include "SkRecordDraw.h" +#include "SkRecorder.h" +#include "SkTypes.h" SkPictureRecorder::~SkPictureRecorder() { - SkSafeSetNull(fCanvas); + this->reset(); +} + +void SkPictureRecorder::reset() { + SkSafeSetNull(fPictureRecord); + SkSafeSetNull(fRecorder); + SkDELETE(fRecord); + fRecord = NULL; } SkCanvas* SkPictureRecorder::beginRecording(int width, int height, SkBBHFactory* bbhFactory /* = NULL */, uint32_t recordFlags /* = 0 */) { - SkSafeSetNull(fCanvas); // terminate any prior recording(s) - + this->reset(); // terminate any prior recording(s) fWidth = width; fHeight = height; @@ -28,49 +37,70 @@ SkCanvas* SkPictureRecorder::beginRecording(int width, int height, if (NULL != bbhFactory) { SkAutoTUnref tree((*bbhFactory)(width, height)); SkASSERT(NULL != tree); - fCanvas = SkNEW_ARGS(SkBBoxHierarchyRecord, (size, recordFlags, tree.get())); + fPictureRecord = SkNEW_ARGS(SkBBoxHierarchyRecord, (size, recordFlags, tree.get())); } else { - fCanvas = SkNEW_ARGS(SkPictureRecord, (size, recordFlags)); + fPictureRecord = SkNEW_ARGS(SkPictureRecord, (size, recordFlags)); } - fCanvas->beginRecording(); + fPictureRecord->beginRecording(); + return this->getRecordingCanvas(); +} + +SkCanvas* SkPictureRecorder::EXPERIMENTAL_beginRecording(int width, int height, + SkBBHFactory* bbhFactory /* = NULL */) { + this->reset(); + fWidth = width; + fHeight = height; - return fCanvas; + // TODO: plumb bbhFactory through + fRecord = SkNEW(SkRecord); + fRecorder = SkNEW_ARGS(SkRecorder, (fRecord, width, height)); + return this->getRecordingCanvas(); } SkCanvas* SkPictureRecorder::getRecordingCanvas() { - return fCanvas; + if (NULL != fRecorder) { + return fRecorder; + } + return fPictureRecord; } SkPicture* SkPictureRecorder::endRecording() { - if (NULL == fCanvas) { - return NULL; - } + SkPicture* picture = NULL; - fCanvas->endRecording(); + if (NULL != fRecorder) { + // TODO: picture = SkNEW_ARGS(SkPicture, (fWidth, fHeight, fRecord)); + // fRecord = NULL; + } - const bool deepCopyOps = false; - SkAutoTUnref picture(SkNEW_ARGS(SkPicture, (fWidth, fHeight, - *fCanvas, deepCopyOps))); - SkSafeSetNull(fCanvas); + if (NULL != fPictureRecord) { + fPictureRecord->endRecording(); + const bool deepCopyOps = false; + picture = SkNEW_ARGS(SkPicture, (fWidth, fHeight, *fPictureRecord, deepCopyOps)); + } - return picture.detach(); + this->reset(); + return picture; } void SkPictureRecorder::internalOnly_EnableOpts(bool enableOpts) { - if (NULL != fCanvas) { - fCanvas->internalOnly_EnableOpts(enableOpts); + if (NULL != fPictureRecord) { + fPictureRecord->internalOnly_EnableOpts(enableOpts); } } void SkPictureRecorder::partialReplay(SkCanvas* canvas) const { - if (NULL == fCanvas || NULL == canvas) { - // Not recording or nothing to replay into + if (NULL == canvas) { return; } - const bool deepCopyOps = true; - SkAutoTUnref picture(SkNEW_ARGS(SkPicture, (fWidth, fHeight, - *fCanvas, deepCopyOps))); - picture->draw(canvas); + if (NULL != fRecorder) { + SkRecordDraw(*fRecord, canvas); + } + + if (NULL != fPictureRecord) { + const bool deepCopyOps = true; + SkPicture picture(fWidth, fHeight, *fPictureRecord, deepCopyOps); + picture.draw(canvas); + } } diff --git a/src/record/SkRecord.h b/src/core/SkRecord.h similarity index 100% rename from src/record/SkRecord.h rename to src/core/SkRecord.h diff --git a/src/record/SkRecordDraw.cpp b/src/core/SkRecordDraw.cpp similarity index 100% rename from src/record/SkRecordDraw.cpp rename to src/core/SkRecordDraw.cpp diff --git a/src/record/SkRecordDraw.h b/src/core/SkRecordDraw.h similarity index 100% rename from src/record/SkRecordDraw.h rename to src/core/SkRecordDraw.h diff --git a/src/record/SkRecordOpts.cpp b/src/core/SkRecordOpts.cpp similarity index 100% rename from src/record/SkRecordOpts.cpp rename to src/core/SkRecordOpts.cpp diff --git a/src/record/SkRecordOpts.h b/src/core/SkRecordOpts.h similarity index 100% rename from src/record/SkRecordOpts.h rename to src/core/SkRecordOpts.h diff --git a/src/record/SkRecordPattern.h b/src/core/SkRecordPattern.h similarity index 100% rename from src/record/SkRecordPattern.h rename to src/core/SkRecordPattern.h diff --git a/src/record/SkRecorder.cpp b/src/core/SkRecorder.cpp similarity index 100% rename from src/record/SkRecorder.cpp rename to src/core/SkRecorder.cpp diff --git a/src/record/SkRecorder.h b/src/core/SkRecorder.h similarity index 100% rename from src/record/SkRecorder.h rename to src/core/SkRecorder.h diff --git a/src/record/SkRecording.cpp b/src/core/SkRecording.cpp similarity index 95% rename from src/record/SkRecording.cpp rename to src/core/SkRecording.cpp index ad57aa25a5..94fabce912 100644 --- a/src/record/SkRecording.cpp +++ b/src/core/SkRecording.cpp @@ -5,7 +5,7 @@ * found in the LICENSE file. */ -#include "SkRecording.h" +#include "../../include/record/SkRecording.h" #include "SkRecord.h" #include "SkRecordOpts.h" diff --git a/src/record/SkRecords.h b/src/core/SkRecords.h similarity index 100% rename from src/record/SkRecords.h rename to src/core/SkRecords.h diff --git a/tests/RecordingTest.cpp b/tests/RecordingTest.cpp index fbe8448359..8de52da7ec 100644 --- a/tests/RecordingTest.cpp +++ b/tests/RecordingTest.cpp @@ -7,7 +7,7 @@ #include "Test.h" -#include "SkRecording.h" +#include "../include/record/SkRecording.h" // Minimally exercise the public SkRecording API. diff --git a/tools/bench_playback.cpp b/tools/bench_playback.cpp index f07fa8e300..26fa1c7ee8 100644 --- a/tools/bench_playback.cpp +++ b/tools/bench_playback.cpp @@ -11,10 +11,11 @@ #include "SkOSFile.h" #include "SkPicture.h" #include "SkPictureRecorder.h" -#include "SkRecording.h" #include "SkStream.h" #include "SkString.h" +#include "../include/record/SkRecording.h" + #include "BenchTimer.h" #include "Stats.h" diff --git a/tools/bench_record.cpp b/tools/bench_record.cpp index a8d7a8a0e5..0024c2ccdb 100644 --- a/tools/bench_record.cpp +++ b/tools/bench_record.cpp @@ -11,7 +11,6 @@ #include "SkOSFile.h" #include "SkPicture.h" #include "SkPictureRecorder.h" -#include "SkRecording.h" #include "SkStream.h" #include "SkString.h" @@ -65,16 +64,13 @@ static SkBBHFactory* parse_FLAGS_bbh() { } static void rerecord(const SkPicture& src, SkBBHFactory* bbhFactory) { + SkPictureRecorder recorder; if (FLAGS_skr) { - EXPERIMENTAL::SkRecording recording(src.width(), src.height()); - src.draw(recording.canvas()); - // Release and delete the SkPlayback so that recording optimizes its SkRecord. - SkDELETE(recording.releasePlayback()); + src.draw(recorder.EXPERIMENTAL_beginRecording(src.width(), src.height(), bbhFactory)); } else { - SkPictureRecorder recorder; src.draw(recorder.beginRecording(src.width(), src.height(), bbhFactory)); - SkAutoTUnref dst(recorder.endRecording()); } + SkAutoTUnref pic(recorder.endRecording()); } static void bench_record(const SkPicture& src, -- 2.34.1