From 7f2000fab6ce14430a264e5b3673a54452f797e8 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 25 Oct 2023 14:42:58 -0700 Subject: [PATCH 1/3] Impl --- lib/ui/platform_dispatcher.dart | 58 +++++++++++---------------------- lib/ui/window.dart | 8 ++--- shell/common/animator.cc | 11 +------ shell/common/animator.h | 3 +- 4 files changed, 24 insertions(+), 56 deletions(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index aac7885656309..e8f7d83805baf 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -308,9 +308,8 @@ class PlatformDispatcher { _invoke(onMetricsChanged, _onMetricsChangedZone); } - // A debug-only variable that stores the [FlutterView]s for which - // [FlutterView.render] has already been called during the current - // [onBeginFrame]/[onDrawFrame] callback sequence. + // The [FlutterView]s for which [FlutterView.render] has already been called + // during the current [onBeginFrame]/[onDrawFrame] callback sequence. // // It is null outside the scope of those callbacks indicating that calls to // [FlutterView.render] must be ignored. Furthermore, if a given [FlutterView] @@ -320,16 +319,9 @@ class PlatformDispatcher { // Between [onBeginFrame] and [onDrawFrame] the properties value is // temporarily stored in `_renderedViewsBetweenCallbacks` so that it survives // the gap between the two callbacks. - // - // In release build, this variable is null, and therefore the calling rule is - // not enforced. This is because the check might hurt cold startup delay; - // see https://github.com/flutter/engine/pull/46919. - Set? _debugRenderedViews; - // A debug-only variable that temporarily stores the `_renderedViews` value - // between `_beginFrame` and `_drawFrame`. - // - // In release build, this variable is null. - Set? _debugRenderedViewsBetweenCallbacks; + Set? _renderedViews; + // The `_renderedViews` value between `_beginFrame` and `_drawFrame`. + Set? _renderedViewsBetweenCallbacks; /// A callback invoked when any view begins a frame. /// @@ -351,12 +343,9 @@ class PlatformDispatcher { // Called from the engine, via hooks.dart void _beginFrame(int microseconds) { - assert(_debugRenderedViews == null); - assert(_debugRenderedViewsBetweenCallbacks == null); - assert(() { - _debugRenderedViews = {}; - return true; - }()); + assert(_renderedViews == null); + assert(_renderedViewsBetweenCallbacks == null); + _renderedViews = {}; _invoke1( onBeginFrame, @@ -364,13 +353,10 @@ class PlatformDispatcher { Duration(microseconds: microseconds), ); - assert(_debugRenderedViews != null); - assert(_debugRenderedViewsBetweenCallbacks == null); - assert(() { - _debugRenderedViewsBetweenCallbacks = _debugRenderedViews; - _debugRenderedViews = null; - return true; - }()); + assert(_renderedViews != null); + assert(_renderedViewsBetweenCallbacks == null); + _renderedViewsBetweenCallbacks = _renderedViews; + _renderedViews = null; } /// A callback that is invoked for each frame after [onBeginFrame] has @@ -388,22 +374,16 @@ class PlatformDispatcher { // Called from the engine, via hooks.dart void _drawFrame() { - assert(_debugRenderedViews == null); - assert(_debugRenderedViewsBetweenCallbacks != null); - assert(() { - _debugRenderedViews = _debugRenderedViewsBetweenCallbacks; - _debugRenderedViewsBetweenCallbacks = null; - return true; - }()); + assert(_renderedViews == null); + assert(_renderedViewsBetweenCallbacks != null); + _renderedViews = _renderedViewsBetweenCallbacks; + _renderedViewsBetweenCallbacks = null; _invoke(onDrawFrame, _onDrawFrameZone); - assert(_debugRenderedViews != null); - assert(_debugRenderedViewsBetweenCallbacks == null); - assert(() { - _debugRenderedViews = null; - return true; - }()); + assert(_renderedViews != null); + assert(_renderedViewsBetweenCallbacks == null); + _renderedViews = null; } /// A callback that is invoked when pointer data is available. diff --git a/lib/ui/window.dart b/lib/ui/window.dart index 022227be017c4..71b9ea56a48d0 100644 --- a/lib/ui/window.dart +++ b/lib/ui/window.dart @@ -355,14 +355,10 @@ class FlutterView { /// painting. void render(Scene scene) { // Duplicated calls or calls outside of onBeginFrame/onDrawFrame (indicated - // by _debugRenderedViews being null) are ignored. See _debugRenderedViews. + // by _renderedViews being null) are ignored. See _renderedViews. // TODO(dkwingsmt): We should change this skip into an assertion. // https://github.com/flutter/flutter/issues/137073 - bool validRender = true; - assert(() { - validRender = platformDispatcher._debugRenderedViews?.add(this) ?? false; - return true; - }()); + final bool validRender = platformDispatcher._renderedViews?.add(this) ?? false; if (validRender) { _render(viewId, scene as _NativeScene); } diff --git a/shell/common/animator.cc b/shell/common/animator.cc index 1696e5009db24..1e5d3a7696d32 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -179,18 +179,9 @@ void Animator::EndFrame() { void Animator::Render(int64_t view_id, std::unique_ptr layer_tree, float device_pixel_ratio) { - // Animator::Render should be called between BeginFrame and EndFrame, - // which is indicated by frame_timings_recorder_ being non-null. - // This might happen on release build, and is guarded by PlatformDispatcher on - // debug build. - // TODO(dkwingsmt): We should change this skip into an assertion. - // https://github.com/flutter/flutter/issues/137073 - if (frame_timings_recorder_ == nullptr) { - return; - } - has_rendered_ = true; + FML_CHECK(frame_timings_recorder_ != nullptr); TRACE_EVENT_WITH_FRAME_NUMBER(frame_timings_recorder_, "flutter", "Animator::Render", /*flow_id_count=*/0, /*flow_ids=*/nullptr); diff --git a/shell/common/animator.h b/shell/common/animator.h index 5358ed6b14001..b8b6cdcc1d331 100644 --- a/shell/common/animator.h +++ b/shell/common/animator.h @@ -58,7 +58,8 @@ class Animator final { /// /// This method must be called during a vsync callback, or /// technically, between Animator::BeginFrame and Animator::EndFrame - /// (both private methods). Otherwise, this call will be ignored. + /// (both private methods). Otherwise, an assertion will be + /// triggered. /// void Render(int64_t view_id, std::unique_ptr layer_tree, From f755327488dc6350083e46ed1b936eae609f6ed5 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 26 Oct 2023 13:52:44 -0700 Subject: [PATCH 2/3] Update animator.cc --- shell/common/animator.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shell/common/animator.cc b/shell/common/animator.cc index 1e5d3a7696d32..572233699e1b0 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -179,9 +179,10 @@ void Animator::EndFrame() { void Animator::Render(int64_t view_id, std::unique_ptr layer_tree, float device_pixel_ratio) { + FML_CHECK(frame_timings_recorder_ != nullptr); + has_rendered_ = true; - FML_CHECK(frame_timings_recorder_ != nullptr); TRACE_EVENT_WITH_FRAME_NUMBER(frame_timings_recorder_, "flutter", "Animator::Render", /*flow_id_count=*/0, /*flow_ids=*/nullptr); From 9d45d631d35f203570dce2ad9d40d44f573b6d91 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 26 Oct 2023 14:26:34 -0700 Subject: [PATCH 3/3] Format --- shell/common/animator.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/common/animator.cc b/shell/common/animator.cc index 572233699e1b0..1494ca2832afa 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -180,7 +180,7 @@ void Animator::Render(int64_t view_id, std::unique_ptr layer_tree, float device_pixel_ratio) { FML_CHECK(frame_timings_recorder_ != nullptr); - + has_rendered_ = true; TRACE_EVENT_WITH_FRAME_NUMBER(frame_timings_recorder_, "flutter",