Skip to content

Commit

Permalink
Reland "Remove layer integral offset snapping" (flutter#17915)
Browse files Browse the repository at this point in the history
This reverts commit b5aedb3 and relands flutter#17712.

Fixes flutter/flutter#53288 and flutter/flutter#41654.

Together with flutter#17791, this reland addresses some of Jim's concerns in the original PR flutter#17712.

The major part of this PR is still the same as the original PR, and the performance / golden image impacts should be the same.
  • Loading branch information
liyuqian authored May 1, 2020
1 parent 77d0271 commit 4e29736
Show file tree
Hide file tree
Showing 19 changed files with 59 additions and 150 deletions.
9 changes: 9 additions & 0 deletions flow/layers/container_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@ void ContainerLayer::PaintChildren(PaintContext& context) const {
}
}

void ContainerLayer::TryToPrepareRasterCache(PrerollContext* context,
Layer* layer,
const SkMatrix& matrix) {
if (!context->has_platform_view && context->raster_cache &&
SkRect::Intersects(context->cull_rect, layer->paint_bounds())) {
context->raster_cache->Prepare(context, layer, matrix);
}
}

#if defined(OS_FUCHSIA)

void ContainerLayer::UpdateScene(SceneUpdateContext& context) {
Expand Down
14 changes: 14 additions & 0 deletions flow/layers/container_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,20 @@ class ContainerLayer : public Layer {
// For OpacityLayer to restructure to have a single child.
void ClearChildren() { layers_.clear(); }

// Try to prepare the raster cache for a given layer.
//
// The raster cache would fail if either of the followings is true:
// 1. The context has a platform view.
// 2. The context does not have a valid raster cache.
// 3. The layer's paint bounds does not intersect with the cull rect.
//
// We make this a static function instead of a member function that directy
// uses the "this" pointer as the layer because we sometimes need to raster
// cache a child layer and one can't access its child's protected method.
static void TryToPrepareRasterCache(PrerollContext* context,
Layer* layer,
const SkMatrix& matrix);

private:
std::vector<std::shared_ptr<Layer>> layers_;

Expand Down
15 changes: 1 addition & 14 deletions flow/layers/image_filter_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,26 +28,13 @@ void ImageFilterLayer::Preroll(PrerollContext* context,
set_paint_bounds(child_paint_bounds_);
}

if (!context->has_platform_view && context->raster_cache &&
SkRect::Intersects(context->cull_rect, paint_bounds())) {
SkMatrix ctm = matrix;
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
ctm = RasterCache::GetIntegralTransCTM(ctm);
#endif
context->raster_cache->Prepare(context, this, ctm);
}
TryToPrepareRasterCache(context, this, matrix);
}

void ImageFilterLayer::Paint(PaintContext& context) const {
TRACE_EVENT0("flutter", "ImageFilterLayer::Paint");
FML_DCHECK(needs_painting());

#ifndef SUPPORT_FRACTIONAL_TRANSLATION
SkAutoCanvasRestore save(context.leaf_nodes_canvas, true);
context.leaf_nodes_canvas->setMatrix(RasterCache::GetIntegralTransCTM(
context.leaf_nodes_canvas->getTotalMatrix()));
#endif

if (context.raster_cache &&
context.raster_cache->Draw(this, *context.leaf_nodes_canvas)) {
return;
Expand Down
68 changes: 25 additions & 43 deletions flow/layers/image_filter_layer_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,11 @@ TEST_F(ImageFilterLayerTest, EmptyFilter) {
layer->Paint(paint_context());
EXPECT_EQ(mock_canvas().draw_calls(),
std::vector({
MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
MockCanvas::DrawCall{
1, MockCanvas::SaveLayerData{child_bounds, filter_paint,
nullptr, 2}},
0, MockCanvas::SaveLayerData{child_bounds, filter_paint,
nullptr, 1}},
MockCanvas::DrawCall{
2, MockCanvas::DrawPathData{child_path, child_paint}},
MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
1, MockCanvas::DrawPathData{child_path, child_paint}},
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}},
}));
}
Expand Down Expand Up @@ -96,14 +93,11 @@ TEST_F(ImageFilterLayerTest, SimpleFilter) {
layer->Paint(paint_context());
EXPECT_EQ(mock_canvas().draw_calls(),
std::vector({
MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
MockCanvas::DrawCall{
1, MockCanvas::SaveLayerData{child_bounds, filter_paint,
nullptr, 2}},
0, MockCanvas::SaveLayerData{child_bounds, filter_paint,
nullptr, 1}},
MockCanvas::DrawCall{
2, MockCanvas::DrawPathData{child_path, child_paint}},
MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
1, MockCanvas::DrawPathData{child_path, child_paint}},
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}},
}));
}
Expand Down Expand Up @@ -132,14 +126,11 @@ TEST_F(ImageFilterLayerTest, SimpleFilterBounds) {
layer->Paint(paint_context());
EXPECT_EQ(mock_canvas().draw_calls(),
std::vector({
MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
MockCanvas::DrawCall{
1, MockCanvas::SaveLayerData{child_bounds, filter_paint,
nullptr, 2}},
0, MockCanvas::SaveLayerData{child_bounds, filter_paint,
nullptr, 1}},
MockCanvas::DrawCall{
2, MockCanvas::DrawPathData{child_path, child_paint}},
MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
1, MockCanvas::DrawPathData{child_path, child_paint}},
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}},
}));
}
Expand Down Expand Up @@ -177,19 +168,16 @@ TEST_F(ImageFilterLayerTest, MultipleChildren) {
SkPaint filter_paint;
filter_paint.setImageFilter(layer_filter);
layer->Paint(paint_context());
EXPECT_EQ(mock_canvas().draw_calls(),
std::vector(
{MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
MockCanvas::DrawCall{
1, MockCanvas::SaveLayerData{children_bounds, filter_paint,
nullptr, 2}},
MockCanvas::DrawCall{
2, MockCanvas::DrawPathData{child_path1, child_paint1}},
MockCanvas::DrawCall{
2, MockCanvas::DrawPathData{child_path2, child_paint2}},
MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}}));
EXPECT_EQ(
mock_canvas().draw_calls(),
std::vector({MockCanvas::DrawCall{
0, MockCanvas::SaveLayerData{children_bounds,
filter_paint, nullptr, 1}},
MockCanvas::DrawCall{
1, MockCanvas::DrawPathData{child_path1, child_paint1}},
MockCanvas::DrawCall{
1, MockCanvas::DrawPathData{child_path2, child_paint2}},
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}}));
}

TEST_F(ImageFilterLayerTest, Nested) {
Expand Down Expand Up @@ -237,22 +225,16 @@ TEST_F(ImageFilterLayerTest, Nested) {
layer1->Paint(paint_context());
EXPECT_EQ(mock_canvas().draw_calls(),
std::vector({
MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
MockCanvas::DrawCall{1, MockCanvas::SetMatrixData{SkMatrix()}},
MockCanvas::DrawCall{
1, MockCanvas::SaveLayerData{children_bounds, filter_paint1,
nullptr, 2}},
0, MockCanvas::SaveLayerData{children_bounds, filter_paint1,
nullptr, 1}},
MockCanvas::DrawCall{
2, MockCanvas::DrawPathData{child_path1, child_paint1}},
MockCanvas::DrawCall{2, MockCanvas::SaveData{3}},
MockCanvas::DrawCall{3, MockCanvas::SetMatrixData{SkMatrix()}},
1, MockCanvas::DrawPathData{child_path1, child_paint1}},
MockCanvas::DrawCall{
3, MockCanvas::SaveLayerData{child_path2.getBounds(),
filter_paint2, nullptr, 4}},
1, MockCanvas::SaveLayerData{child_path2.getBounds(),
filter_paint2, nullptr, 2}},
MockCanvas::DrawCall{
4, MockCanvas::DrawPathData{child_path2, child_paint2}},
MockCanvas::DrawCall{4, MockCanvas::RestoreData{3}},
MockCanvas::DrawCall{3, MockCanvas::RestoreData{2}},
2, MockCanvas::DrawPathData{child_path2, child_paint2}},
MockCanvas::DrawCall{2, MockCanvas::RestoreData{1}},
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}},
}));
Expand Down
17 changes: 2 additions & 15 deletions flow/layers/opacity_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,7 @@ void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {

{
set_paint_bounds(paint_bounds().makeOffset(offset_.fX, offset_.fY));
if (!context->has_platform_view && context->raster_cache &&
SkRect::Intersects(context->cull_rect, paint_bounds())) {
SkMatrix ctm = child_matrix;
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
ctm = RasterCache::GetIntegralTransCTM(ctm);
#endif
context->raster_cache->Prepare(context, container, ctm);
}
TryToPrepareRasterCache(context, container, child_matrix);
}
}

Expand All @@ -69,11 +62,6 @@ void OpacityLayer::Paint(PaintContext& context) const {
SkAutoCanvasRestore save(context.internal_nodes_canvas, true);
context.internal_nodes_canvas->translate(offset_.fX, offset_.fY);

#ifndef SUPPORT_FRACTIONAL_TRANSLATION
context.internal_nodes_canvas->setMatrix(RasterCache::GetIntegralTransCTM(
context.leaf_nodes_canvas->getTotalMatrix()));
#endif

if (context.raster_cache &&
context.raster_cache->Draw(GetChildContainer(),
*context.leaf_nodes_canvas, &paint)) {
Expand All @@ -83,8 +71,7 @@ void OpacityLayer::Paint(PaintContext& context) const {
// Skia may clip the content with saveLayerBounds (although it's not a
// guaranteed clip). So we have to provide a big enough saveLayerBounds. To do
// so, we first remove the offset from paint bounds since it's already in the
// matrix. Then we round out the bounds because of our
// RasterCache::GetIntegralTransCTM optimization.
// matrix. Then we round out the bounds.
//
// Note that the following lines are only accessible when the raster cache is
// not available (e.g., when we're using the software backend in golden
Expand Down
39 changes: 0 additions & 39 deletions flow/layers/opacity_layer_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,6 @@ TEST_F(OpacityLayerTest, FullyOpaque) {
const SkMatrix initial_transform = SkMatrix::MakeTrans(0.5f, 0.5f);
const SkMatrix layer_transform =
SkMatrix::MakeTrans(layer_offset.fX, layer_offset.fY);
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
const SkMatrix integral_layer_transform = RasterCache::GetIntegralTransCTM(
SkMatrix::Concat(initial_transform, layer_transform));
#endif
const SkPaint child_paint = SkPaint(SkColors::kGreen);
const SkRect expected_layer_bounds =
layer_transform.mapRect(child_path.getBounds());
Expand All @@ -86,10 +82,6 @@ TEST_F(OpacityLayerTest, FullyOpaque) {
auto expected_draw_calls = std::vector(
{MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer_transform}},
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
MockCanvas::DrawCall{
1, MockCanvas::SetMatrixData{integral_layer_transform}},
#endif
MockCanvas::DrawCall{
1, MockCanvas::SaveLayerData{opacity_bounds, opacity_paint, nullptr,
2}},
Expand All @@ -107,10 +99,6 @@ TEST_F(OpacityLayerTest, FullyTransparent) {
const SkMatrix initial_transform = SkMatrix::MakeTrans(0.5f, 0.5f);
const SkMatrix layer_transform =
SkMatrix::MakeTrans(layer_offset.fX, layer_offset.fY);
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
const SkMatrix integral_layer_transform = RasterCache::GetIntegralTransCTM(
SkMatrix::Concat(initial_transform, layer_transform));
#endif
const SkPaint child_paint = SkPaint(SkColors::kGreen);
const SkRect expected_layer_bounds =
layer_transform.mapRect(child_path.getBounds());
Expand All @@ -133,10 +121,6 @@ TEST_F(OpacityLayerTest, FullyTransparent) {
auto expected_draw_calls = std::vector(
{MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer_transform}},
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
MockCanvas::DrawCall{
1, MockCanvas::SetMatrixData{integral_layer_transform}},
#endif
MockCanvas::DrawCall{1, MockCanvas::SaveData{2}},
MockCanvas::DrawCall{
2, MockCanvas::ClipRectData{kEmptyRect, SkClipOp::kIntersect,
Expand All @@ -155,10 +139,6 @@ TEST_F(OpacityLayerTest, HalfTransparent) {
const SkMatrix initial_transform = SkMatrix::MakeTrans(0.5f, 0.5f);
const SkMatrix layer_transform =
SkMatrix::MakeTrans(layer_offset.fX, layer_offset.fY);
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
const SkMatrix integral_layer_transform = RasterCache::GetIntegralTransCTM(
SkMatrix::Concat(initial_transform, layer_transform));
#endif
const SkPaint child_paint = SkPaint(SkColors::kGreen);
const SkRect expected_layer_bounds =
layer_transform.mapRect(child_path.getBounds());
Expand All @@ -185,10 +165,6 @@ TEST_F(OpacityLayerTest, HalfTransparent) {
auto expected_draw_calls = std::vector(
{MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer_transform}},
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
MockCanvas::DrawCall{
1, MockCanvas::SetMatrixData{integral_layer_transform}},
#endif
MockCanvas::DrawCall{
1, MockCanvas::SaveLayerData{opacity_bounds, opacity_paint, nullptr,
2}},
Expand All @@ -211,13 +187,6 @@ TEST_F(OpacityLayerTest, Nested) {
SkMatrix::MakeTrans(layer1_offset.fX, layer1_offset.fY);
const SkMatrix layer2_transform =
SkMatrix::MakeTrans(layer2_offset.fX, layer2_offset.fY);
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
const SkMatrix integral_layer1_transform = RasterCache::GetIntegralTransCTM(
SkMatrix::Concat(initial_transform, layer1_transform));
const SkMatrix integral_layer2_transform = RasterCache::GetIntegralTransCTM(
SkMatrix::Concat(SkMatrix::Concat(initial_transform, layer1_transform),
layer2_transform));
#endif
const SkPaint child1_paint = SkPaint(SkColors::kRed);
const SkPaint child2_paint = SkPaint(SkColors::kBlue);
const SkPaint child3_paint = SkPaint(SkColors::kGreen);
Expand Down Expand Up @@ -278,21 +247,13 @@ TEST_F(OpacityLayerTest, Nested) {
auto expected_draw_calls = std::vector(
{MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
MockCanvas::DrawCall{1, MockCanvas::ConcatMatrixData{layer1_transform}},
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
MockCanvas::DrawCall{
1, MockCanvas::SetMatrixData{integral_layer1_transform}},
#endif
MockCanvas::DrawCall{
1, MockCanvas::SaveLayerData{opacity1_bounds, opacity1_paint,
nullptr, 2}},
MockCanvas::DrawCall{
2, MockCanvas::DrawPathData{child1_path, child1_paint}},
MockCanvas::DrawCall{2, MockCanvas::SaveData{3}},
MockCanvas::DrawCall{3, MockCanvas::ConcatMatrixData{layer2_transform}},
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
MockCanvas::DrawCall{
3, MockCanvas::SetMatrixData{integral_layer2_transform}},
#endif
MockCanvas::DrawCall{
3, MockCanvas::SaveLayerData{opacity2_bounds, opacity2_paint,
nullptr, 4}},
Expand Down
11 changes: 1 addition & 10 deletions flow/layers/picture_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,7 @@ void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
if (auto* cache = context->raster_cache) {
TRACE_EVENT0("flutter", "PictureLayer::RasterCache (Preroll)");

SkMatrix ctm = matrix;
ctm.postTranslate(offset_.x(), offset_.y());
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
ctm = RasterCache::GetIntegralTransCTM(ctm);
#endif
cache->Prepare(context->gr_context, sk_picture, ctm,
cache->Prepare(context->gr_context, sk_picture, matrix,
context->dst_color_space, is_complex_, will_change_);
}

Expand All @@ -44,10 +39,6 @@ void PictureLayer::Paint(PaintContext& context) const {

SkAutoCanvasRestore save(context.leaf_nodes_canvas, true);
context.leaf_nodes_canvas->translate(offset_.x(), offset_.y());
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
context.leaf_nodes_canvas->setMatrix(RasterCache::GetIntegralTransCTM(
context.leaf_nodes_canvas->getTotalMatrix()));
#endif

if (context.raster_cache &&
context.raster_cache->Draw(*picture(), *context.leaf_nodes_canvas)) {
Expand Down
19 changes: 5 additions & 14 deletions flow/layers/picture_layer_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@
#include "flutter/testing/mock_canvas.h"
#include "third_party/skia/include/core/SkPicture.h"

#ifndef SUPPORT_FRACTIONAL_TRANSLATION
#include "flutter/flow/raster_cache.h"
#endif

namespace flutter {
namespace testing {

Expand Down Expand Up @@ -85,16 +81,11 @@ TEST_F(PictureLayerTest, SimplePicture) {
EXPECT_FALSE(layer->needs_system_composite());

layer->Paint(paint_context());
auto expected_draw_calls = std::vector(
{MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
MockCanvas::DrawCall{1,
MockCanvas::ConcatMatrixData{layer_offset_matrix}},
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
MockCanvas::DrawCall{
1, MockCanvas::SetMatrixData{RasterCache::GetIntegralTransCTM(
layer_offset_matrix)}},
#endif
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}});
auto expected_draw_calls =
std::vector({MockCanvas::DrawCall{0, MockCanvas::SaveData{1}},
MockCanvas::DrawCall{
1, MockCanvas::ConcatMatrixData{layer_offset_matrix}},
MockCanvas::DrawCall{1, MockCanvas::RestoreData{0}}});
EXPECT_EQ(mock_canvas().draw_calls(), expected_draw_calls);
}

Expand Down
7 changes: 0 additions & 7 deletions flow/raster_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,6 @@ class RasterCache {
return bounds;
}

static SkMatrix GetIntegralTransCTM(const SkMatrix& ctm) {
SkMatrix result = ctm;
result[SkMatrix::kMTransX] = SkScalarRoundToScalar(ctm.getTranslateX());
result[SkMatrix::kMTransY] = SkScalarRoundToScalar(ctm.getTranslateY());
return result;
}

// Return true if the cache is generated.
//
// We may return false and not generate the cache if
Expand Down
Loading

0 comments on commit 4e29736

Please sign in to comment.