-
Notifications
You must be signed in to change notification settings - Fork 639
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
[css-values-5] issues with interpolation rules for calc-size() #10220
Comments
Nope, it takes one as well:
Right, it's meant to defer to that preceding section, which gives the behavior when you have two calc-size()s. That might indeed mean that they end up not being interpolable. How could I make this more clear? |
Ah, ok. This is what happens when the spec gets written halfway through the implementation and I don't carefully reread all the bits I've already implemented before the spec existed. That said, the behavior specified here behaves weirdly for percentages -- an animation between As to clarifying the sentence to be clearer, how about (ignoring any of the other issues):
|
Right, but the alternative is that that value pair doesn't interpolate at all, because it's two different intrinsic behaviors. I decided it was more useful to allow it to work in the case where the % is resolving against a definite size, than to strictly preserve percent-ness when auto-upgrading percentages into calc-size() (which would usually just fail to transition at all).
Yeah, that suggested text sounds good, thanks! |
…terpolation paragraph for clarity. #10220
The rules for animation of calc-size() intentionally allow animation between calc-size() expressions typed as something other than a percentage (e.g., a sizing keyword) and a percentage. This animation erases the percentage-ness from the type at all intermediate values. This means the animation works correctly when the percentage basis is definite, but is incorrect when the percentage basis is indefinite. This is an intentional behavior compromise discussed in w3c/csswg-drafts#10220 . This adds a few tests that would hit the removed DCHECK(). Bug: 313072 Change-Id: If5abd44475742272a4635293ddc12837f3ae7153
The rules for animation of calc-size() intentionally allow animation between calc-size() expressions typed as something other than a percentage (e.g., a sizing keyword) and a percentage. This animation erases the percentage-ness from the type at all intermediate values. This means the animation works correctly when the percentage basis is definite, but is incorrect when the percentage basis is indefinite. This is an intentional behavior compromise discussed in w3c/csswg-drafts#10220 . This adds a few tests that would hit the removed DCHECK(). Bug: 313072 Change-Id: If5abd44475742272a4635293ddc12837f3ae7153 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5480311 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: David Baron <dbaron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1292038}
The rules for animation of calc-size() intentionally allow animation between calc-size() expressions typed as something other than a percentage (e.g., a sizing keyword) and a percentage. This animation erases the percentage-ness from the type at all intermediate values. This means the animation works correctly when the percentage basis is definite, but is incorrect when the percentage basis is indefinite. This is an intentional behavior compromise discussed in w3c/csswg-drafts#10220 . This adds a few tests that would hit the removed DCHECK(). Bug: 313072 Change-Id: If5abd44475742272a4635293ddc12837f3ae7153 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5480311 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: David Baron <dbaron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1292038}
The rules for animation of calc-size() intentionally allow animation between calc-size() expressions typed as something other than a percentage (e.g., a sizing keyword) and a percentage. This animation erases the percentage-ness from the type at all intermediate values. This means the animation works correctly when the percentage basis is definite, but is incorrect when the percentage basis is indefinite. This is an intentional behavior compromise discussed in w3c/csswg-drafts#10220 . This adds a few tests that would hit the removed DCHECK(). Bug: 313072 Change-Id: If5abd44475742272a4635293ddc12837f3ae7153 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5480311 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: David Baron <dbaron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1292038}
…with calc-size()., a=testonly Automatic update from web-platform-tests Remove DCHECK() that is no longer valid with calc-size(). The rules for animation of calc-size() intentionally allow animation between calc-size() expressions typed as something other than a percentage (e.g., a sizing keyword) and a percentage. This animation erases the percentage-ness from the type at all intermediate values. This means the animation works correctly when the percentage basis is definite, but is incorrect when the percentage basis is indefinite. This is an intentional behavior compromise discussed in w3c/csswg-drafts#10220 . This adds a few tests that would hit the removed DCHECK(). Bug: 313072 Change-Id: If5abd44475742272a4635293ddc12837f3ae7153 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5480311 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: David Baron <dbaron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1292038} -- wpt-commits: 0c9b2d5a9efd8fa975f319b634fbb50b446e87c5 wpt-pr: 45858
…with calc-size()., a=testonly Automatic update from web-platform-tests Remove DCHECK() that is no longer valid with calc-size(). The rules for animation of calc-size() intentionally allow animation between calc-size() expressions typed as something other than a percentage (e.g., a sizing keyword) and a percentage. This animation erases the percentage-ness from the type at all intermediate values. This means the animation works correctly when the percentage basis is definite, but is incorrect when the percentage basis is indefinite. This is an intentional behavior compromise discussed in w3c/csswg-drafts#10220 . This adds a few tests that would hit the removed DCHECK(). Bug: 313072 Change-Id: If5abd44475742272a4635293ddc12837f3ae7153 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5480311 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: David Baron <dbaron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1292038} -- wpt-commits: 0c9b2d5a9efd8fa975f319b634fbb50b446e87c5 wpt-pr: 45858
One other issue with the rules is that they have frequent references to "the result’s calc-size basis". However, it's not clear to me whether this is intended to recursively extract the basis. For example, can you interpolate between My understanding is the intent of allowing Then the question is: if it is allowed, is the correct interpolation behavior to un-nest the (I suppose one could ask the same question about the rules for interpolation when the basis is a |
Ooh, you're right, it looks like I lost the text for this when moving it from the issue to the spec. I'll fix; the intent is indeed that the basis is recursive.
The behavior I want to preserve is that the obvious "simplification" one can theoretically do (ignoring, for the moment, billion laughs) results in identical behavior. In other words, The other behavior I want to preserve is that calc-size() interpolation is always linear - whatever size the endpoints resolve to, the interpolation acts as if you're just going between those two sizes. The above condition preserves this, tho I suddenly realize my interpolation rules for a len-per basis violate it - going from This is fixable, tho. I think what I want to do is, effectively, to pretend that a calc-size() with a len-per basis is already a "nested" calc-size, so I can move the actual len-per calculation into the sum argument. In other words, Does this sound reasonable? Both the "sequence of calculations" un-nesting concept, and the application of that to a len-per basis? Thoughts on actually defining a keyword for a percentage basis? |
I know I thought of doing this early in the design, actually, I just thought it was awkward to have to write It's possible I might be able to combine the approaches, tho, and say that |
Wait, damn, I actually still lose linear interpolation in this model; the spec specifies that you interpolate the basises. And I can't just naively expand the nested function into the outer one, even if I made "sequence of calculations" actually valid in the grammar, because both sides of the interpolation might have this. Example: The first will result in a final size of 50px (200px * .5 * .5). The second will result in 600px (200px + 100px * 2). Ideally we want the halfway point of the interpolation to be halfway between these two, at 325px. But how do we represent that? We don't get that result in the current spec text; current spec tells us to interpolate the basis and the calculation, resulting in (after simplification) What we want is to produce some form where only the final calculation is what's interpolated, so we don't get the double-interpolation quadratic behavior. A failed attempt I made just now was to allow "sequence of calculations" to be expressed directly in the syntax, but that still runs us into issues - simplifying into I think what we actually need is the ability to represent a calculation sequence as a first-class concept. Let's pretend that Then we can instead simplify the functions to That final calculation, then (again assuming the auto size is 200px) would be We don't necessarily need to make this a generic functionality; we could specialize it to calc-size() if we wanted, since we don't need it anywhere else so far. But if we eat the complexity of a calc sequence here, seems like we might as well do it generally anyway. |
All right, sorry, let's restate the important parts from these stream-of-consciousness posts.
Proposal:
This gives us linear interpolation in all cases, and slightly simplifies the spec text around nested calc-size() or percentage basises. |
I expect to have more thoughts on this later... but a first thought: What was the use case for allowing an inner |
The use-case is I also suspect that "stacked calculations" is probably something that might end up being useful in the future. At least, the concept of "store a sub-calculation in a local variable so you can use it repeatedly" has already been brought up in other issues (like |
Alternate proposal: instead of doing the stacked computations to avoid blowout, just substitute directly and allow UAs to fail if the substitution gets too large. That's what we already do with variables, after all. If it gets "too large" just treat the property as IACVT. That way we don't have to invent anything new. In other words, |
Yeah, I think that sounds fine, at least for now. |
I believe I initially implemented these before the spec was written (or before I was aware that it was). This updates the interpolation rules to follow https://drafts.csswg.org/css-values-5/#interp-calc-size plus the clarifications in w3c/csswg-drafts#10220 . This doesn't implement the IACVT (invalid at computed value time) aspect of the clarification in w3c/csswg-drafts#10220 and therefore adds both TODO comments and failing tests. (The tests are useful to check for lack of crashes; when I first wrote them they did crash and I needed to fix the crashes.) Bug: 313072 Change-Id: Ia9903bd138c3afb58ef12da45171920d28de8fa6
I believe I initially implemented these before the spec was written (or before I was aware that it was). This updates the interpolation rules to follow https://drafts.csswg.org/css-values-5/#interp-calc-size plus the clarifications in w3c/csswg-drafts#10220 . This doesn't implement the IACVT (invalid at computed value time) aspect of the clarification in w3c/csswg-drafts#10220 and therefore adds both TODO comments and failing tests. (The tests are useful to check for lack of crashes; when I first wrote them they did crash and I needed to fix the crashes.) Bug: 313072 Change-Id: Ia9903bd138c3afb58ef12da45171920d28de8fa6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5546294 Commit-Queue: David Baron <dbaron@chromium.org> Reviewed-by: Daniil Sakhapov <sakhapov@chromium.org> Cr-Commit-Position: refs/heads/main@{#1304382}
I believe I initially implemented these before the spec was written (or before I was aware that it was). This updates the interpolation rules to follow https://drafts.csswg.org/css-values-5/#interp-calc-size plus the clarifications in w3c/csswg-drafts#10220 . This doesn't implement the IACVT (invalid at computed value time) aspect of the clarification in w3c/csswg-drafts#10220 and therefore adds both TODO comments and failing tests. (The tests are useful to check for lack of crashes; when I first wrote them they did crash and I needed to fix the crashes.) Bug: 313072 Change-Id: Ia9903bd138c3afb58ef12da45171920d28de8fa6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5546294 Commit-Queue: David Baron <dbaron@chromium.org> Reviewed-by: Daniil Sakhapov <sakhapov@chromium.org> Cr-Commit-Position: refs/heads/main@{#1304382}
So I've mostly implemented that in Chromium in this CL, with the exception of the expansion failure cases. I didn't see a clean way to make them IACVT. Instead, they result in a 0 length. That's probably not ideal. With not too much more work I think I could make them instead fail to interpolate, though. But it's not clear to me if there's a good way to make the animation system say that an interpolation result is IACVT in a case where we have said that the values can be interpolated. |
I think that's fine; this is already an extreme corner case. I'm happy to spec whatever is reasonable to fall out of that case. I'll open a more general issue for IACVT-in-animations, just in case. |
Okay, I added simplification rules:
The interpolation rules have been slightly simplified, as a result, since now the basis is always a keyword by computed-value time. Notably, this now means that interpolation is properly linear at all times (there's a And per #10369, Brian also agrees that if the interpolated value would be IACVT, it should fall back to a discrete transition. |
Are there cases where substitution is needed other than interpolation? In other words, why does anything ever need to be IACVT here if we decide that values where interpolation would lead to too long a value can't be interpolated? |
No, there's not; the substitution behavior is necessary solely to produce linear interpolation behavior. So we could limit it solely to interpolation. I'm just generally inclined to do these sorts of canonicalization more eagerly. |
…terpolation rules., a=testonly Automatic update from web-platform-tests Correct implementation of calc-size() interpolation rules. I believe I initially implemented these before the spec was written (or before I was aware that it was). This updates the interpolation rules to follow https://drafts.csswg.org/css-values-5/#interp-calc-size plus the clarifications in w3c/csswg-drafts#10220 . This doesn't implement the IACVT (invalid at computed value time) aspect of the clarification in w3c/csswg-drafts#10220 and therefore adds both TODO comments and failing tests. (The tests are useful to check for lack of crashes; when I first wrote them they did crash and I needed to fix the crashes.) Bug: 313072 Change-Id: Ia9903bd138c3afb58ef12da45171920d28de8fa6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5546294 Commit-Queue: David Baron <dbaron@chromium.org> Reviewed-by: Daniil Sakhapov <sakhapov@chromium.org> Cr-Commit-Position: refs/heads/main@{#1304382} -- wpt-commits: 5d96f029e638e780bac314057ee680a0fe76ecde wpt-pr: 46413
…terpolation rules., a=testonly Automatic update from web-platform-tests Correct implementation of calc-size() interpolation rules. I believe I initially implemented these before the spec was written (or before I was aware that it was). This updates the interpolation rules to follow https://drafts.csswg.org/css-values-5/#interp-calc-size plus the clarifications in w3c/csswg-drafts#10220 . This doesn't implement the IACVT (invalid at computed value time) aspect of the clarification in w3c/csswg-drafts#10220 and therefore adds both TODO comments and failing tests. (The tests are useful to check for lack of crashes; when I first wrote them they did crash and I needed to fix the crashes.) Bug: 313072 Change-Id: Ia9903bd138c3afb58ef12da45171920d28de8fa6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5546294 Commit-Queue: David Baron <dbaron@chromium.org> Reviewed-by: Daniil Sakhapov <sakhapov@chromium.org> Cr-Commit-Position: refs/heads/main@{#1304382} -- wpt-commits: 5d96f029e638e780bac314057ee680a0fe76ecde wpt-pr: 46413
…with calc-size()., a=testonly Automatic update from web-platform-tests Remove DCHECK() that is no longer valid with calc-size(). The rules for animation of calc-size() intentionally allow animation between calc-size() expressions typed as something other than a percentage (e.g., a sizing keyword) and a percentage. This animation erases the percentage-ness from the type at all intermediate values. This means the animation works correctly when the percentage basis is definite, but is incorrect when the percentage basis is indefinite. This is an intentional behavior compromise discussed in w3c/csswg-drafts#10220 . This adds a few tests that would hit the removed DCHECK(). Bug: 313072 Change-Id: If5abd44475742272a4635293ddc12837f3ae7153 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5480311 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: David Baron <dbaron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1292038} -- wpt-commits: 0c9b2d5a9efd8fa975f319b634fbb50b446e87c5 wpt-pr: 45858
The rules for interpolating
calc-size()
(see #626) currently say:I see a few issues with this sentence.
First, the expression
calc-size(value)
at the end doesn't make sense, becausecalc-size()
takes two arguments. I think it might be that what this meant to say is "behaving as if the non-calc-size()
value value werecalc-size(
value, size)
. But there are other possible options here (although I think they have problems and this one is probably correct).Second, this sentence seems to imply that all such combinations can be interpolated. It seems to me, particularly if the above interpretation is correct (rather than instead being
calc-size(any,
value)
for<length-percentage>
values), that many such combinations cannot be interpolated, given the rules at the start of the section saying when twocalc-size()
values can be interpolated.The text was updated successfully, but these errors were encountered: