New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 668925 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature



Sign in to add a comment

SkNWayCanvas should use kConservativeRasterClip_InitFlag

Project Member Reported by vmi...@chromium.org, Nov 28 2016

Issue description

As SkNWayCanvas is a pass-through canvas, I think it should use the kConservativeRasterClip_InitFlag to disable fine clip rasterization.

We use SkNWayCanvas in a number of places, collecting images in display lists, and replacing images.

On nytimes.com, computing the fine raster is ~50% of time spent in cc::DisplayItemList::GenerateDiscardableImagesMetadata(), and ~5% of our raster thread time.

cc::DisplayItemList::GenerateDiscardableImagesMetadata() at: nytimes_repaint.stacks_27112016.html#{"id":5927,"stack":[0],"merged":false,"ignore":[],"hide":[]}

 
nytimes_repaint.stacks_27112016.html
4.0 MB View Download

Comment 1 by vmi...@chromium.org, Nov 28 2016

Cc: ericrk@chromium.org
Components: Internals>Skia Internals>Compositing>Rasterization
Testing this change gives some good improvements on MotionMark, especially on the clip heavy tests.

Animometer Score   572.41    630.85  +10.21%
Multiply           925.00    925.00    ---
Canvas arcs        572.25    625.48  +9.30%
Leaves           1,122.23  1,354.98  +20.74%
Paths            2,052.96  2,363.54  +15.13%
Canvas Lines     9,471.95  9,668.96  +2.08%
Focus              105.59    112.73  +6.76%
Images              55.21     77.92  +41.13%
Design             173.02    174.54  +0.88%
Suits              566.27    576.16  +1.75%

HTML suite                       378.59    520.59  +37.51%
CSS bouncing circles           1,415.57  1,421.41  +0.41%
CSS bouncing clipped rects       309.41    350.41  +13.25%
CSS bouncing gradient circles  1,153.06  1,189.65  +3.17%
CSS bouncing blend circles       223.53    227.07  +1.58%
CSS bouncing filter circles      164.24    173.56  +5.67%
CSS bouncing SVG images           55.10    401.72  +629.07%
CSS bouncing tagged images       992.45  1,138.34  +14.70%
Leaves 2.0                       906.44    996.70  +9.96%
DOM particles, SVG masks         110.77    263.86  +138.21%
Composited Transforms            594.24    594.81  +0.10%

SVG suite                        617.62    968.67  +56.84%
SVG bouncing circles           3,805.45  3,924.35  +3.12%
SVG bouncing clipped rects       296.04    345.95  +16.86%
SVG bouncing gradient circles  2,115.92  2,145.08  +1.38%
SVG bouncing SVG images           52.53    409.61  +679.76%
SVG bouncing PNG images          717.71    718.97  +0.18%

Comment 2 by bsalo...@google.com, Nov 28 2016

Cc: reed@google.com
I agree that NWay should use the conservative flag.

Comment 3 by vmi...@chromium.org, Nov 28 2016

Our AnalysisCanvas should also use the conservative flag.

Is it possible to expose the flags outside of Skia, or make the conservative clip default SkCanvas behavior and change SkBitmapCanvas to overwrite it?

Comment 4 by vmi...@chromium.org, Nov 29 2016

Cc: chrishtr@chromium.org
fyi
Cc: mtklein@chromium.org
Owner: mtklein@chromium.org
I struggle to have an opinion on making this public, but I do agree NWay should use this and it's super easy.  CL incoming for that.

Comment 8 by vmi...@chromium.org, Nov 30 2016

Perhaps AnalysisCanvas should also subclass SkNWayCanvas to get this.

There's also the lonely and private SkNoSaveLayerCanvas.
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9491858d66bfa61bb5d4641ea8df6df9fdae5a0

commit f9491858d66bfa61bb5d4641ea8df6df9fdae5a0
Author: skia-deps-roller <skia-deps-roller@chromium.org>
Date: Wed Nov 30 08:57:33 2016

Roll src/third_party/skia/ 56b507925..84aa30d39 (24 commits).

https://skia.googlesource.com/skia.git/+log/56b50792531c..84aa30d39e7e

$ git log 56b507925..84aa30d39 --date=short --no-merges --format='%ad %ae %s'
2016-11-29 liyuqian Add additional guard to the Analytic AA change
2016-11-29 bsalomon Make pipeline getter a GrDrawBatch::pipeline() a protected method.
2016-11-29 msarett Fixes for SkColorLookUpTable::interp3D
2016-11-29 mtklein support a8
2016-11-20 fmalita Fuzzer assert in GradientShaderBase4fContext::TSampler
2016-11-29 bsalomon Remove caps image storage caps hack.
2016-11-29 bsalomon Rename SkSL::GLSLCapsFactory to SkSL::ShaderCapsFactory
2016-11-29 mtklein Make SkNWayCanvas use conservative clipping.
2016-11-29 brianosman In GetResourceAsBitmap, don't crash if the resource is missing
2016-11-29 liyuqian Add the missing shift to the dy
2016-11-29 bsalomon Rename vars and functions from 'glslcaps'->'shadercaps'
2016-11-29 mtklein teach MSAN about maskload
2016-11-29 bsalomon Merge GrGLSLCaps into GrShaderCaps
2016-11-28 msarett Delete unnecessary SkSurface_Base API
2016-11-28 msarett Remove duplicate storage of fCanvas in SkOverdrawCanvas
2016-11-29 mtklein gather_i8
2016-11-29 kjlubick Fix fuzzRange
2016-11-29 bsalomon Rm assert that image texture array is null unless GrCaps has images support.
2016-11-29 liyuqian Use AdditiveBlitter for partial rows
2016-11-28 jvanverth Add #define for Nsight compatibility
2016-11-29 jcgregorio Revert "Use /MD for Windows builds."
2016-11-29 liyuqian Compute slope using fSnappedY
2016-11-29 kjlubick Fix DrawFunctions fuzzer to initialize bitmaps
2016-11-28 bsalomon Fix documents for creating a GPU surface.

BUG= 668925 , 668907 

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
TBR=brianosman@google.com

Review-Url: https://codereview.chromium.org/2534343002
Cr-Commit-Position: refs/heads/master@{#435183}

[modify] https://crrev.com/f9491858d66bfa61bb5d4641ea8df6df9fdae5a0/DEPS

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 1 2016

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/ee424acbb87bf999be132a437527e656e37541d7

commit ee424acbb87bf999be132a437527e656e37541d7
Author: Florin Malita <fmalita@chromium.org>
Date: Thu Dec 01 17:47:59 2016

SkNoDrawCanvas - a public non-drawing canvas base class

TODO:

  - convert SkDeferredCanvas, SkLiteRecorder, etc. to the new base
  - remove unused SkNoSaveLayerCanvas

BUG= chromium:668925 
R=reed@google.com,mtklein@google.com

Change-Id: Ie9af577477a6b9eaa5ef55523287ad1635dca116
Reviewed-on: https://skia-review.googlesource.com/5349
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>

[modify] https://crrev.com/ee424acbb87bf999be132a437527e656e37541d7/gn/utils.gni
[modify] https://crrev.com/ee424acbb87bf999be132a437527e656e37541d7/include/core/SkCanvas.h
[add] https://crrev.com/ee424acbb87bf999be132a437527e656e37541d7/include/utils/SkNoDrawCanvas.h
[modify] https://crrev.com/ee424acbb87bf999be132a437527e656e37541d7/include/utils/SkNoSaveLayerCanvas.h
[modify] https://crrev.com/ee424acbb87bf999be132a437527e656e37541d7/src/core/SkCanvas.cpp

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 1 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b8f77623038030d4997e21048ae009a1e3811dd

commit 7b8f77623038030d4997e21048ae009a1e3811dd
Author: skia-deps-roller <skia-deps-roller@chromium.org>
Date: Thu Dec 01 22:48:31 2016

Roll src/third_party/skia/ 71b762f2a..97aadfce6 (23 commits).

https://skia.googlesource.com/skia.git/+log/71b762f2aca1..97aadfce654c

$ git log 71b762f2a..97aadfce6 --date=short --no-merges --format='%ad %ae %s'
2016-12-01 msarett Update skimage version, test CMYK images
2016-12-01 msarett Revert "SkColorSpaceXform bug fixes"
2016-12-01 mtklein Avoid creating std::function in run_pipeline().
2016-12-01 raftias Added CMYK support for ICC profiles.
2016-12-01 brianosman Revert "Add RasterPipeline implementation for SkColorSpaceXform"
2016-12-01 robertphillips Remove use of makeDeferredRenderTargetContextWithFallback
2016-12-01 brianosman Revert of Enable sRGB on iOS, make sRGB decode support optional (patchset #11 id:200001 of https://codereview.chromium.org/2539993002/ )
2016-12-01 msarett Add RasterPipeline implementation for SkColorSpaceXform
2016-12-01 robertphillips Add animating blur image filter GM/slide/bench
2016-12-01 fmalita SkNoDrawCanvas - a public non-drawing canvas base class
2016-11-29 msarett Add srgb and f16 modes to fiddle
2016-12-01 halcanary third_party/ktx: put WriteBitmapToKTX back
2016-12-01 reed Revert "Revert "Revert "remove (empty) SkXfermode.h"""
2016-12-01 brianosman Two (related) changes here:
2016-12-01 bsalomon Rename GrDrawBatch->GrDrawOp
2016-12-01 mtklein Revert "Added CMYK support for ICC profiles."
2016-11-30 raftias Added CMYK support for ICC profiles.
2016-11-30 mtklein move all memset() logic into blitter
2016-12-01 bsalomon Rename GrBatch to GrOp
2016-12-01 bsalomon Remove pipeline info dump from GrDrawBatch
2016-12-01 reed Revert "Revert "remove (empty) SkXfermode.h""
2016-12-01 halcanary No SkEncodeImageAsKTX for ANDROID_FRAMEWORK
2016-11-30 msarett SkColorSpaceXform bug fixes

BUG= 668179 , 668925 

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
TBR=brianosman@google.com

Review-Url: https://codereview.chromium.org/2545913002
Cr-Commit-Position: refs/heads/master@{#435746}

[modify] https://crrev.com/7b8f77623038030d4997e21048ae009a1e3811dd/DEPS

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ae0797ad640d7b8f55de130790ddc32583f059ab

commit ae0797ad640d7b8f55de130790ddc32583f059ab
Author: fmalita <fmalita@chromium.org>
Date: Fri Dec 02 18:16:41 2016

Ensure conservative raster clipping for AnalysisCanvas

Inherit from SkNoDrawCanvas, which uses conservative clipping.

BUG= 668925 
R=reed@google.com

Review-Url: https://codereview.chromium.org/2548573004
Cr-Commit-Position: refs/heads/master@{#435979}

[modify] https://crrev.com/ae0797ad640d7b8f55de130790ddc32583f059ab/skia/ext/analysis_canvas.cc
[modify] https://crrev.com/ae0797ad640d7b8f55de130790ddc32583f059ab/skia/ext/analysis_canvas.h

Strange, I'm not seeing a performance improvement on ToT.

If I apply the fForceConservativeRects myself at chrome position r431915 then I get a large speedup.  Must bisect...
Please ignore #13 - there's an unrelated performance regression in DiscardableImageMap.
fmalita: Thanks for adding SkNoDrawCanvas.

I ran into a peril with this class: if someone subclasses SkNoDrawCanvas and doesn't override all of the onDraw methods, then SkNoDrawCanvas does draw to a bitmap internally.
Owner: fmalita@chromium.org
Status: Assigned (was: Available)
As initialized, SkNoDrawCanvas is using SkNoPixelsBitmapDevice => a bitmap not backed by any pixels.  But we abort rasterization pretty late, at the blitter level (SkNullBlitter) - so you're probably noticing all the work we do before that.

I think it makes sense to stub out SkNoDrawCanvas with no-op draw overrides, to reduce the overhead in this case.

Project Member

Comment 17 by bugdroid1@chromium.org, Dec 5 2016

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/d8c278af51f0e5b3f98f66320acb827310b50e2c

commit d8c278af51f0e5b3f98f66320acb827310b50e2c
Author: Florin Malita <fmalita@chromium.org>
Date: Sun Dec 04 16:44:52 2016

Add no-op draw overrides to SkNoDrawCanvas

SkNoDrawCanvas is not backed by pixels, but for draw ops not intercepted
by clients we abort rasterization failry late (SkNullBlitter).

BUG= chromium:668925 
R=reed@google.com

Change-Id: I4cd80dbbc262936d33410275051ea0b9c04fbc6c
Reviewed-on: https://skia-review.googlesource.com/5543
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Mike Reed <reed@google.com>

[modify] https://crrev.com/d8c278af51f0e5b3f98f66320acb827310b50e2c/include/utils/SkNoDrawCanvas.h

Project Member

Comment 18 by bugdroid1@chromium.org, Dec 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/37f5bc05d896e5328d3cb5484240e7fc9015963b

commit 37f5bc05d896e5328d3cb5484240e7fc9015963b
Author: skia-deps-roller <skia-deps-roller@chromium.org>
Date: Mon Dec 05 18:03:18 2016

Roll src/third_party/skia/ 55360b11c..91db12d89 (5 commits).

https://skia.googlesource.com/skia.git/+log/55360b11c7da..91db12d89c21

$ git log 55360b11c..91db12d89 --date=short --no-merges --format='%ad %ae %s'
2016-12-02 raftias Color-correct Gray JPEG image decoding via ICC profiles.
2016-12-05 brianosman In DM, do quick test to reject context by type first
2016-12-05 brianosman Make ANGLE SampleApp work again
2016-12-04 fmalita Add no-op draw overrides to SkNoDrawCanvas
2016-12-02 mtklein Manual byte -> float conversion.

BUG= 668925 

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
TBR=herb@google.com

Review-Url: https://codereview.chromium.org/2549073003
Cr-Commit-Position: refs/heads/master@{#436347}

[modify] https://crrev.com/37f5bc05d896e5328d3cb5484240e7fc9015963b/DEPS

Status: Fixed (was: Assigned)

Sign in to add a comment