Listen for display refresh changes and report them correctly (flutter#29800)

dnfield · web-flow · commit f4bc821501ad · 2021-11-18T13:09:26.000-08:00
This makes sure the frame timings recorder and vsync waiter implementations get the correct refresh rate if or when it adjusts at runtime. This should be OK because we'll only query the refresh rate when the display metrics actually change, which is much less frequent than every frame. I experimented with an NDK implementation in the previous patch, but that vastly restricts the API levels we can support, and currently on API 30 and 31 it just calls Java methods through JNI anyway. I've refactored the way Display updates are reported so that AndroidDisplay can just dynamically get the refresh rate correctly. These values are used by a service protocol extension and by some Stopwatches on the CompositorContext. See also flutter#29765 Fixes flutter#93688 Fixes flutter#93698
diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter
@@ -696,6 +696,7 @@ FILE: ../../../flutter/shell/common/canvas_spy_unittests.cc
 FILE: ../../../flutter/shell/common/context_options.cc
 FILE: ../../../flutter/shell/common/context_options.h
 FILE: ../../../flutter/shell/common/dart_native_benchmarks.cc
+FILE: ../../../flutter/shell/common/display.cc
 FILE: ../../../flutter/shell/common/display.h
 FILE: ../../../flutter/shell/common/display_manager.cc
 FILE: ../../../flutter/shell/common/display_manager.h
@@ -775,6 +776,8 @@ FILE: ../../../flutter/shell/platform/android/AndroidManifest.xml
 FILE: ../../../flutter/shell/platform/android/android_context_gl.cc
 FILE: ../../../flutter/shell/platform/android/android_context_gl.h
 FILE: ../../../flutter/shell/platform/android/android_context_gl_unittests.cc
+FILE: ../../../flutter/shell/platform/android/android_display.cc
+FILE: ../../../flutter/shell/platform/android/android_display.h
 FILE: ../../../flutter/shell/platform/android/android_environment_gl.cc
 FILE: ../../../flutter/shell/platform/android/android_environment_gl.h
 FILE: ../../../flutter/shell/platform/android/android_exports.lst
diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn
@@ -66,6 +66,7 @@ source_set("common") {
     "canvas_spy.h",
     "context_options.cc",
     "context_options.h",
+    "display.cc",
     "display.h",
     "display_manager.cc",
     "display_manager.h",
diff --git a/shell/common/display.cc b/shell/common/display.cc
@@ -0,0 +1,11 @@
+// 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.
+
+#include "flutter/shell/common/display.h"
+
+namespace flutter {
+double Display::GetRefreshRate() const {
+  return refresh_rate_;
+}
+}  // namespace flutter
diff --git a/shell/common/display.h b/shell/common/display.h
@@ -7,6 +7,8 @@
 
 #include <optional>
 
+#include "flutter/fml/macros.h"
+
 namespace flutter {
 
 /// Unique ID per display that is stable until the Flutter application restarts.
@@ -36,18 +38,20 @@ class Display {
   explicit Display(double refresh_rate)
       : display_id_({}), refresh_rate_(refresh_rate) {}
 
-  ~Display() = default;
+  virtual ~Display() = default;
 
   // Get the display's maximum refresh rate in the unit of frame per second.
   // Return `kUnknownDisplayRefreshRate` if the refresh rate is unknown.
-  double GetRefreshRate() const { return refresh_rate_; }
+  virtual double GetRefreshRate() const;
 
   /// Returns the `DisplayId` of the display.
   std::optional<DisplayId> GetDisplayId() const { return display_id_; }
 
  private:
   std::optional<DisplayId> display_id_;
   double refresh_rate_;
+
+  FML_DISALLOW_COPY_AND_ASSIGN(Display);
 };
 
 }  // namespace flutter
diff --git a/shell/common/display_manager.cc b/shell/common/display_manager.cc
@@ -18,30 +18,31 @@ double DisplayManager::GetMainDisplayRefreshRate() const {
   if (displays_.empty()) {
     return kUnknownDisplayRefreshRate;
   } else {
-    return displays_[0].GetRefreshRate();
+    return displays_[0]->GetRefreshRate();
   }
 }
 
-void DisplayManager::HandleDisplayUpdates(DisplayUpdateType update_type,
-                                          std::vector<Display> displays) {
+void DisplayManager::HandleDisplayUpdates(
+    DisplayUpdateType update_type,
+    std::vector<std::unique_ptr<Display>> displays) {
   std::scoped_lock lock(displays_mutex_);
   CheckDisplayConfiguration(displays);
   switch (update_type) {
     case DisplayUpdateType::kStartup:
       FML_CHECK(displays_.empty());
-      displays_ = displays;
+      displays_ = std::move(displays);
       return;
     default:
       FML_CHECK(false) << "Unknown DisplayUpdateType.";
   }
 }
 
 void DisplayManager::CheckDisplayConfiguration(
-    std::vector<Display> displays) const {
+    const std::vector<std::unique_ptr<Display>>& displays) const {
   FML_CHECK(!displays.empty());
   if (displays.size() > 1) {
     for (auto& display : displays) {
-      FML_CHECK(display.GetDisplayId().has_value());
+      FML_CHECK(display->GetDisplayId().has_value());
     }
   }
 }
diff --git a/shell/common/display_manager.h b/shell/common/display_manager.h
@@ -40,18 +40,19 @@ class DisplayManager {
 
   /// Handles the display updates.
   void HandleDisplayUpdates(DisplayUpdateType update_type,
-                            std::vector<Display> displays);
+                            std::vector<std::unique_ptr<Display>> displays);
 
  private:
   /// Guards `displays_` vector.
   mutable std::mutex displays_mutex_;
-  std::vector<Display> displays_;
+  std::vector<std::unique_ptr<Display>> displays_;
 
   /// Checks that the provided display configuration is valid. Currently this
   /// ensures that all the displays have an id in the case there are multiple
   /// displays. In case where there is a single display, it is valid for the
   /// display to not have an id.
-  void CheckDisplayConfiguration(std::vector<Display> displays) const;
+  void CheckDisplayConfiguration(
+      const std::vector<std::unique_ptr<Display>>& displays) const;
 };
 
 }  // namespace flutter
diff --git a/shell/common/shell.cc b/shell/common/shell.cc
@@ -1867,8 +1867,8 @@ void Shell::SetGpuAvailability(GpuAvailability availability) {
 }
 
 void Shell::OnDisplayUpdates(DisplayUpdateType update_type,
-                             std::vector<Display> displays) {
-  display_manager_->HandleDisplayUpdates(update_type, displays);
+                             std::vector<std::unique_ptr<Display>> displays) {
+  display_manager_->HandleDisplayUpdates(update_type, std::move(displays));
 }
 
 fml::TimePoint Shell::GetCurrentTimePoint() {
diff --git a/shell/common/shell.h b/shell/common/shell.h
@@ -373,7 +373,7 @@ class Shell final : public PlatformView::Delegate,
   /// @brief      Notifies the display manager of the updates.
   ///
   void OnDisplayUpdates(DisplayUpdateType update_type,
-                        std::vector<Display> displays);
+                        std::vector<std::unique_ptr<Display>> displays);
 
   //----------------------------------------------------------------------------
   /// @brief Queries the `DisplayManager` for the main display refresh rate.
diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn
@@ -64,6 +64,8 @@ source_set("flutter_shell_native_src") {
     "$root_build_dir/flutter_icu/icudtl.o",
     "android_context_gl.cc",
     "android_context_gl.h",
+    "android_display.cc",
+    "android_display.h",
     "android_environment_gl.cc",
     "android_environment_gl.h",
     "android_external_texture_gl.cc",
diff --git a/shell/platform/android/android_display.cc b/shell/platform/android/android_display.cc
@@ -0,0 +1,19 @@
+// 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.
+
+#include "flutter/shell/platform/android/android_display.h"
+#include "android_display.h"
+
+namespace flutter {
+
+AndroidDisplay::AndroidDisplay(
+    std::shared_ptr<PlatformViewAndroidJNI> jni_facade)
+    : Display(jni_facade->GetDisplayRefreshRate()),
+      jni_facade_(std::move(jni_facade)) {}
+
+double AndroidDisplay::GetRefreshRate() const {
+  return jni_facade_->GetDisplayRefreshRate();
+}
+
+}  // namespace flutter
diff --git a/shell/platform/android/android_display.h b/shell/platform/android/android_display.h
@@ -0,0 +1,33 @@
+// 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.
+
+#ifndef FLUTTER_SHELL_PLATFORM_ANDROID_DISPLAY_H_
+#define FLUTTER_SHELL_PLATFORM_ANDROID_DISPLAY_H_
+
+#include <cstdint>
+
+#include "flutter/fml/macros.h"
+#include "flutter/shell/common/display.h"
+#include "flutter/shell/platform/android/jni/platform_view_android_jni.h"
+
+namespace flutter {
+
+/// A |Display| that listens to refresh rate changes.
+class AndroidDisplay : public Display {
+ public:
+  explicit AndroidDisplay(std::shared_ptr<PlatformViewAndroidJNI> jni_facade);
+  ~AndroidDisplay() = default;
+
+  // |Display|
+  double GetRefreshRate() const override;
+
+ private:
+  std::shared_ptr<PlatformViewAndroidJNI> jni_facade_;
+
+  FML_DISALLOW_COPY_AND_ASSIGN(AndroidDisplay);
+};
+
+}  // namespace flutter
+
+#endif  // FLUTTER_SHELL_PLATFORM_ANDROID_DISPLAY_H_
diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc
@@ -25,6 +25,7 @@
 #include "flutter/shell/common/rasterizer.h"
 #include "flutter/shell/common/run_configuration.h"
 #include "flutter/shell/common/thread_host.h"
+#include "flutter/shell/platform/android/android_display.h"
 #include "flutter/shell/platform/android/android_image_generator.h"
 #include "flutter/shell/platform/android/context/android_context.h"
 #include "flutter/shell/platform/android/platform_view_android.h"
@@ -61,8 +62,10 @@ AndroidShellHolder::AndroidShellHolder(
                 .enable_software_rendering  // use software rendering
         );
         weak_platform_view = platform_view_android->GetWeakPtr();
-        auto display = Display(jni_facade->GetDisplayRefreshRate());
-        shell.OnDisplayUpdates(DisplayUpdateType::kStartup, {display});
+        std::vector<std::unique_ptr<Display>> displays;
+        displays.push_back(std::make_unique<AndroidDisplay>(jni_facade));
+        shell.OnDisplayUpdates(DisplayUpdateType::kStartup,
+                               std::move(displays));
         return platform_view_android;
       };
 
@@ -209,8 +212,10 @@ std::unique_ptr<AndroidShellHolder> AndroidShellHolder::Spawn(
             android_context          // Android context
         );
         weak_platform_view = platform_view_android->GetWeakPtr();
-        auto display = Display(jni_facade->GetDisplayRefreshRate());
-        shell.OnDisplayUpdates(DisplayUpdateType::kStartup, {display});
+        std::vector<std::unique_ptr<Display>> displays;
+        displays.push_back(std::make_unique<AndroidDisplay>(jni_facade));
+        shell.OnDisplayUpdates(DisplayUpdateType::kStartup,
+                               std::move(displays));
         return platform_view_android;
       };
 
diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java
@@ -186,9 +186,15 @@ private static native void nativeInit(
   // END methods related to FlutterLoader
 
   @Nullable private static AsyncWaitForVsyncDelegate asyncWaitForVsyncDelegate;
-  // This should also be updated by FlutterView when it is attached to a Display.
-  // The initial value of 0.0 indicates unknown refresh rate.
-  private static float refreshRateFPS = 0.0f;
+
+  /**
+   * This value is updated by the VsyncWaiter when it is initialized.
+   *
+   * <p>On API 17+, it is updated whenever the default display refresh rate changes.
+   *
+   * <p>It is defaulted to 60.
+   */
+  private static float refreshRateFPS = 60.0f;
 
   // This is set from native code via JNI.
   @Nullable private static String observatoryUri;
@@ -216,19 +222,34 @@ public static String getObservatoryUri() {
     return observatoryUri;
   }
 
-  public static void setRefreshRateFPS(float refreshRateFPS) {
-    if (FlutterJNI.setRefreshRateFPSCalled) {
-      Log.w(TAG, "FlutterJNI.setRefreshRateFPS called more than once");
-    }
-
+  /**
+   * Notifies the engine about the refresh rate of the display when the API level is below 30.
+   *
+   * <p>For API 30 and above, this value is ignored.
+   *
+   * <p>Calling this method multiple times will update the refresh rate for the next vsync period.
+   * However, callers should avoid calling {@link android.view.Display#getRefreshRate} frequently,
+   * since it is expensive on some vendor implementations.
+   *
+   * @param refreshRateFPS The refresh rate in nanoseconds.
+   */
+  public void setRefreshRateFPS(float refreshRateFPS) {
+    // This is ok because it only ever tracks the refresh rate of the main
+    // display. If we ever need to support the refresh rate of other displays
+    // on Android we will need to refactor this. Static lookup makes things a
+    // bit easier on the C++ side.
     FlutterJNI.refreshRateFPS = refreshRateFPS;
-    FlutterJNI.setRefreshRateFPSCalled = true;
   }
 
-  private static boolean setRefreshRateFPSCalled = false;
-
-  // TODO(mattcarroll): add javadocs
-  public static void setAsyncWaitForVsyncDelegate(@Nullable AsyncWaitForVsyncDelegate delegate) {
+  /**
+   * The Android vsync waiter implementation in C++ needs to know when a vsync signal arrives, which
+   * is obtained via Java API. The delegate set here is called on the C++ side when the engine is
+   * ready to wait for the next vsync signal. The delegate is expected to add a postFrameCallback to
+   * the {@link android.view.Choreographer}, and call {@link nativeOnVsync} to notify the engine.
+   *
+   * @param delegate The delegate that will call the engine back on the next vsync signal.
+   */
+  public void setAsyncWaitForVsyncDelegate(@Nullable AsyncWaitForVsyncDelegate delegate) {
     asyncWaitForVsyncDelegate = delegate;
   }
 
@@ -243,9 +264,15 @@ private static void asyncWaitForVsync(final long cookie) {
     }
   }
 
-  // TODO(mattcarroll): add javadocs
-  public static native void nativeOnVsync(
-      long frameDelayNanos, long refreshPeriodNanos, long cookie);
+  /**
+   * Notifies the engine that the Choreographer has signaled a vsync.
+   *
+   * @param frameDelayNanos The time in nanoseconds when the frame started being rendered,
+   *     subtracted from the {@link System#nanoTime} timebase.
+   * @param refreshPeriodNanos The display refresh period in nanoseconds.
+   * @param cookie An opaque handle to the C++ VSyncWaiter object.
+   */
+  public native void nativeOnVsync(long frameDelayNanos, long refreshPeriodNanos, long cookie);
 
   // TODO(mattcarroll): add javadocs
   @NonNull
@@ -337,8 +364,7 @@ public long performNativeAttach(@NonNull FlutterJNI flutterJNI) {
    * #attachToNative()}.
    *
    * <p>Static methods that should be only called once such as {@link #init(Context, String[],
-   * String, String, String, long)} or {@link #setRefreshRateFPS(float)} shouldn't be called again
-   * on the spawned FlutterJNI instance.
+   * String, String, String, long)} shouldn't be called again on the spawned FlutterJNI instance.
    */
   @UiThread
   @NonNull
diff --git a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java
@@ -15,7 +15,6 @@
 import android.os.Handler;
 import android.os.Looper;
 import android.os.SystemClock;
-import android.view.Display;
 import android.view.WindowManager;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
@@ -148,18 +147,19 @@ public void startInitialization(@NonNull Context applicationContext, @NonNull Se
       initStartTimestampMillis = SystemClock.uptimeMillis();
       flutterApplicationInfo = ApplicationInfoLoader.load(appContext);
 
-      float fps;
-      if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
-        final DisplayManager dm = appContext.getSystemService(DisplayManager.class);
-        final Display primaryDisplay = dm.getDisplay(Display.DEFAULT_DISPLAY);
-        fps = primaryDisplay.getRefreshRate();
+      VsyncWaiter waiter;
+      if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR1 /* 17 */) {
+        final DisplayManager dm =
+            (DisplayManager) appContext.getSystemService(Context.DISPLAY_SERVICE);
+        waiter = VsyncWaiter.getInstance(dm, flutterJNI);
       } else {
-        fps =
+        float fps =
             ((WindowManager) appContext.getSystemService(Context.WINDOW_SERVICE))
                 .getDefaultDisplay()
                 .getRefreshRate();
+        waiter = VsyncWaiter.getInstance(fps, flutterJNI);
       }
-      VsyncWaiter.getInstance(fps).init();
+      waiter.init();
 
       // Use a background thread for initialization tasks that require disk access.
       Callable<InitResult> initTask =
diff --git a/shell/platform/android/io/flutter/view/VsyncWaiter.java b/shell/platform/android/io/flutter/view/VsyncWaiter.java
diff --git a/shell/platform/android/test/io/flutter/view/VsyncWaiterTest.java b/shell/platform/android/test/io/flutter/view/VsyncWaiterTest.java
diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc
diff --git a/tools/android_lint/project.xml b/tools/android_lint/project.xml

Original file line number	Diff line number	Diff line change
`@@ -18,30 +18,31 @@ double DisplayManager::GetMainDisplayRefreshRate() const {`
`18`	`18`	`if (displays_.empty()) {`
`19`	`19`	`return kUnknownDisplayRefreshRate;`
`20`	`20`	`} else {`
`21`		`- return displays_[0].GetRefreshRate();`
	`21`	`+ return displays_[0]->GetRefreshRate();`
`22`	`22`	`}`
`23`	`23`	`}`
`24`	`24`
`25`		`-void DisplayManager::HandleDisplayUpdates(DisplayUpdateType update_type,`
`26`		`- std::vector<Display> displays) {`
	`25`	`+void DisplayManager::HandleDisplayUpdates(`
	`26`	`+ DisplayUpdateType update_type,`
	`27`	`+ std::vector<std::unique_ptr<Display>> displays) {`
`27`	`28`	`std::scoped_lock lock(displays_mutex_);`
`28`	`29`	`CheckDisplayConfiguration(displays);`
`29`	`30`	`switch (update_type) {`
`30`	`31`	`case DisplayUpdateType::kStartup:`
`31`	`32`	`FML_CHECK(displays_.empty());`
`32`		`- displays_ = displays;`
	`33`	`+ displays_ = std::move(displays);`
`33`	`34`	`return;`
`34`	`35`	`default:`
`35`	`36`	`FML_CHECK(false) << "Unknown DisplayUpdateType.";`
`36`	`37`	`}`
`37`	`38`	`}`
`38`	`39`
`39`	`40`	`void DisplayManager::CheckDisplayConfiguration(`
`40`		`- std::vector<Display> displays) const {`
	`41`	`+ const std::vector<std::unique_ptr<Display>>& displays) const {`
`41`	`42`	`FML_CHECK(!displays.empty());`
`42`	`43`	`if (displays.size() > 1) {`
`43`	`44`	`for (auto& display : displays) {`
`44`		`- FML_CHECK(display.GetDisplayId().has_value());`
	`45`	`+ FML_CHECK(display->GetDisplayId().has_value());`
`45`	`46`	`}`
`46`	`47`	`}`
`47`	`48`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1867,8 +1867,8 @@ void Shell::SetGpuAvailability(GpuAvailability availability) {`
`1867`	`1867`	`}`
`1868`	`1868`
`1869`	`1869`	`void Shell::OnDisplayUpdates(DisplayUpdateType update_type,`
`1870`		`- std::vector<Display> displays) {`
`1871`		`- display_manager_->HandleDisplayUpdates(update_type, displays);`
	`1870`	`+ std::vector<std::unique_ptr<Display>> displays) {`
	`1871`	`+ display_manager_->HandleDisplayUpdates(update_type, std::move(displays));`
`1872`	`1872`	`}`
`1873`	`1873`
`1874`	`1874`	`fml::TimePoint Shell::GetCurrentTimePoint() {`