This happens to Flexbox too.

Issue 781241 is about LogicalLeft(), but in this case, Offset() is (0, 0) for all cells/items. Maybe we can't compute if parent is not NG and that we need to rely on LayoutBox::Location()?

I think I understood what's happening. If parent is not LayoutNG, the parent may set Location() after children are laid out, and that fragment.Offset() is unreliable.

I'm not sure if this is solvable...trying to work around in paint code.

Morten has filed bug 606350 about Flexbox, where setting position after layout causes problems elsewhere too.

But I'm not sure why setting position after layout is a problem here -- when/where do we create the paint fragments?

Sorry, this is about NGPhysicalFragment::Offset(), I should be more clear.The bug 606350 looks like non-NG, no?

We create NGPaintFragments in pre-paint phase, but offset we use is NGPhysicalFragment::Offset(). For table cells and flex items, they're all (0, 0), while their LayoutObject has correct Location.

We probably need to set the offset when parent layout is done, but the parent layout is non-NG.

Yes, it's non-NG, but I understand the problem is that the position is set after layout? Maybe I misunderstood.

How come NGPhysicalFragment::Offset() is (0,0)? I guess I am not sure how we create the physical fragments in this situation.

No, sorry, I didn't explain well. It's within the layout cycle, after children are laid out, parent moves the children as part of its layout cycle.
1. LayoutTableCell::UpdateLayout.
2. LayoutTableSection determines the Location of the LayoutTableCell and LayoutTableCell::SetLocation.

When the LayoutTableCell is NG but LayoutTableSection is not, we determine the Offset within the LayoutNGBlockFlow::UpdateLayout(), which is step 1. This is too early, but we currently don't have a good chance to set NGPhysicalFragment::Offset when non-NG parent layout is complete.

Hmm, does this make sense? It's a bit hard to describe.

I think code is easier to explain, can you review |NGPhysicalFragment::IsPlacedByLayoutNG| in https://chromium-review.googlesource.com/c/chromium/src/+/780821 ?

Thanks, see my comment there.

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

commit acf42d2b0e6e2192d0e4b31b6a9a7ab5f7fbfe35
Author: Koji Ishii <kojii@chromium.org>
Date: Thu Nov 30 09:36:04 2017

[LayoutNG] Use AdjustPaintOffsetScope in NGBoxFragmentPainter

This patch changes NGBoxFragmentPainter to use AdjustPaintOffsetScope
to adjust paint offset rather than accumulating fragment offsets.

Before this change, NGBoxFragmentPainter accepts offsets with its own
offset accumulated. This was not consistent with other painters and
that callers need to change which offsets to pass.

Using AdjustPaintOffsetScope at the beginning of Paint() makes
NGBoxFragmentPainter consistent with other painters, and supports
SlimmingPaintV175 better.

Note that inline offset is incorrect in some cases. I'll work on it
in following patches.

Bug:  714962 , 788590
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ibb783218e8ec039d5d0abe41c4414f6fcc8d20b4
Reviewed-on: https://chromium-review.googlesource.com/780821
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520488}
[modify] https://crrev.com/acf42d2b0e6e2192d0e4b31b6a9a7ab5f7fbfe35/third_party/WebKit/Source/core/layout/ng/layout_ng_block_flow.cc
[modify] https://crrev.com/acf42d2b0e6e2192d0e4b31b6a9a7ab5f7fbfe35/third_party/WebKit/Source/core/layout/ng/layout_ng_mixin.cc
[modify] https://crrev.com/acf42d2b0e6e2192d0e4b31b6a9a7ab5f7fbfe35/third_party/WebKit/Source/core/layout/ng/ng_physical_fragment.cc
[modify] https://crrev.com/acf42d2b0e6e2192d0e4b31b6a9a7ab5f7fbfe35/third_party/WebKit/Source/core/layout/ng/ng_physical_fragment.h
[modify] https://crrev.com/acf42d2b0e6e2192d0e4b31b6a9a7ab5f7fbfe35/third_party/WebKit/Source/core/paint/AdjustPaintOffsetScope.h
[modify] https://crrev.com/acf42d2b0e6e2192d0e4b31b6a9a7ab5f7fbfe35/third_party/WebKit/Source/core/paint/ng/ng_box_fragment_painter.cc
[modify] https://crrev.com/acf42d2b0e6e2192d0e4b31b6a9a7ab5f7fbfe35/third_party/WebKit/Source/core/paint/ng/ng_box_fragment_painter.h