Commit a911ce26 authored by George Steel's avatar George Steel Committed by Commit Bot

Fix TransformOperations::DependsOnBoxSize false positives

Allow checks which skip a prefix, as needed for proper behavior in
TransformOperations::BlendRemainingByUsingMatrixInterpolation().
This method will now only create InterpolatedTransformationOperations
which are actually box-size dependent (as originally intended) ans
create a matrix otherwise. Add a DCHECK for this property.

Note that the interpolation defined in the css-transforms-1/2 specs
assume that transform styles are calculated after layout, with matrix
interpolation always producing a Matrix3DTransformOperation. As style
is calculated before layout, this isn't possible in cases where the
inputs to matrix interpolation are layout-dependent, so we have
InterpolatedTransformOperation to defer interpolation in these cases.

There is a currently CSSWG resolution to spec this behavior and give
InterpolatedTransformOperation a defined serialization, having
interpolation return a matrix as per the current spec where possible
(the inputs of matrix interpolation are layout-independent) and an
interpolated operation otherwise (where the current spec is
inconsistent). The added DCHECK enforces this behavior.
Resolution: https://github.com/w3c/csswg-drafts/issues/2854

This is to prepare for using DependsOnBoxSize for invalidation of
compositor animations, enabling acceleration of percent-containing
transform.
Design Doc: https://docs.google.com/document/d/1zgr5CHRMpvlqodn1e0eM9J3MjL2eEMfAHrHsZUK7gMM/

Bug: 389359
Change-Id: I2302546225ac52f47104d2dd77b02f874d2f6ef8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2432194Reviewed-by: default avatarKevin Ellis <kevers@chromium.org>
Reviewed-by: default avatarIan Vollick <vollick@chromium.org>
Commit-Queue: George Steel <gtsteel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813278}
parent 0667f4d1
...@@ -81,7 +81,8 @@ class PLATFORM_EXPORT InterpolatedTransformOperation final ...@@ -81,7 +81,8 @@ class PLATFORM_EXPORT InterpolatedTransformOperation final
} }
bool DependsOnBoxSize() const override { bool DependsOnBoxSize() const override {
return from_.DependsOnBoxSize() || to_.DependsOnBoxSize(); return from_.DependsOnBoxSize(starting_index_) ||
to_.DependsOnBoxSize(starting_index_);
} }
InterpolatedTransformOperation(const TransformOperations& from, InterpolatedTransformOperation(const TransformOperations& from,
...@@ -91,7 +92,11 @@ class PLATFORM_EXPORT InterpolatedTransformOperation final ...@@ -91,7 +92,11 @@ class PLATFORM_EXPORT InterpolatedTransformOperation final
: from_(from), : from_(from),
to_(to), to_(to),
starting_index_(starting_index), starting_index_(starting_index),
progress_(progress) {} progress_(progress) {
// This should only be generated during interpolation when it is impossible
// to create a Matrix3DTransformOperation due to layout-dependence.
DCHECK(DependsOnBoxSize());
}
const TransformOperations from_; const TransformOperations from_;
const TransformOperations to_; const TransformOperations to_;
......
...@@ -118,7 +118,8 @@ TransformOperations::BlendRemainingByUsingMatrixInterpolation( ...@@ -118,7 +118,8 @@ TransformOperations::BlendRemainingByUsingMatrixInterpolation(
double progress) const { double progress) const {
// Not safe to use a cached transform if any of the operations are size // Not safe to use a cached transform if any of the operations are size
// dependent. // dependent.
if (DependsOnBoxSize() || from.DependsOnBoxSize()) { if (DependsOnBoxSize(matching_prefix_length) ||
from.DependsOnBoxSize(matching_prefix_length)) {
return InterpolatedTransformOperation::Create( return InterpolatedTransformOperation::Create(
from, *this, matching_prefix_length, progress); from, *this, matching_prefix_length, progress);
} }
......
...@@ -109,9 +109,9 @@ class PLATFORM_EXPORT TransformOperations { ...@@ -109,9 +109,9 @@ class PLATFORM_EXPORT TransformOperations {
return false; return false;
} }
bool DependsOnBoxSize() const { bool DependsOnBoxSize(wtf_size_t skip_prefix = 0) const {
for (auto& operation : operations_) { for (wtf_size_t i = skip_prefix; i < operations_.size(); i++) {
if (operation->DependsOnBoxSize()) if (operations_[i]->DependsOnBoxSize())
return true; return true;
} }
return false; return false;
......
...@@ -611,4 +611,42 @@ TEST(TransformOperationsTest, InterpolatedTransformBlendIdentityTest) { ...@@ -611,4 +611,42 @@ TEST(TransformOperationsTest, InterpolatedTransformBlendIdentityTest) {
EXPECT_TRANSFORMATION_MATRIX(mat_d1, mat_d3); EXPECT_TRANSFORMATION_MATRIX(mat_d1, mat_d3);
EXPECT_TRANSFORMATION_MATRIX(mat_d2, mat_d3); EXPECT_TRANSFORMATION_MATRIX(mat_d2, mat_d3);
} }
TEST(TransformOperationsTest, BlendPercentPrefixTest) {
TransformOperations ops_a, ops_b;
ops_a.Operations().push_back(TranslateTransformOperation::Create(
Length::Percent(100), Length::Fixed(0), TransformOperation::kTranslate));
ops_a.Operations().push_back(
RotateTransformOperation::Create(180, TransformOperation::kRotate));
ops_b.Operations().push_back(TranslateTransformOperation::Create(
Length::Fixed(0), Length::Percent(50), TransformOperation::kTranslate));
ops_b.Operations().push_back(
ScaleTransformOperation::Create(2, 2, TransformOperation::kScale));
EXPECT_TRUE(ops_a.DependsOnBoxSize());
EXPECT_FALSE(ops_a.DependsOnBoxSize(1));
EXPECT_TRUE(ops_b.DependsOnBoxSize());
EXPECT_FALSE(ops_b.DependsOnBoxSize(1));
TransformOperations ops_c = ops_a.Blend(ops_b, 0.5);
EXPECT_TRUE(ops_c.DependsOnBoxSize());
ASSERT_EQ(ops_c.Operations().size(), 2u);
ASSERT_TRUE(IsA<TranslateTransformOperation>(*ops_c.Operations()[0]));
// Even though both transform lists contain percents, the matrix interpolated
// part does not, so it should interpolate to a matrix and not defer to an
// InterpolatedTransformOperation.
ASSERT_TRUE(IsA<Matrix3DTransformOperation>(*ops_c.Operations()[1]));
TransformationMatrix mat_c =
To<Matrix3DTransformOperation>(*ops_c.Operations()[1]).Matrix();
auto translate_ref = TranslateTransformOperation::Create(
Length::Percent(50), Length::Percent(25), TransformOperation::kTranslate);
// scale(1.5) rotate(90deg)
TransformationMatrix matrix_ref(0, 1.5, -1.5, 0, 0, 0);
EXPECT_EQ(*ops_c.Operations()[0], *translate_ref);
EXPECT_TRANSFORMATION_MATRIX(mat_c, matrix_ref);
}
} // namespace blink } // namespace blink
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