From 8e45c3777d886ba3fe239bb549d06b0693692152 Mon Sep 17 00:00:00 2001 From: "yunchao.he" Date: Sun, 14 Sep 2014 18:59:04 -0700 Subject: [PATCH] Picture Recording: fix the performance bottleneck in SkDeferredCanvas::isFullFrame blink skips all pending commands during picture recording if it is drawing an opaque full-frame geometry or image. This may improve performance for some edge cases. To recognize an opaque full-frame drawing should be cheap enough. Otherwise, the overhead will offset the improvement. Unfortunately, data from perf for content_shell on Nexus7 shows that SkDeferredCanvas::isFullFrame is far from cheap. Table below shows that how much isFullFrame() costs in the whole render process. benchmark percentage my local benchmark(draw 1000 sprites) 4.1% speedReading 2.8% FishIETank(1000 fishes) 1.5% GUIMark3 Bitmap 2.0% By contrast, real recording (SkGPipeCanvas::drawBitmapRectToRect) and real rasterization (GrDrawTarget::drawRect) cost ~4% and ~6% in the whole render process respectively. Apparently, SkDeferredCanvas::isFullFrame() is nontrivial. getDeviceSize() is the main contributor to this hotspot. The change simply save the canvasSize and reuse it among drawings if it is not a fresh frame. This change cut off ~65% (or improved ~2 times) of isFullFrame(). telemetry smoothness canvas_tough_test didn't show obvious improvement or regression. BUG=411166 R=junov@chromium.org, tomhudson@google.com, reed@google.com Author: yunchao.he@intel.com Review URL: https://codereview.chromium.org/545813002 --- include/utils/SkDeferredCanvas.h | 8 +++++ src/utils/SkDeferredCanvas.cpp | 13 +++++++- tests/DeferredCanvasTest.cpp | 53 ++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/include/utils/SkDeferredCanvas.h b/include/utils/SkDeferredCanvas.h index 907cb91575..5f781f8859 100644 --- a/include/utils/SkDeferredCanvas.h +++ b/include/utils/SkDeferredCanvas.h @@ -88,6 +88,11 @@ public: */ bool isFreshFrame() const; + /** + * Returns canvas's size. + */ + SkISize getCanvasSize() const; + /** * Returns true if the canvas has recorded draw commands that have * not yet been played back. @@ -250,6 +255,9 @@ private: size_t fBitmapSizeThreshold; bool fDeferredDrawing; + mutable SkISize fCachedCanvasSize; + mutable bool fCachedCanvasSizeDirty; + friend class SkDeferredCanvasTester; // for unit testing typedef SkCanvas INHERITED; }; diff --git a/src/utils/SkDeferredCanvas.cpp b/src/utils/SkDeferredCanvas.cpp index b46e92a4a0..cce5dde538 100644 --- a/src/utils/SkDeferredCanvas.cpp +++ b/src/utils/SkDeferredCanvas.cpp @@ -526,6 +526,8 @@ SkDeferredCanvas::SkDeferredCanvas(SkDeferredDevice* device) : SkCanvas (device) void SkDeferredCanvas::init() { fBitmapSizeThreshold = kDeferredCanvasBitmapSizeThreshold; fDeferredDrawing = true; // On by default + fCachedCanvasSize.setEmpty(); + fCachedCanvasSizeDirty = true; } void SkDeferredCanvas::setMaxRecordingStorage(size_t maxStorage) { @@ -589,6 +591,14 @@ bool SkDeferredCanvas::isFreshFrame() const { return this->getDeferredDevice()->isFreshFrame(); } +SkISize SkDeferredCanvas::getCanvasSize() const { + if (fCachedCanvasSizeDirty) { + fCachedCanvasSize = this->getBaseLayerSize(); + fCachedCanvasSizeDirty = false; + } + return fCachedCanvasSize; +} + bool SkDeferredCanvas::hasPendingCommands() const { return this->getDeferredDevice()->hasPendingCommands(); } @@ -609,6 +619,7 @@ SkSurface* SkDeferredCanvas::setSurface(SkSurface* surface) { // all pending commands, which can help to seamlessly recover from // a lost accelerated graphics context. deferredDevice->setSurface(surface); + fCachedCanvasSizeDirty = true; return surface; } @@ -632,7 +643,7 @@ SkImage* SkDeferredCanvas::newImageSnapshot() { bool SkDeferredCanvas::isFullFrame(const SkRect* rect, const SkPaint* paint) const { SkCanvas* canvas = this->drawingCanvas(); - SkISize canvasSize = this->getDeviceSize(); + SkISize canvasSize = this->getCanvasSize(); if (rect) { if (!canvas->getTotalMatrix().rectStaysRect()) { return false; // conservative diff --git a/tests/DeferredCanvasTest.cpp b/tests/DeferredCanvasTest.cpp index 61af550e20..89454256aa 100644 --- a/tests/DeferredCanvasTest.cpp +++ b/tests/DeferredCanvasTest.cpp @@ -839,6 +839,58 @@ static void TestDeferredCanvasCreateCompatibleDevice(skiatest::Reporter* reporte REPORTER_ASSERT(reporter, notificationCounter.fStorageAllocatedChangedCount == 1); } +static void TestDeferredCanvasGetCanvasSize(skiatest::Reporter* reporter) { + SkRect rect; + rect.setXYWH(SkIntToScalar(0), SkIntToScalar(0), SkIntToScalar(gWidth), SkIntToScalar(gHeight)); + SkRect clip; + clip.setXYWH(SkIntToScalar(0), SkIntToScalar(0), SkIntToScalar(1), SkIntToScalar(1)); + + SkPaint paint; + SkISize size = SkISize::Make(gWidth, gHeight); + + SkAutoTUnref surface(createSurface(0xFFFFFFFF)); + SkAutoTUnref canvas(SkDeferredCanvas::Create(surface.get())); + + for (int i = 0; i < 2; ++i) { + if (i == 1) { + SkSurface* newSurface = SkSurface::NewRasterPMColor(4, 4); + canvas->setSurface(newSurface); + size = SkISize::Make(4, 4); + } + + // verify that canvas size is correctly initialized or set + REPORTER_ASSERT(reporter, size == canvas->getCanvasSize()); + + // Verify that clear, clip and draw the canvas will not change its size + canvas->clear(0x00000000); + canvas->clipRect(clip, SkRegion::kIntersect_Op, false); + canvas->drawRect(rect, paint); + REPORTER_ASSERT(reporter, size == canvas->getCanvasSize()); + + // Verify that flush the canvas will not change its size + canvas->flush(); + REPORTER_ASSERT(reporter, size == canvas->getCanvasSize()); + + // Verify that clear canvas with saved state will not change its size + canvas->save(); + canvas->clear(0xFFFFFFFF); + REPORTER_ASSERT(reporter, size == canvas->getCanvasSize()); + + // Verify that restore canvas state will not change its size + canvas->restore(); + REPORTER_ASSERT(reporter, size == canvas->getCanvasSize()); + + // Verify that clear within a layer will not change canvas size + canvas->saveLayer(&clip, &paint); + canvas->clear(0x00000000); + REPORTER_ASSERT(reporter, size == canvas->getCanvasSize()); + + // Verify that restore from a layer will not change canvas size + canvas->restore(); + REPORTER_ASSERT(reporter, size == canvas->getCanvasSize()); + } +} + DEF_TEST(DeferredCanvas_CPU, reporter) { TestDeferredCanvasFlush(reporter); TestDeferredCanvasSilentFlush(reporter); @@ -850,6 +902,7 @@ DEF_TEST(DeferredCanvas_CPU, reporter) { TestDeferredCanvasBitmapSizeThreshold(reporter); TestDeferredCanvasCreateCompatibleDevice(reporter); TestDeferredCanvasWritePixelsToSurface(reporter); + TestDeferredCanvasGetCanvasSize(reporter); TestDeferredCanvasSurface(reporter, NULL); TestDeferredCanvasSetSurface(reporter, NULL); }