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

Issue 2335773002: Revert of SkLiteRecorder: don't tell SkCanvas about clips (Closed)

Created:
4 years, 3 months ago by liyuqian
Modified:
4 years, 3 months ago
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Target Ref:
refs/heads/master
Project:
skia
Visibility:
Public.

Description

Revert of SkLiteRecorder: don't tell SkCanvas about clips (patchset #1 id:1 of https://codereview.chromium.org/2227833004/ ) Reason for revert: This CL breaks Android CTS tests CanvasStateTests#testClipPathReturnValues, CanvasStateTests#testClipRectReturnValues, and CanvasStateTests#testQuickReject. Reverting it won't significantly affect Android's performance. See the following test result of testInvalidateTree Jank test. (Derek said that this is the only test closely related to this CL.) **After reverting** INSTRUMENTATION_STATUS: gfx-max-slow-draw=0.0 INSTRUMENTATION_STATUS: gfx-max-high-input-latency=0.0 INSTRUMENTATION_STATUS: gfx-max-slow-ui-thread=100.0 INSTRUMENTATION_STATUS: gfx-avg-slow-bitmap-uploads=100.0 INSTRUMENTATION_STATUS: gfx-avg-missed-vsync=72.96467105972435 INSTRUMENTATION_STATUS: gfx-avg-high-input-latency=0.0 INSTRUMENTATION_STATUS: gfx-max-slow-bitmap-uploads=100.0 INSTRUMENTATION_STATUS: gfx-max-missed-vsync=73.56181150550796 INSTRUMENTATION_STATUS: gfx-avg-frame-time-90=69.0 INSTRUMENTATION_STATUS: gfx-avg-frame-time-95=69.0 INSTRUMENTATION_STATUS: gfx-avg-frame-time-99=69.0 INSTRUMENTATION_STATUS: gfx-max-jank=100.0 INSTRUMENTATION_STATUS: gfx-avg-slow-draw=0.0 INSTRUMENTATION_STATUS: gfx-avg-slow-ui-thread=100.0 INSTRUMENTATION_STATUS: gfx-max-frame-time-90=69 INSTRUMENTATION_STATUS: gfx-max-frame-time-95=69 INSTRUMENTATION_STATUS: gfx-max-frame-time-99=69 INSTRUMENTATION_STATUS: gfx-avg-jank=100.0 INSTRUMENTATION_STATUS_CODE: -1 . Test results for InstrumentationTestRunner=. Time: 77.404 **Before Reverting** gfx-max-slow-draw=1.1135857461024499 INSTRUMENTATION_STATUS: gfx-max-high-input-latency=0.0 INSTRUMENTATION_STATUS: gfx-max-slow-ui-thread=100.0 INSTRUMENTATION_STATUS: gfx-avg-slow-bitmap-uploads=100.0 INSTRUMENTATION_STATUS: gfx-avg-missed-vsync=70.79687591131983 INSTRUMENTATION_STATUS: gfx-avg-high-input-latency=0.0 INSTRUMENTATION_STATUS: gfx-max-slow-bitmap-uploads=100.0 INSTRUMENTATION_STATUS: gfx-max-missed-vsync=76.94174757281553 INSTRUMENTATION_STATUS: gfx-avg-frame-time-90=67.66666666666667 INSTRUMENTATION_STATUS: gfx-avg-frame-time-95=67.66666666666667 INSTRUMENTATION_STATUS: gfx-avg-frame-time-99=69.0 INSTRUMENTATION_STATUS: gfx-max-jank=100.0 INSTRUMENTATION_STATUS: gfx-avg-slow-draw=0.5317509395832639 INSTRUMENTATION_STATUS: gfx-avg-slow-ui-thread=100.0 INSTRUMENTATION_STATUS: gfx-max-frame-time-90=69 INSTRUMENTATION_STATUS: gfx-max-frame-time-95=69 INSTRUMENTATION_STATUS: gfx-max-frame-time-99=69 INSTRUMENTATION_STATUS: gfx-avg-jank=100.0 INSTRUMENTATION_STATUS_CODE: -1 . Test results for InstrumentationTestRunner=. Time: 77.411 OK (1 test) Original issue's description: > SkLiteRecorder: don't tell SkCanvas about clips > > If no one reads our clip, and we don't expect quickReject() to help, > we can probably get away without maintaining a proper clip stack. > > This puts us at about 0.6-0.7x of previous record cost. > > BUG=skia: > GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2227833004 > > Committed: https://skia.googlesource.com/skia/+/8369e32a05543c2619b1d7e740fe6ff6d2f03af3 TBR=reed@google.com,mtklein@google.com,mtklein@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=skia: Committed: https://skia.googlesource.com/skia/+/91c4271310f11c29d008e8bd92446170308cf3b4

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -0 lines) Patch
M src/core/SkLiteRecorder.cpp View 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (5 generated)
liyuqian
Created Revert of SkLiteRecorder: don't tell SkCanvas about clips
4 years, 3 months ago (2016-09-12 19:10:45 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2335773002/1
4 years, 3 months ago (2016-09-12 19:10:50 UTC) #3
commit-bot: I haz the power
4 years, 3 months ago (2016-09-12 20:00:52 UTC) #8
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://skia.googlesource.com/skia/+/91c4271310f11c29d008e8bd92446170308cf3b4

Powered by Google App Engine
This is Rietveld 408576698