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

Author: arbreng

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,15 +447,18 @@ 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;
   pointer_data.change = GetChangeFromPointerEventPhase(pointer.phase);
   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