Ah, sorry for the trouble. I'm looking forward to work on it, but there's a good risk that we may not match to legacy for the vertical position in vertical flow, as I'm seeing cases where legacy does incorrect positioning. Even for this 1px off, I haven't figured out which is more correct yet.

For inline, especially in vertical flow, it's better not to have a ref file rendered in legacy.

Oh, I see. Sorry, I thought this off-by-one thing was a plain bug that you were going to fix, but if it's legacy that's actually off by one in some cases, I guess I have to do something with the tests. I really don't want to degrade them to pixel tests, though...

Feel free to close this bug, if you realize that NG is doing it right. :)

Well.. this is strange. I cannot reproduce any of this today, although the issue is still there (the attached test case still fails).

I'd expect https://chromium-review.googlesource.com/c/chromium/src/+/1092573 to cause all the tests mentioned above to regress, but no!

Oh well, if they suddenly start to regress because of this issue, I think the best fix is to change all the tests to use horizontal-tb for all text (the pass condition). The tests really only care about block positioning anyway. That would make the tests more human neck friendly, too. :)

Yeah, I'm still trying to narrow this down. Sometimes legacy is off by many pixels, sometimes NG is off by 1 pixel. I made a few tests to know which is correct, but then hit legacy bug to dig further...

From this test:

http://jsbin.com/didamuq/edit?html,output

NG positions vlr and vrl at the same pixel, while legacy doesn't. Odd that legacy even positions the block at different position, but regardless, NG seems to render the text at the same position.

#5 maybe an abspos bug in legacy. Another test without using abspos:

http://jsbin.com/nivaruw/edit?html,output

Both look correct now, no differences are observed. Maybe there's a condition to kick the one pixel off?

Re my confusion in #c3: Turns out the regressions were caused by a local patch. :) I'm about to introduce support for creating anonymous LayoutNGTableCell objects. Currently, anonymous cells use legacy layout, while non-anonymous ones use NG. No reason for this distinction of course, but by fixing that, the 8 fast/pagination/ tests (which happen to create anonymous table cells) would regress.

Thanks, but it's still true that there seems to be some cases. Maybe just rounding error, maybe we're fixing, but I'll keep this for a while until we know it better.

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

commit 6531bb6b83fc644141153c8248fdf03603d3c990
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Thu Jun 14 08:55:05 2018

Change some fast/pagination/ tests to write pass conditions in horizontal-tb.

Apart from making it more human neck friendly, those which were in
vertical-lr would expose bug 850504 (once support for anonymous NG table
cells is enabled), which really isn't an interesting thing to test in
these tests. To be consistent, rewrite all tests to use horizontal-tb,
not just those that previously used vertical-lr.

viewport-y-vertical-XX-rtl additionally need a fixed horizontal offset
for the absolutely positioned box in the ref. Not sure what went wrong,
but both NG and legacy decided to act up in identical ways.

Bug: 850504
Change-Id: I888db0680c5d543c9ac19a1ae8ab9ca791d8b259
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/1099384
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567197}
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-x-horizontal-tb-ltr-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-x-horizontal-tb-ltr.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-x-horizontal-tb-rtl-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-x-horizontal-tb-rtl.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-x-vertical-lr-ltr-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-x-vertical-lr-ltr.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-x-vertical-lr-rtl-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-x-vertical-lr-rtl.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-x-vertical-rl-ltr-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-x-vertical-rl-ltr.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-x-vertical-rl-rtl-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-x-vertical-rl-rtl.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-y-horizontal-tb-ltr-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-y-horizontal-tb-ltr.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-y-horizontal-tb-rtl-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-y-horizontal-tb-rtl.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-y-vertical-lr-ltr-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-y-vertical-lr-ltr.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-y-vertical-lr-rtl-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-y-vertical-lr-rtl.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-y-vertical-rl-ltr-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-y-vertical-rl-ltr.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-y-vertical-rl-rtl-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/div-y-vertical-rl-rtl.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-x-horizontal-tb-ltr-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-x-horizontal-tb-ltr.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-x-horizontal-tb-rtl-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-x-horizontal-tb-rtl.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-x-vertical-lr-ltr-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-x-vertical-lr-ltr.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-x-vertical-lr-rtl-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-x-vertical-lr-rtl.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-x-vertical-rl-ltr-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-x-vertical-rl-ltr.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-x-vertical-rl-rtl-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-x-vertical-rl-rtl.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-y-horizontal-tb-ltr-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-y-horizontal-tb-ltr.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-y-horizontal-tb-rtl-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-y-horizontal-tb-rtl.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-y-vertical-lr-ltr-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-y-vertical-lr-ltr.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-y-vertical-lr-rtl-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-y-vertical-lr-rtl.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-y-vertical-rl-ltr-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-y-vertical-rl-ltr.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-y-vertical-rl-rtl-expected.html
[modify] https://crrev.com/6531bb6b83fc644141153c8248fdf03603d3c990/third_party/WebKit/LayoutTests/fast/pagination/viewport-y-vertical-rl-rtl.html

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

commit 8ba44c463621ef8236ead03616ecb5b7c87ea03b
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Thu Jun 21 19:28:12 2018

[LayoutNG] Link vertical-lr off-by-1px multicol test failures to dedicated bug.

TBR=kojii@chromium.org

Bug: 850504
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I80780e6e19db3932094085a5fdf12a0497ab2774
Reviewed-on: https://chromium-review.googlesource.com/1110364
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569354}
[modify] https://crrev.com/8ba44c463621ef8236ead03616ecb5b7c87ea03b/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG

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

commit a691d430a7c037e96c227b55f17387c89d2d0252
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Tue Oct 09 15:03:44 2018

[LayoutNG] Mark fieldset-vertical.html with a relevant bug.

This fails because vertical-lr is rendered off-by-one pixel compared to
legacy layout. Fieldsets are still laid out by legacy when NG is
enabled, while the ref is rendered by NG.

TBR=kojii@chromium.org

Bug: 850504
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Icd528a31035eb72f840f864bdee649b684f702d2
Reviewed-on: https://chromium-review.googlesource.com/c/1270898
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597917}
[modify] https://crrev.com/a691d430a7c037e96c227b55f17387c89d2d0252/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG

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

commit 7251c36ab0f9a070f0df584911ccf0f7901c3662
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Wed Nov 21 15:53:12 2018

[LayoutNG] Mark tests that fail due to off-by-one in vertical-lr.

TBR=kojii@chromium.org

Bug: 850504
Change-Id: I617a48c931a6acf3bb7ca560d59b1136eea59d44
Reviewed-on: https://chromium-review.googlesource.com/c/1346457
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610077}
[modify] https://crrev.com/7251c36ab0f9a070f0df584911ccf0f7901c3662/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG

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

commit a75914d27fa0ca3d6fde9095bfceb38750c5bcd2
Author: Ian Kilpatrick <ikilpatrick@chromium.org>
Date: Fri Jan 18 05:09:11 2019

[LayoutNG] Rebaseline fast/writing-mode/{background-vertical-lr,basic-vertical-line}.html

Also skips:
external/wpt/html/rendering/non-replaced-elements/the-fieldset-and-legend-elements/fieldset-vertical.html

LayoutNG has more correct rules for placing elements inside a linebox
in the vertical-lr writing-mode.

See testcase:
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=6525

Legacy layout will try to snap children to the "bottom" of the linebox
*IF* there is an inline level child whose ascender is larger than the
default ascender size (I think).

In the above testcase, play around with the height of the atomic-inline
to see this logic in action.

Bug: 850504
Change-Id: I433bc005cfdff38a48b94d83481a356bfc88ed5f
Reviewed-on: https://chromium-review.googlesource.com/c/1418734
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624014}
[modify] https://crrev.com/a75914d27fa0ca3d6fde9095bfceb38750c5bcd2/third_party/blink/web_tests/FlagExpectations/enable-blink-features=LayoutNG
[add] https://crrev.com/a75914d27fa0ca3d6fde9095bfceb38750c5bcd2/third_party/blink/web_tests/flag-specific/enable-blink-features=LayoutNG/fast/writing-mode/background-vertical-lr-expected.png
[add] https://crrev.com/a75914d27fa0ca3d6fde9095bfceb38750c5bcd2/third_party/blink/web_tests/flag-specific/enable-blink-features=LayoutNG/fast/writing-mode/basic-vertical-line-expected.png