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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2334873002: [LayoutNG] Don't implement ContainerSize() using the physical constraint space. (Closed)

Created:
4 years, 3 months ago by mstensho (USE GERRIT)
Modified:
4 years, 3 months ago
Reviewers:
cbiesinger, ikilpatrick, eae
CC:
chromium-reviews, ojan+watch_chromium.org, szager+layoutwatch_chromium.org, zoltan1, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, blink-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[LayoutNG] Don't implement ContainerSize() using the physical constraint space. There will typically be derived constraint spaces setting a different containing block size, and those are the ones we need to use when resolving percentages or auto inline size. When a derived constraint space changes writing-mode, we now need to make sure that the size is transposed. There is a unit test that would fail otherwise. BUG=635619

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -7 lines) Patch
M third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_constraint_space.cc View 2 chunks +7 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_constraint_space_test.cc View 1 chunk +15 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_units.h View 1 chunk +7 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (2 generated)
mstensho (USE GERRIT)
Does this make sense at all? What we're currently doing is definitely wrong, but I'm ...
4 years, 3 months ago (2016-09-13 18:37:15 UTC) #2
cbiesinger
On 2016/09/13 18:37:15, mstensho wrote: > Does this make sense at all? What we're currently ...
4 years, 3 months ago (2016-09-13 19:05:01 UTC) #3
mstensho (USE GERRIT)
On 2016/09/13 19:05:01, cbiesinger wrote: > On 2016/09/13 18:37:15, mstensho wrote: > > Does this ...
4 years, 3 months ago (2016-09-13 19:13:16 UTC) #4
cbiesinger
Oh, you're right :( So, here's how this should work, I think: In kStateInit, we ...
4 years, 3 months ago (2016-09-13 19:18:23 UTC) #6
eae
On 2016/09/13 19:18:23, cbiesinger wrote: > Oh, you're right :( > > So, here's how ...
4 years, 3 months ago (2016-09-15 09:24:15 UTC) #7
mstensho (USE GERRIT)
4 years, 3 months ago (2016-09-15 12:00:56 UTC) #8 
On 2016/09/15 09:24:15, eae wrote:
> On 2016/09/13 19:18:23, cbiesinger wrote:
> > Oh, you're right :(
> > 
> > So, here's how this should work, I think:
> > 
> > In kStateInit, we need to create a constraint space with a new container
size
> > (and a new size equal to new container size) but based on the input one so
we
> > get the exclusions right.
> > 
> > Then for each child, we call LayoutOpportunities() to find a location for
that
> > child. This will create constraint spaces with the same ContainerSize but
> > different Size and Offset. That's what we pass to the child's layout.
> > 
> > (And it's in NGBox::Layout where we convert the writing mode to the child's
> > writing mode)
> 
> Sounds reasonable to me.

OK, I'll just close this one.

Powered by Google App Engine
This is Rietveld 408576698