Turn on RasterCache based on view hierarchy (flutter#13762)

Michael Klimushyn · web-flow · commit 8a99d1074821 · 2019-11-08T17:14:50.000-08:00
This is a duplicate of flutter/engine#13360 with the test switched to use the software backend instead of the GL backend. After some debugging and testing on another GL embedder I think the issue with the test is some bug having to do with the GL implementation in the test harness specifically. Fixes flutter#38903
diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter
@@ -878,6 +878,7 @@ FILE: ../../../flutter/shell/platform/embedder/fixtures/main.dart
 FILE: ../../../flutter/shell/platform/embedder/fixtures/scene_without_custom_compositor.png
 FILE: ../../../flutter/shell/platform/embedder/fixtures/scene_without_custom_compositor_with_xform.png
 FILE: ../../../flutter/shell/platform/embedder/fixtures/verifyb143464703.png
+FILE: ../../../flutter/shell/platform/embedder/fixtures/verifyb143464703_soft_noxform.png
 FILE: ../../../flutter/shell/platform/embedder/platform_view_embedder.cc
 FILE: ../../../flutter/shell/platform/embedder/platform_view_embedder.h
 FILE: ../../../flutter/shell/platform/embedder/vsync_waiter_embedder.cc
diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc
@@ -26,14 +26,28 @@ void ContainerLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
 void ContainerLayer::PrerollChildren(PrerollContext* context,
                                      const SkMatrix& child_matrix,
                                      SkRect* child_paint_bounds) {
+  // Platform views have no children, so context->has_platform_view should
+  // always be false.
+  FML_DCHECK(!context->has_platform_view);
+  bool child_has_platform_view = false;
   for (auto& layer : layers_) {
+    // Reset context->has_platform_view to false so that layers aren't treated
+    // as if they have a platform view based on one being previously found in a
+    // sibling tree.
+    context->has_platform_view = false;
+
     layer->Preroll(context, child_matrix);
 
     if (layer->needs_system_composite()) {
       set_needs_system_composite(true);
     }
     child_paint_bounds->join(layer->paint_bounds());
+
+    child_has_platform_view =
+        child_has_platform_view || context->has_platform_view;
   }
+
+  context->has_platform_view = child_has_platform_view;
 }
 
 void ContainerLayer::PaintChildren(PaintContext& context) const {
diff --git a/flow/layers/layer.h b/flow/layers/layer.h
@@ -58,6 +58,7 @@ struct PrerollContext {
   TextureRegistry& texture_registry;
   const bool checkerboard_offscreen_layers;
   float total_elevation = 0.0f;
+  bool has_platform_view = false;
 };
 
 // Represents a single composited layer. Created on the UI thread but then
diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc
@@ -46,7 +46,7 @@ void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
   set_paint_bounds(paint_bounds().makeOffset(offset_.fX, offset_.fY));
   // See |EnsureSingleChild|.
   FML_DCHECK(layers().size() == 1);
-  if (context->view_embedder == nullptr && context->raster_cache &&
+  if (!context->has_platform_view && context->raster_cache &&
       SkRect::Intersects(context->cull_rect, paint_bounds())) {
     Layer* child = layers()[0].get();
     SkMatrix ctm = child_matrix;
@@ -75,11 +75,7 @@ void OpacityLayer::Paint(PaintContext& context) const {
   // See |EnsureSingleChild|.
   FML_DCHECK(layers().size() == 1);
 
-  // Embedded platform views are changing the canvas in the middle of the paint
-  // traversal. To make sure we paint on the right canvas, when the embedded
-  // platform views preview is enabled (context.view_embedded is not null) we
-  // don't use the cache.
-  if (context.view_embedder == nullptr && context.raster_cache) {
+  if (context.raster_cache) {
     const SkMatrix& ctm = context.leaf_nodes_canvas->getTotalMatrix();
     RasterCacheResult child_cache =
         context.raster_cache->Get(layers()[0].get(), ctm);
diff --git a/flow/layers/platform_view_layer.cc b/flow/layers/platform_view_layer.cc
@@ -23,6 +23,7 @@ void PlatformViewLayer::Preroll(PrerollContext* context,
                       "does not support embedding";
     return;
   }
+  context->has_platform_view = true;
   std::unique_ptr<EmbeddedViewParams> params =
       std::make_unique<EmbeddedViewParams>();
   params->offsetPixels =
diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc
@@ -157,27 +157,28 @@ void RasterCache::Prepare(PrerollContext* context,
   entry.access_count = ClampSize(entry.access_count + 1, 0, access_threshold_);
   entry.used_this_frame = true;
   if (!entry.image.is_valid()) {
-    entry.image = Rasterize(context->gr_context, ctm, context->dst_color_space,
-                            checkerboard_images_, layer->paint_bounds(),
-                            [layer, context](SkCanvas* canvas) {
-                              SkISize canvas_size = canvas->getBaseLayerSize();
-                              SkNWayCanvas internal_nodes_canvas(
-                                  canvas_size.width(), canvas_size.height());
-                              internal_nodes_canvas.addCanvas(canvas);
-                              Layer::PaintContext paintContext = {
-                                  (SkCanvas*)&internal_nodes_canvas,
-                                  canvas,
-                                  context->gr_context,
-                                  nullptr,
-                                  context->raster_time,
-                                  context->ui_time,
-                                  context->texture_registry,
-                                  context->raster_cache,
-                                  context->checkerboard_offscreen_layers};
-                              if (layer->needs_painting()) {
-                                layer->Paint(paintContext);
-                              }
-                            });
+    entry.image = Rasterize(
+        context->gr_context, ctm, context->dst_color_space,
+        checkerboard_images_, layer->paint_bounds(),
+        [layer, context](SkCanvas* canvas) {
+          SkISize canvas_size = canvas->getBaseLayerSize();
+          SkNWayCanvas internal_nodes_canvas(canvas_size.width(),
+                                             canvas_size.height());
+          internal_nodes_canvas.addCanvas(canvas);
+          Layer::PaintContext paintContext = {
+              (SkCanvas*)&internal_nodes_canvas,
+              canvas,
+              context->gr_context,
+              nullptr,
+              context->raster_time,
+              context->ui_time,
+              context->texture_registry,
+              context->has_platform_view ? nullptr : context->raster_cache,
+              context->checkerboard_offscreen_layers};
+          if (layer->needs_painting()) {
+            layer->Paint(paintContext);
+          }
+        });
   }
 }
 
@@ -250,6 +251,10 @@ void RasterCache::Clear() {
   layer_cache_.clear();
 }
 
+size_t RasterCache::GetCachedEntriesCount() const {
+  return layer_cache_.size() + picture_cache_.size();
+}
+
 void RasterCache::SetCheckboardCacheImages(bool checkerboard) {
   if (checkerboard_images_ == checkerboard) {
     return;
diff --git a/flow/raster_cache.h b/flow/raster_cache.h
@@ -100,6 +100,8 @@ class RasterCache {
 
   void SetCheckboardCacheImages(bool checkerboard);
 
+  size_t GetCachedEntriesCount() const;
+
  private:
   struct Entry {
     bool used_this_frame = false;
diff --git a/shell/common/shell.cc b/shell/common/shell.cc
@@ -523,7 +523,7 @@ const TaskRunners& Shell::GetTaskRunners() const {
   return task_runners_;
 }
 
-fml::WeakPtr<Rasterizer> Shell::GetRasterizer() {
+fml::WeakPtr<Rasterizer> Shell::GetRasterizer() const {
   FML_DCHECK(is_setup_);
   return weak_rasterizer_;
 }
diff --git a/shell/common/shell.h b/shell/common/shell.h
@@ -210,7 +210,7 @@ class Shell final : public PlatformView::Delegate,
   ///
   /// @return     A weak pointer to the rasterizer.
   ///
-  fml::WeakPtr<Rasterizer> GetRasterizer();
+  fml::WeakPtr<Rasterizer> GetRasterizer() const;
 
   //------------------------------------------------------------------------------
   /// @brief      Engines may only be accessed on the UI thread. This method is
diff --git a/shell/platform/embedder/BUILD.gn b/shell/platform/embedder/BUILD.gn
@@ -98,6 +98,7 @@ test_fixtures("fixtures") {
     "fixtures/scene_without_custom_compositor.png",
     "fixtures/scene_without_custom_compositor_with_xform.png",
     "fixtures/verifyb143464703.png",
+    "fixtures/verifyb143464703_soft_noxform.png",
   ]
 }
 
@@ -125,6 +126,7 @@ if (current_toolchain == host_toolchain) {
     deps = [
       ":embedder",
       ":fixtures",
+      "$flutter_root/flow",
       "$flutter_root/lib/ui",
       "$flutter_root/runtime",
       "$flutter_root/testing:dart",
diff --git a/shell/platform/embedder/embedder_engine.cc b/shell/platform/embedder/embedder_engine.cc
@@ -247,4 +247,9 @@ bool EmbedderEngine::RunTask(const FlutterTask* task) {
                                 task->task);
 }
 
+const Shell& EmbedderEngine::GetShell() const {
+  FML_DCHECK(shell_);
+  return *shell_.get();
+}
+
 }  // namespace flutter
diff --git a/shell/platform/embedder/embedder_engine.h b/shell/platform/embedder/embedder_engine.h
@@ -80,6 +80,8 @@ class EmbedderEngine {
 
   bool RunTask(const FlutterTask* task);
 
+  const Shell& GetShell() const;
+
  private:
   const std::unique_ptr<EmbedderThreadHost> thread_host_;
   TaskRunners task_runners_;
diff --git a/shell/platform/embedder/fixtures/main.dart b/shell/platform/embedder/fixtures/main.dart
@@ -206,6 +206,50 @@ void can_composite_platform_views() {
   window.scheduleFrame();
 }
 
+@pragma('vm:entry-point')
+void can_composite_platform_views_with_opacity() {
+  window.onBeginFrame = (Duration duration) {
+    SceneBuilder builder = SceneBuilder();
+
+    // Root node
+    builder.pushOffset(1.0, 2.0);
+
+    // First sibling layer (no platform view, should be cached)
+    builder.pushOpacity(127);
+    builder.addPicture(Offset(1.0, 1.0), CreateSimplePicture());
+    builder.pop();
+
+    // Second sibling layer (platform view, should not be cached)
+    builder.pushOpacity(127);
+    builder.addPlatformView(42, width: 123.0, height: 456.0);
+    builder.pop();
+
+    // Third sibling layer (no platform view, should be cached)
+    builder.pushOpacity(127);
+    builder.addPicture(Offset(2.0, 1.0), CreateSimplePicture());
+    builder.pop();
+
+    signalNativeTest(); // Signal 2
+    window.render(builder.build());
+  };
+  signalNativeTest(); // Signal 1
+  window.scheduleFrame();
+}
+
+@pragma('vm:entry-point')
+void can_composite_with_opacity() {
+  window.onBeginFrame = (Duration duration) {
+    SceneBuilder builder = SceneBuilder();
+    builder.pushOpacity(127);
+    builder.addPicture(Offset(1.0, 1.0), CreateSimplePicture());
+    builder.pop(); // offset
+    signalNativeTest(); // Signal 2
+    window.render(builder.build());
+  };
+  signalNativeTest(); // Signal 1
+  window.scheduleFrame();
+}
+
 Picture CreateColoredBox(Color color, Size size) {
   Paint paint = Paint();
   paint.color = color;
diff --git a/shell/platform/embedder/fixtures/verifyb143464703_soft_noxform.png b/shell/platform/embedder/fixtures/verifyb143464703_soft_noxform.png
diff --git a/shell/platform/embedder/tests/embedder_assertions.h b/shell/platform/embedder/tests/embedder_assertions.h
@@ -9,6 +9,7 @@
 
 #include "flutter/fml/logging.h"
 #include "flutter/shell/platform/embedder/embedder.h"
+#include "flutter/shell/platform/embedder/embedder_engine.h"
 #include "flutter/testing/assertions.h"
 #include "gtest/gtest.h"
 #include "third_party/skia/include/core/SkPoint.h"
@@ -261,4 +262,8 @@ inline FlutterTransformation FlutterTransformationMake(const SkMatrix& matrix) {
   return transformation;
 }
 
+inline flutter::EmbedderEngine* ToEmbedderEngine(const FlutterEngine& engine) {
+  return reinterpret_cast<flutter::EmbedderEngine*>(engine);
+}
+
 #endif  // FLUTTER_SHELL_PLATFORM_EMBEDDER_TESTS_EMBEDDER_ASSERTIONS_H_
diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc

Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,7 @@ void PlatformViewLayer::Preroll(PrerollContext* context,`
`23`	`23`	`"does not support embedding";`
`24`	`24`	`return;`
`25`	`25`	`}`
	`26`	`+ context->has_platform_view = true;`
`26`	`27`	`std::unique_ptr<EmbeddedViewParams> params =`
`27`	`28`	`std::make_unique<EmbeddedViewParams>();`
`28`	`29`	`params->offsetPixels =`
Original file line number	Diff line number	Diff line change
`@@ -523,7 +523,7 @@ const TaskRunners& Shell::GetTaskRunners() const {`
`523`	`523`	`return task_runners_;`
`524`	`524`	`}`
`525`	`525`
`526`		`-fml::WeakPtr<Rasterizer> Shell::GetRasterizer() {`
	`526`	`+fml::WeakPtr<Rasterizer> Shell::GetRasterizer() const {`
`527`	`527`	`FML_DCHECK(is_setup_);`
`528`	`528`	`return weak_rasterizer_;`
`529`	`529`	`}`
Original file line number	Diff line number	Diff line change
`@@ -98,6 +98,7 @@ test_fixtures("fixtures") {`
`98`	`98`	`"fixtures/scene_without_custom_compositor.png",`
`99`	`99`	`"fixtures/scene_without_custom_compositor_with_xform.png",`
`100`	`100`	`"fixtures/verifyb143464703.png",`
	`101`	`+ "fixtures/verifyb143464703_soft_noxform.png",`
`101`	`102`	`]`
`102`	`103`	`}`
`103`	`104`
`@@ -125,6 +126,7 @@ if (current_toolchain == host_toolchain) {`
`125`	`126`	`deps = [`
`126`	`127`	`":embedder",`
`127`	`128`	`":fixtures",`
	`129`	`+ "$flutter_root/flow",`
`128`	`130`	`"$flutter_root/lib/ui",`
`129`	`131`	`"$flutter_root/runtime",`
`130`	`132`	`"$flutter_root/testing:dart",`