From bc9f524c04046f26939fdf0a1d15829d3ec9a362 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 14 Jul 2020 22:40:04 -0700 Subject: [PATCH 01/17] Hint the VM when a layer or picture goes out of scope --- flow/compositor_context.cc | 14 +++++++++----- flow/compositor_context.h | 16 ++++++++++++++-- flow/layers/layer.cc | 5 +++-- flow/layers/layer.h | 6 +++++- flow/layers/layer_tree.cc | 1 + flow/layers/picture_layer.cc | 16 ++++++++++++---- flow/layers/picture_layer.h | 3 ++- flow/raster_cache.cc | 12 ++++++++---- flow/raster_cache.h | 12 +++++++++--- lib/ui/compositing/scene_builder.cc | 2 +- runtime/runtime_controller.cc | 3 ++- runtime/runtime_controller.h | 5 ++++- shell/common/engine.cc | 7 ++++++- shell/common/engine.h | 10 ++++++++++ shell/common/rasterizer.cc | 5 +++++ shell/common/rasterizer.h | 9 ++++++++- shell/common/shell.cc | 5 +++++ shell/common/shell.h | 3 +++ 18 files changed, 107 insertions(+), 27 deletions(-) diff --git a/flow/compositor_context.cc b/flow/compositor_context.cc index e94ffd53f53d7..2d58ab0f1547f 100644 --- a/flow/compositor_context.cc +++ b/flow/compositor_context.cc @@ -9,8 +9,9 @@ namespace flutter { -CompositorContext::CompositorContext(fml::Milliseconds frame_budget) - : raster_time_(frame_budget), ui_time_(frame_budget) {} +CompositorContext::CompositorContext(Delegate& delegate, + fml::Milliseconds frame_budget) + : delegate_(delegate), raster_time_(frame_budget), ui_time_(frame_budget) {} CompositorContext::~CompositorContext() = default; @@ -23,8 +24,11 @@ void CompositorContext::BeginFrame(ScopedFrame& frame, } void CompositorContext::EndFrame(ScopedFrame& frame, - bool enable_instrumentation) { - raster_cache_.SweepAfterFrame(); + bool enable_instrumentation, + size_t freed_hint) { + freed_hint += raster_cache_.SweepAfterFrame(); + delegate_.OnCompositorEndFrame(freed_hint); + if (enable_instrumentation) { raster_time_.Stop(); } @@ -64,7 +68,7 @@ CompositorContext::ScopedFrame::ScopedFrame( } CompositorContext::ScopedFrame::~ScopedFrame() { - context_.EndFrame(*this, instrumentation_enabled_); + context_.EndFrame(*this, instrumentation_enabled_, uncached_external_size_); } RasterStatus CompositorContext::ScopedFrame::Raster( diff --git a/flow/compositor_context.h b/flow/compositor_context.h index e3c786d156126..6edf18f175f97 100644 --- a/flow/compositor_context.h +++ b/flow/compositor_context.h @@ -37,6 +37,11 @@ enum class RasterStatus { class CompositorContext { public: + class Delegate { + public: + virtual void OnCompositorEndFrame(size_t freed_hint) = 0; + }; + class ScopedFrame { public: ScopedFrame(CompositorContext& context, @@ -67,6 +72,8 @@ class CompositorContext { virtual RasterStatus Raster(LayerTree& layer_tree, bool ignore_raster_cache); + void add_external_size(size_t size) { uncached_external_size_ += size; } + private: CompositorContext& context_; GrContext* gr_context_; @@ -76,11 +83,13 @@ class CompositorContext { const bool instrumentation_enabled_; const bool surface_supports_readback_; fml::RefPtr raster_thread_merger_; + size_t uncached_external_size_ = 0; FML_DISALLOW_COPY_AND_ASSIGN(ScopedFrame); }; - CompositorContext(fml::Milliseconds frame_budget = fml::kDefaultFrameBudget); + CompositorContext(Delegate& delegate, + fml::Milliseconds frame_budget = fml::kDefaultFrameBudget); virtual ~CompositorContext(); @@ -108,6 +117,7 @@ class CompositorContext { Stopwatch& ui_time() { return ui_time_; } private: + Delegate& delegate_; RasterCache raster_cache_; TextureRegistry texture_registry_; Counter frame_count_; @@ -116,7 +126,9 @@ class CompositorContext { void BeginFrame(ScopedFrame& frame, bool enable_instrumentation); - void EndFrame(ScopedFrame& frame, bool enable_instrumentation); + void EndFrame(ScopedFrame& frame, + bool enable_instrumentation, + size_t freed_hint); FML_DISALLOW_COPY_AND_ASSIGN(CompositorContext); }; diff --git a/flow/layers/layer.cc b/flow/layers/layer.cc index 97da04f7f54c8..7a84dae48335e 100644 --- a/flow/layers/layer.cc +++ b/flow/layers/layer.cc @@ -9,10 +9,11 @@ namespace flutter { -Layer::Layer() +Layer::Layer(size_t external_size) : paint_bounds_(SkRect::MakeEmpty()), unique_id_(NextUniqueID()), - needs_system_composite_(false) {} + needs_system_composite_(false), + external_size_(external_size) {} Layer::~Layer() = default; diff --git a/flow/layers/layer.h b/flow/layers/layer.h index 4986ac1f3af77..72adfe3f58253 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -71,13 +71,14 @@ struct PrerollContext { // Informs whether a layer needs to be system composited. bool child_scene_layer_exists_below = false; #endif + size_t uncached_external_size = 0; }; // Represents a single composited layer. Created on the UI thread but then // subquently used on the Rasterizer thread. class Layer { public: - Layer(); + Layer(size_t external_size = 0); virtual ~Layer(); virtual void Preroll(PrerollContext* context, const SkMatrix& matrix); @@ -185,6 +186,8 @@ class Layer { uint64_t unique_id() const { return unique_id_; } + size_t external_size() const { return external_size_; } + protected: #if defined(LEGACY_FUCHSIA_EMBEDDER) bool child_layer_exists_below_ = false; @@ -194,6 +197,7 @@ class Layer { SkRect paint_bounds_; uint64_t unique_id_; bool needs_system_composite_; + size_t external_size_ = 0; static uint64_t NextUniqueID(); diff --git a/flow/layers/layer_tree.cc b/flow/layers/layer_tree.cc index 0826dee610a41..0723c661da869 100644 --- a/flow/layers/layer_tree.cc +++ b/flow/layers/layer_tree.cc @@ -58,6 +58,7 @@ bool LayerTree::Preroll(CompositorContext::ScopedFrame& frame, frame_device_pixel_ratio_}; root_layer_->Preroll(&context, frame.root_surface_transformation()); + frame.add_external_size(context.uncached_external_size); return context.surface_needs_readback; } diff --git a/flow/layers/picture_layer.cc b/flow/layers/picture_layer.cc index d5d6a34b573e8..067a59d782398 100644 --- a/flow/layers/picture_layer.cc +++ b/flow/layers/picture_layer.cc @@ -11,8 +11,10 @@ namespace flutter { PictureLayer::PictureLayer(const SkPoint& offset, SkiaGPUObject picture, bool is_complex, - bool will_change) - : offset_(offset), + bool will_change, + size_t external_size) + : Layer(external_size), + offset_(offset), picture_(std::move(picture)), is_complex_(is_complex), will_change_(will_change) {} @@ -26,6 +28,7 @@ void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { SkPicture* sk_picture = picture(); + bool cached = false; if (auto* cache = context->raster_cache) { TRACE_EVENT0("flutter", "PictureLayer::RasterCache (Preroll)"); @@ -34,8 +37,13 @@ void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { #ifndef SUPPORT_FRACTIONAL_TRANSLATION ctm = RasterCache::GetIntegralTransCTM(ctm); #endif - cache->Prepare(context->gr_context, sk_picture, ctm, - context->dst_color_space, is_complex_, will_change_); + cached = cache->Prepare(context->gr_context, sk_picture, ctm, + context->dst_color_space, is_complex_, will_change_, + external_size()); + } + + if (!cached) { + context->uncached_external_size += external_size(); } SkRect bounds = sk_picture->cullRect().makeOffset(offset_.x(), offset_.y()); diff --git a/flow/layers/picture_layer.h b/flow/layers/picture_layer.h index e733e7455ca6c..c86361a9aaaae 100644 --- a/flow/layers/picture_layer.h +++ b/flow/layers/picture_layer.h @@ -18,7 +18,8 @@ class PictureLayer : public Layer { PictureLayer(const SkPoint& offset, SkiaGPUObject picture, bool is_complex, - bool will_change); + bool will_change, + size_t external_size); SkPicture* picture() const { return picture_.get().get(); } diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index ca3c400b99c94..3af46988a4c44 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -141,6 +141,7 @@ void RasterCache::Prepare(PrerollContext* context, Entry& entry = layer_cache_[cache_key]; entry.access_count++; entry.used_this_frame = true; + entry.external_size = layer->external_size(); if (!entry.image) { entry.image = RasterizeLayer(context, layer, ctm, checkerboard_images_); } @@ -181,7 +182,8 @@ bool RasterCache::Prepare(GrContext* context, const SkMatrix& transformation_matrix, SkColorSpace* dst_color_space, bool is_complex, - bool will_change) { + bool will_change, + size_t external_size) { // Disabling caching when access_threshold is zero is historic behavior. if (access_threshold_ == 0) { return false; @@ -207,6 +209,7 @@ bool RasterCache::Prepare(GrContext* context, // Creates an entry, if not present prior. Entry& entry = picture_cache_[cache_key]; + entry.external_size = external_size; if (entry.access_count < access_threshold_) { // Frame threshold has not yet been reached. return false; @@ -260,11 +263,12 @@ bool RasterCache::Draw(const Layer* layer, return false; } -void RasterCache::SweepAfterFrame() { - SweepOneCacheAfterFrame(picture_cache_); - SweepOneCacheAfterFrame(layer_cache_); +size_t RasterCache::SweepAfterFrame() { + size_t removed_size = SweepOneCacheAfterFrame(picture_cache_); + removed_size += SweepOneCacheAfterFrame(layer_cache_); picture_cached_this_frame_ = 0; TraceStatsToTimeline(); + return removed_size; } void RasterCache::Clear() { diff --git a/flow/raster_cache.h b/flow/raster_cache.h index f7b1467fc12c9..67a867189fe41 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -139,7 +139,8 @@ class RasterCache { const SkMatrix& transformation_matrix, SkColorSpace* dst_color_space, bool is_complex, - bool will_change); + bool will_change, + size_t external_size = 0); void Prepare(PrerollContext* context, Layer* layer, const SkMatrix& ctm); @@ -158,7 +159,8 @@ class RasterCache { SkCanvas& canvas, SkPaint* paint = nullptr) const; - void SweepAfterFrame(); + /// Returns the amount of external bytes freed by the sweep. + size_t SweepAfterFrame(); void Clear(); @@ -174,17 +176,20 @@ class RasterCache { struct Entry { bool used_this_frame = false; size_t access_count = 0; + size_t external_size = 0; std::unique_ptr image; }; template - static void SweepOneCacheAfterFrame(Cache& cache) { + static size_t SweepOneCacheAfterFrame(Cache& cache) { std::vector dead; + size_t removed_size = 0; for (auto it = cache.begin(); it != cache.end(); ++it) { Entry& entry = it->second; if (!entry.used_this_frame) { dead.push_back(it); + removed_size += entry.external_size; } entry.used_this_frame = false; } @@ -192,6 +197,7 @@ class RasterCache { for (auto it : dead) { cache.erase(it); } + return removed_size; } const size_t access_threshold_; diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 1c0fa9bd5597f..b60a5f1c67caa 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -220,7 +220,7 @@ void SceneBuilder::addPicture(double dx, pictureRect.offset(offset.x(), offset.y()); auto layer = std::make_unique( offset, UIDartState::CreateGPUObject(picture->picture()), !!(hints & 1), - !!(hints & 2)); + !!(hints & 2), picture->GetAllocationSize()); AddLayer(std::move(layer)); } diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index aa557d2abf716..b5508f8cf1ede 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -219,7 +219,7 @@ bool RuntimeController::ReportTimings(std::vector timings) { return false; } -bool RuntimeController::NotifyIdle(int64_t deadline) { +bool RuntimeController::NotifyIdle(int64_t deadline, size_t freed_hint) { std::shared_ptr root_isolate = root_isolate_.lock(); if (!root_isolate) { return false; @@ -227,6 +227,7 @@ bool RuntimeController::NotifyIdle(int64_t deadline) { tonic::DartState::Scope scope(root_isolate); + Dart_HintFreed(freed_hint); Dart_NotifyIdle(deadline); // Idle notifications being in isolate scope are part of the contract. diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index ad89cbeae064b..33b65d89b6071 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -329,9 +329,12 @@ class RuntimeController final : public WindowClient { /// system's monotonic time. The clock can be accessed via /// `Dart_TimelineGetMicros`. /// + /// @param[in] freed_hint A hint of the number of bytes potentially freed + /// since the last call to NotifyIdle if a GC were run. + /// /// @return If the idle notification was forwarded to the running isolate. /// - bool NotifyIdle(int64_t deadline); + bool NotifyIdle(int64_t deadline, size_t freed_hint); //---------------------------------------------------------------------------- /// @brief Returns if the root isolate is running. The isolate must be diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 7428cbf49828e..00f6c4936621e 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -234,11 +234,16 @@ void Engine::ReportTimings(std::vector timings) { runtime_controller_->ReportTimings(std::move(timings)); } +void Engine::HintFreed(size_t size) { + hint_freed_bytes_since_last_idle_ += size; +} + void Engine::NotifyIdle(int64_t deadline) { auto trace_event = std::to_string(deadline - Dart_TimelineGetMicros()); TRACE_EVENT1("flutter", "Engine::NotifyIdle", "deadline_now_delta", trace_event.c_str()); - runtime_controller_->NotifyIdle(deadline); + runtime_controller_->NotifyIdle(deadline, hint_freed_bytes_since_last_idle_); + hint_freed_bytes_since_last_idle_ = 0; } std::pair Engine::GetUIIsolateReturnCode() { diff --git a/shell/common/engine.h b/shell/common/engine.h index e92a91a955742..dc259d496abcc 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -451,6 +451,15 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// void BeginFrame(fml::TimePoint frame_time); + + //---------------------------------------------------------------------------- + /// @brief Notifies the engine that native bytes might be freed if a + /// garbage collection ran now. + /// + /// @param[in] size The number of bytes freed. + /// + void HintFreed(size_t size); + //---------------------------------------------------------------------------- /// @brief Notifies the engine that the UI task runner is not expected to /// undertake a new frame workload till a specified timepoint. The @@ -772,6 +781,7 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { FontCollection font_collection_; ImageDecoder image_decoder_; TaskRunners task_runners_; + size_t hint_freed_bytes_since_last_idle_ = 0; fml::WeakPtrFactory weak_factory_; // |RuntimeDelegate| diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 9735854f41842..19367419423bb 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -41,6 +41,7 @@ Rasterizer::Rasterizer( : Rasterizer(delegate, std::move(task_runners), std::make_unique( + *this, delegate.GetFrameBudget()), is_gpu_disabled_sync_switch) {} @@ -276,6 +277,10 @@ sk_sp Rasterizer::ConvertToRasterImage(sk_sp image) { }); } +void Rasterizer::OnCompositorEndFrame(size_t freed_hint) { + delegate_.OnCompositorFrameEnd(freed_hint); +} + RasterStatus Rasterizer::DoDraw( std::unique_ptr layer_tree) { FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread()); diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index c436c3bda91e5..685263590de8f 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -38,7 +38,7 @@ namespace flutter { /// and the on-screen render surface. The compositor context has all the GPU /// state necessary to render frames to the render surface. /// -class Rasterizer final : public SnapshotDelegate { +class Rasterizer final : public SnapshotDelegate, public CompositorContext::Delegate { public: //---------------------------------------------------------------------------- /// @brief Used to forward events from the rasterizer to interested @@ -68,6 +68,8 @@ class Rasterizer final : public SnapshotDelegate { /// virtual void OnFrameRasterized(const FrameTiming& frame_timing) = 0; + virtual void OnCompositorFrameEnd(size_t freed_hint) = 0; + /// Time limit for a smooth frame. See `Engine::GetDisplayRefreshRate`. virtual fml::Milliseconds GetFrameBudget() = 0; @@ -79,6 +81,8 @@ class Rasterizer final : public SnapshotDelegate { // TODO(dnfield): remove once embedders have caught up. class DummyDelegate : public Delegate { void OnFrameRasterized(const FrameTiming&) override {} + void OnCompositorFrameEnd(size_t freed_hint) override {} + fml::Milliseconds GetFrameBudget() override { return fml::kDefaultFrameBudget; } @@ -455,6 +459,9 @@ class Rasterizer final : public SnapshotDelegate { // |SnapshotDelegate| sk_sp ConvertToRasterImage(sk_sp image) override; + // |CompositorContext::Delegate| + void OnCompositorEndFrame(size_t freed_hint) override; + sk_sp ScreenshotLayerTreeAsImage( flutter::LayerTree* tree, flutter::CompositorContext& compositor_context, diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 976d1befb37b5..04bfdb38a784c 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1195,6 +1195,11 @@ void Shell::OnFrameRasterized(const FrameTiming& timing) { } } +void Shell::OnCompositorFrameEnd(size_t freed_hint) { + FML_DCHECK(engine_); + engine_->HintFreed(freed_hint); +} + fml::Milliseconds Shell::GetFrameBudget() { if (display_refresh_rate_ > 0) { return fml::RefreshRateToFrameBudget(display_refresh_rate_.load()); diff --git a/shell/common/shell.h b/shell/common/shell.h index fd1a30d3f4e35..44b4974eec232 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -527,6 +527,9 @@ class Shell final : public PlatformView::Delegate, // |Rasterizer::Delegate| void OnFrameRasterized(const FrameTiming&) override; + // |Rasterizer::Delegate| + void OnCompositorFrameEnd(size_t freed_hint) override; + // |Rasterizer::Delegate| fml::Milliseconds GetFrameBudget() override; From b351806e69108c661ece440288850c595cf57f76 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 17 Jul 2020 01:56:05 -0700 Subject: [PATCH 02/17] tests --- ci/licenses_golden/licenses_flutter | 1 + flow/layers/layer_tree_unittests.cc | 16 +++- flow/layers/picture_layer_unittests.cc | 12 ++- lib/ui/BUILD.gn | 1 + lib/ui/fixtures/ui_test.dart | 58 ++++++++++++ lib/ui/painting/image_release_unittests.cc | 103 +++++++++++++++++++++ shell/common/shell.cc | 5 +- shell/common/shell_test.cc | 19 ++++ shell/common/shell_test.h | 1 + shell/common/shell_unittests.cc | 6 +- 10 files changed, 211 insertions(+), 11 deletions(-) create mode 100644 lib/ui/painting/image_release_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index a6f62772e1551..e9614dd49d180 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -339,6 +339,7 @@ FILE: ../../../flutter/lib/ui/painting/image_encoding.cc FILE: ../../../flutter/lib/ui/painting/image_encoding.h FILE: ../../../flutter/lib/ui/painting/image_filter.cc FILE: ../../../flutter/lib/ui/painting/image_filter.h +FILE: ../../../flutter/lib/ui/painting/image_release_unittests.cc FILE: ../../../flutter/lib/ui/painting/image_shader.cc FILE: ../../../flutter/lib/ui/painting/image_shader.h FILE: ../../../flutter/lib/ui/painting/matrix.cc diff --git a/flow/layers/layer_tree_unittests.cc b/flow/layers/layer_tree_unittests.cc index 1215b72f78726..fe9332c8493a2 100644 --- a/flow/layers/layer_tree_unittests.cc +++ b/flow/layers/layer_tree_unittests.cc @@ -15,11 +15,16 @@ namespace flutter { namespace testing { -class LayerTreeTest : public CanvasTest { +class CompositorDelegate : public CompositorContext::Delegate { + public: + +}; + +class LayerTreeTest : public CanvasTest, public CompositorContext::Delegate { public: LayerTreeTest() : layer_tree_(SkISize::Make(64, 64), 100.0f, 1.0f), - compositor_context_(fml::kDefaultFrameBudget), + compositor_context_(*this, fml::kDefaultFrameBudget), root_transform_(SkMatrix::Translate(1.0f, 1.0f)), scoped_frame_(compositor_context_.AcquireFrame(nullptr, &mock_canvas(), @@ -33,11 +38,18 @@ class LayerTreeTest : public CanvasTest { CompositorContext::ScopedFrame& frame() { return *scoped_frame_.get(); } const SkMatrix& root_transform() { return root_transform_; } + void OnCompositorEndFrame(size_t freed_hint) override { + last_freed_hint_ = freed_hint; + } + + size_t last_freed_hint() { return last_freed_hint_; } + private: LayerTree layer_tree_; CompositorContext compositor_context_; SkMatrix root_transform_; std::unique_ptr scoped_frame_; + size_t last_freed_hint_ = 0; }; TEST_F(LayerTreeTest, PaintingEmptyLayerDies) { diff --git a/flow/layers/picture_layer_unittests.cc b/flow/layers/picture_layer_unittests.cc index dc9e6080c1508..879b9c3acdc28 100644 --- a/flow/layers/picture_layer_unittests.cc +++ b/flow/layers/picture_layer_unittests.cc @@ -24,7 +24,7 @@ using PictureLayerTest = SkiaGPUObjectLayerTest; TEST_F(PictureLayerTest, PaintBeforePrerollInvalidPictureDies) { const SkPoint layer_offset = SkPoint::Make(0.0f, 0.0f); auto layer = std::make_shared( - layer_offset, SkiaGPUObject(), false, false); + layer_offset, SkiaGPUObject(), false, false, 0); EXPECT_DEATH_IF_SUPPORTED(layer->Paint(paint_context()), "picture_\\.get\\(\\)"); @@ -35,7 +35,7 @@ TEST_F(PictureLayerTest, PaintBeforePreollDies) { const SkRect picture_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 20.5f, 21.5f); auto mock_picture = SkPicture::MakePlaceholder(picture_bounds); auto layer = std::make_shared( - layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false); + layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false, 0); EXPECT_EQ(layer->paint_bounds(), SkRect::MakeEmpty()); EXPECT_DEATH_IF_SUPPORTED(layer->Paint(paint_context()), @@ -47,7 +47,7 @@ TEST_F(PictureLayerTest, PaintingEmptyLayerDies) { const SkRect picture_bounds = SkRect::MakeEmpty(); auto mock_picture = SkPicture::MakePlaceholder(picture_bounds); auto layer = std::make_shared( - layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false); + layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false, 0); layer->Preroll(preroll_context(), SkMatrix()); EXPECT_EQ(layer->paint_bounds(), SkRect::MakeEmpty()); @@ -62,7 +62,7 @@ TEST_F(PictureLayerTest, PaintingEmptyLayerDies) { TEST_F(PictureLayerTest, InvalidPictureDies) { const SkPoint layer_offset = SkPoint::Make(0.0f, 0.0f); auto layer = std::make_shared( - layer_offset, SkiaGPUObject(), false, false); + layer_offset, SkiaGPUObject(), false, false, 0); // Crashes reading a nullptr. EXPECT_DEATH_IF_SUPPORTED(layer->Preroll(preroll_context(), SkMatrix()), ""); @@ -75,7 +75,9 @@ TEST_F(PictureLayerTest, SimplePicture) { const SkRect picture_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 20.5f, 21.5f); auto mock_picture = SkPicture::MakePlaceholder(picture_bounds); auto layer = std::make_shared( - layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false); + layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false, 1000); + + EXPECT_EQ(layer->external_size(), 1000ul); layer->Preroll(preroll_context(), SkMatrix()); EXPECT_EQ(layer->paint_bounds(), diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index 0699795130746..bc0ef8d061c1e 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -178,6 +178,7 @@ if (enable_unittests) { public_configs = [ "//flutter:export_dynamic_symbols" ] sources = [ + "painting/image_release_unittests.cc", "painting/vertices_unittests.cc", "window/pointer_data_packet_converter_unittests.cc", ] diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index ecd42cc041bf5..15301c3898c6f 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -3,6 +3,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; import 'dart:typed_data'; import 'dart:ui'; @@ -44,3 +45,60 @@ void frameCallback(FrameInfo info) { @pragma('vm:entry-point') void messageCallback(dynamic data) { } + +@pragma('vm:entry-point') +Future pumpImage() async { + final Completer completer = Completer(); + const int width = 8000; + const int height = 8000; + decodeImageFromPixels( + Uint8List.fromList(List.filled(width * height * 4, 0xFF)), + width, + height, + PixelFormat.rgba8888, + (Image image) => completer.complete(image), + ); + + final Image image = await completer.future; + + final FrameCallback renderBlank = (Duration duration) { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + canvas.drawRect(Rect.largest, Paint()); + final Picture picture = recorder.endRecording(); + + final SceneBuilder builder = SceneBuilder(); + builder.addPicture(Offset.zero, picture); + + final Scene scene = builder.build(); + window.render(scene); + scene.dispose(); + window.onBeginFrame = (Duration duration) => _done(); + window.scheduleFrame(); + }; + + final FrameCallback renderImage = (Duration duration) { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + canvas.drawImage(image, Offset.zero, Paint()); + final Picture picture = recorder.endRecording(); + + final SceneBuilder builder = SceneBuilder(); + builder.addPicture(Offset.zero, picture); + + _captureImageAndPicture(image, picture); + + final Scene scene = builder.build(); + window.render(scene); + scene.dispose(); + picture.dispose(); + image.dispose(); + window.onBeginFrame = renderBlank; + window.scheduleFrame(); + }; + + window.onBeginFrame = renderImage; + window.scheduleFrame(); +} +void _captureImageAndPicture(Image image, Picture picture) native 'CaptureImageAndPicture'; +Future _done() native 'Done'; diff --git a/lib/ui/painting/image_release_unittests.cc b/lib/ui/painting/image_release_unittests.cc new file mode 100644 index 0000000000000..837542c63e816 --- /dev/null +++ b/lib/ui/painting/image_release_unittests.cc @@ -0,0 +1,103 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#define FML_USED_ON_EMBEDDER + +#include "flutter/common/task_runners.h" +#include "flutter/fml/synchronization/waitable_event.h" +#include "flutter/lib/ui/painting/image.h" +#include "flutter/lib/ui/painting/picture.h" +#include "flutter/runtime/dart_vm.h" +#include "flutter/shell/common/shell_test.h" +#include "flutter/shell/common/thread_host.h" +#include "flutter/testing/testing.h" + +namespace flutter { +namespace testing { + +template +T* GetNativePeer(Dart_NativeArguments args, int index) { + auto handle = Dart_GetNativeArgument(args, index); + intptr_t peer = 0; + EXPECT_FALSE(Dart_IsError(Dart_GetNativeInstanceField( + handle, tonic::DartWrappable::kPeerIndex, &peer))); + return reinterpret_cast(peer); +} + +TEST_F(ShellTest, ImageReleasedAfterFrame) { + fml::AutoResetWaitableEvent message_latch; + + sk_sp current_image; + sk_sp current_picture; + auto nativeCaptureImageAndPicture = [&](Dart_NativeArguments args) { + CanvasImage* image = GetNativePeer(args, 0); + Picture* picture = GetNativePeer(args, 1); + ASSERT_FALSE(image->image()->unique()); + ASSERT_FALSE(picture->picture()->unique()); + current_image = image->image(); + current_picture = picture->picture(); + }; + + auto nativeDone = [&](Dart_NativeArguments args) { message_latch.Signal(); }; + + Settings settings = CreateSettingsForFixture(); + TaskRunners task_runners("test", // label + GetCurrentTaskRunner(), // platform + CreateNewThread(), // raster + CreateNewThread(), // ui + CreateNewThread() // io + ); + + AddNativeCallback("CaptureImageAndPicture", + CREATE_NATIVE_ENTRY(nativeCaptureImageAndPicture)); + AddNativeCallback("Done", CREATE_NATIVE_ENTRY(nativeDone)); + + std::unique_ptr shell = CreateShell(std::move(settings), task_runners); + + ASSERT_TRUE(shell->IsSetup()); + + SetViewportMetrics(shell.get(), 800, 600); + + shell->GetPlatformView()->NotifyCreated(); + + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("pumpImage"); + + shell->RunEngine(std::move(configuration), [&](auto result) { + ASSERT_EQ(result, Engine::RunStatus::Success); + }); + + message_latch.Wait(); + + ASSERT_TRUE(current_picture); + ASSERT_TRUE(current_image); + + ASSERT_FALSE(current_picture->unique()); + ASSERT_FALSE(current_image->unique()); + + // Drain the raster task runner to get to the end of the frame. + fml::AutoResetWaitableEvent latch; + task_runners.GetRasterTaskRunner()->PostTask([&latch]() { latch.Signal(); }); + latch.Wait(); + + // Tell the engine we're idle. + task_runners.GetUITaskRunner()->PostTask( + [&latch, engine = shell->GetEngine()]() { + ASSERT_TRUE(engine); + engine->NotifyIdle(Dart_TimelineGetMicros() + 10000); + latch.Signal(); + }); + latch.Wait(); + + + EXPECT_TRUE(current_picture->unique()); + current_picture.reset(); + EXPECT_TRUE(current_image->unique()); + + shell->GetPlatformView()->NotifyDestroyed(); + DestroyShell(std::move(shell), std::move(task_runners)); +} + +} // namespace testing +} // namespace flutter diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 04bfdb38a784c..981a4cc950a36 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1196,8 +1196,9 @@ void Shell::OnFrameRasterized(const FrameTiming& timing) { } void Shell::OnCompositorFrameEnd(size_t freed_hint) { - FML_DCHECK(engine_); - engine_->HintFreed(freed_hint); + if (engine_) { + engine_->HintFreed(freed_hint); + } } fml::Milliseconds Shell::GetFrameBudget() { diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index c8ebf642e056f..14cda75a6d91d 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -96,6 +96,25 @@ void ShellTest::VSyncFlush(Shell* shell, bool& will_draw_new_frame) { latch.Wait(); } +void ShellTest::SetViewportMetrics(Shell* shell, double width, double height) { + flutter::ViewportMetrics viewport_metrics = { + 1, width, height, flutter::kUnsetDepth, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; + // Set viewport to nonempty, and call Animator::BeginFrame to make the layer + // tree pipeline nonempty. Without either of this, the layer tree below + // won't be rasterized. + fml::AutoResetWaitableEvent latch; + shell->GetTaskRunners().GetUITaskRunner()->PostTask( + [&latch, engine = shell->weak_engine_, viewport_metrics]() { + engine->SetViewportMetrics(std::move(viewport_metrics)); + const auto frame_begin_time = fml::TimePoint::Now(); + const auto frame_end_time = + frame_begin_time + fml::TimeDelta::FromSecondsF(1.0 / 60.0); + engine->animator_->BeginFrame(frame_begin_time, frame_end_time); + latch.Signal(); + }); + latch.Wait(); +} + void ShellTest::PumpOneFrame(Shell* shell, double width, double height, diff --git a/shell/common/shell_test.h b/shell/common/shell_test.h index 2b46881c3c68f..ebc117542d3c9 100644 --- a/shell/common/shell_test.h +++ b/shell/common/shell_test.h @@ -57,6 +57,7 @@ class ShellTest : public FixtureTest { /// the `will_draw_new_frame` to true. static void VSyncFlush(Shell* shell, bool& will_draw_new_frame); + static void SetViewportMetrics(Shell* shell, double width, double height); /// Given the root layer, this callback builds the layer tree to be rasterized /// in PumpOneFrame. using LayerTreeBuilder = diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index a2b04b8a44543..9f1609d6b81d2 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -507,7 +507,8 @@ TEST_F(ShellTest, this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0)); auto picture_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({sk_picture, queue}), false, false); + flutter::SkiaGPUObject({sk_picture, queue}), false, false, + 0); root->Add(picture_layer); }; @@ -1074,7 +1075,8 @@ TEST_F(ShellTest, Screenshot) { this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0)); auto picture_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({sk_picture, queue}), false, false); + flutter::SkiaGPUObject({sk_picture, queue}), false, false, + 0); root->Add(picture_layer); }; From d921c887031934fe09382087dd88635024214af5 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 13 Aug 2020 09:16:18 -0700 Subject: [PATCH 03/17] format --- flow/layers/layer_tree_unittests.cc | 1 - flow/layers/picture_layer_unittests.cc | 9 ++++++--- lib/ui/painting/image_release_unittests.cc | 1 - shell/common/engine.h | 1 - shell/common/rasterizer.h | 3 ++- 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/flow/layers/layer_tree_unittests.cc b/flow/layers/layer_tree_unittests.cc index fe9332c8493a2..3c6a109f47ef5 100644 --- a/flow/layers/layer_tree_unittests.cc +++ b/flow/layers/layer_tree_unittests.cc @@ -17,7 +17,6 @@ namespace testing { class CompositorDelegate : public CompositorContext::Delegate { public: - }; class LayerTreeTest : public CanvasTest, public CompositorContext::Delegate { diff --git a/flow/layers/picture_layer_unittests.cc b/flow/layers/picture_layer_unittests.cc index 879b9c3acdc28..b7bfce854ed82 100644 --- a/flow/layers/picture_layer_unittests.cc +++ b/flow/layers/picture_layer_unittests.cc @@ -35,7 +35,8 @@ TEST_F(PictureLayerTest, PaintBeforePreollDies) { const SkRect picture_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 20.5f, 21.5f); auto mock_picture = SkPicture::MakePlaceholder(picture_bounds); auto layer = std::make_shared( - layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false, 0); + layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false, + 0); EXPECT_EQ(layer->paint_bounds(), SkRect::MakeEmpty()); EXPECT_DEATH_IF_SUPPORTED(layer->Paint(paint_context()), @@ -47,7 +48,8 @@ TEST_F(PictureLayerTest, PaintingEmptyLayerDies) { const SkRect picture_bounds = SkRect::MakeEmpty(); auto mock_picture = SkPicture::MakePlaceholder(picture_bounds); auto layer = std::make_shared( - layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false, 0); + layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false, + 0); layer->Preroll(preroll_context(), SkMatrix()); EXPECT_EQ(layer->paint_bounds(), SkRect::MakeEmpty()); @@ -75,7 +77,8 @@ TEST_F(PictureLayerTest, SimplePicture) { const SkRect picture_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 20.5f, 21.5f); auto mock_picture = SkPicture::MakePlaceholder(picture_bounds); auto layer = std::make_shared( - layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false, 1000); + layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false, + 1000); EXPECT_EQ(layer->external_size(), 1000ul); diff --git a/lib/ui/painting/image_release_unittests.cc b/lib/ui/painting/image_release_unittests.cc index 837542c63e816..e9a2b5af1e736 100644 --- a/lib/ui/painting/image_release_unittests.cc +++ b/lib/ui/painting/image_release_unittests.cc @@ -90,7 +90,6 @@ TEST_F(ShellTest, ImageReleasedAfterFrame) { }); latch.Wait(); - EXPECT_TRUE(current_picture->unique()); current_picture.reset(); EXPECT_TRUE(current_image->unique()); diff --git a/shell/common/engine.h b/shell/common/engine.h index 073a329e4bfc8..97165f42a69fa 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -465,7 +465,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// void BeginFrame(fml::TimePoint frame_time); - //---------------------------------------------------------------------------- /// @brief Notifies the engine that native bytes might be freed if a /// garbage collection ran now. diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index 1f8121a1f3635..31d10585e8d59 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -38,7 +38,8 @@ namespace flutter { /// and the on-screen render surface. The compositor context has all the GPU /// state necessary to render frames to the render surface. /// -class Rasterizer final : public SnapshotDelegate, public CompositorContext::Delegate { +class Rasterizer final : public SnapshotDelegate, + public CompositorContext::Delegate { public: //---------------------------------------------------------------------------- /// @brief Used to forward events from the rasterizer to interested From b05fac35c6f21a6c29d4b67c76707784b6cf62d2 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 13 Aug 2020 09:58:00 -0700 Subject: [PATCH 04/17] no lint --- lib/ui/painting/image_release_unittests.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ui/painting/image_release_unittests.cc b/lib/ui/painting/image_release_unittests.cc index e9a2b5af1e736..3cbb67dd8ea00 100644 --- a/lib/ui/painting/image_release_unittests.cc +++ b/lib/ui/painting/image_release_unittests.cc @@ -1,6 +1,7 @@ // Copyright 2013 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +// FLUTTER_NOLINT #define FML_USED_ON_EMBEDDER From 21a9cd9fbff0110b8a16897d3c0c0945b9dc38a2 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 13 Aug 2020 17:29:21 -0700 Subject: [PATCH 05/17] fix fuchsia, fix test for AOT --- flow/compositor_context.cc | 7 ++-- flow/compositor_context.h | 7 ++-- flow/layers/layer_tree_unittests.cc | 6 +++- lib/ui/painting/image_release_unittests.cc | 36 +++++++++---------- shell/common/rasterizer.cc | 8 +---- shell/common/rasterizer.h | 10 ++---- shell/common/shell.cc | 2 +- shell/common/shell.h | 8 +++-- .../fuchsia/flutter/compositor_context.cc | 4 ++- .../fuchsia/flutter/compositor_context.h | 3 +- shell/platform/fuchsia/flutter/engine.cc | 1 + 11 files changed, 46 insertions(+), 46 deletions(-) diff --git a/flow/compositor_context.cc b/flow/compositor_context.cc index 895c394a562df..01e23e057196c 100644 --- a/flow/compositor_context.cc +++ b/flow/compositor_context.cc @@ -9,9 +9,10 @@ namespace flutter { -CompositorContext::CompositorContext(Delegate& delegate, - fml::Milliseconds frame_budget) - : delegate_(delegate), raster_time_(frame_budget), ui_time_(frame_budget) {} +CompositorContext::CompositorContext(Delegate& delegate) + : delegate_(delegate), + raster_time_(delegate.GetFrameBudget()), + ui_time_(delegate.GetFrameBudget()) {} CompositorContext::~CompositorContext() = default; diff --git a/flow/compositor_context.h b/flow/compositor_context.h index ced2d34b3c91f..d9448b829f737 100644 --- a/flow/compositor_context.h +++ b/flow/compositor_context.h @@ -39,7 +39,11 @@ class CompositorContext { public: class Delegate { public: + /// Called at the end of a frame with approximately how many bytes mightbe + /// freed if a GC ran now. virtual void OnCompositorEndFrame(size_t freed_hint) = 0; + /// Time limit for a smooth frame. See `Engine::GetDisplayRefreshRate`. + virtual fml::Milliseconds GetFrameBudget() = 0; }; class ScopedFrame { @@ -88,8 +92,7 @@ class CompositorContext { FML_DISALLOW_COPY_AND_ASSIGN(ScopedFrame); }; - CompositorContext(Delegate& delegate, - fml::Milliseconds frame_budget = fml::kDefaultFrameBudget); + explicit CompositorContext(Delegate& delegate); virtual ~CompositorContext(); diff --git a/flow/layers/layer_tree_unittests.cc b/flow/layers/layer_tree_unittests.cc index 3c6a109f47ef5..052bc2f1df7d9 100644 --- a/flow/layers/layer_tree_unittests.cc +++ b/flow/layers/layer_tree_unittests.cc @@ -23,7 +23,7 @@ class LayerTreeTest : public CanvasTest, public CompositorContext::Delegate { public: LayerTreeTest() : layer_tree_(SkISize::Make(64, 64), 100.0f, 1.0f), - compositor_context_(*this, fml::kDefaultFrameBudget), + compositor_context_(*this), root_transform_(SkMatrix::Translate(1.0f, 1.0f)), scoped_frame_(compositor_context_.AcquireFrame(nullptr, &mock_canvas(), @@ -41,6 +41,10 @@ class LayerTreeTest : public CanvasTest, public CompositorContext::Delegate { last_freed_hint_ = freed_hint; } + fml::Milliseconds GetFrameBudget() override { + return fml::kDefaultFrameBudget; + } + size_t last_freed_hint() { return last_freed_hint_; } private: diff --git a/lib/ui/painting/image_release_unittests.cc b/lib/ui/painting/image_release_unittests.cc index 3cbb67dd8ea00..0383dac8b6054 100644 --- a/lib/ui/painting/image_release_unittests.cc +++ b/lib/ui/painting/image_release_unittests.cc @@ -43,11 +43,12 @@ TEST_F(ShellTest, ImageReleasedAfterFrame) { auto nativeDone = [&](Dart_NativeArguments args) { message_latch.Signal(); }; Settings settings = CreateSettingsForFixture(); + auto task_runner = CreateNewThread(); TaskRunners task_runners("test", // label GetCurrentTaskRunner(), // platform - CreateNewThread(), // raster - CreateNewThread(), // ui - CreateNewThread() // io + task_runner, // raster + task_runner, // ui + task_runner // io ); AddNativeCallback("CaptureImageAndPicture", @@ -74,22 +75,19 @@ TEST_F(ShellTest, ImageReleasedAfterFrame) { ASSERT_TRUE(current_picture); ASSERT_TRUE(current_image); - ASSERT_FALSE(current_picture->unique()); - ASSERT_FALSE(current_image->unique()); - - // Drain the raster task runner to get to the end of the frame. - fml::AutoResetWaitableEvent latch; - task_runners.GetRasterTaskRunner()->PostTask([&latch]() { latch.Signal(); }); - latch.Wait(); - - // Tell the engine we're idle. - task_runners.GetUITaskRunner()->PostTask( - [&latch, engine = shell->GetEngine()]() { - ASSERT_TRUE(engine); - engine->NotifyIdle(Dart_TimelineGetMicros() + 10000); - latch.Signal(); - }); - latch.Wait(); + // AOT modes are fast enough that we get here before the task runner has + // had a chance to drain. Make sure that if the picture or image are not + // already unique, at least one idle notification has a chance to process + // after rasterization has occurred. + if (!current_picture->unique() || !current_image->unique()) { + fml::AutoResetWaitableEvent latch; + task_runner->PostTask([&latch, engine = shell->GetEngine()]() { + ASSERT_TRUE(engine); + engine->NotifyIdle(Dart_TimelineGetMicros() + 10000); + latch.Signal(); + }); + latch.Wait(); + } EXPECT_TRUE(current_picture->unique()); current_picture.reset(); diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index c0ecdf59534b8..7c02539b76b69 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -27,9 +27,7 @@ static constexpr std::chrono::milliseconds kSkiaCleanupExpiration(15000); Rasterizer::Rasterizer(Delegate& delegate) : Rasterizer(delegate, - std::make_unique( - *this, - delegate.GetFrameBudget())) {} + std::make_unique(delegate)) {} Rasterizer::Rasterizer( Delegate& delegate, @@ -264,10 +262,6 @@ sk_sp Rasterizer::ConvertToRasterImage(sk_sp image) { }); } -void Rasterizer::OnCompositorEndFrame(size_t freed_hint) { - delegate_.OnCompositorFrameEnd(freed_hint); -} - RasterStatus Rasterizer::DoDraw( std::unique_ptr layer_tree) { FML_DCHECK(delegate_.GetTaskRunners() diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index c13bad3ce5523..6482a9c4b77fc 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -38,8 +38,7 @@ namespace flutter { /// and the on-screen render surface. The compositor context has all the GPU /// state necessary to render frames to the render surface. /// -class Rasterizer final : public SnapshotDelegate, - public CompositorContext::Delegate { +class Rasterizer final : public SnapshotDelegate { public: //---------------------------------------------------------------------------- /// @brief Used to forward events from the rasterizer to interested @@ -51,7 +50,7 @@ class Rasterizer final : public SnapshotDelegate, /// are made on the GPU task runner. Any delegate must ensure that /// they can handle the threading implications. /// - class Delegate { + class Delegate : public CompositorContext::Delegate { public: //-------------------------------------------------------------------------- /// @brief Notifies the delegate that a frame has been rendered. The @@ -69,8 +68,6 @@ class Rasterizer final : public SnapshotDelegate, /// virtual void OnFrameRasterized(const FrameTiming& frame_timing) = 0; - virtual void OnCompositorFrameEnd(size_t freed_hint) = 0; - /// Time limit for a smooth frame. See `Engine::GetDisplayRefreshRate`. virtual fml::Milliseconds GetFrameBudget() = 0; @@ -415,9 +412,6 @@ class Rasterizer final : public SnapshotDelegate, // |SnapshotDelegate| sk_sp ConvertToRasterImage(sk_sp image) override; - // |CompositorContext::Delegate| - void OnCompositorEndFrame(size_t freed_hint) override; - sk_sp ScreenshotLayerTreeAsImage( flutter::LayerTree* tree, flutter::CompositorContext& compositor_context, diff --git a/shell/common/shell.cc b/shell/common/shell.cc index b23f28dbd8d86..2c016b4cb713b 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1200,7 +1200,7 @@ void Shell::OnFrameRasterized(const FrameTiming& timing) { } } -void Shell::OnCompositorFrameEnd(size_t freed_hint) { +void Shell::OnCompositorEndFrame(size_t freed_hint) { if (engine_) { engine_->HintFreed(freed_hint); } diff --git a/shell/common/shell.h b/shell/common/shell.h index 60b5fd058c6fe..75819ade3511e 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -12,6 +12,7 @@ #include "flutter/common/settings.h" #include "flutter/common/task_runners.h" +#include "flutter/flow/compositor_context.h" #include "flutter/flow/surface.h" #include "flutter/flow/texture.h" #include "flutter/fml/closure.h" @@ -528,13 +529,14 @@ class Shell final : public PlatformView::Delegate, void OnFrameRasterized(const FrameTiming&) override; // |Rasterizer::Delegate| - void OnCompositorFrameEnd(size_t freed_hint) override; + fml::TimePoint GetLatestFrameTargetTime() const override; // |Rasterizer::Delegate| + // |CompositorContext::Delegate| fml::Milliseconds GetFrameBudget() override; - // |Rasterizer::Delegate| - fml::TimePoint GetLatestFrameTargetTime() const override; + // |CompositorContext::Delegate| + void OnCompositorEndFrame(size_t freed_hint) override; // |ServiceProtocol::Handler| fml::RefPtr GetServiceProtocolHandlerTaskRunner( diff --git a/shell/platform/fuchsia/flutter/compositor_context.cc b/shell/platform/fuchsia/flutter/compositor_context.cc index b0bbfc7ecbc28..f37a58f47cabf 100644 --- a/shell/platform/fuchsia/flutter/compositor_context.cc +++ b/shell/platform/fuchsia/flutter/compositor_context.cc @@ -65,13 +65,15 @@ class ScopedFrame final : public flutter::CompositorContext::ScopedFrame { }; CompositorContext::CompositorContext( + flutter::CompositorContext::Delegate& delegate, std::string debug_label, fuchsia::ui::views::ViewToken view_token, scenic::ViewRefPair view_ref_pair, fidl::InterfaceHandle session, fml::closure session_error_callback, zx_handle_t vsync_event_handle) - : debug_label_(std::move(debug_label)), + : flutter::CompositorContext(delegate), + debug_label_(std::move(debug_label)), session_connection_( debug_label_, std::move(view_token), diff --git a/shell/platform/fuchsia/flutter/compositor_context.h b/shell/platform/fuchsia/flutter/compositor_context.h index 2fdcb02ce963b..5802e1ec97243 100644 --- a/shell/platform/fuchsia/flutter/compositor_context.h +++ b/shell/platform/fuchsia/flutter/compositor_context.h @@ -21,7 +21,8 @@ namespace flutter_runner { // Fuchsia. class CompositorContext final : public flutter::CompositorContext { public: - CompositorContext(std::string debug_label, + CompositorContext(CompositorContext::Delegate& delegate, + std::string debug_label, fuchsia::ui::views::ViewToken view_token, scenic::ViewRefPair view_ref_pair, fidl::InterfaceHandle session, diff --git a/shell/platform/fuchsia/flutter/engine.cc b/shell/platform/fuchsia/flutter/engine.cc index a23c488b64708..79490d705b092 100644 --- a/shell/platform/fuchsia/flutter/engine.cc +++ b/shell/platform/fuchsia/flutter/engine.cc @@ -221,6 +221,7 @@ Engine::Engine(Delegate& delegate, TRACE_DURATION("flutter", "CreateCompositorContext"); compositor_context = std::make_unique( + shell, thread_label, // debug label std::move(view_token), // scenic view we attach our tree to std::move(view_ref_pair), // scenic view ref/view ref control From 5bc0710382ff3e39eff602d43e2003ddc3b458c9 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 13 Aug 2020 20:46:04 -0700 Subject: [PATCH 06/17] missing ctors --- shell/common/skp_shader_warmup_unittests.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/shell/common/skp_shader_warmup_unittests.cc b/shell/common/skp_shader_warmup_unittests.cc index 689f2d9322fcc..ec81003c8f04f 100644 --- a/shell/common/skp_shader_warmup_unittests.cc +++ b/shell/common/skp_shader_warmup_unittests.cc @@ -153,7 +153,8 @@ class SkpWarmupTest : public ShellTest { auto picture_layer = std::make_shared( SkPoint::Make(0, 0), SkiaGPUObject(picture, queue), /* is_complex */ false, - /* will_change */ false); + /* will_change */ false, + /* external_size */ 0); root->Add(picture_layer); }; PumpOneFrame(shell.get(), picture->cullRect().width(), @@ -235,7 +236,8 @@ TEST_F(SkpWarmupTest, Image) { auto picture_layer = std::make_shared( SkPoint::Make(0, 0), SkiaGPUObject(picture, queue), /* is_complex */ false, - /* will_change */ false); + /* will_change */ false, + /* external_size */ 0); root->Add(picture_layer); }; From bd326274e70e9a189967b165dd59c097dc7a63e6 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 14 Aug 2020 09:34:49 -0700 Subject: [PATCH 07/17] zra review --- flow/layers/layer_tree_unittests.cc | 2 + lib/ui/fixtures/ui_test.dart | 7 ++-- lib/ui/painting/image_release_unittests.cc | 49 +++++++++++----------- runtime/runtime_controller.cc | 2 + 4 files changed, 32 insertions(+), 28 deletions(-) diff --git a/flow/layers/layer_tree_unittests.cc b/flow/layers/layer_tree_unittests.cc index 052bc2f1df7d9..dfbd7e858a4e7 100644 --- a/flow/layers/layer_tree_unittests.cc +++ b/flow/layers/layer_tree_unittests.cc @@ -37,10 +37,12 @@ class LayerTreeTest : public CanvasTest, public CompositorContext::Delegate { CompositorContext::ScopedFrame& frame() { return *scoped_frame_.get(); } const SkMatrix& root_transform() { return root_transform_; } + // |CompositorContext::Delegate| void OnCompositorEndFrame(size_t freed_hint) override { last_freed_hint_ = freed_hint; } + // |CompositorContext::Delegate| fml::Milliseconds GetFrameBudget() override { return fml::kDefaultFrameBudget; } diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index 2f8106d9d2587..6ddd27e48a393 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -71,6 +71,9 @@ Future encodeImageProducesExternalUint8List() async { _validateExternal(result); }); } +void _encodeImage(Image i, int format, void Function(Uint8List result)) + native 'EncodeImage'; +void _validateExternal(Uint8List result) native 'ValidateExternal'; @pragma('vm:entry-point') Future pumpImage() async { @@ -126,9 +129,5 @@ Future pumpImage() async { window.onBeginFrame = renderImage; window.scheduleFrame(); } - void _captureImageAndPicture(Image image, Picture picture) native 'CaptureImageAndPicture'; Future _done() native 'Done'; -void _encodeImage(Image i, int format, void Function(Uint8List result)) - native 'EncodeImage'; -void _validateExternal(Uint8List result) native 'ValidateExternal'; diff --git a/lib/ui/painting/image_release_unittests.cc b/lib/ui/painting/image_release_unittests.cc index 0383dac8b6054..b50d16ffbf6bb 100644 --- a/lib/ui/painting/image_release_unittests.cc +++ b/lib/ui/painting/image_release_unittests.cc @@ -1,7 +1,6 @@ // Copyright 2013 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// FLUTTER_NOLINT #define FML_USED_ON_EMBEDDER @@ -17,27 +16,30 @@ namespace flutter { namespace testing { -template -T* GetNativePeer(Dart_NativeArguments args, int index) { - auto handle = Dart_GetNativeArgument(args, index); - intptr_t peer = 0; - EXPECT_FALSE(Dart_IsError(Dart_GetNativeInstanceField( - handle, tonic::DartWrappable::kPeerIndex, &peer))); - return reinterpret_cast(peer); -} +class ImageReleaseTest : public ShellTest { + public: + template + T* GetNativePeer(Dart_NativeArguments args, int index) { + auto handle = Dart_GetNativeArgument(args, index); + intptr_t peer = 0; + EXPECT_FALSE(Dart_IsError(Dart_GetNativeInstanceField( + handle, tonic::DartWrappable::kPeerIndex, &peer))); + return reinterpret_cast(peer); + } -TEST_F(ShellTest, ImageReleasedAfterFrame) { fml::AutoResetWaitableEvent message_latch; + sk_sp current_image_; + sk_sp current_picture_; +}; - sk_sp current_image; - sk_sp current_picture; +TEST_F(ImageReleaseTest, ImageReleasedAfterFrame) { auto nativeCaptureImageAndPicture = [&](Dart_NativeArguments args) { CanvasImage* image = GetNativePeer(args, 0); Picture* picture = GetNativePeer(args, 1); ASSERT_FALSE(image->image()->unique()); ASSERT_FALSE(picture->picture()->unique()); - current_image = image->image(); - current_picture = picture->picture(); + current_image_ = image->image(); + current_picture_ = picture->picture(); }; auto nativeDone = [&](Dart_NativeArguments args) { message_latch.Signal(); }; @@ -72,26 +74,25 @@ TEST_F(ShellTest, ImageReleasedAfterFrame) { message_latch.Wait(); - ASSERT_TRUE(current_picture); - ASSERT_TRUE(current_image); + ASSERT_TRUE(current_picture_); + ASSERT_TRUE(current_image_); // AOT modes are fast enough that we get here before the task runner has // had a chance to drain. Make sure that if the picture or image are not // already unique, at least one idle notification has a chance to process // after rasterization has occurred. - if (!current_picture->unique() || !current_image->unique()) { - fml::AutoResetWaitableEvent latch; - task_runner->PostTask([&latch, engine = shell->GetEngine()]() { + if (!current_picture_->unique() || !current_image_->unique()) { + task_runner->PostTask([&, engine = shell->GetEngine()]() { ASSERT_TRUE(engine); engine->NotifyIdle(Dart_TimelineGetMicros() + 10000); - latch.Signal(); + message_latch.Signal(); }); - latch.Wait(); + message_latch.Wait(); } - EXPECT_TRUE(current_picture->unique()); - current_picture.reset(); - EXPECT_TRUE(current_image->unique()); + EXPECT_TRUE(current_picture_->unique()); + current_picture_.reset(); + EXPECT_TRUE(current_image_->unique()); shell->GetPlatformView()->NotifyDestroyed(); DestroyShell(std::move(shell), std::move(task_runners)); diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 3d4d303e1cd5d..9c81d6a759cdb 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -241,6 +241,8 @@ bool RuntimeController::NotifyIdle(int64_t deadline, size_t freed_hint) { tonic::DartState::Scope scope(root_isolate); + // Dart will use the freed hint at the next idle notification. Make sure to + // Update it with our latest value before calling NotifyIdle. Dart_HintFreed(freed_hint); Dart_NotifyIdle(deadline); From 3684db4abb369807d02ff34a8f72cb4512738424 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 14 Aug 2020 12:01:31 -0700 Subject: [PATCH 08/17] merge --- shell/common/shell_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 616c2ba997d10..e006b25fbc9c9 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -1367,7 +1367,7 @@ TEST_F(ShellTest, OnServiceProtocolEstimateRasterCacheMemoryWorks) { auto picture_layer = std::make_shared( SkPoint::Make(0, 0), flutter::SkiaGPUObject({MakeSizedPicture(100, 100), queue}), - false, false); + false, false, 0); picture_layer->set_paint_bounds(SkRect::MakeWH(100, 100)); // 2. Rasterize the picture and the picture layer in the raster cache. From d506040f1daf3bacebdaf648de8d1411fdec1b86 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 14 Aug 2020 13:10:44 -0700 Subject: [PATCH 09/17] onDrawFrame --- lib/ui/fixtures/ui_test.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index 6ddd27e48a393..8375092dcf7ae 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -102,7 +102,9 @@ Future pumpImage() async { final Scene scene = builder.build(); window.render(scene); scene.dispose(); - window.onBeginFrame = (Duration duration) => _done(); + window.onBeginFrame = (Duration duration) { + window.onDrawFrame = _done; + }; window.scheduleFrame(); }; From 3094415f627f31fb03ece6d20f12efc81b8ca1d6 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 18 Aug 2020 17:39:37 -0700 Subject: [PATCH 10/17] chinmay review --- flow/layers/layer_tree_unittests.cc | 4 ---- lib/ui/fixtures/ui_test.dart | 4 ++-- lib/ui/painting/image_release_unittests.cc | 11 +++++++---- shell/common/shell.cc | 15 +++++++++------ 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/flow/layers/layer_tree_unittests.cc b/flow/layers/layer_tree_unittests.cc index dfbd7e858a4e7..d9b19e3bdb671 100644 --- a/flow/layers/layer_tree_unittests.cc +++ b/flow/layers/layer_tree_unittests.cc @@ -15,10 +15,6 @@ namespace flutter { namespace testing { -class CompositorDelegate : public CompositorContext::Delegate { - public: -}; - class LayerTreeTest : public CanvasTest, public CompositorContext::Delegate { public: LayerTreeTest() diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index 8375092dcf7ae..8b1a896b75918 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -103,7 +103,7 @@ Future pumpImage() async { window.render(scene); scene.dispose(); window.onBeginFrame = (Duration duration) { - window.onDrawFrame = _done; + window.onDrawFrame = _onBeginFrameDone; }; window.scheduleFrame(); }; @@ -132,4 +132,4 @@ Future pumpImage() async { window.scheduleFrame(); } void _captureImageAndPicture(Image image, Picture picture) native 'CaptureImageAndPicture'; -Future _done() native 'Done'; +Future _onBeginFrameDone() native 'OnBeginFrameDone'; diff --git a/lib/ui/painting/image_release_unittests.cc b/lib/ui/painting/image_release_unittests.cc index b50d16ffbf6bb..34654b7849901 100644 --- a/lib/ui/painting/image_release_unittests.cc +++ b/lib/ui/painting/image_release_unittests.cc @@ -33,7 +33,7 @@ class ImageReleaseTest : public ShellTest { }; TEST_F(ImageReleaseTest, ImageReleasedAfterFrame) { - auto nativeCaptureImageAndPicture = [&](Dart_NativeArguments args) { + auto native_capture_image_and_picture = [&](Dart_NativeArguments args) { CanvasImage* image = GetNativePeer(args, 0); Picture* picture = GetNativePeer(args, 1); ASSERT_FALSE(image->image()->unique()); @@ -42,7 +42,9 @@ TEST_F(ImageReleaseTest, ImageReleasedAfterFrame) { current_picture_ = picture->picture(); }; - auto nativeDone = [&](Dart_NativeArguments args) { message_latch.Signal(); }; + auto native_on_begin_frame_done = [&](Dart_NativeArguments args) { + message_latch.Signal(); + }; Settings settings = CreateSettingsForFixture(); auto task_runner = CreateNewThread(); @@ -54,8 +56,9 @@ TEST_F(ImageReleaseTest, ImageReleasedAfterFrame) { ); AddNativeCallback("CaptureImageAndPicture", - CREATE_NATIVE_ENTRY(nativeCaptureImageAndPicture)); - AddNativeCallback("Done", CREATE_NATIVE_ENTRY(nativeDone)); + CREATE_NATIVE_ENTRY(native_capture_image_and_picture)); + AddNativeCallback("OnBeginFrameDone", + CREATE_NATIVE_ENTRY(native_on_begin_frame_done)); std::unique_ptr shell = CreateShell(std::move(settings), task_runners); diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 1837060d0103e..b11f343fe8313 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -625,7 +625,7 @@ void Shell::OnPlatformViewCreated(std::unique_ptr surface) { // a synchronous fashion. fml::AutoResetWaitableEvent latch; auto raster_task = - fml::MakeCopyable([&waiting_for_first_frame = waiting_for_first_frame_, + fml::MakeCopyable([& waiting_for_first_frame = waiting_for_first_frame_, rasterizer = rasterizer_->GetWeakPtr(), // surface = std::move(surface), // &latch]() mutable { @@ -990,7 +990,7 @@ void Shell::OnAnimatorDraw(fml::RefPtr> pipeline, } task_runners_.GetRasterTaskRunner()->PostTask( - [&waiting_for_first_frame = waiting_for_first_frame_, + [& waiting_for_first_frame = waiting_for_first_frame_, &waiting_for_first_frame_condition = waiting_for_first_frame_condition_, rasterizer = rasterizer_->GetWeakPtr(), pipeline = std::move(pipeline)]() { @@ -1206,9 +1206,12 @@ void Shell::OnFrameRasterized(const FrameTiming& timing) { } void Shell::OnCompositorEndFrame(size_t freed_hint) { - if (engine_) { - engine_->HintFreed(freed_hint); - } + task_runners_.GetUITaskRunner()->PostTask( + [engine = engine_->GetWeakPtr(), freed_hint = freed_hint]() { + if (engine) { + engine->HintFreed(freed_hint); + } + }); } fml::Milliseconds Shell::GetFrameBudget() { @@ -1525,7 +1528,7 @@ fml::Status Shell::WaitForFirstFrame(fml::TimeDelta timeout) { std::unique_lock lock(waiting_for_first_frame_mutex_); bool success = waiting_for_first_frame_condition_.wait_for( lock, std::chrono::milliseconds(timeout.ToMilliseconds()), - [&waiting_for_first_frame = waiting_for_first_frame_] { + [& waiting_for_first_frame = waiting_for_first_frame_] { return !waiting_for_first_frame.load(); }); if (success) { From 6a1a6287cbfcb98b2f717aec1910b8eeba5f172e Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 18 Aug 2020 17:48:46 -0700 Subject: [PATCH 11/17] format --- shell/common/shell.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/common/shell.cc b/shell/common/shell.cc index b11f343fe8313..fafe3bb2817ba 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -625,7 +625,7 @@ void Shell::OnPlatformViewCreated(std::unique_ptr surface) { // a synchronous fashion. fml::AutoResetWaitableEvent latch; auto raster_task = - fml::MakeCopyable([& waiting_for_first_frame = waiting_for_first_frame_, + fml::MakeCopyable([&waiting_for_first_frame = waiting_for_first_frame_, rasterizer = rasterizer_->GetWeakPtr(), // surface = std::move(surface), // &latch]() mutable { @@ -990,7 +990,7 @@ void Shell::OnAnimatorDraw(fml::RefPtr> pipeline, } task_runners_.GetRasterTaskRunner()->PostTask( - [& waiting_for_first_frame = waiting_for_first_frame_, + [&waiting_for_first_frame = waiting_for_first_frame_, &waiting_for_first_frame_condition = waiting_for_first_frame_condition_, rasterizer = rasterizer_->GetWeakPtr(), pipeline = std::move(pipeline)]() { @@ -1528,7 +1528,7 @@ fml::Status Shell::WaitForFirstFrame(fml::TimeDelta timeout) { std::unique_lock lock(waiting_for_first_frame_mutex_); bool success = waiting_for_first_frame_condition_.wait_for( lock, std::chrono::milliseconds(timeout.ToMilliseconds()), - [& waiting_for_first_frame = waiting_for_first_frame_] { + [&waiting_for_first_frame = waiting_for_first_frame_] { return !waiting_for_first_frame.load(); }); if (success) { From d743620b940fee7f77d43ce43b4554a0b0a88ec4 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 18 Aug 2020 21:14:07 -0700 Subject: [PATCH 12/17] fix ctor --- shell/common/shell_unittests.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 4c1672243ac5e..bb19173424a89 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -511,7 +511,8 @@ TEST_F(ShellTest, ExternalEmbedderNoThreadMerger) { this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0)); auto picture_layer = std::make_shared( SkPoint::Make(10, 10), - flutter::SkiaGPUObject({sk_picture, queue}), false, false); + flutter::SkiaGPUObject({sk_picture, queue}), false, false, + 0); root->Add(picture_layer); }; From 13efdb918f38b996d482d4e6d77e32e893ca3936 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 18 Aug 2020 21:35:44 -0700 Subject: [PATCH 13/17] more bad merge --- shell/common/shell_test.cc | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index 0d38cfc90cd05..62ddc1527511a 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -98,7 +98,22 @@ void ShellTest::VSyncFlush(Shell* shell, bool& will_draw_new_frame) { void ShellTest::SetViewportMetrics(Shell* shell, double width, double height) { flutter::ViewportMetrics viewport_metrics = { - 1, width, height, flutter::kUnsetDepth, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; + 1, // device pixel ratio + width, // physical width + height, // physical height + 0, // padding top + 0, // padding right + 0, // padding bottom + 0, // padding left + 0, // view inset top + 0, // view inset right + 0, // view inset bottom + 0, // view inset left + 0, // gesture inset top + 0, // gesture inset right + 0, // gesture inset bottom + 0 // gesture inset left + }; // Set viewport to nonempty, and call Animator::BeginFrame to make the layer // tree pipeline nonempty. Without either of this, the layer tree below // won't be rasterized. From 478d773405d6366bea5bd0629c48b8d8617fc8ee Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 19 Aug 2020 08:51:29 -0700 Subject: [PATCH 14/17] fix thread issues --- flow/compositor_context.h | 3 +++ shell/common/shell.cc | 9 +++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/flow/compositor_context.h b/flow/compositor_context.h index d9448b829f737..b17dc907420da 100644 --- a/flow/compositor_context.h +++ b/flow/compositor_context.h @@ -41,7 +41,10 @@ class CompositorContext { public: /// Called at the end of a frame with approximately how many bytes mightbe /// freed if a GC ran now. + /// + /// This method is called from the raster task runner. virtual void OnCompositorEndFrame(size_t freed_hint) = 0; + /// Time limit for a smooth frame. See `Engine::GetDisplayRefreshRate`. virtual fml::Milliseconds GetFrameBudget() = 0; }; diff --git a/shell/common/shell.cc b/shell/common/shell.cc index fafe3bb2817ba..0656952b4768a 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -625,7 +625,7 @@ void Shell::OnPlatformViewCreated(std::unique_ptr surface) { // a synchronous fashion. fml::AutoResetWaitableEvent latch; auto raster_task = - fml::MakeCopyable([&waiting_for_first_frame = waiting_for_first_frame_, + fml::MakeCopyable([& waiting_for_first_frame = waiting_for_first_frame_, rasterizer = rasterizer_->GetWeakPtr(), // surface = std::move(surface), // &latch]() mutable { @@ -990,7 +990,7 @@ void Shell::OnAnimatorDraw(fml::RefPtr> pipeline, } task_runners_.GetRasterTaskRunner()->PostTask( - [&waiting_for_first_frame = waiting_for_first_frame_, + [& waiting_for_first_frame = waiting_for_first_frame_, &waiting_for_first_frame_condition = waiting_for_first_frame_condition_, rasterizer = rasterizer_->GetWeakPtr(), pipeline = std::move(pipeline)]() { @@ -1206,8 +1206,9 @@ void Shell::OnFrameRasterized(const FrameTiming& timing) { } void Shell::OnCompositorEndFrame(size_t freed_hint) { + FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread()); task_runners_.GetUITaskRunner()->PostTask( - [engine = engine_->GetWeakPtr(), freed_hint = freed_hint]() { + [engine = weak_engine_, freed_hint = freed_hint]() { if (engine) { engine->HintFreed(freed_hint); } @@ -1528,7 +1529,7 @@ fml::Status Shell::WaitForFirstFrame(fml::TimeDelta timeout) { std::unique_lock lock(waiting_for_first_frame_mutex_); bool success = waiting_for_first_frame_condition_.wait_for( lock, std::chrono::milliseconds(timeout.ToMilliseconds()), - [&waiting_for_first_frame = waiting_for_first_frame_] { + [& waiting_for_first_frame = waiting_for_first_frame_] { return !waiting_for_first_frame.load(); }); if (success) { From 0fb859cb9ebcef79bf5b8652a611d72f35442819 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 19 Aug 2020 10:17:22 -0700 Subject: [PATCH 15/17] format! --- shell/common/shell.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 0656952b4768a..47cceb5598bc2 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -625,7 +625,7 @@ void Shell::OnPlatformViewCreated(std::unique_ptr surface) { // a synchronous fashion. fml::AutoResetWaitableEvent latch; auto raster_task = - fml::MakeCopyable([& waiting_for_first_frame = waiting_for_first_frame_, + fml::MakeCopyable([&waiting_for_first_frame = waiting_for_first_frame_, rasterizer = rasterizer_->GetWeakPtr(), // surface = std::move(surface), // &latch]() mutable { @@ -990,7 +990,7 @@ void Shell::OnAnimatorDraw(fml::RefPtr> pipeline, } task_runners_.GetRasterTaskRunner()->PostTask( - [& waiting_for_first_frame = waiting_for_first_frame_, + [&waiting_for_first_frame = waiting_for_first_frame_, &waiting_for_first_frame_condition = waiting_for_first_frame_condition_, rasterizer = rasterizer_->GetWeakPtr(), pipeline = std::move(pipeline)]() { @@ -1529,7 +1529,7 @@ fml::Status Shell::WaitForFirstFrame(fml::TimeDelta timeout) { std::unique_lock lock(waiting_for_first_frame_mutex_); bool success = waiting_for_first_frame_condition_.wait_for( lock, std::chrono::milliseconds(timeout.ToMilliseconds()), - [& waiting_for_first_frame = waiting_for_first_frame_] { + [&waiting_for_first_frame = waiting_for_first_frame_] { return !waiting_for_first_frame.load(); }); if (success) { From e4be6ecb5567d75cec14f04ebf4dd1f83a9a1e32 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 19 Aug 2020 11:44:53 -0700 Subject: [PATCH 16/17] deflake --- lib/ui/fixtures/ui_test.dart | 57 ++++++++++------------ lib/ui/painting/image_release_unittests.cc | 51 +++++++++++++------ 2 files changed, 62 insertions(+), 46 deletions(-) diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index 8b1a896b75918..478c919066562 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -77,19 +77,6 @@ void _validateExternal(Uint8List result) native 'ValidateExternal'; @pragma('vm:entry-point') Future pumpImage() async { - final Completer completer = Completer(); - const int width = 8000; - const int height = 8000; - decodeImageFromPixels( - Uint8List.fromList(List.filled(width * height * 4, 0xFF)), - width, - height, - PixelFormat.rgba8888, - (Image image) => completer.complete(image), - ); - - final Image image = await completer.future; - final FrameCallback renderBlank = (Duration duration) { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); @@ -109,23 +96,33 @@ Future pumpImage() async { }; final FrameCallback renderImage = (Duration duration) { - final PictureRecorder recorder = PictureRecorder(); - final Canvas canvas = Canvas(recorder); - canvas.drawImage(image, Offset.zero, Paint()); - final Picture picture = recorder.endRecording(); - - final SceneBuilder builder = SceneBuilder(); - builder.addPicture(Offset.zero, picture); - - _captureImageAndPicture(image, picture); - - final Scene scene = builder.build(); - window.render(scene); - scene.dispose(); - picture.dispose(); - image.dispose(); - window.onBeginFrame = renderBlank; - window.scheduleFrame(); + const int width = 8000; + const int height = 8000; + final Completer completer = Completer(); + decodeImageFromPixels( + Uint8List.fromList(List.filled(width * height * 4, 0xFF)), + width, + height, + PixelFormat.rgba8888, + (Image image) => completer.complete(image), + ); + completer.future.then((Image image) { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + canvas.drawImage(image, Offset.zero, Paint()); + final Picture picture = recorder.endRecording(); + + final SceneBuilder builder = SceneBuilder(); + builder.addPicture(Offset.zero, picture); + + _captureImageAndPicture(image, picture); + + final Scene scene = builder.build(); + window.render(scene); + scene.dispose(); + window.onBeginFrame = renderBlank; + window.scheduleFrame(); + }); }; window.onBeginFrame = renderImage; diff --git a/lib/ui/painting/image_release_unittests.cc b/lib/ui/painting/image_release_unittests.cc index 34654b7849901..18b9ad75aafd7 100644 --- a/lib/ui/painting/image_release_unittests.cc +++ b/lib/ui/painting/image_release_unittests.cc @@ -27,9 +27,23 @@ class ImageReleaseTest : public ShellTest { return reinterpret_cast(peer); } - fml::AutoResetWaitableEvent message_latch; - sk_sp current_image_; + // Used to wait on Dart callbacks or Shell task runner flushing + fml::AutoResetWaitableEvent message_latch_; + // Used to wait for the finalization of the picture Dart_Handle + fml::AutoResetWaitableEvent picture_finalizer_latch_; + static void picture_finalizer(void* isolate_callback_data, void* peer) { + auto latch = reinterpret_cast(peer); + latch->Signal(); + } + // Used to wait for the finalization of the image Dart_Handle + fml::AutoResetWaitableEvent image_finalizer_latch_; + static void image_finalizer(void* isolate_callback_data, void* peer) { + auto latch = reinterpret_cast(peer); + latch->Signal(); + } + sk_sp current_picture_; + sk_sp current_image_; }; TEST_F(ImageReleaseTest, ImageReleasedAfterFrame) { @@ -40,10 +54,16 @@ TEST_F(ImageReleaseTest, ImageReleasedAfterFrame) { ASSERT_FALSE(picture->picture()->unique()); current_image_ = image->image(); current_picture_ = picture->picture(); + Dart_NewFinalizableHandle( + Dart_HandleFromWeakPersistent(picture->dart_wrapper()), + &picture_finalizer_latch_, 0, &picture_finalizer); + Dart_NewFinalizableHandle( + Dart_HandleFromWeakPersistent(image->dart_wrapper()), + &image_finalizer_latch_, 0, &image_finalizer); }; auto native_on_begin_frame_done = [&](Dart_NativeArguments args) { - message_latch.Signal(); + message_latch_.Signal(); }; Settings settings = CreateSettingsForFixture(); @@ -75,27 +95,26 @@ TEST_F(ImageReleaseTest, ImageReleasedAfterFrame) { ASSERT_EQ(result, Engine::RunStatus::Success); }); - message_latch.Wait(); + message_latch_.Wait(); ASSERT_TRUE(current_picture_); ASSERT_TRUE(current_image_); - // AOT modes are fast enough that we get here before the task runner has - // had a chance to drain. Make sure that if the picture or image are not - // already unique, at least one idle notification has a chance to process - // after rasterization has occurred. - if (!current_picture_->unique() || !current_image_->unique()) { - task_runner->PostTask([&, engine = shell->GetEngine()]() { - ASSERT_TRUE(engine); - engine->NotifyIdle(Dart_TimelineGetMicros() + 10000); - message_latch.Signal(); - }); - message_latch.Wait(); - } + // Make sure we get at least one good sized NotifyIdle here. + task_runner->PostTask([&, engine = shell->GetEngine()]() { + ASSERT_TRUE(engine); + engine->NotifyIdle(Dart_TimelineGetMicros() + 10000000); + message_latch_.Signal(); + }); + message_latch_.Wait(); + picture_finalizer_latch_.Wait(); EXPECT_TRUE(current_picture_->unique()); current_picture_.reset(); + + image_finalizer_latch_.Wait(); EXPECT_TRUE(current_image_->unique()); + current_image_.reset(); shell->GetPlatformView()->NotifyDestroyed(); DestroyShell(std::move(shell), std::move(task_runners)); From 3207082a8a98e35cff1d7ec7e7f169978300bf2e Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 19 Aug 2020 12:47:47 -0700 Subject: [PATCH 17/17] Remove flaky test --- ci/licenses_golden/licenses_flutter | 1 - lib/ui/BUILD.gn | 1 - lib/ui/painting/image_release_unittests.cc | 124 --------------------- 3 files changed, 126 deletions(-) delete mode 100644 lib/ui/painting/image_release_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 7def265089588..5389f652c873c 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -342,7 +342,6 @@ FILE: ../../../flutter/lib/ui/painting/image_encoding.h FILE: ../../../flutter/lib/ui/painting/image_encoding_unittests.cc FILE: ../../../flutter/lib/ui/painting/image_filter.cc FILE: ../../../flutter/lib/ui/painting/image_filter.h -FILE: ../../../flutter/lib/ui/painting/image_release_unittests.cc FILE: ../../../flutter/lib/ui/painting/image_shader.cc FILE: ../../../flutter/lib/ui/painting/image_shader.h FILE: ../../../flutter/lib/ui/painting/immutable_buffer.cc diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index 5bda272bf989b..b0454b381b55c 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -181,7 +181,6 @@ if (enable_unittests) { sources = [ "painting/image_encoding_unittests.cc", - "painting/image_release_unittests.cc", "painting/vertices_unittests.cc", "window/platform_configuration_unittests.cc", "window/pointer_data_packet_converter_unittests.cc", diff --git a/lib/ui/painting/image_release_unittests.cc b/lib/ui/painting/image_release_unittests.cc deleted file mode 100644 index 18b9ad75aafd7..0000000000000 --- a/lib/ui/painting/image_release_unittests.cc +++ /dev/null @@ -1,124 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#define FML_USED_ON_EMBEDDER - -#include "flutter/common/task_runners.h" -#include "flutter/fml/synchronization/waitable_event.h" -#include "flutter/lib/ui/painting/image.h" -#include "flutter/lib/ui/painting/picture.h" -#include "flutter/runtime/dart_vm.h" -#include "flutter/shell/common/shell_test.h" -#include "flutter/shell/common/thread_host.h" -#include "flutter/testing/testing.h" - -namespace flutter { -namespace testing { - -class ImageReleaseTest : public ShellTest { - public: - template - T* GetNativePeer(Dart_NativeArguments args, int index) { - auto handle = Dart_GetNativeArgument(args, index); - intptr_t peer = 0; - EXPECT_FALSE(Dart_IsError(Dart_GetNativeInstanceField( - handle, tonic::DartWrappable::kPeerIndex, &peer))); - return reinterpret_cast(peer); - } - - // Used to wait on Dart callbacks or Shell task runner flushing - fml::AutoResetWaitableEvent message_latch_; - // Used to wait for the finalization of the picture Dart_Handle - fml::AutoResetWaitableEvent picture_finalizer_latch_; - static void picture_finalizer(void* isolate_callback_data, void* peer) { - auto latch = reinterpret_cast(peer); - latch->Signal(); - } - // Used to wait for the finalization of the image Dart_Handle - fml::AutoResetWaitableEvent image_finalizer_latch_; - static void image_finalizer(void* isolate_callback_data, void* peer) { - auto latch = reinterpret_cast(peer); - latch->Signal(); - } - - sk_sp current_picture_; - sk_sp current_image_; -}; - -TEST_F(ImageReleaseTest, ImageReleasedAfterFrame) { - auto native_capture_image_and_picture = [&](Dart_NativeArguments args) { - CanvasImage* image = GetNativePeer(args, 0); - Picture* picture = GetNativePeer(args, 1); - ASSERT_FALSE(image->image()->unique()); - ASSERT_FALSE(picture->picture()->unique()); - current_image_ = image->image(); - current_picture_ = picture->picture(); - Dart_NewFinalizableHandle( - Dart_HandleFromWeakPersistent(picture->dart_wrapper()), - &picture_finalizer_latch_, 0, &picture_finalizer); - Dart_NewFinalizableHandle( - Dart_HandleFromWeakPersistent(image->dart_wrapper()), - &image_finalizer_latch_, 0, &image_finalizer); - }; - - auto native_on_begin_frame_done = [&](Dart_NativeArguments args) { - message_latch_.Signal(); - }; - - Settings settings = CreateSettingsForFixture(); - auto task_runner = CreateNewThread(); - TaskRunners task_runners("test", // label - GetCurrentTaskRunner(), // platform - task_runner, // raster - task_runner, // ui - task_runner // io - ); - - AddNativeCallback("CaptureImageAndPicture", - CREATE_NATIVE_ENTRY(native_capture_image_and_picture)); - AddNativeCallback("OnBeginFrameDone", - CREATE_NATIVE_ENTRY(native_on_begin_frame_done)); - - std::unique_ptr shell = CreateShell(std::move(settings), task_runners); - - ASSERT_TRUE(shell->IsSetup()); - - SetViewportMetrics(shell.get(), 800, 600); - - shell->GetPlatformView()->NotifyCreated(); - - auto configuration = RunConfiguration::InferFromSettings(settings); - configuration.SetEntrypoint("pumpImage"); - - shell->RunEngine(std::move(configuration), [&](auto result) { - ASSERT_EQ(result, Engine::RunStatus::Success); - }); - - message_latch_.Wait(); - - ASSERT_TRUE(current_picture_); - ASSERT_TRUE(current_image_); - - // Make sure we get at least one good sized NotifyIdle here. - task_runner->PostTask([&, engine = shell->GetEngine()]() { - ASSERT_TRUE(engine); - engine->NotifyIdle(Dart_TimelineGetMicros() + 10000000); - message_latch_.Signal(); - }); - message_latch_.Wait(); - picture_finalizer_latch_.Wait(); - - EXPECT_TRUE(current_picture_->unique()); - current_picture_.reset(); - - image_finalizer_latch_.Wait(); - EXPECT_TRUE(current_image_->unique()); - current_image_.reset(); - - shell->GetPlatformView()->NotifyDestroyed(); - DestroyShell(std::move(shell), std::move(task_runners)); -} - -} // namespace testing -} // namespace flutter