[Web Animations] Add support for composite operations for software animations
Created attachment 438469 [details] Patch for EWS
Created attachment 438499 [details] Patch for EWS
Created attachment 438651 [details] Patch for EWS
Created attachment 438652 [details] Patch for EWS
<rdar://problem/83495091>
Created attachment 441708 [details] Patch for EWS
Created attachment 441716 [details] Patch for EWS
Created attachment 441867 [details] Patch for EWS
Created attachment 441868 [details] Patch for EWS
Created attachment 441992 [details] Patch for EWS
Created attachment 442025 [details] Patch
Created attachment 442027 [details] Patch
Comment on attachment 442027 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=442027&action=review > Source/WebCore/animation/CSSPropertyAnimation.cpp:1343 > + shadow = shadow ? shadow->next() : 0; nullptr instead of 0 > Source/WebCore/animation/CSSPropertyAnimation.cpp:1388 > + shadow = shadow ? shadow->next() : 0; ditto Also, why not make this a static function so you don't need to duplicate it? > Source/WebCore/platform/animation/AnimationUtilities.h:57 > + return static_cast<unsigned>(lround(to > from ? static_cast<double>(from) + static_cast<double>(to - from) * context.progress : static_cast<double>(from) - static_cast<double>(from - to) * context.progress)); to > from ? from + (to-from) * p : from - (from-to) * p to > from ? from + to*p - from*p : from - from*p + to*p to > from ? from + to*p - from*p : from + to*p - from*p Aren't both sides the same? This explains why you don't check for to > from in the other blends. > Source/WebCore/platform/graphics/transforms/AffineTransform.cpp:387 > + if (compositeOperation != CompositeOperation::Replace) { I think it might be more understandable if you explicitly test for Add or Accumulate > Source/WebCore/platform/graphics/transforms/RotateTransformOperation.cpp:63 > - FloatPoint3D vector { static_cast<float>(op.m_x), static_cast<float>(op.m_y), static_cast<float>(op.m_z) }; > - vector.normalize(); > - return vector; > + auto length = std::hypot(op.m_x, op.m_y, op.m_z); > + return { static_cast<float>(op.m_x / length), static_cast<float>(op.m_y / length), static_cast<float>(op.m_z / length) }; Seems unrelated, but fine. > Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:1641 > + if (from != to) Don't think you need this test.
(In reply to Dean Jackson from comment #13) > Comment on attachment 442027 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=442027&action=review > > > Source/WebCore/animation/CSSPropertyAnimation.cpp:1343 > > + shadow = shadow ? shadow->next() : 0; > > nullptr instead of 0 > > > Source/WebCore/animation/CSSPropertyAnimation.cpp:1388 > > + shadow = shadow ? shadow->next() : 0; > > ditto Will fix in commit. > Also, why not make this a static function so you don't need to duplicate it? I'll definitely look into that. > > Source/WebCore/platform/animation/AnimationUtilities.h:57 > > + return static_cast<unsigned>(lround(to > from ? static_cast<double>(from) + static_cast<double>(to - from) * context.progress : static_cast<double>(from) - static_cast<double>(from - to) * context.progress)); > > to > from ? from + (to-from) * p : from - (from-to) * p > to > from ? from + to*p - from*p : from - from*p + to*p > to > from ? from + to*p - from*p : from + to*p - from*p > > Aren't both sides the same? They are! Will fix in commit. > > Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:1641 > > + if (from != to) > > Don't think you need this test. Correct, will fix in commit.
Created attachment 442130 [details] Patch for landing
(In reply to Antoine Quint from comment #14) > (In reply to Dean Jackson from comment #13) > > Comment on attachment 442027 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=442027&action=review > > > > > Source/WebCore/animation/CSSPropertyAnimation.cpp:1343 > > > + shadow = shadow ? shadow->next() : 0; > > > > Also, why not make this a static function so you don't need to duplicate it? > > I'll definitely look into that. I've looked into it and addressed it in the patch for landing.
Created attachment 442176 [details] Patch for landing
Created attachment 443528 [details] Patch for landing
Committed r285397 (243954@main): <https://commits.webkit.org/243954@main>
*** Bug 224738 has been marked as a duplicate of this bug. ***