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

Issue 2289363005: Improve usage of window rectangles (Closed)

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

Description

Improve usage of window rectangles * Skips non-AA diff rect elements and replaces them with window rectangles. * Places window rectangles in the interiors of antialiased diff rects. * Arranges two overlapping window rectangles in a plus shape inside of diff rounded rects. * Enables window rectangles when clearing and generating clip masks. GTX 960 perf result (with vs. without window rectangles): glinst4 msaa16 gpu keymobi_pinterest.skp 0.48 -> 0.17 [ 35%] 2.77 -> 1.49 [ 54%] 0.22 -> 0.16 [ 70%] keymobi_digg_com.skp 0.42 -> 0.23 [ 55%] 2.34 -> 1.08 [ 46%] 0.25 -> 0.21 [ 83%] desk_jsfiddlebigcar.skp 0.28 -> 0.16 [ 59%] 1.70 -> 0.96 [ 57%] 0.19 -> 0.14 [ 70%] top25desk_wordpress.skp 0.45 -> 0.18 [ 40%] 2.78 -> 1.53 [ 55%] 0.21 -> 0.19 [ 94%] top25desk_weather_com.skp 2.01 -> 1.93 [ 96%] 23.5 -> 2.54 [ 11%] 1.90 -> 1.68 [ 88%] keymobi_blogger.skp 0.57 -> 0.37 [ 65%] 2.87 -> 1.54 [ 54%] 0.43 -> 0.33 [ 77%] keymobi_linkedin.skp 0.32 -> 0.17 [ 51%] 1.93 -> 1.04 [ 54%] 0.17 -> 0.15 [ 91%] keymobi_bing_com_search_... 0.29 -> 0.25 [ 83%] 1.85 -> 1.23 [ 66%] 0.50 -> 0.24 [ 48%] keymobi_theverge_com_201... 1.00 -> 0.67 [ 68%] 9.46 -> 3.84 [ 41%] 0.72 -> 0.65 [ 90%] keymobi_sfgate_com_.skp 1.56 -> 1.13 [ 72%] 4.49 -> 2.86 [ 64%] 1.54 -> 1.11 [ 72%] ... GEOMEAN (All 79 blink skps) 1.04 -> 0.90 [ 86%] 4.22 -> 2.81 [ 67%] 0.95 -> 0.89 [ 94%] BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2289363005 Committed: https://skia.googlesource.com/skia/+/db42be9a326c747ff92ed1da8c3536c5b3e8e22b Committed: https://skia.googlesource.com/skia/+/bf4a8f90c87dddf6290aa774536715e55e6a12f5

Patch Set 1 #

Patch Set 2 : rebase #

Patch Set 3 : rebase #

Patch Set 4 : Improve usage of window rectangles #

Total comments: 5

Patch Set 5 : comments #

Total comments: 3

Patch Set 6 : Improve usage of window rectangles #

Patch Set 7 : rebase #

Patch Set 8 : #include wrangling #

Patch Set 9 : comments #

Patch Set 10 : rebase #

Patch Set 11 : Improve usage of window rectangles #

Unified diffs Side-by-side diffs Delta from patch set Stats (+688 lines, -192 lines) Patch
A gm/windowrectangles.cpp View 1 2 3 4 5 6 7 8 9 10 1 chunk +300 lines, -0 lines 0 comments Download
M gyp/gpu.gypi View 1 2 chunks +2 lines, -0 lines 0 comments Download
M include/gpu/GrTypesPriv.h View 2 chunks +0 lines, -27 lines 0 comments Download
M include/private/GrSurfaceProxy.h View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M src/gpu/GrAppliedClip.h View 1 2 3 4 5 4 chunks +13 lines, -10 lines 0 comments Download
M src/gpu/GrClipStackClip.cpp View 1 2 3 4 5 6 7 8 9 3 chunks +13 lines, -19 lines 0 comments Download
M src/gpu/GrDrawContext.cpp View 1 2 3 4 5 6 1 chunk +6 lines, -3 lines 0 comments Download
M src/gpu/GrDrawTarget.cpp View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M src/gpu/GrFixedClip.h View 1 2 3 4 5 2 chunks +17 lines, -22 lines 0 comments Download
M src/gpu/GrFixedClip.cpp View 1 2 3 4 5 2 chunks +34 lines, -10 lines 0 comments Download
M src/gpu/GrPipeline.h View 4 chunks +5 lines, -5 lines 0 comments Download
M src/gpu/GrPipeline.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M src/gpu/GrReducedClip.h View 4 chunks +21 lines, -11 lines 0 comments Download
M src/gpu/GrReducedClip.cpp View 1 2 3 4 5 6 7 8 9 10 7 chunks +111 lines, -8 lines 0 comments Download
M src/gpu/GrRenderTarget.cpp View 1 chunk +5 lines, -0 lines 0 comments Download
M src/gpu/GrRenderTargetPriv.h View 1 chunk +1 line, -1 line 0 comments Download
A src/gpu/GrScissorState.h View 1 chunk +40 lines, -0 lines 0 comments Download
M src/gpu/GrWindowRectangles.h View 4 chunks +6 lines, -15 lines 0 comments Download
A src/gpu/GrWindowRectsState.h View 1 2 3 4 5 1 chunk +60 lines, -0 lines 0 comments Download
M src/gpu/batches/GrClearBatch.h View 1 2 3 4 5 6 1 chunk +3 lines, -0 lines 0 comments Download
M src/gpu/gl/GrGLGpu.h View 1 2 3 4 5 6 3 chunks +20 lines, -22 lines 0 comments Download
M src/gpu/gl/GrGLGpu.cpp View 1 2 3 4 5 6 5 chunks +20 lines, -17 lines 0 comments Download
M src/gpu/vk/GrVkGpuCommandBuffer.cpp View 1 2 3 4 5 6 7 8 9 2 chunks +2 lines, -0 lines 0 comments Download
M tests/ProxyTest.cpp View 1 2 3 4 5 6 2 chunks +5 lines, -3 lines 0 comments Download
M tests/WindowRectanglesTest.cpp View 2 chunks +0 lines, -16 lines 0 comments Download

Messages

Total messages: 30 (11 generated)
csmartdalton
4 years, 3 months ago (2016-08-31 07:03:12 UTC) #3
bsalomon
Adding Rob to look at use of GrRenderTarget here. We have to figure out how ...
4 years, 3 months ago (2016-08-31 14:00:52 UTC) #5
robertphillips
On 2016/08/31 14:00:52, bsalomon wrote: > Adding Rob to look at use of GrRenderTarget here. ...
4 years, 3 months ago (2016-08-31 14:23:22 UTC) #6
bsalomon
https://codereview.chromium.org/2289363005/diff/60001/src/gpu/GrAppliedClip.h File src/gpu/GrAppliedClip.h (right): https://codereview.chromium.org/2289363005/diff/60001/src/gpu/GrAppliedClip.h#newcode39 src/gpu/GrAppliedClip.h:39: GrWindowRectsState& SK_WARN_UNUSED_RESULT addWindowRectangles() { Can this be a setter ...
4 years, 3 months ago (2016-08-31 14:40:39 UTC) #7
csmartdalton
https://codereview.chromium.org/2289363005/diff/60001/src/gpu/GrAppliedClip.h File src/gpu/GrAppliedClip.h (right): https://codereview.chromium.org/2289363005/diff/60001/src/gpu/GrAppliedClip.h#newcode39 src/gpu/GrAppliedClip.h:39: GrWindowRectsState& SK_WARN_UNUSED_RESULT addWindowRectangles() { On 2016/08/31 14:40:39, bsalomon wrote: ...
4 years, 3 months ago (2016-08-31 15:04:35 UTC) #8
bsalomon
https://codereview.chromium.org/2289363005/diff/80001/src/gpu/GrFixedClip.h File src/gpu/GrFixedClip.h (right): https://codereview.chromium.org/2289363005/diff/80001/src/gpu/GrFixedClip.h#newcode34 src/gpu/GrFixedClip.h:34: GrWindowRectsState& windowRectsState() { return fWindowRectsState; } On 2016/08/31 15:04:34, ...
4 years, 3 months ago (2016-08-31 17:50:46 UTC) #10
csmartdalton
https://codereview.chromium.org/2289363005/diff/80001/src/gpu/GrFixedClip.h File src/gpu/GrFixedClip.h (right): https://codereview.chromium.org/2289363005/diff/80001/src/gpu/GrFixedClip.h#newcode34 src/gpu/GrFixedClip.h:34: GrWindowRectsState& windowRectsState() { return fWindowRectsState; } On 2016/08/31 17:50:46, ...
4 years, 3 months ago (2016-08-31 19:43:36 UTC) #11
csmartdalton
Window rectangles are alive and well on the bot. This is ready to land.
4 years, 3 months ago (2016-09-01 21:38:14 UTC) #12
bsalomon
lgtm, but perhaps the GM could use some additional comments as it has a somewhat ...
4 years, 3 months ago (2016-09-02 13:09:17 UTC) #13
csmartdalton
On 2016/09/02 13:09:17, bsalomon wrote: > lgtm, but perhaps the GM could use some additional ...
4 years, 3 months ago (2016-09-02 17:34:55 UTC) #14
bsalomon
On 2016/09/02 17:34:55, csmartdalton wrote: > On 2016/09/02 13:09:17, bsalomon wrote: > > lgtm, but ...
4 years, 3 months ago (2016-09-02 17:36:24 UTC) #15
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/2289363005/160001
4 years, 3 months ago (2016-09-02 18:07:51 UTC) #17
commit-bot: I haz the power
Committed patchset #9 (id:160001) as https://skia.googlesource.com/skia/+/db42be9a326c747ff92ed1da8c3536c5b3e8e22b
4 years, 3 months ago (2016-09-02 18:36:29 UTC) #19
caryclark
A revert of this CL (patchset #9 id:160001) has been created in https://codereview.chromium.org/2312173002/ by caryclark@google.com. ...
4 years, 3 months ago (2016-09-06 13:44:00 UTC) #20
csmartdalton
Patchset 10 fixes the issue. Once the bots finish this should be ready to re-land.
4 years, 3 months ago (2016-09-06 15:33:56 UTC) #23
csmartdalton
Bots were successful. This is ready to land. (Also notice the new GEOMEAN row of ...
4 years, 3 months ago (2016-09-06 16:27:40 UTC) #25
bsalomon
On 2016/09/06 16:27:40, csmartdalton wrote: > Bots were successful. This is ready to land. > ...
4 years, 3 months ago (2016-09-06 16:30:01 UTC) #26
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/2289363005/200001
4 years, 3 months ago (2016-09-06 16:34:49 UTC) #28
commit-bot: I haz the power
4 years, 3 months ago (2016-09-06 17:01:10 UTC) #30
Message was sent while issue was closed.
Committed patchset #11 (id:200001) as
https://skia.googlesource.com/skia/+/bf4a8f90c87dddf6290aa774536715e55e6a12f5

Powered by Google App Engine
This is Rietveld 408576698