1752658 - Some refactor around DoFlexLayout

Status: RESOLVED FIXED

(Core :: Layout: Flexbox, task)

Description

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

It's been a while since I think about flexbox algorithm & fragmentation. This time when I look at DoFlexLayout and its data flow, I found some opportunities to make it clearer. See the upcoming patches.

Comment 1

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1752658 Part 1 - Compute gap sizes only in flex container's first-in-flow.

Move the assertion for unconstrained isize to the beginning of Reflow() because
we check it in all cases -- in GetMainSizeFromReflowInput when a flex container
is row-oriented, or in the old code computing gap size and ComputeCrossSize()
when a flex container is column-oriented.

Comment 2

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1752658 Part 2 - Remove aConsumedBSize from DoFlexLayout() and ComputeCrossSize().

DoFlexLayout() and ComputeCrossSize() are only called in first-in-flow, so
aConsumedBSize are always 0.

Depends on D137361

Comment 3

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1752658 Part 3 - Reduce scope of the struts array in Reflow().

The StrutInfo array is used only within DoFlexLayout, so we can restart the
flex layout within DoFlexLayout to make Reflow() simpler.

Depends on D137362

Comment 4

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1752658 Part 4 - Aggregate DoFlexLayout's output data in a struct instead of output arguments.

I found in/out parameters confusing when reasoning the data flow. Aggregating
DoFlexLayout's output data in a struct also reduces DoFlexLayout's number of
arguments.

Depends on D137363

Comment 5

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1752658 Part 5 - Pass flex container's tentative cross size into DoFlexLayout and others.

We've got the tentative cross size before calling DoFlexLayout() in Reflow(). In
DoFlexLayout and a few other methods and just use that value.

Depends on D137364

Comment 6

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1752658 Part 6 - Pass FlexLayoutResult into ReflowChildren().

Currently, we pass all the five fields in FlexLayoutResult separately into
ReflowChildren(), but we really should just pass FlexLayoutResult instead.

Depends on D137365

Comment 7

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1752658 Part 7 - Generalize GenerateFlexLines to populate FlexLayoutResult.

Depends on D138100

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/88181dec11e8
Part 1 - Compute gap sizes only in flex container's first-in-flow. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/112d24a3a280
Part 2 - Remove aConsumedBSize from DoFlexLayout() and ComputeCrossSize(). r=dholbert
https://hg.mozilla.org/integration/autoland/rev/25f556801924
Part 3 - Reduce scope of the struts array in Reflow(). r=dholbert
https://hg.mozilla.org/integration/autoland/rev/dd2291e298d0
Part 4 - Aggregate DoFlexLayout's output data in a struct instead of output arguments. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/68cc1c64bb66
Part 5 - Pass flex container's tentative cross size into DoFlexLayout and others. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/8eebe46df776
Part 6 - Pass FlexLayoutResult into ReflowChildren(). r=dholbert
https://hg.mozilla.org/integration/autoland/rev/d295fb16a0cb
Part 7 - Generalize GenerateFlexLines to populate FlexLayoutResult. r=dholbert

Comment 9

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/88181dec11e8
https://hg.mozilla.org/mozilla-central/rev/112d24a3a280
https://hg.mozilla.org/mozilla-central/rev/25f556801924
https://hg.mozilla.org/mozilla-central/rev/dd2291e298d0
https://hg.mozilla.org/mozilla-central/rev/68cc1c64bb66
https://hg.mozilla.org/mozilla-central/rev/8eebe46df776
https://hg.mozilla.org/mozilla-central/rev/d295fb16a0cb