Commit fbc2fc80 authored by Xida Chen's avatar Xida Chen Committed by Commit Bot

Treat alpha channel differently when compositing animations

Currently our implementation of InterpolationType::Composite is too
general and could result in errors when applied to color.

Let's use an example to demonstrate how it works.
Suppose we have to keyframes to animation
keyframe[0]: (10, 0, 0, 1), composite: add
keyframe[1]: (30, 0, 0, 1), composite: replace
and the underlying color of the animation target is (50, 0, 0, 1).
We want to calculate the interpolation result at 1.2, and we will
use alpha channel because that's easier to demonstrate.

The first step is computing the interpolated result at 1.2 for the
two keyframes, and our implementation gives correct result where the
alpha is 1 * (-0.2) + 1 *1.2 = 1. In other words, the result is
(34, 0, 0, 1).

Next step is to composite the interpolated result onto the underlying
value. The alpha channel is 1 * (-0.2) + 1 = 0.8.
We can see that the alpha channel is wrong, and because of that, the
other channel would be wrong because they operate on pre-multiplied
numbers.

This CL treats alpha channel differently for color. When
the underlying and the other one has the same alpha channel, we keep
that value and do not apply the formula.

Here is the spec indicates that alpha channel needs to be always
clamped to range [0, 1]:
https://www.w3.org/TR/css-color-3/#alphavaluedt

Bug: 981326
Change-Id: I34429d0a463bdc83099d58ad23874086d861dc9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1715786
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: default avatarStephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686064}
parent 1e42f9c7
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "third_party/blink/renderer/core/animation/color_property_functions.h" #include "third_party/blink/renderer/core/animation/color_property_functions.h"
#include "third_party/blink/renderer/core/animation/interpolable_value.h"
#include "third_party/blink/renderer/core/css/css_color_value.h" #include "third_party/blink/renderer/core/css/css_color_value.h"
#include "third_party/blink/renderer/core/css/css_identifier_value.h" #include "third_party/blink/renderer/core/css/css_identifier_value.h"
#include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h" #include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
...@@ -295,4 +296,37 @@ const CSSValue* CSSColorInterpolationType::CreateCSSValue( ...@@ -295,4 +296,37 @@ const CSSValue* CSSColorInterpolationType::CreateCSSValue(
return CSSColorValue::Create(color.Rgb()); return CSSColorValue::Create(color.Rgb());
} }
void CSSColorInterpolationType::Composite(
UnderlyingValueOwner& underlying_value_owner,
double underlying_fraction,
const InterpolationValue& value,
double interpolation_fraction) const {
DCHECK(!underlying_value_owner.Value().non_interpolable_value);
DCHECK(!value.non_interpolable_value);
InterpolableList& underlying_list = ToInterpolableList(
*underlying_value_owner.MutableValue().interpolable_value);
const InterpolableList& other_list =
ToInterpolableList(*value.interpolable_value);
// Both lists should have kUnvisited and kVisited.
DCHECK(underlying_list.length() == kInterpolableColorPairIndexCount);
DCHECK(other_list.length() == kInterpolableColorPairIndexCount);
for (wtf_size_t i = 0; i < underlying_list.length(); i++) {
InterpolableList& underlying =
ToInterpolableList(*underlying_list.GetMutable(i));
const InterpolableList& other = ToInterpolableList(*other_list.Get(i));
DCHECK(underlying.length() == kInterpolableColorIndexCount);
DCHECK(other.length() == kInterpolableColorIndexCount);
for (wtf_size_t j = 0; j < underlying.length(); j++) {
DCHECK(underlying.Get(j)->IsNumber());
DCHECK(other.Get(j)->IsNumber());
InterpolableNumber& underlying_number =
ToInterpolableNumber(*underlying.GetMutable(j));
const InterpolableNumber& other_number =
ToInterpolableNumber(*other.Get(j));
if (j != kAlpha || underlying_number.Value() != other_number.Value())
underlying_number.ScaleAndAdd(underlying_fraction, other_number);
}
}
}
} // namespace blink } // namespace blink
...@@ -26,6 +26,10 @@ class CSSColorInterpolationType : public CSSInterpolationType { ...@@ -26,6 +26,10 @@ class CSSColorInterpolationType : public CSSInterpolationType {
void ApplyStandardPropertyValue(const InterpolableValue&, void ApplyStandardPropertyValue(const InterpolableValue&,
const NonInterpolableValue*, const NonInterpolableValue*,
StyleResolverState&) const final; StyleResolverState&) const final;
void Composite(UnderlyingValueOwner& underlying_value_owner,
double underlying_fraction,
const InterpolationValue& value,
double interpolation_fraction) const final;
static std::unique_ptr<InterpolableValue> CreateInterpolableColor( static std::unique_ptr<InterpolableValue> CreateInterpolableColor(
const Color&); const Color&);
......
<!DOCTYPE html>
<meta charset="UTF-8">
<style>
.target {
width: 40px;
height: 40px;
background-color: black;
}
.expected {
background-color: green;
}
</style>
<body>
<script src="../interpolation/resources/interpolation-test.js"></script>
<script>
assertComposition({
property: 'color',
underlying: 'rgb(50, 50, 50)',
addFrom: 'rgb(10, 10, 10)',
replaceTo: 'rgb(30, 30, 30)',
}, [
{at: 0, is: 'rgb(60, 60, 60)'},
{at: 0.2, is: 'rgb(54, 54, 54)'},
{at: 1, is: 'rgb(30, 30, 30)'},
{at: 1.2, is: 'rgb(24, 24, 24)'},
{at: 1.5, is: 'rgb(15, 15, 15)'},
]);
assertComposition({
property: 'color',
underlying: 'rgb(60, 60, 60)',
addFrom: 'rgb(0, 0, 0)',
replaceTo: 'rgb(50, 50, 50)',
}, [
{at: 0, is: 'rgb(60, 60, 60)'},
{at: 0.5, is: 'rgb(55, 55, 55)'},
{at: 1, is: 'rgb(50, 50, 50)'},
{at: 1.2, is: 'rgb(48, 48, 48)'},
{at: 1.5, is: 'rgb(45, 45, 45)'},
]);
</script>
</body>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment