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
Committed: https://skia.googlesource.com/skia/+/8e45c3777d886ba3fe239bb549d06b0693692152
R=junov@chromium.org, tomhudson@google.com, reed@google.com
Author: yunchao.he@intel.com
Review URL: https://codereview.chromium.org/545813002
diff --git a/include/utils/SkDeferredCanvas.h b/include/utils/SkDeferredCanvas.h
index 907cb91..5f781f8 100644
--- a/include/utils/SkDeferredCanvas.h
+++ b/include/utils/SkDeferredCanvas.h
@@ -89,6 +89,11 @@
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 @@
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 b46e92a..cce5dde 100644
--- a/src/utils/SkDeferredCanvas.cpp
+++ b/src/utils/SkDeferredCanvas.cpp
@@ -526,6 +526,8 @@
void SkDeferredCanvas::init() {
fBitmapSizeThreshold = kDeferredCanvasBitmapSizeThreshold;
fDeferredDrawing = true; // On by default
+ fCachedCanvasSize.setEmpty();
+ fCachedCanvasSizeDirty = true;
}
void SkDeferredCanvas::setMaxRecordingStorage(size_t maxStorage) {
@@ -589,6 +591,14 @@
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 @@
// 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 @@
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 61af550..2938326 100644
--- a/tests/DeferredCanvasTest.cpp
+++ b/tests/DeferredCanvasTest.cpp
@@ -839,6 +839,59 @@
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<SkSurface> surface(createSurface(0xFFFFFFFF));
+ SkAutoTUnref<SkDeferredCanvas> canvas(SkDeferredCanvas::Create(surface.get()));
+ SkSurface* newSurface = SkSurface::NewRasterPMColor(4, 4);
+ SkAutoTUnref<SkSurface> aur(newSurface);
+
+ for (int i = 0; i < 2; ++i) {
+ if (i == 1) {
+ 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 +903,7 @@
TestDeferredCanvasBitmapSizeThreshold(reporter);
TestDeferredCanvasCreateCompatibleDevice(reporter);
TestDeferredCanvasWritePixelsToSurface(reporter);
+ TestDeferredCanvasGetCanvasSize(reporter);
TestDeferredCanvasSurface(reporter, NULL);
TestDeferredCanvasSetSurface(reporter, NULL);
}
