New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 616600 link

Starred by 1 user
Status: Fixed
Owner:
█ wkorman@chromium.org
Last visit > 30 days ago
Closed: Jul 2016
Cc:
pdr@chromium.org
wangxianzhu@chromium.org
kojii@chromium.org
chrishtr@chromium.org
vmp...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocking:
issue 529938



Sign in to add a comment

Paint invalidation rects incorrect for position: relative writing-mode: vertical-rl

Project Member Reported by wkorman@chromium.org, Jun 1 2016 
To repro:

1. open imported/csswg-test/css-writing-modes-3/box-offsets-rel-pos-vrl-004.xht in content shell
2. open dev tools inspector
3. turn on 'paint flashing' in Rendering settings
4. select any of the divs with 'blue-relatively-positioned' class
5. toggle background-color, observe paint flashing in incorrect locations and only partial repainting of blue boxes

Issue found while working on  http://crbug.com/529938 . This test is currently disabled on Linux, but we see failures on Mac/Windows on trybots with http://crrev.com/1484163002 and that patch will change the failure mode to where only one (top left) of the four blue boxes render correctly initially. Failure on Linux with patch looks similar to Mac/Windows.

Comment 1 by wkorman@chromium.org, Jun 1 2016 

A simpler test case attached.
test.html
259 bytes View Download

Comment 2 by wkorman@chromium.org, Jun 6 2016

Cc: pdr@chromium.org
Even simpler test case attached that removes the background yellow rect, though it looks more complex because we've added code to log the bounding client rect to the console for easier comparison to the paint invalidation rect.
test.html
413 bytes View Download

Comment 3 by wkorman@chromium.org, Jun 7 2016

Summary: Paint invalidation rects incorrect for position: relative writing-mode: vertical-rl (was: Paint invalidation rects incorrect for imported/csswg-test/css-writing-modes-3/box-offsets-rel-pos-vrl-004.xht)

Comment 4 by wkorman@chromium.org, Jun 9 2016 

I am working through tracing logic in detail involved in:

(1) painting (PaintLayerClipper::calculateRects() implicated via tracing from BoxPainter::paintFillLayer())
(2) getBoundingClientRect (LayoutBox::mapLocalToAncestor())
(3) paint invalidation/visual rects (LayoutBox::mapToVisualRectInAncestorSpace())

to better understand how things work across a range of test cases. (1) and (2) seem to produce correct output, but (3) is broken. I expect fix will be some mix of adding/removing a flipForWritingMode(), using topLeftLocation() rather than locationOffset(), or similar, but I have to better understand to solve.

Comment 5 by wkorman@chromium.org, Jun 15 2016 

http://crrev.com/2073563002 is in flight to address this.

I'm working through what looks like ~11 remaining failing LayoutTests. There are try jobs running on current patchset to provide visibility.

Most recently fixed LayoutTableCell.

Remaining failures look like they involve multicol, overflow pagination, position: absolute, and text selection.

Comment 6 by wkorman@chromium.org, Jun 16 2016 

Notes on multicol: logic in MultiColumnFragmentainerGroup::fragmentsBoundingBox() requires review/fixing.

PASS text in fast/multicol/tall-line-in-short-block.html is ending up with empty visual rect.

Comment 7 by wkorman@chromium.org, Jun 20 2016 

Current layout test results for rtree with vertical-rl change from patchset 38 of http://crrev.com/1484163002:

https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/87286/layout-test-results/results.html

Majority are multicol related hence that's current focus.

Comment 8 by wkorman@chromium.org, Jun 29 2016 

These tests I happened to just see in TestExpectations could be the same issue as we're working to solve in this bug:

# These tests pass but images do not match because of position: absolute in vertical flow bug
 crbug.com/492664  imported/csswg-test/css-writing-modes-3/block-flow-direction-vrl-009.xht [ Failure ]
 crbug.com/492664  imported/csswg-test/css-writing-modes-3/clip-rect-vlr-003.xht [ Failure ]
 crbug.com/492664  imported/csswg-test/css-writing-modes-3/clip-rect-vlr-005.xht [ Failure ]
 crbug.com/492664  imported/csswg-test/css-writing-modes-3/clip-rect-vlr-007.xht [ Failure ]
 crbug.com/492664  imported/csswg-test/css-writing-modes-3/clip-rect-vlr-009.xht [ Failure ]
 crbug.com/492664  imported/csswg-test/css-writing-modes-3/clip-rect-vrl-002.xht [ Failure ]
 crbug.com/492664  imported/csswg-test/css-writing-modes-3/clip-rect-vrl-004.xht [ Failure ]
 crbug.com/492664  imported/csswg-test/css-writing-modes-3/clip-rect-vrl-006.xht [ Failure ]
 crbug.com/492664  imported/csswg-test/css-writing-modes-3/clip-rect-vrl-008.xht [ Failure ]
 crbug.com/492664  imported/csswg-test/css-writing-modes-3/line-box-direction-vrl-009.xht [ Failure ]
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 13 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a7e6a571947df4ac9a93a8d43882c6d8588514b6

commit a7e6a571947df4ac9a93a8d43882c6d8588514b6
Author: wkorman <wkorman@chromium.org>
Date: Wed Jul 13 02:44:16 2016

Rework mapToVisualRectInAncestorSpace to handle flipped blocks correctly.

In particular, we were incorrectly calculating rects for out-of-flow
positioned elements.

Follows canonical logic in PaintLayer::updateLayerPosition().

BUG= 616600 

Review-Url: https://codereview.chromium.org/2073563002
Cr-Commit-Position: refs/heads/master@{#404925}

[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutBox.h
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutFlowThread.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutInline.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutTableCell.h
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutView.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/README.md
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/VisualRectMappingTest.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 13 2016

Labels: merge-merged-2795
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a7e6a571947df4ac9a93a8d43882c6d8588514b6

commit a7e6a571947df4ac9a93a8d43882c6d8588514b6
Author: wkorman <wkorman@chromium.org>
Date: Wed Jul 13 02:44:16 2016

Rework mapToVisualRectInAncestorSpace to handle flipped blocks correctly.

In particular, we were incorrectly calculating rects for out-of-flow
positioned elements.

Follows canonical logic in PaintLayer::updateLayerPosition().

BUG= 616600 

Review-Url: https://codereview.chromium.org/2073563002
Cr-Commit-Position: refs/heads/master@{#404925}

[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutBox.h
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutFlowThread.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutInline.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutTableCell.h
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/LayoutView.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/README.md
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/VisualRectMappingTest.cpp
[modify] https://crrev.com/a7e6a571947df4ac9a93a8d43882c6d8588514b6/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/626303cb7dcd51ade39ee4ca6a4b7cb83e933a5f

commit 626303cb7dcd51ade39ee4ca6a4b7cb83e933a5f
Author: wkorman <wkorman@chromium.org>
Date: Thu Jul 14 23:59:34 2016

Remove obsolete comment in LayoutFlowThread::mapToVisualRectInAncestorSpace.

In http://crrev.com/2073563002 we reworked mapToVisualRectInAncestorSpace so
that it expects rect to be in physical coordinates, *not* physical coordinates
with flipped block-flow direction.

BUG= 616600 
TBR=chrishtr

Review-Url: https://codereview.chromium.org/2151093002
Cr-Commit-Position: refs/heads/master@{#405641}

[modify] https://crrev.com/626303cb7dcd51ade39ee4ca6a4b7cb83e933a5f/third_party/WebKit/Source/core/layout/LayoutFlowThread.cpp
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 16 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d15111e9284f91a482eb87639656844996b31adf

commit d15111e9284f91a482eb87639656844996b31adf
Author: Rebaseline Bot <blink-rebaseline-bot@chromium.org>
Date: Sat Jul 16 03:00:09 2016

Auto-rebaseline for r405917

https://chromium.googlesource.com/chromium/src/+/09f0997bf

BUG= 616600 
TBR=chrishtr@chromium.org

Review URL: https://codereview.chromium.org/2153333002 .

Cr-Commit-Position: refs/heads/master@{#405926}

[modify] https://crrev.com/d15111e9284f91a482eb87639656844996b31adf/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/d15111e9284f91a482eb87639656844996b31adf/third_party/WebKit/LayoutTests/paint/invalidation/composited-table-row-expected.txt
[rename] https://crrev.com/d15111e9284f91a482eb87639656844996b31adf/third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/composited-table-row-expected.png
[add] https://crrev.com/d15111e9284f91a482eb87639656844996b31adf/third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/composited-table-row-expected.txt
[add] https://crrev.com/d15111e9284f91a482eb87639656844996b31adf/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/composited-table-row-expected.png
[add] https://crrev.com/d15111e9284f91a482eb87639656844996b31adf/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/composited-table-row-expected.png
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 19 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cac5d64bc07cde6d153ee9c7c9e6367aa07f3c39

commit cac5d64bc07cde6d153ee9c7c9e6367aa07f3c39
Author: chrishtr <chrishtr@chromium.org>
Date: Tue Jul 19 01:56:34 2016

Flip for writing mode exactly along the containing block chain.

container() actually implements containg block. (Contrary to the name
containingBlock() in the other method, it implements something slightly different.)

BUG= 616600 

Review-Url: https://codereview.chromium.org/2154913002
Cr-Commit-Position: refs/heads/master@{#406177}

[modify] https://crrev.com/cac5d64bc07cde6d153ee9c7c9e6367aa07f3c39/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/cac5d64bc07cde6d153ee9c7c9e6367aa07f3c39/third_party/WebKit/Source/core/layout/LayoutObject.cpp

Comment 17 by chrishtr@chromium.org, Jul 20 2016 

I think these are all done? Close the bug?

\o/

Comment 18 by wkorman@chromium.org, Jul 20 2016 

These ruby tests are still failing at ToT with rtree patch:

fast/ruby/overhang-vertical.html
fast/ruby/overhang-vertical-no-overlap1.html
fast/ruby/overhang-vertical-no-overlap2.html
fast/writing-mode/japanese-ruby-vertical-rl.html

https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/88537/layout-test-results/results.html

Comment 20 by wkorman@chromium.org, Jul 20 2016 

fast/writing-mode/Kusa-Makura-background-canvas.html is also failing on Mac only for some reason, looks like similar Ruby issue:

https://storage.googleapis.com/chromium-layout-test-archives/mac_blink_rel/68061/layout-test-results/results.html

Comment 21 by wkorman@chromium.org, Jul 20 2016

Status: Fixed (was: Started)
Ruby tests are being tracked separately in  crbug.com/628323  so closing this one.
►

Sign in to add a comment