Skip to content

Commit

Permalink
Treat alpha channel differently when compositing animations
Browse files Browse the repository at this point in the history
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: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686064}
  • Loading branch information
xidachen authored and Commit Bot committed Aug 12, 2019
1 parent 1e42f9c commit fbc2fc8
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 89 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "base/memory/ptr_util.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_identifier_value.h"
#include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
Expand Down Expand Up @@ -295,4 +296,37 @@ const CSSValue* CSSColorInterpolationType::CreateCSSValue(
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
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ class CSSColorInterpolationType : public CSSInterpolationType {
void ApplyStandardPropertyValue(const InterpolableValue&,
const NonInterpolableValue*,
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(
const Color&);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<!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>

This file was deleted.

0 comments on commit fbc2fc8

Please sign in to comment.