Summary: | [css-ui] Fix interpolation of accent-color | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Aditya Keerthi <akeerthi> | ||||||
Component: | Animations | Assignee: | Aditya Keerthi <akeerthi> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | dino, graouts, graouts, koivisto, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | Other | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Aditya Keerthi
2021-10-08 11:53:43 PDT
Created attachment 440652 [details]
Patch
Comment on attachment 440652 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440652&action=review Very nice! Thanks for catching this and fixing it Aditya. > Source/WebCore/animation/CSSPropertyAnimation.cpp:1490 > + auto& blendingRenderStyle = context.progress < 0.5 ? from : to; If canInterpolate(from, to) is false, then progress should have already been set to either 0 or 1 in CSSPropertyAnimation::blendProperties(). I think you should ASSERT() that you're only dealing with 0 or 1 here and change this line to `auto& blendingRenderStyle = context.progress ? from : to;`. Comment on attachment 440652 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440652&action=review Very nice! Thanks for catching this and fixing it Aditya. >> Source/WebCore/animation/CSSPropertyAnimation.cpp:1490 >> + auto& blendingRenderStyle = context.progress < 0.5 ? from : to; > > If canInterpolate(from, to) is false, then progress should have already been set to either 0 or 1 in CSSPropertyAnimation::blendProperties(). I think you should ASSERT() that you're only dealing with 0 or 1 here and change this line to `auto& blendingRenderStyle = context.progress ? from : to;`. If canInterpolate(from, to) is false, then progress should have already been set to either 0 or 1 in CSSPropertyAnimation::blendProperties(). I think you should ASSERT() that you're only dealing with 0 or 1 here and change this line to `auto& blendingRenderStyle = context.progress ? from : to;`. Created attachment 440865 [details]
Patch for landing
(In reply to Antoine Quint from comment #4) > Comment on attachment 440652 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=440652&action=review > > Very nice! Thanks for catching this and fixing it Aditya. Thanks for the review! > >> Source/WebCore/animation/CSSPropertyAnimation.cpp:1490 > >> + auto& blendingRenderStyle = context.progress < 0.5 ? from : to; > > > > If canInterpolate(from, to) is false, then progress should have already been set to either 0 or 1 in CSSPropertyAnimation::blendProperties(). I think you should ASSERT() that you're only dealing with 0 or 1 here and change this line to `auto& blendingRenderStyle = context.progress ? from : to;`. > > If canInterpolate(from, to) is false, then progress should have already been > set to either 0 or 1 in CSSPropertyAnimation::blendProperties(). I think you > should ASSERT() that you're only dealing with 0 or 1 here and change this > line to `auto& blendingRenderStyle = context.progress ? from : to;`. Added the assertion. I think you meant `auto& blendingRenderStyle = context.progress ? to : from;` :) (In reply to Aditya Keerthi from comment #6) > (In reply to Antoine Quint from comment #4) > > Comment on attachment 440652 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=440652&action=review > I think you meant `auto& blendingRenderStyle = context.progress ? to : > from;` :) Absolutely! Committed r283980 (242826@main): <https://commits.webkit.org/242826@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 440865 [details]. |