Skip to content

Remove overrides_transform_style() and get_used_transform_style() - #388

Merged
mrobinson merged 1 commit into
servo:mainfrom
mrobinson:complete-overrides_transform_style
Jun 16, 2026
Merged

Remove overrides_transform_style() and get_used_transform_style()#388
mrobinson merged 1 commit into
servo:mainfrom
mrobinson:complete-overrides_transform_style

Conversation

@mrobinson

@mrobinson mrobinson commented Jun 12, 2026

Copy link
Copy Markdown
Member

It's not possible to properly calculate these only looking at style,
because the results depends on the used overflow value.

Servo PR: servo/servo#45629

Comment thread style/properties/properties.mako.rs Outdated
Comment on lines +2149 to +2150
!matches!(self.get_box().clone_overflow_x(), Overflow::Visible | Overflow::Clip) ||
!matches!(self.get_box().clone_overflow_y(), Overflow::Visible | Overflow::Clip) ||

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this check the used overflow? E.g. a table row is a transformable element so transform-style applies, but overflow doesn't. Or propagation of overflow to the viewport.

If so, this should move to Servo. May make sense to move it regardless.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I've modified this PR to simply remove this functionality, have moved it to servo/servo#45629, and ensured that there the used overflow is considered when determining the used TransformStyle.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems you didn't push the removal?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the typical situation where I had this in my working directory, but uncommitted. I've pushed it now.

@mrobinson
mrobinson force-pushed the complete-overrides_transform_style branch from 88ea543 to 033af4f Compare June 13, 2026 06:04
@mrobinson mrobinson changed the title Make the implementation of overrides_transform_style() more complete Remove overrides_transform_style() and get_used_transform_style() Jun 13, 2026
mrobinson added a commit to mrobinson/servo that referenced this pull request Jun 13, 2026
Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-iamge`, and `mask-boarder-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
servo-wpt-sync pushed a commit to servo/wpt that referenced this pull request Jun 13, 2026
Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-iamge`, and `mask-boarder-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
@nicoburns

nicoburns commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

One thing I've thought about for these kind of values is that it could be useful to pre-compute the bits that can be computed purely from style information and store them on the ComputedValues/ComputedValuesInner in a bitset (I am imagining that this would be a bit of Servo-only code that runs during the style pass as/just after the computed values for a node are computed). That would make it much cheaper to check rather than doing comparisons on several style properties each time (some of which like the transforms can be quite large).

Not sure how much different it would make to performance, but it'd be an easy optimization to implement, and might also be quite nice in terms of "making the code structure reflect the spec"?

@mrobinson

Copy link
Copy Markdown
Member Author

Not sure how much different it would make to performance, but it'd be an easy optimization to implement, and might also be quite nice in terms of "making the code structure reflect the spec"?

That's an interesting idea. I think our plan in Servo is to cache these kinds of things on the Fragment, but maybe there is room for multiple layers of caching.

It's not possible to properly calculate these only looking at style,
because the results depends on the used `overflow` value.

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
@mrobinson
mrobinson force-pushed the complete-overrides_transform_style branch from 033af4f to 33eb615 Compare June 14, 2026 18:26
mrobinson added a commit to mrobinson/servo that referenced this pull request Jun 14, 2026
Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-iamge`, and `mask-boarder-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
servo-wpt-sync pushed a commit to servo/wpt that referenced this pull request Jun 14, 2026
Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-iamge`, and `mask-boarder-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
mrobinson added a commit to mrobinson/servo that referenced this pull request Jun 14, 2026
Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-iamge`, and `mask-boarder-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
servo-wpt-sync pushed a commit to servo/wpt that referenced this pull request Jun 14, 2026
Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-iamge`, and `mask-boarder-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
@mrobinson
mrobinson added this pull request to the merge queue Jun 16, 2026
Merged via the queue into servo:main with commit 85b34b0 Jun 16, 2026
5 checks passed
@mrobinson
mrobinson deleted the complete-overrides_transform_style branch June 16, 2026 11:40
mrobinson added a commit to mrobinson/servo that referenced this pull request Jun 16, 2026
Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-iamge`, and `mask-boarder-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
servo-wpt-sync pushed a commit to servo/wpt that referenced this pull request Jun 16, 2026
Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-iamge`, and `mask-boarder-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
mrobinson added a commit to mrobinson/servo that referenced this pull request Jun 16, 2026
Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-iamge`, and `mask-boarder-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
servo-wpt-sync pushed a commit to servo/wpt that referenced this pull request Jun 16, 2026
Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-iamge`, and `mask-boarder-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
mrobinson added a commit to mrobinson/servo that referenced this pull request Jun 16, 2026
Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-iamge`, and `mask-boarder-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
servo-wpt-sync pushed a commit to servo/wpt that referenced this pull request Jun 16, 2026
Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-iamge`, and `mask-boarder-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
servo-wpt-sync pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 16, 2026
Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-iamge`, and `mask-boarder-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
veyndan pushed a commit to veyndan/servo that referenced this pull request Jun 16, 2026
servo#45629)

Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-image`, and `mask-border-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Testing: This adds a new crashtest. Three other WPT tests start to pass.
Fixes: servo#42476.

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
lando-worker Bot pushed a commit to mozilla-firefox/firefox that referenced this pull request Jun 18, 2026
…`TransformStyle` more complete, a=testonly

Automatic update from web-platform-tests
layout: Make the implementation of used `TransformStyle` more complete

Stylo was using a very incomplete method for calculating the used
`TransformStyle` for boxes, which didn't incorporate all of the grouping
properties from the specification. This change moves the functionality
into Servo (as cannot properly compute this value without
`FragmentFlags`) and makes it more complete. We still do not process the
value of `contain`, `mask-iamge`, and `mask-boarder-source` as we do not
support those values yet.

Incidentally, this also fixes a panic that we were causing in WebRender,
probably by going down a path that Gecko does not exercise.

Stylo PR: servo/stylo#388

Signed-off-by: Martin Robinson <martin@abandonedwig.info>

--

wpt-commits: 3b5dbefa6117860e83e16416778955a035962e05
wpt-pr: 60593
mu-mostafa98 pushed a commit to mu-mostafa98/stylo that referenced this pull request Jun 22, 2026
Fixes a compile warning from servo#388.

Signed-off-by: Oriol Brufau <obrufau@igalia.com>
Loirooriol pushed a commit that referenced this pull request Jul 1, 2026
…#388)

It's not possible to properly calculate these only looking at style,
because the results depends on the used `overflow` value.

Servo PR: servo/servo#45629

Signed-off-by: Martin Robinson <martin@abandonedwig.info>
Loirooriol added a commit that referenced this pull request Jul 1, 2026
Fixes a compile warning from #388.

Signed-off-by: Oriol Brufau <obrufau@igalia.com>
lando-worker Bot pushed a commit to mozilla-firefox/firefox that referenced this pull request Jul 7, 2026
…sform_style()`. r=firefox-style-system-reviewers,emilio

It's not possible to properly calculate these only looking at style,
because the results depends on the used `overflow` value.

Imports servo/stylo#388
Imports servo/stylo#398

Differential Revision: https://phabricator.services.mozilla.com/D310917
github-actions Bot pushed a commit to longvatrong111/stylo that referenced this pull request Jul 8, 2026
…sform_style()`. r=firefox-style-system-reviewers,emilio

It's not possible to properly calculate these only looking at style,
because the results depends on the used `overflow` value.

Imports servo#388
Imports servo#398

Differential Revision: https://phabricator.services.mozilla.com/D310917
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants