Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(625)

Issue 653153004: Remove GraphicsContext's deferred save mechanism. (Closed)

Created:
6 years, 2 months ago by f(malita)
Modified:
6 years, 2 months ago
CC:
blink-reviews, krit, Rik, jbroman, mkwst+moarreviews_chromium.org, danakj, pdr+graphicswatchlist_chromium.org, Stephen Chennney, rwlbuis, reed1, mtklein
Project:
blink
Visibility:
Public.

Description

Remove GraphicsContext's deferred save mechanism. The new Skia recording backend is not as sensitive to the number of draw ops as the old one -> we don't have to bend over backwards to suppress unneeded saves at the GC level. While this results in a slight increase in the number of recorded ops, the unnecessary saves and restores are optimized out by the bounding box hierarchy - so there is no playback/rasterization time penalty (Skia also has an explicit save/restore optimizer which can be used for this purpose if we ever decide to drop the BBH). Local perf measurements show no rasterization impact and a minor (<1%) improvement for record time. R=schenney@chromium.org,senorblanco@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183750

Patch Set 1 #

Patch Set 2 : saveCount() still needed for asserts #

Patch Set 3 : implement saveCount() based on m_paintStateStack #

Patch Set 4 : minor cleanup #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -75 lines) Patch
M Source/platform/graphics/GraphicsContext.h View 1 2 4 chunks +4 lines, -33 lines 0 comments Download
M Source/platform/graphics/GraphicsContext.cpp View 1 2 3 17 chunks +19 lines, -42 lines 0 comments Download

Messages

Total messages: 8 (2 generated)
f(malita)
6 years, 2 months ago (2014-10-15 00:44:46 UTC) #1
Stephen Chennney
Glad to see it go. LGTM
6 years, 2 months ago (2014-10-15 13:46:43 UTC) #2
Stephen White
LGTM too.
6 years, 2 months ago (2014-10-15 13:56:34 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/653153004/60001
6 years, 2 months ago (2014-10-15 13:58:22 UTC) #5
commit-bot: I haz the power
Committed patchset #4 (id:60001) as 183750
6 years, 2 months ago (2014-10-15 14:01:32 UTC) #6
mtklein
6 years, 2 months ago (2014-10-15 14:02:34 UTC) #8
Message was sent while issue was closed.
Neat!

Powered by Google App Engine
This is Rietveld 408576698