1714238 - Share data structure for SVGPathData::mData and nsStyleSVGRest::mD

Status: RESOLVED FIXED

(Core :: SVG, enhancement)

Description

[Boris Chiou [:boris]]

We should use the similar data structure for both SVGPathData::mData and nsStyleSVGReset::mD (which is added by Bug 1340422). For now we still need a conversion from SVGPathData::mData to nsStyleSVGReset::mD when handling SVG d attribute on path element. If we share the same data structue (e.g. use StylePathComand everywhere), we can avoid unwanted conversion, for optimization.

Note: nsStyleSVGReset::mD uses the same data structure with offset-path path() function.

Comment 1

[Boris Chiou [:boris]]

I update the dependency because this bug is still dependent on the removal of SVGPathSeg code, so we don't have to keep SVGPathData::mData for the DOM API. However, the shipment of d property is not strongly dependent on this bug (i.e. we just need to do some more conversions). So shipping the d property without this should be fine.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1714238 - Use a single SVG path representation. r=boris,longsonr

This removes a bunch of special and complex code from the SVG path code,
and simplifies fixing stuff like bug 1903361.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1714238 - Fix some tentative WPT tests to align with CSS animations and new Gecko behavior / Safari. r=longsonr

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Hey Brian / Daniel, just sanity-checking... I don't think there's a good reason why our CSS animation behavior and SMIL behavior for path interpolation is different right? https://phabricator.services.mozilla.com/D214594 in particular shows the behavior difference. I'm aligning on the CSS behavior of always absolutizing paths before interpolation, rather than the SMIL behavior which is relativizing or absolutizing depending on which one is the right side of the interpolation (this code).

I guess https://svgwg.org/svg2-draft/paths.html#DProperty doesn't say how to interpolate one way or the other...

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I filed https://github.com/w3c/svgwg/issues/947

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

I'm aligning on the CSS behavior of always absolutizing paths before interpolation, rather than the SMIL behavior which is relativizing or absolutizing depending on which one is the right side of the interpolation

That sounds fine to me. I don't recall there being a particular reason to prefer the behavior that you're replacing here (and it sounds like it doesn't have any impact on the geometry, if I'm understanding correctly)

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ff2a3385da3e
Use a single SVG path representation. r=boris,longsonr,webidl,smaug
https://hg.mozilla.org/integration/autoland/rev/2233e7aef1d7
Fix some tentative WPT tests to align with CSS animations and new Gecko behavior / Safari. r=longsonr

Comment 8

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

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

Comment 9

[Brian Birtles (:birtles)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

Hey Brian / Daniel, just sanity-checking... I don't think there's a good reason why our CSS animation behavior and SMIL behavior for path interpolation is different right?

I'm afraid I don't recall any particular reason for aligning with the value on the RHS. It looks like heycam wrote that code. I think aligning with CSS is fine, particularly because the SVG spec doesn't even seem to require converting between relative/absolute commands ("exactly the same number and types of path data commands", emphasis added)

Comment 10

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/ff2a3385da3e
https://hg.mozilla.org/mozilla-central/rev/2233e7aef1d7

Comment 11

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

Upstream PR merged by moz-wptsync-bot

Comment 12

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

Upstream PR merged by moz-wptsync-bot