diff options
author | 2014-09-15 06:00:49 -0700 | |
---|---|---|
committer | 2014-09-15 06:00:49 -0700 | |
commit | 5087b2c06762d0ea8b471f63953a587ecafaae0f (patch) | |
tree | 55631074f2966ace9ab8e89338c383df1c12d9ea /src/utils/SkDeferredCanvas.cpp | |
parent | e6cb48382db869f8e194b83aec80fd495be76db7 (diff) |
Revert of Picture Recording: fix the performance bottleneck in SkDeferredCanvas::isFullFrame (patchset #7 id:140001 of https://codereview.chromium.org/545813002/)
Reason for revert:
This is leaking memory:
http://108.170.220.120:10117/builders/Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug-ASAN/builds/2516/steps/RunDM/logs/stdio
Original issue's description:
> 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, yunchao.he@intel.com
TBR=junov@chromium.org, reed@google.com, tomhudson@google.com, yunchao.he@intel.com
NOTREECHECKS=true
NOTRY=true
BUG=411166
Author: mtklein@google.com
Review URL: https://codereview.chromium.org/571053002
Diffstat (limited to 'src/utils/SkDeferredCanvas.cpp')
-rw-r--r-- | src/utils/SkDeferredCanvas.cpp | 13 |
1 files changed, 1 insertions, 12 deletions
diff --git a/src/utils/SkDeferredCanvas.cpp b/src/utils/SkDeferredCanvas.cpp index cce5dde538..b46e92a4a0 100644 --- a/src/utils/SkDeferredCanvas.cpp +++ b/src/utils/SkDeferredCanvas.cpp @@ -526,8 +526,6 @@ 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) { @@ -591,14 +589,6 @@ 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(); } @@ -619,7 +609,6 @@ 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; } @@ -643,7 +632,7 @@ SkImage* SkDeferredCanvas::newImageSnapshot() { bool SkDeferredCanvas::isFullFrame(const SkRect* rect, const SkPaint* paint) const { SkCanvas* canvas = this->drawingCanvas(); - SkISize canvasSize = this->getCanvasSize(); + SkISize canvasSize = this->getDeviceSize(); if (rect) { if (!canvas->getTotalMatrix().rectStaysRect()) { return false; // conservative |