1598156 - [motion-1] Support "<basic-shape> || <coord-box>" for offset-path

Status: RESOLVED FIXED

(Core :: CSS Transitions and Animations, enhancement, P3)

Description

[Boris Chiou [:boris]]

Implement basic shapes for offset-path.

Note: The syntax might be changed because of w3c/fxtf-drafts/#369.

Comment 2

[Boris Chiou [:boris]]

Attachment: Bug 1598156 - Part 1: Rework GenericBasicShape.

The definition of <basic-shape> includes other types of basic shapes,
e.g. path(), xywh(), rect(). So we put them together to match the spec.
However, some properties only use the subset of basic shapes, so we use
the bitflags to choose the supported basic shape at the parse time.

Also, remove StyleBasicShapeType because no one uses it.

Only refactoring in this patch.

Comment 3

[Boris Chiou [:boris]]

Attachment: Bug 1598156 - Part 2: Add IgnoreFillRule for BasicShape parser.

For outline shapes, fill-rule should be ignored. We add the flag in
the parser of BasicShape, so offset-path can use this flag to ignore
fill-rule.

Note: "IgnoreFillRule" means the fill-rule is still in the function
syntax, but we don't use it. Alwasy using the default fill-rule so we don't
serialize it and it doesn't affect the interpolation.

No behavir change in this patch.

Comment 4

[Boris Chiou [:boris]]

Attachment: Bug 1598156 - Part 3: Introduce ShapePosition for BasicShape.

The omitted postion has special meaning for offset-path. It defaults to using
offset-position as the circle/ellipse center if the explicit center is
not given. So we have to introduce ShapePosition::None to represent this
case.

We omit this component if it is not given, when serializing offet-path,
and preserve the value even if it specifies the default value, for the
same reason.

Note: No behavior change on shape-outside and clip-path in this patch.

https://drafts.fxtf.org/motion-1/#valdef-offset-path-basic-shape

Comment 5

[Boris Chiou [:boris]]

Attachment: Bug 1598156 - Part 4: Use BasicShape for OffsetPath in style system.

We rewrite the data structure of OffsetPath to support all basic shapes.
However, We don't build the gfx::Path for basic shapes other than path()
for now.

Also, will add <coord-box> in the next patch.

Note: The serialize of the specified value of <position> should be
updated because we have to convert the keyword into percentage per the
spec. This should be done by Bug 1832691, so I didn't add tests here.

Comment 6

[Boris Chiou [:boris]]

Attachment: Bug 1598156 - Part 5: Add coord-box into offset-path property.

This patch makes sure the syntax of offset-path is:
offset-path: none | <offset-path> || <coord-box>.

Comment 7

[Boris Chiou [:boris]]

Attachment: Bug 1598156 - Part 1: Factor out the building of gfx::path into separate functions.

So we can reuse these gfx::Path creaters.

Comment 8

[Boris Chiou [:boris]]

Attachment: Bug 1598156 - Part 6: Build path for basic shapes.

Comment 9

[Boris Chiou [:boris]]

Attachment: Bug 1598156 - Part 8: Support Compositor animations for all basic shapes.

Comment 10

[Daniel Holbert [:dholbert]]

FWIW, I think this will make us pass these two interop-2023 WPTs:
https://wpt.fyi/results/css/motion/offset-path-shape.html (fixed by part 4, in style system at least)
https://wpt.fyi/results/css/motion/offset-path-geometry-box.html (fixed by part 5, in style system at least)

Comment 11

[Boris Chiou [:boris]]

Yes. And I still need one extra patch to make sure it renders properly for coord-box. Also, there are some new tests on the upstream repo:
offset-path-coord-box-001.html
offset-path-coord-box-002.html
offset-path-coord-box-003.html
I will check them as well.

Comment 12

[Phabricator Automation]

Comment on attachment 9336917 [details]
Bug 1598156 - Part 1: Rework GenericBasicShape.

Revision D179624 was moved to bug 1837305. Setting attachment 9336917 [details] to obsolete.

Comment 13

[Phabricator Automation]

Comment on attachment 9336918 [details]
Bug 1598156 - Part 2: Add IgnoreFillRule for BasicShape parser.

Revision D179625 was moved to bug 1837305. Setting attachment 9336918 [details] to obsolete.

Comment 14

[Phabricator Automation]

Comment on attachment 9336919 [details]
Bug 1598156 - Part 3: Introduce ShapePosition for BasicShape.

Revision D179626 was moved to bug 1837305. Setting attachment 9336919 [details] to obsolete.

Comment 15

[Phabricator Automation]

Comment on attachment 9336920 [details]
Bug 1598156 - Part 4: Use BasicShape for OffsetPath in style system.

Revision D179627 was moved to bug 1837305. Setting attachment 9336920 [details] to obsolete.

Comment 16

[Phabricator Automation]

Comment on attachment 9336921 [details]
Bug 1598156 - Part 5: Add coord-box into offset-path property.

Revision D179628 was moved to bug 1837305. Setting attachment 9336921 [details] to obsolete.

Comment 17

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #10)

FWIW, I think this will make us pass these two interop-2023 WPTs: [...]

Presumably/hopefully this also makes us pass all of the 17 tests that were just added to interop-2023 in https://github.com/web-platform-tests/interop/issues/327#issuecomment-1582309275

Comment 18

[Boris Chiou [:boris]]

Attachment: Bug 1598156 - Part 3: Remove RayReferenceData.

The data in PathReferenceData could be used for basic shape as well, so
I'd like to rename it. Also, use nsPoint for the current position and
use nsRect for the rect of containing block so we can reuse some
functions in ShapeUtils without redundant conversions, for Bug 1598156.

Also, fix the missing serialization of mBorderBox in layers message.

Comment 19

[Boris Chiou [:boris]]

Attachment: Bug 1598156 - Part 2: Rename Path as Shape in OffsetPathData.

Because we'd like to use it for all basic shapes.

No behavior change.

Comment 20

[Boris Chiou [:boris]]

Attachment: Bug 1598156 - Part 4: Rename ComputeRayOrigin as ComputePosition.

We'd like to reuse it to compute the center position for circle() and
ellipse(), so use a better function name.

No behavior change.

Comment 21

[Boris Chiou [:boris]]

Attachment: Bug 1598156 - Part 5: Add some helper functions to check the type of StyleOffsetPath.

The data structure of StyleOffsetPath is complicated, so adding some
useful functions to simplify the code.

No behavior change.

Comment 22

[Boris Chiou [:boris]]

Attachment: Bug 1598156 - Part 7: Support <coord-box> without any function.

I tweak css/motion/offset-path-coord-box-001.html a little bit to
avoid using floating-point number in the reference file.

For offset-path-coord-box-002.html and offset-path-coord-box-003.html,
I make the border width larger so it'd be clear to see the difference.
Also the original reference files are not correct because they always use
border-box, so I also fix them to use the correct box.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ef9f8655ad63
Part 1: Factor out the building of gfx::path into separate functions. r=emilio
https://hg.mozilla.org/integration/autoland/rev/4cfb1ecae1ef
Part 2: Rename Path as Shape in OffsetPathData. r=emilio
https://hg.mozilla.org/integration/autoland/rev/ffab15c6d340
Part 3: Remove RayReferenceData. r=emilio
https://hg.mozilla.org/integration/autoland/rev/4ecee4746253
Part 4: Rename ComputeRayOrigin as ComputePosition. r=emilio
https://hg.mozilla.org/integration/autoland/rev/d05733192617
Part 5: Add some helper functions to check the type of StyleOffsetPath. r=emilio
https://hg.mozilla.org/integration/autoland/rev/e6f1d47e29a7
Part 6: Build path for basic shapes. r=emilio
https://hg.mozilla.org/integration/autoland/rev/588cd3f02088
Part 7: Support <coord-box> without any function. r=emilio
https://hg.mozilla.org/integration/autoland/rev/e7b972f1cc78
Part 8: Support Compositor animations for all basic shapes. r=emilio,hiro

Comment 24

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/40760 for changes under testing/web-platform/tests

Comment 25

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/ef9f8655ad63
https://hg.mozilla.org/mozilla-central/rev/4cfb1ecae1ef
https://hg.mozilla.org/mozilla-central/rev/ffab15c6d340
https://hg.mozilla.org/mozilla-central/rev/4ecee4746253
https://hg.mozilla.org/mozilla-central/rev/d05733192617
https://hg.mozilla.org/mozilla-central/rev/e6f1d47e29a7
https://hg.mozilla.org/mozilla-central/rev/588cd3f02088
https://hg.mozilla.org/mozilla-central/rev/e7b972f1cc78

Comment 26

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 28

[Dipika]

MDN doc updates for this feature can be tracked via this issue: https://github.com/mdn/content/issues/27747.