From d6c95de025fe83b6c8abf5f311b4280ef975aabe Mon Sep 17 00:00:00 2001 From: Zachary Anderson Date: Mon, 28 Sep 2020 09:41:43 -0700 Subject: [PATCH] Revert "Apply dpr transform to fuchsia accessibility bridge (#21364)" This reverts commit cf1fbf27195a8ee62758483898e0903622d091a3. --- .../fuchsia/flutter/accessibility_bridge.cc | 83 +--------- .../fuchsia/flutter/accessibility_bridge.h | 13 +- .../flutter/accessibility_bridge_unittest.cc | 156 +++++++----------- .../platform/fuchsia/flutter/platform_view.cc | 2 +- 4 files changed, 67 insertions(+), 187 deletions(-) diff --git a/shell/platform/fuchsia/flutter/accessibility_bridge.cc b/shell/platform/fuchsia/flutter/accessibility_bridge.cc index ff47f55e24db1..330f3dc97eb6a 100644 --- a/shell/platform/fuchsia/flutter/accessibility_bridge.cc +++ b/shell/platform/fuchsia/flutter/accessibility_bridge.cc @@ -56,14 +56,9 @@ fuchsia::ui::gfx::BoundingBox AccessibilityBridge::GetNodeLocation( fuchsia::ui::gfx::mat4 AccessibilityBridge::GetNodeTransform( const flutter::SemanticsNode& node) const { - return ConvertSkiaTransformToMat4(node.transform); -} - -fuchsia::ui::gfx::mat4 AccessibilityBridge::ConvertSkiaTransformToMat4( - const SkM44 transform) const { fuchsia::ui::gfx::mat4 value; float* m = value.matrix.data(); - transform.getColMajor(m); + node.transform.getColMajor(m); return value; } @@ -253,8 +248,7 @@ static void PrintNodeSizeError(uint32_t node_id) { } void AccessibilityBridge::AddSemanticsNodeUpdate( - const flutter::SemanticsNodeUpdates update, - float view_pixel_ratio) { + const flutter::SemanticsNodeUpdates update) { if (update.empty()) { return; } @@ -265,7 +259,7 @@ void AccessibilityBridge::AddSemanticsNodeUpdate( std::vector nodes; size_t current_size = 0; - bool has_root_node_update = false; + // TODO(MI4-2498): Actions, Roles, hit test children, additional // flags/states/attr @@ -274,14 +268,6 @@ void AccessibilityBridge::AddSemanticsNodeUpdate( for (const auto& value : update) { size_t this_node_size = sizeof(fuchsia::accessibility::semantics::Node); const auto& flutter_node = value.second; - // We handle root update separately in GetRootNodeUpdate. - // TODO(chunhtai): remove this special case after we remove the inverse - // view pixel ratio transformation in scenic view. - if (flutter_node.id == kRootNodeId) { - root_flutter_semantics_node_ = flutter_node; - has_root_node_update = true; - continue; - } // Store the nodes for later hit testing. nodes_[flutter_node.id] = { .id = flutter_node.id, @@ -316,6 +302,7 @@ void AccessibilityBridge::AddSemanticsNodeUpdate( PrintNodeSizeError(flutter_node.id); return; } + current_size += this_node_size; // If we would exceed the max FIDL message size by appending this node, @@ -332,29 +319,6 @@ void AccessibilityBridge::AddSemanticsNodeUpdate( PrintNodeSizeError(nodes.back().node_id()); } - // Handles root node update. - if (has_root_node_update || last_seen_view_pixel_ratio_ != view_pixel_ratio) { - last_seen_view_pixel_ratio_ = view_pixel_ratio; - size_t root_node_size; - fuchsia::accessibility::semantics::Node root_update = - GetRootNodeUpdate(root_node_size); - // TODO(MI4-2531, FIDL-718): Remove this - // This is defensive. If, despite our best efforts, we ended up with a node - // that is larger than the max fidl size, we send no updates. - if (root_node_size >= kMaxMessageSize) { - PrintNodeSizeError(kRootNodeId); - return; - } - current_size += root_node_size; - // If we would exceed the max FIDL message size by appending this node, - // we should delete/update/commit now. - if (current_size >= kMaxMessageSize) { - tree_ptr_->UpdateSemanticNodes(std::move(nodes)); - nodes.clear(); - } - nodes.push_back(std::move(root_update)); - } - PruneUnreachableNodes(); UpdateScreenRects(); @@ -364,45 +328,6 @@ void AccessibilityBridge::AddSemanticsNodeUpdate( tree_ptr_->CommitUpdates([]() {}); } -fuchsia::accessibility::semantics::Node AccessibilityBridge::GetRootNodeUpdate( - size_t& node_size) { - fuchsia::accessibility::semantics::Node root_fuchsia_node; - std::vector child_ids; - node_size = sizeof(fuchsia::accessibility::semantics::Node); - for (int32_t flutter_child_id : - root_flutter_semantics_node_.childrenInTraversalOrder) { - child_ids.push_back(FlutterIdToFuchsiaId(flutter_child_id)); - } - // Applies the inverse view pixel ratio transformation to the root node. - float inverse_view_pixel_ratio = 1.f / last_seen_view_pixel_ratio_; - SkM44 inverse_view_pixel_ratio_transform; - inverse_view_pixel_ratio_transform.setScale(inverse_view_pixel_ratio, - inverse_view_pixel_ratio, 1.f); - - SkM44 result = root_flutter_semantics_node_.transform * - inverse_view_pixel_ratio_transform; - nodes_[root_flutter_semantics_node_.id] = { - .id = root_flutter_semantics_node_.id, - .flags = root_flutter_semantics_node_.flags, - .rect = root_flutter_semantics_node_.rect, - .transform = result, - .children_in_hit_test_order = - root_flutter_semantics_node_.childrenInHitTestOrder, - }; - root_fuchsia_node.set_node_id(root_flutter_semantics_node_.id) - .set_role(GetNodeRole(root_flutter_semantics_node_)) - .set_location(GetNodeLocation(root_flutter_semantics_node_)) - .set_transform(ConvertSkiaTransformToMat4(result)) - .set_attributes( - GetNodeAttributes(root_flutter_semantics_node_, &node_size)) - .set_states(GetNodeStates(root_flutter_semantics_node_, &node_size)) - .set_actions(GetNodeActions(root_flutter_semantics_node_, &node_size)) - .set_child_ids(child_ids); - node_size += kNodeIdSize * - root_flutter_semantics_node_.childrenInTraversalOrder.size(); - return root_fuchsia_node; -} - void AccessibilityBridge::UpdateScreenRects() { std::unordered_set visited_nodes; UpdateScreenRects(kRootNodeId, SkM44{}, &visited_nodes); diff --git a/shell/platform/fuchsia/flutter/accessibility_bridge.h b/shell/platform/fuchsia/flutter/accessibility_bridge.h index 5ba1d981b0aaa..bbfa18eae1621 100644 --- a/shell/platform/fuchsia/flutter/accessibility_bridge.h +++ b/shell/platform/fuchsia/flutter/accessibility_bridge.h @@ -82,8 +82,7 @@ class AccessibilityBridge void SetSemanticsEnabled(bool enabled); // Adds a semantics node update to the buffer of node updates to apply. - void AddSemanticsNodeUpdate(const flutter::SemanticsNodeUpdates update, - float view_pixel_ratio); + void AddSemanticsNodeUpdate(const flutter::SemanticsNodeUpdates update); // Notifies the bridge of a 'hover move' touch exploration event. zx_status_t OnHoverMove(double x, double y); @@ -116,8 +115,6 @@ class AccessibilityBridge AccessibilityBridge::Delegate& delegate_; static constexpr int32_t kRootNodeId = 0; - flutter::SemanticsNode root_flutter_semantics_node_; - float last_seen_view_pixel_ratio_ = 1.f; fidl::Binding binding_; fuchsia::accessibility::semantics::SemanticsManagerPtr fuchsia_semantics_manager_; @@ -127,21 +124,15 @@ class AccessibilityBridge // Assists with pruning unreachable nodes and hit testing. std::unordered_map nodes_; - fuchsia::accessibility::semantics::Node GetRootNodeUpdate(size_t& node_size); - // Derives the BoundingBox of a Flutter semantics node from its // rect and elevation. fuchsia::ui::gfx::BoundingBox GetNodeLocation( const flutter::SemanticsNode& node) const; - // Gets mat4 transformation from a Flutter semantics node. + // Converts a Flutter semantics node's transformation to a mat4. fuchsia::ui::gfx::mat4 GetNodeTransform( const flutter::SemanticsNode& node) const; - // Converts a Flutter semantics node's transformation to a mat4. - fuchsia::ui::gfx::mat4 ConvertSkiaTransformToMat4( - const SkM44 transform) const; - // Derives the attributes for a Fuchsia semantics node from a Flutter // semantics node. fuchsia::accessibility::semantics::Attributes GetNodeAttributes( diff --git a/shell/platform/fuchsia/flutter/accessibility_bridge_unittest.cc b/shell/platform/fuchsia/flutter/accessibility_bridge_unittest.cc index bd4dc20722792..d57c7781ee79e 100644 --- a/shell/platform/fuchsia/flutter/accessibility_bridge_unittest.cc +++ b/shell/platform/fuchsia/flutter/accessibility_bridge_unittest.cc @@ -179,13 +179,11 @@ TEST_F(AccessibilityBridgeTest, DeletesChildrenTransitively) { node0.childrenInTraversalOrder = {1}; node0.childrenInHitTestOrder = {1}; - accessibility_bridge_->AddSemanticsNodeUpdate( - { - {0, node0}, - {1, node1}, - {2, node2}, - }, - 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({ + {0, node0}, + {1, node1}, + {2, node2}, + }); RunLoopUntilIdle(); EXPECT_EQ(0, semantics_manager_.DeleteCount()); @@ -199,11 +197,9 @@ TEST_F(AccessibilityBridgeTest, DeletesChildrenTransitively) { // Remove the children node0.childrenInTraversalOrder.clear(); node0.childrenInHitTestOrder.clear(); - accessibility_bridge_->AddSemanticsNodeUpdate( - { - {0, node0}, - }, - 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({ + {0, node0}, + }); RunLoopUntilIdle(); EXPECT_EQ(1, semantics_manager_.DeleteCount()); @@ -221,7 +217,7 @@ TEST_F(AccessibilityBridgeTest, PopulatesRoleButton) { node0.id = 0; node0.flags = static_cast(flutter::SemanticsFlags::kIsButton); - accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}, 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}); RunLoopUntilIdle(); EXPECT_EQ(1U, semantics_manager_.LastUpdatedNodes().size()); @@ -237,7 +233,7 @@ TEST_F(AccessibilityBridgeTest, PopulatesRoleImage) { node0.id = 0; node0.flags = static_cast(flutter::SemanticsFlags::kIsImage); - accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}, 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}); RunLoopUntilIdle(); EXPECT_EQ(1U, semantics_manager_.LastUpdatedNodes().size()); @@ -253,7 +249,7 @@ TEST_F(AccessibilityBridgeTest, PopulatesRoleSlider) { node0.id = 0; node0.actions |= static_cast(flutter::SemanticsAction::kIncrease); - accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}, 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}); RunLoopUntilIdle(); EXPECT_EQ(1U, semantics_manager_.LastUpdatedNodes().size()); @@ -269,7 +265,7 @@ TEST_F(AccessibilityBridgeTest, PopulatesRoleHeader) { node0.id = 0; node0.flags = static_cast(flutter::SemanticsFlags::kIsHeader); - accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}, 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}); RunLoopUntilIdle(); EXPECT_EQ(1U, semantics_manager_.LastUpdatedNodes().size()); @@ -291,7 +287,7 @@ TEST_F(AccessibilityBridgeTest, PopulatesCheckedState) { node0.flags |= static_cast(flutter::SemanticsFlags::kIsChecked); node0.value = "value"; - accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}, 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}); RunLoopUntilIdle(); EXPECT_EQ(0, semantics_manager_.DeleteCount()); @@ -325,7 +321,7 @@ TEST_F(AccessibilityBridgeTest, PopulatesSelectedState) { // IsHidden = false node0.flags = static_cast(flutter::SemanticsFlags::kIsSelected); - accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}, 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}); RunLoopUntilIdle(); EXPECT_EQ(0, semantics_manager_.DeleteCount()); @@ -348,20 +344,6 @@ TEST_F(AccessibilityBridgeTest, PopulatesSelectedState) { EXPECT_FALSE(semantics_manager_.UpdateOverflowed()); } -TEST_F(AccessibilityBridgeTest, ApplyViewPixelRatioToRoot) { - flutter::SemanticsNode node0; - node0.id = 0; - node0.flags = static_cast(flutter::SemanticsFlags::kIsSelected); - - accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}, 1.25f); - RunLoopUntilIdle(); - const auto& fuchsia_node = semantics_manager_.LastUpdatedNodes().at(0u); - EXPECT_EQ(fuchsia_node.node_id(), static_cast(node0.id)); - EXPECT_EQ(fuchsia_node.transform().matrix[0], 0.8f); - EXPECT_EQ(fuchsia_node.transform().matrix[5], 0.8f); - EXPECT_EQ(fuchsia_node.transform().matrix[10], 1.f); -} - TEST_F(AccessibilityBridgeTest, PopulatesHiddenState) { flutter::SemanticsNode node0; node0.id = 0; @@ -371,7 +353,7 @@ TEST_F(AccessibilityBridgeTest, PopulatesHiddenState) { // IsHidden = true node0.flags = static_cast(flutter::SemanticsFlags::kIsHidden); - accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}, 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}); RunLoopUntilIdle(); EXPECT_EQ(0, semantics_manager_.DeleteCount()); @@ -403,7 +385,7 @@ TEST_F(AccessibilityBridgeTest, PopulatesActions) { node0.actions |= static_cast(flutter::SemanticsAction::kIncrease); node0.actions |= static_cast(flutter::SemanticsAction::kDecrease); - accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}, 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}}); RunLoopUntilIdle(); EXPECT_EQ(0, semantics_manager_.DeleteCount()); @@ -440,13 +422,11 @@ TEST_F(AccessibilityBridgeTest, TruncatesLargeLabel) { node0.childrenInTraversalOrder = {1, 2}; node0.childrenInHitTestOrder = {1, 2}; - accessibility_bridge_->AddSemanticsNodeUpdate( - { - {0, node0}, - {1, node1}, - {2, bad_node}, - }, - 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({ + {0, node0}, + {1, node1}, + {2, bad_node}, + }); RunLoopUntilIdle(); // Nothing to delete, but we should have broken @@ -486,13 +466,11 @@ TEST_F(AccessibilityBridgeTest, TruncatesLargeValue) { node0.childrenInTraversalOrder = {1, 2}; node0.childrenInHitTestOrder = {1, 2}; - accessibility_bridge_->AddSemanticsNodeUpdate( - { - {0, node0}, - {1, node1}, - {2, bad_node}, - }, - 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({ + {0, node0}, + {1, node1}, + {2, bad_node}, + }); RunLoopUntilIdle(); EXPECT_EQ(0, semantics_manager_.DeleteCount()); @@ -543,15 +521,13 @@ TEST_F(AccessibilityBridgeTest, SplitsLargeUpdates) { node1.childrenInTraversalOrder = {3, 4}; node1.childrenInHitTestOrder = {3, 4}; - accessibility_bridge_->AddSemanticsNodeUpdate( - { - {0, node0}, - {1, node1}, - {2, node2}, - {3, node3}, - {4, node4}, - }, - 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({ + {0, node0}, + {1, node1}, + {2, node2}, + {3, node3}, + {4, node4}, + }); RunLoopUntilIdle(); // Nothing to delete, but we should have broken into groups (4, 3, 2), (1, 0) @@ -569,11 +545,9 @@ TEST_F(AccessibilityBridgeTest, HandlesCycles) { node0.id = 0; node0.childrenInTraversalOrder.push_back(0); node0.childrenInHitTestOrder.push_back(0); - accessibility_bridge_->AddSemanticsNodeUpdate( - { - {0, node0}, - }, - 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({ + {0, node0}, + }); RunLoopUntilIdle(); EXPECT_EQ(0, semantics_manager_.DeleteCount()); @@ -588,12 +562,10 @@ TEST_F(AccessibilityBridgeTest, HandlesCycles) { node1.id = 1; node1.childrenInTraversalOrder = {0}; node1.childrenInHitTestOrder = {0}; - accessibility_bridge_->AddSemanticsNodeUpdate( - { - {0, node0}, - {1, node1}, - }, - 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({ + {0, node0}, + {1, node1}, + }); RunLoopUntilIdle(); EXPECT_EQ(0, semantics_manager_.DeleteCount()); @@ -630,7 +602,7 @@ TEST_F(AccessibilityBridgeTest, BatchesLargeMessages) { } update.insert(std::make_pair(0, std::move(node0))); - accessibility_bridge_->AddSemanticsNodeUpdate(update, 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate(update); RunLoopUntilIdle(); EXPECT_EQ(0, semantics_manager_.DeleteCount()); @@ -644,11 +616,9 @@ TEST_F(AccessibilityBridgeTest, BatchesLargeMessages) { // Remove the children node0.childrenInTraversalOrder.clear(); node0.childrenInHitTestOrder.clear(); - accessibility_bridge_->AddSemanticsNodeUpdate( - { - {0, node0}, - }, - 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({ + {0, node0}, + }); RunLoopUntilIdle(); EXPECT_EQ(1, semantics_manager_.DeleteCount()); @@ -683,15 +653,13 @@ TEST_F(AccessibilityBridgeTest, HitTest) { node0.childrenInTraversalOrder = {1, 2, 3, 4}; node0.childrenInHitTestOrder = {1, 2, 3, 4}; - accessibility_bridge_->AddSemanticsNodeUpdate( - { - {0, node0}, - {1, node1}, - {2, node2}, - {3, node3}, - {4, node4}, - }, - 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({ + {0, node0}, + {1, node1}, + {2, node2}, + {3, node3}, + {4, node4}, + }); RunLoopUntilIdle(); uint32_t hit_node_id; @@ -736,13 +704,11 @@ TEST_F(AccessibilityBridgeTest, HitTestOverlapping) { node0.childrenInTraversalOrder = {1, 2}; node0.childrenInHitTestOrder = {2, 1}; - accessibility_bridge_->AddSemanticsNodeUpdate( - { - {0, node0}, - {1, node1}, - {2, node2}, - }, - 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({ + {0, node0}, + {1, node1}, + {2, node2}, + }); RunLoopUntilIdle(); uint32_t hit_node_id; @@ -765,12 +731,10 @@ TEST_F(AccessibilityBridgeTest, Actions) { node0.childrenInTraversalOrder = {1}; node0.childrenInHitTestOrder = {1}; - accessibility_bridge_->AddSemanticsNodeUpdate( - { - {0, node0}, - {1, node1}, - }, - 1.f); + accessibility_bridge_->AddSemanticsNodeUpdate({ + {0, node0}, + {1, node1}, + }); RunLoopUntilIdle(); auto handled_callback = [](bool handled) { EXPECT_TRUE(handled); }; diff --git a/shell/platform/fuchsia/flutter/platform_view.cc b/shell/platform/fuchsia/flutter/platform_view.cc index 3f15dac57da37..958b6046f6af9 100644 --- a/shell/platform/fuchsia/flutter/platform_view.cc +++ b/shell/platform/fuchsia/flutter/platform_view.cc @@ -574,7 +574,7 @@ void PlatformView::DispatchSemanticsAction(int32_t node_id, void PlatformView::UpdateSemantics( flutter::SemanticsNodeUpdates update, flutter::CustomAccessibilityActionUpdates actions) { - accessibility_bridge_->AddSemanticsNodeUpdate(update, view_pixel_ratio_); + accessibility_bridge_->AddSemanticsNodeUpdate(update); } // Channel handler for kAccessibilityChannel