Closed Bug 1674450 Opened 1 year ago Closed 1 year ago

Convert SizeComputationInput's computed margin, border/padding, padding to logical coordinates

Categories

(Core :: Layout, task)

task

Tracking

()

RESOLVED FIXED
84 Branch
Tracking Status
firefox84 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

References

(Blocks 1 open bug)

Details

Attachments

(8 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

I'm planning to post a patch stack doing:

  1. Some cleanups for readability.
  2. Remove writable reference to physical margin, border/padding, and padding in https://searchfox.org/mozilla-central/rev/d866b96d74ec2a63f09ee418f048d23f4fd379a2/layout/generic/ReflowInput.h#126-130
  3. Convert the optional margin and padding argument on ReflowInput::Init to logical coordinate in https://searchfox.org/mozilla-central/rev/d866b96d74ec2a63f09ee418f048d23f4fd379a2/layout/generic/ReflowInput.h#824-825
  4. Finally, convert SizeComputationInput's computed margin, border/padding, and padding logical coordinates https://searchfox.org/mozilla-central/rev/d866b96d74ec2a63f09ee418f048d23f4fd379a2/layout/generic/ReflowInput.h#172-182

This is a pure rename to make the argument meaning clearer. In the later
parts, we might use wm as an alias to
SizeComputationInput::mWritingMode. It's important to differentiate
between mWritingMode and containing block's writing mode.

Depends on D95364

I favor two-argument SetComputedLogicalMargin() for clarity in this pat

Spoiler alert: the one-argument SetComputedLogicalMargin() will be removed in a later part.

Depends on D95365

Similar to the optional aContainingBlockSize parameter, both border and
padding should use logical coordinates in ReflowInput::mFrame's writing
mode.

Table frames that need to override border and padding can be simplified a bit.
However, DR_init_constraints_cookie and DR_init_offsets_cookie become more
complex, but they're only for debugging. I'm not planning to update their
internal APIs.

Depends on D95368

The one-argument SetComputedLogicalMargin(),
SetComputedLogicalBorderPadding(), and SetComputedLogicalPadding() are
not changed because the next part are going to delete them.

Depends on D95369

The two-argument versions are clearer and easier to use because they
work for any pair of writing mode and logical margin.

Depends on D95370

One addition conversion I'd love to do is to add WritingMode argument to ComputedLogicalMargin(), ComputedLogicalBorderPadding(), and ComputedLogicalPadding(), and adapt all the callers. https://searchfox.org/mozilla-central/rev/d866b96d74ec2a63f09ee418f048d23f4fd379a2/layout/generic/ReflowInput.h#132-140

We have a lot of parent frame's having to do something like ComputedLogicalMargin().ConvertTo(parentWM, childWM) after getting its child's margin. It would be nice to just write ComputedLogicalMargin(parentWM). This also makes the API similar to nsIFrame::GetLogicalUsedMargin(WritingMode).

As the patch stack in this bug is growing large, maybe do this in a follow-up.

Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ce2fcebace69
Part 1 - Move a ReflowInput's constructor down in the file so that both constructors are near. r=layout-reviewers,jfkthame
https://hg.mozilla.org/integration/autoland/rev/d7318b9d7389
Part 2 - Rename aWM to aCBWM for some SizeComputationInput's methods. r=layout-reviewers,jfkthame
https://hg.mozilla.org/integration/autoland/rev/931eb62aaa5a
Part 3 - Remove SizeComputationInput::ComputedPhysicalMargin() that returns a writable-reference. r=layout-reviewers,jfkthame
https://hg.mozilla.org/integration/autoland/rev/f80eefc2409e
Part 4 - Remove SizeComputationInput::ComputedPhysicalPadding() that returns a writable-reference. r=layout-reviewers,jfkthame
https://hg.mozilla.org/integration/autoland/rev/c94697ae9905
Part 5 - Remove SizeComputationInput::ComputedPhysicalBorderPadding() that returns a writable-reference. r=layout-reviewers,jfkthame
https://hg.mozilla.org/integration/autoland/rev/f1ab1d9462e7
Part 6 - Convert ReflowInput::Init()'s optional border and padding parameters to Maybe<LogicalMargin>. r=layout-reviewers,jfkthame
https://hg.mozilla.org/integration/autoland/rev/46a25fbea18d
Part 7 - Store SizeComputationInput's computed margin, border/padding, and padding in logical coordinates. r=layout-reviewers,jfkthame
https://hg.mozilla.org/integration/autoland/rev/9fea7d170f8d
Part 8 - Remove SizeComputationInput's one-argument methods to set margin, border/padding, and padding. r=layout-reviewers,jfkthame
Blocks: 1674931
You need to log in before you can comment on or make changes to this bug.