Skip to content
Permalink
Browse files
[web-animations] opacity should use unclamped values for from/to keyf…
…rames with iterationComposite

https://bugs.webkit.org/show_bug.cgi?id=248338

Reviewed by Antti Koivisto.

Our last failure in web-animations/animation-model/keyframe-effects/effect-value-iteration-composite-operation.html
was because of code that looks like this:

    const animation = div.animate({ opacity: [0, 0.4] }, { duration: 1000, iterations: 10, iterationComposite: 'accumulate' });
    animation.currentTime += animation.effect.getComputedTiming().duration * 2.5;
    assert_equals(getComputedStyle(div).opacity, '1', // (0.8 + 1.2) * 0.5
        'Animated opacity style in the middle of the third iteration');

We failed this test is because in the third iteration we would compute from/to kefyrames to be 0.8 and 1.0, instead
of 0.8 and 1.2. The reason this happens is because in KeyframeEffect::setAnimatedPropertiesInStyle(), we would compute
the from/to values by blending the from/to keyframes with the to keyframe with accumulation to match the current iteration
with CSSPropertyAnimation::blendProperties(). That method stores the resulting value in a RenderStyle which means in this
case RenderStyle::setOpacity() is called and the blended value is clamped to the specified [0-1] range for opacity.

Some property types require special blending code to run for accumulation, for instance transforms, filters, colors and
shadows. But for most other types where the encapsulated values are numbers, we can run some simple maths to accumulate
the values by the number of iterations. The math looks something like this:

    iterationIncrement = currentIteration * to;
    from += iterationIncrement;
    to += iterationIncrement;

We implement this logic in the various blending functions for float, int, etc. and add a requiresBlendingForAccumulativeIteration()
virtual method on animation wrappers such that methods that require the more complex blending code to be ran iteratively can do so.
This includes lengths and length-based types that will yield calc() values as well as the aforementioned transforms, filters,
colors and shadows.

Then in KeyframeEffect::setAnimatedPropertiesInStyle(), we call CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration()
to check whether we must run blending code to compute the from/to keyframes or simply let those values be computed in the final call
to CSSPropertyAnimation::blendProperties() at the end of the function when the composite operation is "replace".

To do this, we must also expose the "compositeIteration" and "currentIteration" values to the blending context, so we add those two
members to BlendingContext.

* LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-iteration-composite-operation-expected.txt:
* Source/WebCore/animation/CSSPropertyAnimation.cpp:
(WebCore::CSSPropertyBlendingContext::CSSPropertyBlendingContext):
(WebCore::blendFunc):
(WebCore::AnimationPropertyWrapperBase::requiresBlendingForAccumulativeIteration const):
(WebCore::lengthsRequireBlendingForAccumulativeIteration):
(WebCore::lengthVariantRequiresBlendingForAccumulativeIteration):
(WebCore::CSSPropertyAnimation::blendProperties):
(WebCore::CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration):
* Source/WebCore/animation/CSSPropertyAnimation.h:
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::setAnimatedPropertiesInStyle):
* Source/WebCore/platform/Length.cpp:
(WebCore::blend):
* Source/WebCore/platform/animation/AnimationUtilities.h:
(WebCore::BlendingContext::BlendingContext):
(WebCore::BlendingContext::isReplace const):
(WebCore::blend):

Canonical link: https://commits.webkit.org/257033@main
  • Loading branch information
graouts committed Nov 26, 2022
1 parent 7a2abd4 commit f0e70ac5078d742d598da042f4728d042aca3e98
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 11 deletions.
@@ -8,7 +8,7 @@ PASS iteration composition of <number> type animation
PASS iteration composition of <shape> type animation
PASS iteration composition of <calc()> value animation
PASS iteration composition of <calc()> value animation that the values can'tbe reduced
FAIL iteration composition of opacity animation assert_equals: Animated opacity style at 50s of the third iteration expected "1" but got "0.9"
PASS iteration composition of opacity animation
PASS iteration composition of box-shadow animation
PASS iteration composition of filter blur animation
PASS iteration composition of filter brightness for different unit animation
@@ -85,8 +85,8 @@ static TextStream& operator<<(TextStream& stream, CSSPropertyID property)
struct CSSPropertyBlendingContext : BlendingContext {
const CSSPropertyBlendingClient* client { nullptr };

CSSPropertyBlendingContext(double progress, bool isDiscrete, CompositeOperation compositeOperation, const CSSPropertyBlendingClient* client)
: BlendingContext(progress, isDiscrete, compositeOperation)
CSSPropertyBlendingContext(double progress, bool isDiscrete, CompositeOperation compositeOperation, const CSSPropertyBlendingClient* client, IterationCompositeOperation iterationCompositeOperation = IterationCompositeOperation::Replace, double currentIteration = 0)
: BlendingContext(progress, isDiscrete, compositeOperation, iterationCompositeOperation, currentIteration)
, client(client)
{
}
@@ -104,6 +104,12 @@ static inline double blendFunc(double from, double to, const CSSPropertyBlending

static inline float blendFunc(float from, float to, const CSSPropertyBlendingContext& context)
{
if (context.iterationCompositeOperation == IterationCompositeOperation::Accumulate && context.currentIteration) {
auto iterationIncrement = context.currentIteration * to;
from += iterationIncrement;
to += iterationIncrement;
}

if (context.compositeOperation == CompositeOperation::Replace)
return narrowPrecisionToFloat(from + (to - from) * context.progress);
return narrowPrecisionToFloat(from + from + (to - from) * context.progress);
@@ -720,6 +726,7 @@ class AnimationPropertyWrapperBase {

virtual bool isShorthandWrapper() const { return false; }
virtual bool isAdditiveOrCumulative() const { return true; }
virtual bool requiresBlendingForAccumulativeIteration(const RenderStyle&, const RenderStyle&) const { return false; }
virtual bool equals(const RenderStyle&, const RenderStyle&) const = 0;
virtual bool canInterpolate(const RenderStyle&, const RenderStyle&, CompositeOperation) const { return true; }
virtual void blend(RenderStyle&, const RenderStyle&, const RenderStyle&, const CSSPropertyBlendingContext&) const = 0;
@@ -948,6 +955,12 @@ static bool canInterpolateLengths(const Length& from, const Length& to, bool isL
return false;
}

static bool lengthsRequireBlendingForAccumulativeIteration(const Length& from, const Length& to)
{
// If blending the values can yield a calc() value, we must go through the blending code for iterationComposite.
return from.isCalculated() || to.isCalculated() || from.type() != to.type();
}

class LengthPropertyWrapper : public PropertyWrapperGetter<const Length&> {
WTF_MAKE_FAST_ALLOCATED;
public:
@@ -968,6 +981,11 @@ class LengthPropertyWrapper : public PropertyWrapperGetter<const Length&> {
return canInterpolateLengths(value(from), value(to), m_flags.contains(Flags::IsLengthPercentage));
}

bool requiresBlendingForAccumulativeIteration(const RenderStyle& from, const RenderStyle& to) const final
{
return lengthsRequireBlendingForAccumulativeIteration(value(from), value(to));
}

void blend(RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, const CSSPropertyBlendingContext& context) const override
{
auto valueRange = m_flags.contains(Flags::NegativeLengthsAreInvalid) ? ValueRange::NonNegative : ValueRange::All;
@@ -1004,6 +1022,14 @@ class LengthPointPropertyWrapper : public PropertyWrapperGetter<LengthPoint> {
}

private:
bool requiresBlendingForAccumulativeIteration(const RenderStyle& from, const RenderStyle& to) const final
{
auto fromLengthPoint = value(from);
auto toLengthPoint = value(to);
return lengthsRequireBlendingForAccumulativeIteration(fromLengthPoint.x(), toLengthPoint.x())
|| lengthsRequireBlendingForAccumulativeIteration(fromLengthPoint.y(), toLengthPoint.y());
}

void blend(RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, const CSSPropertyBlendingContext& context) const final
{
(destination.*m_setter)(blendFunc(value(from), value(to), context));
@@ -1034,6 +1060,17 @@ class LengthPointOrAutoPropertyWrapper final : public LengthPointPropertyWrapper
}
};

static bool lengthVariantRequiresBlendingForAccumulativeIteration(const LengthSize& from, const LengthSize& to)
{
return lengthsRequireBlendingForAccumulativeIteration(from.width, to.width)
|| lengthsRequireBlendingForAccumulativeIteration(from.height, to.height);
}

static bool lengthVariantRequiresBlendingForAccumulativeIteration(const GapLength& from, const GapLength& to)
{
return from.isNormal() || to.isNormal() || lengthsRequireBlendingForAccumulativeIteration(from.length(), to.length());
}

template <typename T>
class LengthVariantPropertyWrapper final : public PropertyWrapperGetter<const T&> {
WTF_MAKE_FAST_ALLOCATED;
@@ -1050,6 +1087,11 @@ class LengthVariantPropertyWrapper final : public PropertyWrapperGetter<const T&
return canInterpolateLengthVariants(this->value(from), this->value(to));
}

bool requiresBlendingForAccumulativeIteration(const RenderStyle& from, const RenderStyle& to) const final
{
return lengthVariantRequiresBlendingForAccumulativeIteration(this->value(from), this->value(to));
}

void blend(RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, const CSSPropertyBlendingContext& context) const final
{
(destination.*m_setter)(blendFunc(this->value(from), this->value(to), context));
@@ -1172,6 +1214,16 @@ class LengthBoxPropertyWrapper : public PropertyWrapperGetter<const LengthBox&>
&& canInterpolateLengths(fromLengthBox.left(), toLengthBox.left(), isLengthPercentage);
}

bool requiresBlendingForAccumulativeIteration(const RenderStyle& from, const RenderStyle& to) const final
{
auto& fromLengthBox = value(from);
auto& toLengthBox = value(to);
return lengthsRequireBlendingForAccumulativeIteration(fromLengthBox.top(), toLengthBox.top())
&& lengthsRequireBlendingForAccumulativeIteration(fromLengthBox.right(), toLengthBox.right())
&& lengthsRequireBlendingForAccumulativeIteration(fromLengthBox.bottom(), toLengthBox.bottom())
&& lengthsRequireBlendingForAccumulativeIteration(fromLengthBox.left(), toLengthBox.left());
}

void blend(RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, const CSSPropertyBlendingContext& context) const override
{
if (m_flags.contains(Flags::UsesFillKeyword))
@@ -1376,6 +1428,7 @@ class AcceleratedPropertyWrapper final : public PropertyWrapper<T> {

private:
bool animationIsAccelerated() const final { return true; }
bool requiresBlendingForAccumulativeIteration(const RenderStyle&, const RenderStyle&) const final { return this->property() == CSSPropertyTransform; }
};

template <typename T>
@@ -1414,6 +1467,8 @@ class PropertyWrapperFilter final : public PropertyWrapper<const FilterOperation
;
}

bool requiresBlendingForAccumulativeIteration(const RenderStyle&, const RenderStyle&) const final { return true; }

bool canInterpolate(const RenderStyle& from, const RenderStyle& to, CompositeOperation compositeOperation) const final
{
auto& fromFilterOperations = value(from);
@@ -1502,6 +1557,8 @@ class PropertyWrapperShadow final : public AnimationPropertyWrapperBase {
}

private:
bool requiresBlendingForAccumulativeIteration(const RenderStyle&, const RenderStyle&) const final { return true; }

bool equals(const RenderStyle& a, const RenderStyle& b) const final
{
if (&a == &b)
@@ -1750,6 +1807,8 @@ template <typename T = StyleColor> class PropertyWrapperVisitedAffectedColor : p
}

protected:
bool requiresBlendingForAccumulativeIteration(const RenderStyle&, const RenderStyle&) const final { return true; }

bool equals(const RenderStyle& a, const RenderStyle& b) const override
{
return m_wrapper->equals(a, b) && m_visitedWrapper->equals(a, b);
@@ -3826,7 +3885,7 @@ CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap()
#endif
}

void CSSPropertyAnimation::blendProperties(const CSSPropertyBlendingClient* client, CSSPropertyID property, RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, double progress, CompositeOperation compositeOperation)
void CSSPropertyAnimation::blendProperties(const CSSPropertyBlendingClient* client, CSSPropertyID property, RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, double progress, CompositeOperation compositeOperation, IterationCompositeOperation iterationCompositeOperation, double currentIteration)
{
ASSERT(property != CSSPropertyInvalid && property != CSSPropertyCustom);

@@ -3841,7 +3900,7 @@ void CSSPropertyAnimation::blendProperties(const CSSPropertyBlendingClient* clie
progress = progress < 0.5 ? 0 : 1;
compositeOperation = CompositeOperation::Replace;
}
wrapper->blend(destination, from, to, { progress, isDiscrete, compositeOperation, client });
wrapper->blend(destination, from, to, { progress, isDiscrete, compositeOperation, client, iterationCompositeOperation, currentIteration });
#if !LOG_DISABLED
wrapper->logBlend(from, to, destination, progress);
#endif
@@ -3868,6 +3927,12 @@ bool CSSPropertyAnimation::isPropertyAdditiveOrCumulative(CSSPropertyID property
return wrapper ? wrapper->isAdditiveOrCumulative() : false;
}

bool CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration(CSSPropertyID property, const RenderStyle& a, const RenderStyle& b)
{
auto* wrapper = CSSPropertyAnimationWrapperMap::singleton().wrapperForProperty(property);
return wrapper && wrapper->requiresBlendingForAccumulativeIteration(a, b);
}

bool CSSPropertyAnimation::animationOfPropertyIsAccelerated(CSSPropertyID property)
{
AnimationPropertyWrapperBase* wrapper = CSSPropertyAnimationWrapperMap::singleton().wrapperForProperty(property);
@@ -30,6 +30,7 @@

#include "CSSPropertyNames.h"
#include "CompositeOperation.h"
#include "IterationCompositeOperation.h"
#include <wtf/HashSet.h>

namespace WebCore {
@@ -41,13 +42,14 @@ class CSSPropertyAnimation {
public:
static bool isPropertyAnimatable(CSSPropertyID);
static bool isPropertyAdditiveOrCumulative(CSSPropertyID);
static bool propertyRequiresBlendingForAccumulativeIteration(CSSPropertyID, const RenderStyle& a, const RenderStyle& b);
static bool animationOfPropertyIsAccelerated(CSSPropertyID);
static bool propertiesEqual(CSSPropertyID, const RenderStyle& a, const RenderStyle& b);
static bool canPropertyBeInterpolated(CSSPropertyID, const RenderStyle& a, const RenderStyle& b);
static CSSPropertyID getPropertyAtIndex(int, std::optional<bool>& isShorthand);
static int getNumProperties();

static void blendProperties(const CSSPropertyBlendingClient*, CSSPropertyID, RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, double progress, CompositeOperation);
static void blendProperties(const CSSPropertyBlendingClient*, CSSPropertyID, RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, double progress, CompositeOperation, IterationCompositeOperation = IterationCompositeOperation::Replace, double currentIteration = 0);
static void blendCustomProperty(const AtomString&, RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, double progress);
};

@@ -1531,6 +1531,8 @@ void KeyframeEffect::setAnimatedPropertiesInStyle(RenderStyle& targetStyle, doub
auto startKeyframeStyle = RenderStyle::clone(*startKeyframe.style());
auto endKeyframeStyle = RenderStyle::clone(*endKeyframe.style());

auto usedBlendingForAccumulativeIteration = false;

// 12. For each keyframe in interval endpoints:
// If keyframe has a composite operation that is not replace, or keyframe has no composite operation and the
// composite operation of this keyframe effect is not replace, then perform the following steps:
@@ -1562,7 +1564,8 @@ void KeyframeEffect::setAnimatedPropertiesInStyle(RenderStyle& targetStyle, doub
}

// If this keyframe effect has an iteration composite operation of accumulate,
if (m_iterationCompositeOperation == IterationCompositeOperation::Accumulate) {
if (m_iterationCompositeOperation == IterationCompositeOperation::Accumulate && currentIteration && CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration(cssPropertyId, startKeyframeStyle, endKeyframeStyle)) {
usedBlendingForAccumulativeIteration = true;
// apply the following step current iteration times:
for (auto i = 0; i < currentIteration; ++i) {
// replace the property value of target property on keyframe with the result of combining the
@@ -1608,7 +1611,11 @@ void KeyframeEffect::setAnimatedPropertiesInStyle(RenderStyle& targetStyle, doub
// property specified on the two keyframes in interval endpoints taking the first such value as Vstart and the second as Vend and using transformed
// distance as the interpolation parameter p.
WTF::switchOn(property,
[&] (CSSPropertyID propertyId) { CSSPropertyAnimation::blendProperties(this, propertyId, targetStyle, startKeyframeStyle, endKeyframeStyle, transformedDistance, CompositeOperation::Replace); },
[&] (CSSPropertyID propertyId) {
auto iterationCompositeOperation = usedBlendingForAccumulativeIteration ? IterationCompositeOperation::Replace : m_iterationCompositeOperation;
currentIteration = usedBlendingForAccumulativeIteration ? 0 : currentIteration;
CSSPropertyAnimation::blendProperties(this, propertyId, targetStyle, startKeyframeStyle, endKeyframeStyle, transformedDistance, CompositeOperation::Replace, iterationCompositeOperation, currentIteration);
},
[&] (AtomString customProperty) { CSSPropertyAnimation::blendCustomProperty(customProperty, targetStyle, startKeyframeStyle, endKeyframeStyle, transformedDistance); }
);
};
@@ -322,10 +322,10 @@ Length blend(const Length& from, const Length& to, const BlendingContext& contex
if (from.isCalculated() || to.isCalculated() || (from.type() != to.type()))
return blendMixedTypes(from, to, context);

if (!context.progress && context.compositeOperation == CompositeOperation::Replace)
if (!context.progress && context.isReplace())
return from;

if (context.progress == 1 && context.compositeOperation == CompositeOperation::Replace)
if (context.progress == 1 && context.isReplace())
return to;

LengthType resultType = to.type();
@@ -27,6 +27,7 @@

#include "CompositeOperation.h"
#include "IntPoint.h"
#include "IterationCompositeOperation.h"
#include "LayoutPoint.h"

namespace WebCore {
@@ -35,38 +36,71 @@ struct BlendingContext {
double progress { 0 };
bool isDiscrete { false };
CompositeOperation compositeOperation { CompositeOperation::Replace };
IterationCompositeOperation iterationCompositeOperation { IterationCompositeOperation::Replace };
double currentIteration { 0 };

BlendingContext(double progress = 0, bool isDiscrete = false, CompositeOperation compositeOperation = CompositeOperation::Replace)
BlendingContext(double progress = 0, bool isDiscrete = false, CompositeOperation compositeOperation = CompositeOperation::Replace, IterationCompositeOperation iterationCompositeOperation = IterationCompositeOperation::Replace, double currentIteration = 0)
: progress(progress)
, isDiscrete(isDiscrete)
, compositeOperation(compositeOperation)
, iterationCompositeOperation(iterationCompositeOperation)
, currentIteration(currentIteration)
{
}

bool isReplace() const
{
return compositeOperation == CompositeOperation::Replace && iterationCompositeOperation == IterationCompositeOperation::Replace;
}
};

inline int blend(int from, int to, const BlendingContext& context)
{
if (context.iterationCompositeOperation == IterationCompositeOperation::Accumulate && context.currentIteration) {
auto iterationIncrement = static_cast<int>(context.currentIteration * static_cast<double>(to));
from += iterationIncrement;
to += iterationIncrement;
}

if (context.compositeOperation == CompositeOperation::Replace)
return static_cast<int>(roundTowardsPositiveInfinity(from + (static_cast<double>(to) - from) * context.progress));
return static_cast<int>(roundTowardsPositiveInfinity(static_cast<double>(from) + static_cast<double>(from) + static_cast<double>(to - from) * context.progress));
}

inline unsigned blend(unsigned from, unsigned to, const BlendingContext& context)
{
if (context.iterationCompositeOperation == IterationCompositeOperation::Accumulate && context.currentIteration) {
auto iterationIncrement = static_cast<unsigned>(context.currentIteration * static_cast<double>(to));
from += iterationIncrement;
to += iterationIncrement;
}

if (context.compositeOperation == CompositeOperation::Replace)
return static_cast<unsigned>(lround(from + (static_cast<double>(to) - from) * context.progress));
return static_cast<unsigned>(lround(from + from + (static_cast<double>(to) - from) * context.progress));
}

inline double blend(double from, double to, const BlendingContext& context)
{
if (context.iterationCompositeOperation == IterationCompositeOperation::Accumulate && context.currentIteration) {
auto iterationIncrement = context.currentIteration * to;
from += iterationIncrement;
to += iterationIncrement;
}

if (context.compositeOperation == CompositeOperation::Replace)
return from + (to - from) * context.progress;
return from + from + (to - from) * context.progress;
}

inline float blend(float from, float to, const BlendingContext& context)
{
if (context.iterationCompositeOperation == IterationCompositeOperation::Accumulate && context.currentIteration) {
auto iterationIncrement = static_cast<float>(context.currentIteration * to);
from += iterationIncrement;
to += iterationIncrement;
}

if (context.compositeOperation == CompositeOperation::Replace)
return static_cast<float>(from + (to - from) * context.progress);
return static_cast<float>(from + from + (to - from) * context.progress);

0 comments on commit f0e70ac

Please sign in to comment.