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

Issue 88863002: Land layer squashing behind a flag (Closed)

Created:
7 years ago by shawnsingh
Modified:
7 years ago
CC:
blink-reviews, bemjb+rendering_chromium.org, zoltan1, eae+blinkwatch, leviw+renderwatch, blink-layers+watch_chromium.org, jchaffraix+rendering, hartmanng
Visibility:
Public.

Description

Land layer squashing behind a flag This patch adds appropriate ASSERTs and flag checks to ensure that the feature does not affect tip of tree when the flag is disabled. Tests and bugs will be added in upcoming patches. BUG=261605 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=163777

Patch Set 1 #

Patch Set 2 : rebased, not yet for review #

Patch Set 3 : in progress, still need more cleaning #

Patch Set 4 : fix build error #

Patch Set 5 : fix build error #

Patch Set 6 : not perfect but can be reviewed #

Total comments: 13

Patch Set 7 : a few reviewer points addressed but not all yet #

Patch Set 8 : Addressed reviewer feedback #

Total comments: 42

Patch Set 9 : addressed feedback #

Total comments: 9

Patch Set 10 : Should be good for landing behind flag #

Total comments: 3

Patch Set 11 : rebased and addressed 2 iterations of feedback #

Patch Set 12 : fix minor mistake #

Patch Set 13 : fixed childForSuperLayers issues #

Patch Set 14 : removed bogus asserts #

Unified diffs Side-by-side diffs Delta from patch set Stats (+723 lines, -43 lines) Patch
A LayoutTests/compositing/squashing/squashed-repaints.html View 1 2 3 4 5 6 7 8 1 chunk +130 lines, -0 lines 0 comments Download
A LayoutTests/compositing/squashing/squashed-repaints-expected.txt View 1 2 3 4 5 6 7 1 chunk +167 lines, -0 lines 0 comments Download
M Source/core/rendering/CompositedLayerMapping.h View 1 2 3 4 5 6 7 8 9 10 5 chunks +14 lines, -0 lines 0 comments Download
M Source/core/rendering/CompositedLayerMapping.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 13 chunks +171 lines, -9 lines 0 comments Download
M Source/core/rendering/CompositingReasons.h View 1 2 3 4 5 6 7 8 2 chunks +7 lines, -1 line 0 comments Download
M Source/core/rendering/CompositingState.h View 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderLayer.h View 1 2 3 4 5 6 7 8 4 chunks +11 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderLayer.cpp View 1 2 3 4 5 6 7 8 9 10 11 7 chunks +32 lines, -9 lines 0 comments Download
M Source/core/rendering/RenderLayerClipper.cpp View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/rendering/RenderLayerCompositor.h View 1 2 3 4 5 6 7 8 9 10 2 chunks +20 lines, -1 line 0 comments Download
M Source/core/rendering/RenderLayerCompositor.cpp View 1 2 3 4 5 6 7 8 9 10 12 chunks +111 lines, -18 lines 0 comments Download
M Source/core/rendering/RenderLayerModelObject.h View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/rendering/RenderLayerModelObject.cpp View 1 2 3 4 5 1 chunk +5 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderLayerRepainter.cpp View 1 2 3 4 5 6 7 8 9 10 2 chunks +21 lines, -5 lines 0 comments Download
M Source/core/rendering/RenderObject.cpp View 1 2 3 4 5 6 7 8 9 10 1 chunk +28 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (0 generated)
shawnsingh
Ian, PTAL thanks! the html layout tests are usable interactively, but not really written as ...
7 years ago (2013-11-27 22:24:23 UTC) #1
enne (OOO)
Exciting! https://codereview.chromium.org/88863002/diff/90001/Source/core/rendering/CompositedLayerMapping.cpp File Source/core/rendering/CompositedLayerMapping.cpp (right): https://codereview.chromium.org/88863002/diff/90001/Source/core/rendering/CompositedLayerMapping.cpp#newcode2089 Source/core/rendering/CompositedLayerMapping.cpp:2089: if (m_nextSquashedLayerIndex == 0x0fff0000) { There's just something ...
7 years ago (2013-11-27 23:20:26 UTC) #2
shawnsingh
PTAL, thanks! Responses to previous feedback inline below About the layout test expected result: the ...
7 years ago (2013-12-02 01:55:02 UTC) #3
enne (OOO)
https://codereview.chromium.org/88863002/diff/90001/Source/core/rendering/RenderLayer.h File Source/core/rendering/RenderLayer.h (right): https://codereview.chromium.org/88863002/diff/90001/Source/core/rendering/RenderLayer.h#newcode357 Source/core/rendering/RenderLayer.h:357: CompositedLayerMapping* groupedMapping() const { return m_groupedMapping; } On 2013/12/02 ...
7 years ago (2013-12-02 18:48:59 UTC) #4
shawnsingh
On 2013/12/02 18:48:59, enne wrote: > https://codereview.chromium.org/88863002/diff/90001/Source/core/rendering/RenderLayer.h > File Source/core/rendering/RenderLayer.h (right): > > https://codereview.chromium.org/88863002/diff/90001/Source/core/rendering/RenderLayer.h#newcode357 > ...
7 years ago (2013-12-02 19:46:25 UTC) #5
Ian Vollick
I'm also really excited about this. I've just got lots of what are probably very ...
7 years ago (2013-12-02 21:17:04 UTC) #6
trchen
Unsolicited feedback. :> https://codereview.chromium.org/88863002/diff/130001/Source/core/rendering/CompositedLayerMapping.cpp File Source/core/rendering/CompositedLayerMapping.cpp (right): https://codereview.chromium.org/88863002/diff/130001/Source/core/rendering/CompositedLayerMapping.cpp#newcode1282 Source/core/rendering/CompositedLayerMapping.cpp:1282: if (!m_squashingContainmentLayer) { This can merge ...
7 years ago (2013-12-02 22:56:55 UTC) #7
shawnsingh
new patch uploaded - PTAL! Thanks for all the comments. The patch is much improved: ...
7 years ago (2013-12-05 17:55:24 UTC) #8
Ian Vollick
https://codereview.chromium.org/88863002/diff/130001/Source/core/rendering/CompositedLayerMapping.cpp File Source/core/rendering/CompositedLayerMapping.cpp (right): https://codereview.chromium.org/88863002/diff/130001/Source/core/rendering/CompositedLayerMapping.cpp#newcode795 Source/core/rendering/CompositedLayerMapping.cpp:795: if (m_squashingLayer) { On 2013/12/05 17:55:24, shawnsingh wrote: > ...
7 years ago (2013-12-06 21:11:29 UTC) #9
Ian Vollick
This is looking nice. I've just got a few minor comments. https://codereview.chromium.org/88863002/diff/150001/Source/core/rendering/CompositedLayerMapping.cpp File Source/core/rendering/CompositedLayerMapping.cpp (right): ...
7 years ago (2013-12-07 02:44:42 UTC) #10
Ian Vollick
I see you've posted a new patch set, but I don't see another comment on ...
7 years ago (2013-12-11 16:45:17 UTC) #11
shawnsingh
PTAL - I've addressed your comments now. And rebased to closer to tip of tree. ...
7 years ago (2013-12-11 18:50:33 UTC) #12
Ian Vollick
lgtm https://codereview.chromium.org/88863002/diff/160001/Source/core/rendering/RenderLayerCompositor.cpp File Source/core/rendering/RenderLayerCompositor.cpp (right): https://codereview.chromium.org/88863002/diff/160001/Source/core/rendering/RenderLayerCompositor.cpp#newcode1035 Source/core/rendering/RenderLayerCompositor.cpp:1035: static IntPoint computeOffsetFromAbsolute(RenderLayer* layer) On 2013/12/11 18:50:34, shawnsingh ...
7 years ago (2013-12-11 19:47:22 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/shawnsingh@chromium.org/88863002/190001
7 years ago (2013-12-11 21:54:31 UTC) #14
commit-bot: I haz the power
Retried try job too often on linux_blink for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blink&number=5232
7 years ago (2013-12-12 00:41:08 UTC) #15
Ian Vollick
On 2013/12/12 00:41:08, I haz the power (commit-bot) wrote: > Retried try job too often ...
7 years ago (2013-12-12 05:46:08 UTC) #16
Ian Vollick
On 2013/12/12 00:41:08, I haz the power (commit-bot) wrote: > Retried try job too often ...
7 years ago (2013-12-12 05:46:12 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/shawnsingh@chromium.org/88863002/210001
7 years ago (2013-12-12 05:46:50 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/shawnsingh@chromium.org/88863002/230001
7 years ago (2013-12-12 07:34:24 UTC) #19
commit-bot: I haz the power
7 years ago (2013-12-12 12:18:37 UTC) #20
Message was sent while issue was closed.
Change committed as 163777

Powered by Google App Engine
This is Rietveld 408576698