2038219 - Avoid extra work and allocations when resolving non-length calc expressions.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P3)

Description

[Emilio Cobos Álvarez [:emilio]]

Bug 2035572 introduces a pattern like:

calc.node.with_computed_context(context).resolve()

Where with_computed_context will traverse the tree and allocate a bunch of nodes, and resolve() will do the same again.

Instead, it seems it'd be easier to just pass an Option<&Context> down to resolve() and friends, or something along those lines?

Comment 1

[Sajid Anwar [:sajidanwar]]

Attachment: Bug 2038219 - Resolve CalcNumeric in a single calc tree pass. r=#style

Comment 2

[Sajid Anwar [:sajidanwar]]

Attachment: Bug 2038219 - Re-introduce specified CalcLengthPercentage as newtype. r=#style

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/acdfef6ab551
https://hg.mozilla.org/integration/autoland/rev/0fbdc86f8666
Resolve CalcNumeric in a single calc tree pass. r=firefox-style-system-reviewers,emilio
https://github.com/mozilla-firefox/firefox/commit/15959078236a
https://hg.mozilla.org/integration/autoland/rev/1b655ed2cf31
Re-introduce specified CalcLengthPercentage as newtype. r=firefox-style-system-reviewers,emilio

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/928268942327
https://hg.mozilla.org/integration/autoland/rev/ace061dc3060
Revert "Bug 2038219 - Re-introduce specified CalcLengthPercentage as newtype. r=firefox-style-system-reviewers,emilio" for causing wpt failures @calc-infinity-nan-computed.html.

Comment 5

[agoloman]

Backed out for causing wpt failures @calc-infinity-nan-computed.html.

• Backout link
• Push with failures
• Failure Log

Comment 6

[Sajid Anwar [:sajidanwar]]

Failure here was from having moved the normalize(value).min(f32::MAX).max(f32::MIN) to inside the CalcNumeric::resolve, which made that fire on the computed angle degrees before performing the Infinity/NaN → 0deg replacement. That causes the Infinity values to turn into f32::MAX/MIN which are nonsensical for angles.

Going to push an update to the patch to move the normalize/min/max back out to the individual callers so that it can be skipped for angle.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f6f7bc5b3ba4
https://hg.mozilla.org/integration/autoland/rev/839c4f0555e6
Resolve CalcNumeric in a single calc tree pass. r=firefox-style-system-reviewers,emilio
https://github.com/mozilla-firefox/firefox/commit/95c3711a2d14
https://hg.mozilla.org/integration/autoland/rev/c5b468db452a
Re-introduce specified CalcLengthPercentage as newtype. r=firefox-style-system-reviewers,emilio

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/839c4f0555e6
https://hg.mozilla.org/mozilla-central/rev/c5b468db452a