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

Issue 799403006: [New Multicolumn] Make computeOffsetFromCompositedAncestor() flowthread-aware. (Closed)

Created:
6 years ago by mstensho (USE GERRIT)
Modified:
6 years ago
Reviewers:
chrishtr
CC:
blink-layers+watch_chromium.org, blink-reviews, blink-reviews-rendering, Dominik Röttsches, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, zoltan1
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

[New Multicolumn] Make computeOffsetFromCompositedAncestor() flowthread-aware. This function needs to produce the visual offset, and you cannot get that by calling convertToLayerCoords(). Introducing RenderLayer::visualOffsetFromAncestor() to calculate the correct visual offset. This also allows for reducing convertFromFlowThreadToVisualBoundingBoxInAncestor() by a few lines. We can now add compositing/columns to the virtual/regionbasedmulticol/ testsuite, since both tests there have started to pass. The fast/multicol/composited-layer-single-fragment.html test is based on work by andersr@opera.com The test for hit-testing something 3D-transformed has already been passing for some time (since https://codereview.chromium.org/625903004), but actual painting was wrong, until now. BUG=359877 R=chrishtr@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187413

Patch Set 1 #

Total comments: 4

Patch Set 2 : code review #

Total comments: 4

Patch Set 3 : code review #2 #

Total comments: 4

Patch Set 4 : Simplify test. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+102 lines, -6 lines) Patch
M LayoutTests/TestExpectations View 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/VirtualTestSuites View 1 chunk +5 lines, -0 lines 0 comments Download
A LayoutTests/fast/multicol/composited-layer-single-fragment.html View 1 2 3 1 chunk +35 lines, -0 lines 0 comments Download
A LayoutTests/fast/multicol/composited-layer-single-fragment-expected.html View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
A LayoutTests/fast/multicol/hit-test-translate-z.html View 1 chunk +26 lines, -0 lines 0 comments Download
A + LayoutTests/fast/multicol/hit-test-translate-z-expected.txt View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/virtual/regionbasedmulticol/compositing/columns/README.txt View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/RenderLayer.h View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderLayer.cpp View 1 2 2 chunks +29 lines, -4 lines 0 comments Download
M Source/core/rendering/compositing/CompositedLayerMapping.cpp View 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 13 (1 generated)
mstensho (USE GERRIT)
6 years ago (2014-12-16 15:50:00 UTC) #1
chrishtr
https://codereview.chromium.org/799403006/diff/1/Source/core/rendering/RenderLayer.cpp File Source/core/rendering/RenderLayer.cpp (right): https://codereview.chromium.org/799403006/diff/1/Source/core/rendering/RenderLayer.cpp#newcode1458 Source/core/rendering/RenderLayer.cpp:1458: if (!paginationLayer || paginationLayer == this) { Do you ...
6 years ago (2014-12-16 17:33:45 UTC) #2
mstensho (USE GERRIT)
https://codereview.chromium.org/799403006/diff/1/Source/core/rendering/RenderLayer.cpp File Source/core/rendering/RenderLayer.cpp (right): https://codereview.chromium.org/799403006/diff/1/Source/core/rendering/RenderLayer.cpp#newcode1458 Source/core/rendering/RenderLayer.cpp:1458: if (!paginationLayer || paginationLayer == this) { On 2014/12/16 ...
6 years ago (2014-12-16 22:31:47 UTC) #3
chrishtr
https://codereview.chromium.org/799403006/diff/20001/Source/core/rendering/RenderLayer.cpp File Source/core/rendering/RenderLayer.cpp (right): https://codereview.chromium.org/799403006/diff/20001/Source/core/rendering/RenderLayer.cpp#newcode1469 Source/core/rendering/RenderLayer.cpp:1469: // FIXME: Handle nested fragmentation contexts (crbug.com/423076). For now ...
6 years ago (2014-12-17 19:11:00 UTC) #4
mstensho (USE GERRIT)
https://codereview.chromium.org/799403006/diff/20001/Source/core/rendering/RenderLayer.cpp File Source/core/rendering/RenderLayer.cpp (right): https://codereview.chromium.org/799403006/diff/20001/Source/core/rendering/RenderLayer.cpp#newcode1469 Source/core/rendering/RenderLayer.cpp:1469: // FIXME: Handle nested fragmentation contexts (crbug.com/423076). For now ...
6 years ago (2014-12-17 19:29:53 UTC) #5
chrishtr
https://codereview.chromium.org/799403006/diff/40001/LayoutTests/fast/multicol/composited-layer-single-fragment.html File LayoutTests/fast/multicol/composited-layer-single-fragment.html (right): https://codereview.chromium.org/799403006/diff/40001/LayoutTests/fast/multicol/composited-layer-single-fragment.html#newcode40 LayoutTests/fast/multicol/composited-layer-single-fragment.html:40: <div class="element e1"></div> Can this test be reduced? It's ...
6 years ago (2014-12-17 19:35:55 UTC) #6
mstensho (USE GERRIT)
https://codereview.chromium.org/799403006/diff/40001/Source/core/rendering/RenderLayer.cpp File Source/core/rendering/RenderLayer.cpp (right): https://codereview.chromium.org/799403006/diff/40001/Source/core/rendering/RenderLayer.cpp#newcode1454 Source/core/rendering/RenderLayer.cpp:1454: LayoutPoint RenderLayer::visualOffsetFromAncestor(const RenderLayer* ancestorLayer) const On 2014/12/17 19:35:55, chrishtr ...
6 years ago (2014-12-17 20:49:27 UTC) #7
chrishtr
On 2014/12/17 at 20:49:27, mstensho wrote: > https://codereview.chromium.org/799403006/diff/40001/Source/core/rendering/RenderLayer.cpp > File Source/core/rendering/RenderLayer.cpp (right): > > https://codereview.chromium.org/799403006/diff/40001/Source/core/rendering/RenderLayer.cpp#newcode1454 ...
6 years ago (2014-12-17 21:53:03 UTC) #8
mstensho (USE GERRIT)
Bleh, uploaded a new patch, but forgot to actually mark the test change as done. ...
6 years ago (2014-12-17 22:26:53 UTC) #9
chrishtr
lgtm
6 years ago (2014-12-17 22:31:03 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/799403006/60001
6 years ago (2014-12-17 22:31:21 UTC) #12
commit-bot: I haz the power
6 years ago (2014-12-18 00:43:54 UTC) #13
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187413

Powered by Google App Engine
This is Rietveld 408576698