fuchsia: Don't send ViewportMetrics w/ 0 DPR (#21392)

lib/ui/window/viewport_metrics.cc

@@ -10,6 +10,13 @@ namespace flutter {
 
 ViewportMetrics::ViewportMetrics() = default;
 
+ViewportMetrics::ViewportMetrics(double p_device_pixel_ratio,
+                                 double p_physical_width,
+                                 double p_physical_height)
+    : device_pixel_ratio(p_device_pixel_ratio),
+      physical_width(p_physical_width),
+      physical_height(p_physical_height) {}
+
 ViewportMetrics::ViewportMetrics(double p_device_pixel_ratio,
                                  double p_physical_width,
                                  double p_physical_height,
@@ -44,11 +51,42 @@ ViewportMetrics::ViewportMetrics(double p_device_pixel_ratio,
       physical_system_gesture_inset_left(p_physical_system_gesture_inset_left) {
 }
 
-ViewportMetrics::ViewportMetrics(double p_device_pixel_ratio,
-                                 double p_physical_width,
-                                 double p_physical_height)
-    : device_pixel_ratio(p_device_pixel_ratio),
-      physical_width(p_physical_width),
-      physical_height(p_physical_height) {}
+bool operator==(const ViewportMetrics& a, const ViewportMetrics& b) {
+  return a.device_pixel_ratio == b.device_pixel_ratio &&
+         a.physical_width == b.physical_width &&
+         a.physical_height == b.physical_height &&
+         a.physical_padding_top == b.physical_padding_top &&
+         a.physical_padding_right == b.physical_padding_right &&
+         a.physical_padding_bottom == b.physical_padding_bottom &&
+         a.physical_padding_left == b.physical_padding_left &&
+         a.physical_view_inset_top == b.physical_view_inset_top &&
+         a.physical_view_inset_right == b.physical_view_inset_right &&
+         a.physical_view_inset_bottom == b.physical_view_inset_bottom &&
+         a.physical_view_inset_left == b.physical_view_inset_left &&
+         a.physical_system_gesture_inset_top ==
+             b.physical_system_gesture_inset_top &&
+         a.physical_system_gesture_inset_right ==
+             b.physical_system_gesture_inset_right &&
+         a.physical_system_gesture_inset_bottom ==
+             b.physical_system_gesture_inset_bottom &&
+         a.physical_system_gesture_inset_left ==
+             b.physical_system_gesture_inset_left;
+}
+
+std::ostream& operator<<(std::ostream& os, const ViewportMetrics& a) {
+  os << "DPR: " << a.device_pixel_ratio << " "
+     << "Size: [" << a.physical_width << "W " << a.physical_height << "H] "
+     << "Padding: [" << a.physical_padding_top << "T "
+     << a.physical_padding_right << "R " << a.physical_padding_bottom << "B "
+     << a.physical_padding_left << "L] "
+     << "Insets: [" << a.physical_view_inset_top << "T "
+     << a.physical_view_inset_right << "R " << a.physical_view_inset_bottom
+     << "B " << a.physical_view_inset_left << "L] "
+     << "Gesture Insets: [" << a.physical_system_gesture_inset_top << "T "
+     << a.physical_system_gesture_inset_right << "R "
+     << a.physical_system_gesture_inset_bottom << "B "
+     << a.physical_system_gesture_inset_left << "L]";
+  return os;
+}
 
 }  // namespace flutter

lib/ui/window/viewport_metrics.h

@@ -5,10 +5,15 @@
 #ifndef FLUTTER_LIB_UI_WINDOW_VIEWPORT_METRICS_H_
 #define FLUTTER_LIB_UI_WINDOW_VIEWPORT_METRICS_H_
 
+#include <ostream>
+
 namespace flutter {
 
 struct ViewportMetrics {
   ViewportMetrics();
+  ViewportMetrics(double p_device_pixel_ratio,
+                  double p_physical_width,
+                  double p_physical_height);
   ViewportMetrics(double p_device_pixel_ratio,
                   double p_physical_width,
                   double p_physical_height,
@@ -25,12 +30,6 @@ struct ViewportMetrics {
                   double p_physical_system_gesture_inset_bottom,
                   double p_physical_system_gesture_inset_left);
 
-  // Create a ViewportMetrics instance that doesn't include depth, padding, or
-  // insets.
-  ViewportMetrics(double p_device_pixel_ratio,
-                  double p_physical_width,
-                  double p_physical_height);
-
   double device_pixel_ratio = 1.0;
   double physical_width = 0;
   double physical_height = 0;
@@ -48,24 +47,8 @@ struct ViewportMetrics {
   double physical_system_gesture_inset_left = 0;
 };
 
-struct LogicalSize {
-  double width = 0.0;
-  double height = 0.0;
-};
-
-struct LogicalInset {
-  double left = 0.0;
-  double top = 0.0;
-  double right = 0.0;
-  double bottom = 0.0;
-};
-
-struct LogicalMetrics {
-  LogicalSize size;
-  double scale = 1.0;
-  LogicalInset padding;
-  LogicalInset view_inset;
-};
+bool operator==(const ViewportMetrics& a, const ViewportMetrics& b);
+std::ostream& operator<<(std::ostream& os, const ViewportMetrics& a);
 
 }  // namespace flutter
 

shell/platform/fuchsia/flutter/platform_view.cc

@@ -14,10 +14,11 @@
 #include "flutter/fml/logging.h"
 #include "flutter/lib/ui/window/pointer_data.h"
 #include "flutter/lib/ui/window/window.h"
+#include "third_party/rapidjson/include/rapidjson/document.h"
+#include "third_party/rapidjson/include/rapidjson/stringbuffer.h"
+#include "third_party/rapidjson/include/rapidjson/writer.h"
+
 #include "logging.h"
-#include "rapidjson/document.h"
-#include "rapidjson/stringbuffer.h"
-#include "rapidjson/writer.h"
 #include "runtime/dart/utils/inlines.h"
 #include "vsync_waiter.h"
 
@@ -214,7 +215,8 @@ void PlatformView::OnScenicError(std::string error) {
 void PlatformView::OnScenicEvent(
     std::vector<fuchsia::ui::scenic::Event> events) {
   TRACE_EVENT0("flutter", "PlatformView::OnScenicEvent");
-  bool should_update_metrics = false;
+
+  bool metrics_changed = false;
   for (const auto& event : events) {
     switch (event.Which()) {
       case fuchsia::ui::scenic::Event::Tag::kGfx:
@@ -223,31 +225,52 @@ void PlatformView::OnScenicEvent(
             const fuchsia::ui::gfx::Metrics& metrics =
                 event.gfx().metrics().metrics;
             const float new_view_pixel_ratio = metrics.scale_x;
+            if (new_view_pixel_ratio <= 0.f) {
+              FML_DLOG(ERROR)
+                  << "Got an invalid pixel ratio from Scenic; ignoring: "
+                  << new_view_pixel_ratio;
+              break;
+            }
 
             // Avoid metrics update when possible -- it is computationally
             // expensive.
-            if (view_pixel_ratio_ != new_view_pixel_ratio) {
-              view_pixel_ratio_ = new_view_pixel_ratio;
-              should_update_metrics = true;
+            if (view_pixel_ratio_.has_value() &&
+                *view_pixel_ratio_ == new_view_pixel_ratio) {
+              FML_DLOG(ERROR)
+                  << "Got an identical pixel ratio from Scenic; ignoring: "
+                  << new_view_pixel_ratio;
+              break;
             }
+
+            view_pixel_ratio_ = new_view_pixel_ratio;
+            metrics_changed = true;
             break;
           }
           case fuchsia::ui::gfx::Event::Tag::kViewPropertiesChanged: {
             const fuchsia::ui::gfx::BoundingBox& bounding_box =
                 event.gfx().view_properties_changed().properties.bounding_box;
-            const float new_view_width =
-                std::max(bounding_box.max.x - bounding_box.min.x, 0.0f);
-            const float new_view_height =
-                std::max(bounding_box.max.y - bounding_box.min.y, 0.0f);
+            const std::pair<float, float> new_view_size = {
+                std::max(bounding_box.max.x - bounding_box.min.x, 0.0f),
+                std::max(bounding_box.max.y - bounding_box.min.y, 0.0f)};
+            if (new_view_size.first <= 0.f || new_view_size.second <= 0.f) {
+              FML_DLOG(ERROR)
+                  << "Got an invalid view size from Scenic; ignoring: "
+                  << new_view_size.first << " " << new_view_size.second;
+              break;
+            }
 
             // Avoid metrics update when possible -- it is computationally
             // expensive.
-            if (view_width_ != new_view_width ||
-                view_height_ != new_view_width) {
-              view_width_ = new_view_width;
-              view_height_ = new_view_height;
-              should_update_metrics = true;
+            if (view_logical_size_.has_value() &&
+                *view_logical_size_ == new_view_size) {
+              FML_DLOG(ERROR)
+                  << "Got an identical view size from Scenic; ignoring: "
+                  << new_view_size.first << " " << new_view_size.second;
+              break;
             }
+
+            view_logical_size_ = new_view_size;
+            metrics_changed = true;
             break;
           }
           case fuchsia::ui::gfx::Event::Tag::kViewConnected:
@@ -298,19 +321,22 @@ void PlatformView::OnScenicEvent(
     }
   }
 
-  if (should_update_metrics) {
+  if (view_pixel_ratio_.has_value() && view_logical_size_.has_value() &&
+      metrics_changed) {
+    const float pixel_ratio = *view_pixel_ratio_;
+    const std::pair<float, float> logical_size = *view_logical_size_;
     SetViewportMetrics({
-        view_pixel_ratio_,                 // device_pixel_ratio
-        view_width_ * view_pixel_ratio_,   // physical_width
-        view_height_ * view_pixel_ratio_,  // physical_height
-        0.0f,                              // physical_padding_top
-        0.0f,                              // physical_padding_right
-        0.0f,                              // physical_padding_bottom
-        0.0f,                              // physical_padding_left
-        0.0f,                              // physical_view_inset_top
-        0.0f,                              // physical_view_inset_right
-        0.0f,                              // physical_view_inset_bottom
-        0.0f,                              // physical_view_inset_left
+        pixel_ratio,                        // device_pixel_ratio
+        logical_size.first * pixel_ratio,   // physical_width
+        logical_size.second * pixel_ratio,  // physical_height
+        0.0f,                               // physical_padding_top
+        0.0f,                               // physical_padding_right
+        0.0f,                               // physical_padding_bottom
+        0.0f,                               // physical_padding_left
+        0.0f,                               // physical_view_inset_top
+        0.0f,                               // physical_view_inset_right
+        0.0f,                               // physical_view_inset_bottom
+        0.0f,                               // physical_view_inset_left
         0.0f,  // p_physical_system_gesture_inset_top
         0.0f,  // p_physical_system_gesture_inset_right
         0.0f,  // p_physical_system_gesture_inset_bottom
@@ -421,6 +447,9 @@ bool PlatformView::OnHandlePointerEvent(
       PointerTraceHACK(pointer.radius_major, pointer.radius_minor);
   TRACE_FLOW_END("input", "dispatch_event_to_client", trace_id);
 
+  const float pixel_ratio =
+      view_pixel_ratio_.has_value() ? *view_pixel_ratio_ : 0.f;
+
   flutter::PointerData pointer_data;
   pointer_data.Clear();
   pointer_data.time_stamp = pointer.event_time / 1000;
@@ -428,8 +457,8 @@ bool PlatformView::OnHandlePointerEvent(
   pointer_data.kind = GetKindFromPointerType(pointer.type);
   pointer_data.device = pointer.pointer_id;
   // Pointer events are in logical pixels, so scale to physical.
-  pointer_data.physical_x = pointer.x * view_pixel_ratio_;
-  pointer_data.physical_y = pointer.y * view_pixel_ratio_;
+  pointer_data.physical_x = pointer.x * pixel_ratio;
+  pointer_data.physical_y = pointer.y * pixel_ratio;
   // Buttons are single bit values starting with kMousePrimaryButton = 1.
   pointer_data.buttons = static_cast<uint64_t>(pointer.buttons);
 
@@ -601,7 +630,10 @@ void PlatformView::DispatchSemanticsAction(int32_t node_id,
 void PlatformView::UpdateSemantics(
     flutter::SemanticsNodeUpdates update,
     flutter::CustomAccessibilityActionUpdates actions) {
-  accessibility_bridge_->AddSemanticsNodeUpdate(update, view_pixel_ratio_);
+  const float pixel_ratio =
+      view_pixel_ratio_.has_value() ? *view_pixel_ratio_ : 0.f;
+
+  accessibility_bridge_->AddSemanticsNodeUpdate(update, pixel_ratio);
 }
 
 // Channel handler for kAccessibilityChannel

shell/platform/fuchsia/flutter/platform_view.h

@@ -9,6 +9,7 @@
 #include <fuchsia/ui/scenic/cpp/fidl.h>
 #include <lib/fidl/cpp/binding.h>
 #include <lib/fit/function.h>
+#include <lib/ui/scenic/cpp/id.h>
 
 #include <map>
 #include <set>
@@ -19,8 +20,6 @@
 
 #include "accessibility_bridge.h"
 
-#include <lib/ui/scenic/cpp/id.h>  // nogncheck
-
 namespace flutter_runner {
 
 using OnEnableWireframe = fit::function<void(bool)>;
@@ -29,16 +28,22 @@ using OnUpdateView = fit::function<void(int64_t, bool, bool)>;
 using OnDestroyView = fit::function<void(int64_t)>;
 using OnCreateSurface = fit::function<std::unique_ptr<flutter::Surface>()>;
 
-// The per engine component residing on the platform thread is responsible for
-// all platform specific integrations.
+// PlatformView is the per-engine component residing on the platform thread that
+// is responsible for all platform specific integrations -- particularly
+// integration with the platform's accessibility, input, and windowing features.
+//
+// PlatformView communicates with the Dart code via "platform messages" handled
+// in HandlePlatformMessage.  This communication is bidirectional.  Platform
+// messages are notably responsible for communication related to input and
+// external views / windowing.
 //
 // The PlatformView implements SessionListener and gets Session events but it
-// does *not* actually own the Session itself; that is owned by the Compositor
-// thread.
+// does *not* actually own the Session itself; that is owned by the
+// FuchsiaExternalViewEmbedder on the raster thread.
 class PlatformView final : public flutter::PlatformView,
+                           public AccessibilityBridge::Delegate,
                            private fuchsia::ui::scenic::SessionListener,
-                           public fuchsia::ui::input::InputMethodEditorClient,
-                           public AccessibilityBridge::Delegate {
+                           private fuchsia::ui::input::InputMethodEditorClient {
  public:
   PlatformView(flutter::PlatformView::Delegate& delegate,
                std::string debug_label,
@@ -73,51 +78,6 @@ class PlatformView final : public flutter::PlatformView,
   flutter::PointerDataDispatcherMaker GetDispatcherMaker() override;
 
  private:
-  const std::string debug_label_;
-  // TODO(MI4-2490): remove once ViewRefControl is passed to Scenic and kept
-  // alive there
-  const fuchsia::ui::views::ViewRef view_ref_;
-  fuchsia::ui::views::FocuserPtr focuser_;
-  std::unique_ptr<AccessibilityBridge> accessibility_bridge_;
-
-  fidl::Binding<fuchsia::ui::scenic::SessionListener> session_listener_binding_;
-  fit::closure session_listener_error_callback_;
-  OnEnableWireframe wireframe_enabled_callback_;
-  OnCreateView on_create_view_callback_;
-  OnUpdateView on_update_view_callback_;
-  OnDestroyView on_destroy_view_callback_;
-  OnCreateSurface on_create_surface_callback_;
-
-  int current_text_input_client_ = 0;
-  fidl::Binding<fuchsia::ui::input::InputMethodEditorClient> ime_client_;
-  fuchsia::ui::input::InputMethodEditorPtr ime_;
-  fuchsia::ui::input::ImeServicePtr text_sync_service_;
-
-  fuchsia::sys::ServiceProviderPtr parent_environment_service_provider_;
-
-  // last_text_state_ is the last state of the text input as reported by the IME
-  // or initialized by Flutter. We set it to null if Flutter doesn't want any
-  // input, since then there is no text input state at all.
-  std::unique_ptr<fuchsia::ui::input::TextInputState> last_text_state_;
-
-  std::set<int> down_pointers_;
-  std::map<
-      std::string /* channel */,
-      fit::function<void(
-          fml::RefPtr<flutter::PlatformMessage> /* message */)> /* handler */>
-      platform_message_handlers_;
-  // These are the channels that aren't registered and have been notified as
-  // such. Notifying via logs multiple times results in log-spam. See:
-  // https://github.com/flutter/flutter/issues/55966
-  std::set<std::string /* channel */> unregistered_channels_;
-
-  fml::TimeDelta vsync_offset_;
-  zx_handle_t vsync_event_handle_ = 0;
-
-  float view_width_ = 0.0f;        // Width in logical pixels.
-  float view_height_ = 0.0f;       // Height in logical pixels.
-  float view_pixel_ratio_ = 0.0f;  // Logical / physical pixel ratio.
-
   void RegisterPlatformMessageHandlers();
 
   // |fuchsia::ui::input::InputMethodEditorClient|
@@ -185,6 +145,57 @@ class PlatformView final : public flutter::PlatformView,
   void HandleFlutterPlatformViewsChannelPlatformMessage(
       fml::RefPtr<flutter::PlatformMessage> message);
 
+  const std::string debug_label_;
+  // TODO(MI4-2490): remove once ViewRefControl is passed to Scenic and kept
+  // alive there
+  const fuchsia::ui::views::ViewRef view_ref_;
+  fuchsia::ui::views::FocuserPtr focuser_;
+  std::unique_ptr<AccessibilityBridge> accessibility_bridge_;
+
+  // Logical size and logical->physical ratio.  These are optional to provide
+  // an "unset" state during program startup, before Scenic has sent any
+  // metrics-related events to provide initial values for these.
+  //
+  // The engine internally uses a default size of (0.f 0.f) with a default 1.f
+  // ratio, so there is no need to emit events until Scenic has actually sent a
+  // valid size and ratio.
+  std::optional<std::pair<float, float>> view_logical_size_;
+  std::optional<float> view_pixel_ratio_;
+
+  fidl::Binding<fuchsia::ui::scenic::SessionListener> session_listener_binding_;
+  fit::closure session_listener_error_callback_;
+  OnEnableWireframe wireframe_enabled_callback_;
+  OnCreateView on_create_view_callback_;
+  OnUpdateView on_update_view_callback_;
+  OnDestroyView on_destroy_view_callback_;
+  OnCreateSurface on_create_surface_callback_;
+
+  int current_text_input_client_ = 0;
+  fidl::Binding<fuchsia::ui::input::InputMethodEditorClient> ime_client_;
+  fuchsia::ui::input::InputMethodEditorPtr ime_;
+  fuchsia::ui::input::ImeServicePtr text_sync_service_;
+
+  fuchsia::sys::ServiceProviderPtr parent_environment_service_provider_;
+
+  // last_text_state_ is the last state of the text input as reported by the IME
+  // or initialized by Flutter. We set it to null if Flutter doesn't want any
+  // input, since then there is no text input state at all.
+  std::unique_ptr<fuchsia::ui::input::TextInputState> last_text_state_;
+
+  std::set<int> down_pointers_;
+  std::map<
+      std::string /* channel */,
+      fit::function<void(
+          fml::RefPtr<flutter::PlatformMessage> /* message */)> /* handler */>
+      platform_message_handlers_;
+  // These are the channels that aren't registered and have been notified as
+  // such. Notifying via logs multiple times results in log-spam. See:
+  // https://github.com/flutter/flutter/issues/55966
+  std::set<std::string /* channel */> unregistered_channels_;
+
+  fml::TimeDelta vsync_offset_;
+  zx_handle_t vsync_event_handle_ = 0;
+
   FML_DISALLOW_COPY_AND_ASSIGN(PlatformView);
 };