-
Notifications
You must be signed in to change notification settings - Fork 142
[css-typed-om] Simplifying expressions in CSSMathValue subclasses. #503
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
.add(), .sub(), and .negate() all have special cases. (.sub() has it by virtue of calling into the .add() algo.) .mul() could have a special-case here - check if .div() could have a special case - if this+values are all .invert() could special-case a number type. I agree that .min() and .max() could have the exact same special-case as .add(). Happy to add these in if others think they're good ideas. |
The Working Group just discussed
The full IRC log of that discussion<nainar> Topic: Simplifying expressions in CSSMathValue subclasses<TabAtkins> GitHub: https://github.com//issues/503 <nainar> TabAtkins: Math operations in class - you can say px.add(some other px value) <nainar> TabAtkins: some operations have shortcuts to return the operation. Have it for add, negate, sub. Dont have it for mul, div, invert, min, max <nainar> TabAtkins: Have a sketch in the issue of how to do it. Apply these shortcuts to all operations or none? <nainar> TabAtkins: Add always returns a CSSMathSum or a unit vaoue if easily computable? <nainar> Rossen: Similiar with what we do to calc? You simplify it down <TabAtkins> https://drafts.css-houdini.org/css-typed-om/#dom-cssnumericvalue-add <nainar> TabAtkins: Yes <nainar> Rossen: would align with what we already expose <nainar> TabAtkins: People who want to increment length by 1 unit will still get a length unit and not complicated expression <nainar> dholbert: addition inside of a loop - would be a good idea <nainar> Rossen: it keeps expanding. <nainar> RESOLVED: mul/div/invert/max/min should have shortcut just like add/negate/sub |
To match behaviour specified in [1], we fix the "shortcut" logic for mul, div and invert. Normally, when we mul/div/invert, we would return a new CSSMathProduct/Product/Invert, respectively. However, if all the arguments are CSSUnitValues then we might be able to return a CSSUnitValue (e.g. 1.mul(2) is just 2 and not CSSMathProduct(1, 2)). Whether we shortcut or not depends on the units of the arguments: - mul: Only shortcut if there's at most one non-number value. - div: Only shortcut if the arguments are all number values (note we get this for free since div is just multiplying the inverses). - invert: Only shortcut if its a number value. These rules ensure that the result can be stored as a CSSUnitValue (e.g. 2px.invert cannot be represented as a CSSUnitValue as its unit is px^-1). [1] w3c/css-houdini-drafts#503 Bug: 545318 Change-Id: I0c037c0a9026119097e85409efb0919a68e676f8 Reviewed-on: https://chromium-review.googlesource.com/866525 Reviewed-by: nainar <[email protected]> Commit-Queue: Darren Shen <[email protected]> Cr-Commit-Position: refs/heads/master@{#529354}
In the algorithm for adding CSSNumericValues, we have an explicit step (Step 3) for simplifying the expression if all values are
CSSUnitValues
with the same unit. However, none of the other subclasses seem to have this step, even though theoretically it should be possible (e.g.min(2px, 3px) == 2px
). The inconsistency is a bit confusing and makes addition a special case in terms of implementation. Is there any particular reason for making addition simplify but not the others?The text was updated successfully, but these errors were encountered: