From e7d3b59a7c828c9c51ac383ba24c27fd5ad71b57 Mon Sep 17 00:00:00 2001 From: Michael Klimushyn Date: Wed, 30 Oct 2019 11:14:30 -0700 Subject: [PATCH] Revert "Turn on RasterCache based on view hierarchy (#13360)" This caused EmbedderTest.VerifyB143464703 to fail after merging into master. ``` ../../flutter/shell/platform/embedder/tests/embedder_unittests.cc:3111: Failure Value of: ImageMatchesFixture("verifyb143464703.png", renderered_scene) Actual: false Expected: true [ FAILED ] EmbedderTest.VerifyB143464703 (2507 ms) ``` This reverts commit 3ad3bc76a5825210603d4afd12013de8ba7d7668. --- flow/layers/container_layer.cc | 14 -- flow/layers/layer.h | 1 - flow/layers/opacity_layer.cc | 8 +- flow/layers/platform_view_layer.cc | 1 - flow/raster_cache.cc | 47 +++-- flow/raster_cache.h | 2 - shell/common/shell.cc | 2 +- shell/common/shell.h | 2 +- shell/platform/embedder/BUILD.gn | 1 - shell/platform/embedder/embedder_engine.cc | 5 - shell/platform/embedder/embedder_engine.h | 2 - shell/platform/embedder/fixtures/main.dart | 44 ----- .../embedder/tests/embedder_assertions.h | 5 - .../embedder/tests/embedder_unittests.cc | 176 ------------------ 14 files changed, 29 insertions(+), 281 deletions(-) diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index d5c6a2a03a34a..31a5a255afca9 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -26,28 +26,14 @@ 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 index 66944376e8ce8..da2dcd8b21ce3 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -58,7 +58,6 @@ 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 index a27981650d2bf..014ee6736d92a 100644 --- 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->has_platform_view && context->raster_cache && + if (context->view_embedder == nullptr && context->raster_cache && SkRect::Intersects(context->cull_rect, paint_bounds())) { Layer* child = layers()[0].get(); SkMatrix ctm = child_matrix; @@ -75,7 +75,11 @@ void OpacityLayer::Paint(PaintContext& context) const { // See |EnsureSingleChild|. FML_DCHECK(layers().size() == 1); - if (context.raster_cache) { + // 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) { 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 index 3f72993f97d66..15f9edf9719ee 100644 --- a/flow/layers/platform_view_layer.cc +++ b/flow/layers/platform_view_layer.cc @@ -23,7 +23,6 @@ void PlatformViewLayer::Preroll(PrerollContext* context, "does not support embedding"; return; } - context->has_platform_view = true; std::unique_ptr params = std::make_unique(); params->offsetPixels = diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 40087c15be340..d047b5340c386 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -158,28 +158,27 @@ 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->has_platform_view ? nullptr : 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->raster_cache, + context->checkerboard_offscreen_layers}; + if (layer->needs_painting()) { + layer->Paint(paintContext); + } + }); } } @@ -252,10 +251,6 @@ 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 index 3ea92588e1fd7..f008f2459cc30 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -100,8 +100,6 @@ 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 index 2398507b2a83d..7ead524f929d8 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -517,7 +517,7 @@ const TaskRunners& Shell::GetTaskRunners() const { return task_runners_; } -fml::WeakPtr Shell::GetRasterizer() const { +fml::WeakPtr Shell::GetRasterizer() { FML_DCHECK(is_setup_); return weak_rasterizer_; } diff --git a/shell/common/shell.h b/shell/common/shell.h index c392f08fbb9d4..74b3406ae3141 100644 --- 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 GetRasterizer() const; + fml::WeakPtr GetRasterizer(); //------------------------------------------------------------------------------ /// @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 index 46bb47fe392e9..50b700228dde0 100644 --- a/shell/platform/embedder/BUILD.gn +++ b/shell/platform/embedder/BUILD.gn @@ -125,7 +125,6 @@ if (current_toolchain == host_toolchain) { ":fixtures", "$flutter_root/lib/ui", "$flutter_root/runtime", - "$flutter_root/flow", "$flutter_root/testing:dart", "$flutter_root/testing:opengl", "$flutter_root/testing:skia", diff --git a/shell/platform/embedder/embedder_engine.cc b/shell/platform/embedder/embedder_engine.cc index 631ef6f23a6a4..4fcf7ae8937e1 100644 --- a/shell/platform/embedder/embedder_engine.cc +++ b/shell/platform/embedder/embedder_engine.cc @@ -247,9 +247,4 @@ 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 index 94026d9f9646f..1d3255f4360b5 100644 --- a/shell/platform/embedder/embedder_engine.h +++ b/shell/platform/embedder/embedder_engine.h @@ -80,8 +80,6 @@ class EmbedderEngine { bool RunTask(const FlutterTask* task); - const Shell& GetShell() const; - private: const std::unique_ptr thread_host_; TaskRunners task_runners_; diff --git a/shell/platform/embedder/fixtures/main.dart b/shell/platform/embedder/fixtures/main.dart index 007d240b813cd..44cc8306f7368 100644 --- a/shell/platform/embedder/fixtures/main.dart +++ b/shell/platform/embedder/fixtures/main.dart @@ -206,50 +206,6 @@ 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/tests/embedder_assertions.h b/shell/platform/embedder/tests/embedder_assertions.h index d0c69015b92fd..a4918f56bca13 100644 --- a/shell/platform/embedder/tests/embedder_assertions.h +++ b/shell/platform/embedder/tests/embedder_assertions.h @@ -9,7 +9,6 @@ #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" @@ -262,8 +261,4 @@ inline FlutterTransformation FlutterTransformationMake(const SkMatrix& matrix) { return transformation; } -inline flutter::EmbedderEngine* ToEmbedderEngine(const FlutterEngine& engine) { - return reinterpret_cast(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 index bfeb76e7e961d..3a7697e3eb2a9 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -7,8 +7,6 @@ #include #include "embedder.h" -#include "embedder_engine.h" -#include "flutter/flow/raster_cache.h" #include "flutter/fml/file.h" #include "flutter/fml/make_copyable.h" #include "flutter/fml/mapping.h" @@ -662,180 +660,6 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderToOpenGLFramebuffer) { latch.Wait(); } -//------------------------------------------------------------------------------ -/// Layers in a hierarchy containing a platform view should not be cached. The -/// other layers in the hierarchy should be, however. -TEST_F(EmbedderTest, RasterCacheDisabledWithPlatformViews) { - auto& context = GetEmbedderContext(); - - EmbedderConfigBuilder builder(context); - builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); - builder.SetCompositor(); - builder.SetDartEntrypoint("can_composite_platform_views_with_opacity"); - - context.GetCompositor().SetRenderTargetType( - EmbedderTestCompositor::RenderTargetType::kOpenGLFramebuffer); - - fml::CountDownLatch setup(3); - fml::CountDownLatch verify(1); - - context.GetCompositor().SetNextPresentCallback( - [&](const FlutterLayer** layers, size_t layers_count) { - ASSERT_EQ(layers_count, 3u); - - { - FlutterBackingStore backing_store = *layers[0]->backing_store; - backing_store.struct_size = sizeof(backing_store); - backing_store.type = kFlutterBackingStoreTypeOpenGL; - backing_store.did_update = true; - backing_store.open_gl.type = kFlutterOpenGLTargetTypeFramebuffer; - - FlutterLayer layer = {}; - layer.struct_size = sizeof(layer); - layer.type = kFlutterLayerContentTypeBackingStore; - layer.backing_store = &backing_store; - layer.size = FlutterSizeMake(800.0, 600.0); - layer.offset = FlutterPointMake(0, 0); - - ASSERT_EQ(*layers[0], layer); - } - - { - FlutterPlatformView platform_view = {}; - platform_view.struct_size = sizeof(platform_view); - platform_view.identifier = 42; - - FlutterLayer layer = {}; - layer.struct_size = sizeof(layer); - layer.type = kFlutterLayerContentTypePlatformView; - layer.platform_view = &platform_view; - layer.size = FlutterSizeMake(123.0, 456.0); - layer.offset = FlutterPointMake(1.0, 2.0); - - ASSERT_EQ(*layers[1], layer); - } - - { - FlutterBackingStore backing_store = *layers[2]->backing_store; - backing_store.struct_size = sizeof(backing_store); - backing_store.type = kFlutterBackingStoreTypeOpenGL; - backing_store.did_update = true; - backing_store.open_gl.type = kFlutterOpenGLTargetTypeFramebuffer; - - FlutterLayer layer = {}; - layer.struct_size = sizeof(layer); - layer.type = kFlutterLayerContentTypeBackingStore; - layer.backing_store = &backing_store; - layer.size = FlutterSizeMake(800.0, 600.0); - layer.offset = FlutterPointMake(0.0, 0.0); - - ASSERT_EQ(*layers[2], layer); - } - - setup.CountDown(); - }); - - context.AddNativeCallback( - "SignalNativeTest", - CREATE_NATIVE_ENTRY( - [&setup](Dart_NativeArguments args) { setup.CountDown(); })); - - UniqueEngine engine = builder.LaunchEngine(); - - // Send a window metrics events so frames may be scheduled. - FlutterWindowMetricsEvent event = {}; - event.struct_size = sizeof(event); - event.width = 800; - event.height = 600; - event.pixel_ratio = 1.0; - ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), - kSuccess); - ASSERT_TRUE(engine.is_valid()); - - setup.Wait(); - const flutter::Shell& shell = ToEmbedderEngine(engine.get())->GetShell(); - shell.GetTaskRunners().GetGPUTaskRunner()->PostTask([&] { - const flutter::RasterCache& raster_cache = - shell.GetRasterizer()->compositor_context()->raster_cache(); - // 3 layers total, but one of them had the platform view. So the cache - // should only have 2 entries. - ASSERT_EQ(raster_cache.GetCachedEntriesCount(), 2u); - verify.CountDown(); - }); - - verify.Wait(); -} - -//------------------------------------------------------------------------------ -/// The RasterCache should normally be enabled. -/// -TEST_F(EmbedderTest, RasterCacheEnabled) { - auto& context = GetEmbedderContext(); - - EmbedderConfigBuilder builder(context); - builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); - builder.SetCompositor(); - builder.SetDartEntrypoint("can_composite_with_opacity"); - - context.GetCompositor().SetRenderTargetType( - EmbedderTestCompositor::RenderTargetType::kOpenGLFramebuffer); - - fml::CountDownLatch setup(3); - fml::CountDownLatch verify(1); - - context.GetCompositor().SetNextPresentCallback( - [&](const FlutterLayer** layers, size_t layers_count) { - ASSERT_EQ(layers_count, 1u); - - { - FlutterBackingStore backing_store = *layers[0]->backing_store; - backing_store.struct_size = sizeof(backing_store); - backing_store.type = kFlutterBackingStoreTypeOpenGL; - backing_store.did_update = true; - backing_store.open_gl.type = kFlutterOpenGLTargetTypeFramebuffer; - - FlutterLayer layer = {}; - layer.struct_size = sizeof(layer); - layer.type = kFlutterLayerContentTypeBackingStore; - layer.backing_store = &backing_store; - layer.size = FlutterSizeMake(800.0, 600.0); - layer.offset = FlutterPointMake(0, 0); - - ASSERT_EQ(*layers[0], layer); - } - - setup.CountDown(); - }); - - context.AddNativeCallback( - "SignalNativeTest", - CREATE_NATIVE_ENTRY( - [&setup](Dart_NativeArguments args) { setup.CountDown(); })); - - UniqueEngine engine = builder.LaunchEngine(); - - // Send a window metrics events so frames may be scheduled. - FlutterWindowMetricsEvent event = {}; - event.struct_size = sizeof(event); - event.width = 800; - event.height = 600; - event.pixel_ratio = 1.0; - ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), - kSuccess); - ASSERT_TRUE(engine.is_valid()); - - setup.Wait(); - const flutter::Shell& shell = ToEmbedderEngine(engine.get())->GetShell(); - shell.GetTaskRunners().GetGPUTaskRunner()->PostTask([&] { - const flutter::RasterCache& raster_cache = - shell.GetRasterizer()->compositor_context()->raster_cache(); - ASSERT_EQ(raster_cache.GetCachedEntriesCount(), 1u); - verify.CountDown(); - }); - - verify.Wait(); -} - //------------------------------------------------------------------------------ /// Must be able to render using a custom compositor whose render targets for /// the individual layers are OpenGL textures.