Don't use a binary tree representation for calc() sums / products.
Categories
(Core :: CSS Parsing and Computation, task, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox74 | --- | fixed |
People
(Reporter: emilio, Assigned: emilio)
References
Details
Attachments
(1 file)
|
Assignee | |
Comment 1•4 years ago
|
||
Keep a flat list of sum members. Simpify product and division ASAP.
I want to preserve the tree for a bit longer to implement min / max / clamp.
This doesn't do anything for it that we weren't doing already, but it helps to
eventually keep this specified representation and the equivalent computed
representation for <length-percentage> values.
|
|
Assignee | |
Updated•4 years ago
|
|
||
Comment 2•4 years ago
|
||
(Sorry for posting here and not on Phabricator, but the latter hates me.)
Lines 260-272 straightforwardly distribute scalar multiplication over min/max/clamp, but that's not actually right for negative values: -1 * min(a, b) = max(-a, -b) and vice-versa; -1 * clamp(a, x, b) = clamp(-b, -x, -a). (This might make for a good test case, if there's not one already.)
|
|
Assignee | |
Comment 3•4 years ago
|
||
(In reply to quasicomputational from comment #2)
(Sorry for posting here and not on Phabricator, but the latter hates me.)
Lines 260-272 straightforwardly distribute scalar multiplication over min/max/clamp, but that's not actually right for negative values: -1 * min(a, b) = max(-a, -b) and vice-versa; -1 * clamp(a, x, b) = clamp(-b, -x, -a). (This might make for a good test case, if there's not one already.)
Oh, indeed, d'oh! Will add a test for this.
|
|
Assignee | |
Comment 4•4 years ago
|
||
(In reply to quasicomputational from comment #2)
(Sorry for posting here and not on Phabricator, but the latter hates me.)
Lines 260-272 straightforwardly distribute scalar multiplication over min/max/clamp, but that's not actually right for negative values: -1 * min(a, b) = max(-a, -b) and vice-versa; -1 * clamp(a, x, b) = clamp(-b, -x, -a). (This might make for a good test case, if there's not one already.)
FWIW, this was fixed, and some existing tests caught it. I enabled those tests in automation to get some coverage.
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8e28baddb10b Don't use a binary tree representation for calc() sums / products. r=heycam
|
||
Comment 6•4 years ago
|
||
| bugherder | ||
Description
•