Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Apply dpr transform to fuchsia accessibility bridge #21364

Merged
merged 4 commits into from
Sep 28, 2020
Merged

Apply dpr transform to fuchsia accessibility bridge #21364

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0148d66
Apply dpr transform to fuchsia accessibility bridge
chunhtai Sep 25, 2020
a8042ca
fix format
chunhtai Sep 25, 2020
33c6742
addressing comment
chunhtai Sep 26, 2020
767d75e
fix typo
chunhtai Sep 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 79 additions & 3 deletions shell/platform/fuchsia/flutter/accessibility_bridge.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,14 @@ 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();
node.transform.getColMajor(m);
transform.getColMajor(m);
return value;
}

Expand Down Expand Up @@ -250,7 +255,7 @@ void AccessibilityBridge::AddSemanticsNodeUpdate(

std::vector<fuchsia::accessibility::semantics::Node> nodes;
size_t current_size = 0;

bool has_root_node_update = false;
// TODO(MI4-2498): Actions, Roles, hit test children, additional
// flags/states/attr

Expand All @@ -259,6 +264,14 @@ 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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this right? We don't want to set the ID or flags or whatever else might be present on the root node?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They will be set in GetRootNodeUpdate

}
// Store the nodes for later hit testing.
nodes_[flutter_node.id] = {
.id = flutter_node.id,
Expand Down Expand Up @@ -292,7 +305,6 @@ 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,
Expand All @@ -309,6 +321,30 @@ void AccessibilityBridge::AddSemanticsNodeUpdate(
PrintNodeSizeError(nodes.back().node_id());
}

// Handles root node update.
float new_view_pixel_ratio = delegate_.GetViewPixelRatio();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just pass this into AddSemanticsNodeUpdate directly from the PlatformView (or test)? That is the only place AddSemanticsNodeUpdate is ever called, so it avoids the extra delegate callback and associated complexity

if (has_root_node_update ||
last_seen_view_pixel_ratio_ != new_view_pixel_ratio) {
last_seen_view_pixel_ratio_ = new_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();

Expand All @@ -318,6 +354,46 @@ 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<uint32_t> 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<int32_t> visited_nodes;
UpdateScreenRects(kRootNodeId, SkM44{}, &visited_nodes);
Expand Down
11 changes: 10 additions & 1 deletion shell/platform/fuchsia/flutter/accessibility_bridge.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class AccessibilityBridge
virtual void SetSemanticsEnabled(bool enabled) = 0;
virtual void DispatchSemanticsAction(int32_t node_id,
flutter::SemanticsAction action) = 0;
virtual float GetViewPixelRatio() = 0;
};

// TODO(MI4-2531, FIDL-718): Remove this. We shouldn't be worried about
Expand Down Expand Up @@ -115,6 +116,8 @@ 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<fuchsia::accessibility::semantics::SemanticListener> binding_;
fuchsia::accessibility::semantics::SemanticsManagerPtr
fuchsia_semantics_manager_;
Expand All @@ -124,15 +127,21 @@ class AccessibilityBridge
// Assists with pruning unreachable nodes and hit testing.
std::unordered_map<int32_t, SemanticsNode> 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;

// Converts a Flutter semantics node's transformation to a mat4.
// Gets mat4 transformation from a Flutter semantics node.
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(
Expand Down
17 changes: 17 additions & 0 deletions shell/platform/fuchsia/flutter/accessibility_bridge_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ namespace flutter_runner_test {
class AccessibilityBridgeTestDelegate
: public flutter_runner::AccessibilityBridge::Delegate {
public:
float view_pixel_ratio = 1.f;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For tests it would probably be helpful to set this to some other value so we can check the transforms are applied correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I make use of this float in ApplyViewPixelRatioToRoot test

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that it's used, but I think it should be some value other than 1.0, which doesn't result in a particularly interesting transformation right?

void SetSemanticsEnabled(bool enabled) override { enabled_ = enabled; }
void DispatchSemanticsAction(int32_t node_id,
flutter::SemanticsAction action) override {
actions.push_back(std::make_pair(node_id, action));
}
float GetViewPixelRatio() override { return view_pixel_ratio; }

bool enabled() { return enabled_; }
std::vector<std::pair<int32_t, flutter::SemanticsAction>> actions;
Expand Down Expand Up @@ -269,6 +271,21 @@ TEST_F(AccessibilityBridgeTest, PopulatesSelectedState) {
EXPECT_FALSE(semantics_manager_.UpdateOverflowed());
}

TEST_F(AccessibilityBridgeTest, ApplyViewPixelRatioToRoot) {
accessibility_delegate_.view_pixel_ratio = 1.25f;
flutter::SemanticsNode node0;
node0.id = 0;
node0.flags = static_cast<int>(flutter::SemanticsFlags::kIsSelected);

accessibility_bridge_->AddSemanticsNodeUpdate({{0, node0}});
RunLoopUntilIdle();
const auto& fuchsia_node = semantics_manager_.LastUpdatedNodes().at(0u);
EXPECT_EQ(fuchsia_node.node_id(), static_cast<unsigned int>(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;
Expand Down
6 changes: 6 additions & 0 deletions shell/platform/fuchsia/flutter/platform_view.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,12 @@ void PlatformView::DispatchSemanticsAction(int32_t node_id,
flutter::PlatformView::DispatchSemanticsAction(node_id, action, {});
}

// |flutter::PlatformView|
// |flutter_runner::AccessibilityBridge::Delegate|
float PlatformView::GetViewPixelRatio() {
return view_pixel_ratio_;
}

// |flutter::PlatformView|
void PlatformView::UpdateSemantics(
flutter::SemanticsNodeUpdates update,
Expand Down
2 changes: 2 additions & 0 deletions shell/platform/fuchsia/flutter/platform_view.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ class PlatformView final : public flutter::PlatformView,
// |flutter_runner::AccessibilityBridge::Delegate|
void DispatchSemanticsAction(int32_t node_id,
flutter::SemanticsAction action) override;
// |flutter_runner::AccessibilityBridge::Delegate|
float GetViewPixelRatio() override;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned above, I think we can get rid of this


// |PlatformView|
flutter::PointerDataDispatcherMaker GetDispatcherMaker() override;
Expand Down