Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[fuchsia] HitTesting for fuchsia a11y #15570

Merged
merged 12 commits into from
Jan 22, 2020
Merged

[fuchsia] HitTesting for fuchsia a11y #15570

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1405b2b
HitTesting for fuchsia a11y
dnfield Jan 14, 2020
3b38ae2
test attempt
dnfield Jan 14, 2020
36a8681
update test, avoid cycles in new method
dnfield Jan 14, 2020
b30e145
uncomment
dnfield Jan 14, 2020
358ae6c
CI
dnfield Jan 14, 2020
55d9b9c
Merge remote-tracking branch 'upstream/master' into fuchsia_hit
dnfield Jan 15, 2020
4648271
map2
dnfield Jan 15, 2020
a318f41
Merge remote-tracking branch 'upstream/master' into fuchsia_hit
dnfield Jan 22, 2020
b80421e
early return
dnfield Jan 22, 2020
2d1de23
Merge branch 'fuchsia_hit' of github.com:dnfield/engine into fuchsia_hit
dnfield Jan 22, 2020
c502973
SkScalar
dnfield Jan 22, 2020
f541a08
ci
dnfield Jan 22, 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
90 changes: 85 additions & 5 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 @@ -99,8 +99,8 @@ std::unordered_set<int32_t> AccessibilityBridge::GetDescendants(

auto it = nodes_.find(id);
if (it != nodes_.end()) {
auto const& children = it->second;
for (const auto& child : children) {
const auto& node = it->second;
for (const auto& child : node.children_in_hit_test_order) {
if (descendents.find(child) == descendents.end()) {
to_process.push_back(child);
} else {
Expand Down Expand Up @@ -180,10 +180,18 @@ void AccessibilityBridge::AddSemanticsNodeUpdate(
for (const auto& value : update) {
size_t this_node_size = sizeof(fuchsia::accessibility::semantics::Node);
const auto& flutter_node = value.second;
nodes_[flutter_node.id] =
std::vector<int32_t>(flutter_node.childrenInTraversalOrder);
// Store the nodes for later hit testing.
nodes_[flutter_node.id] = {
.id = flutter_node.id,
.flags = flutter_node.flags,
.rect = flutter_node.rect,
.transform = flutter_node.transform,
.children_in_hit_test_order = flutter_node.childrenInHitTestOrder,
};
fuchsia::accessibility::semantics::Node fuchsia_node;
std::vector<uint32_t> child_ids;
// Send the nodes in traversal order, so the manager can figure out
// traversal.
for (int32_t flutter_child_id : flutter_node.childrenInTraversalOrder) {
child_ids.push_back(FlutterIdToFuchsiaId(flutter_child_id));
}
Expand Down Expand Up @@ -221,13 +229,58 @@ void AccessibilityBridge::AddSemanticsNodeUpdate(
}

PruneUnreachableNodes();
UpdateScreenRects();

tree_ptr_->UpdateSemanticNodes(std::move(nodes));
// TODO(dnfield): Implement the callback here
// https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=35718.
tree_ptr_->CommitUpdates([]() {});
}

void AccessibilityBridge::UpdateScreenRects() {
std::unordered_set<int32_t> visited_nodes;
UpdateScreenRects(kRootNodeId, SkMatrix44::I(), &visited_nodes);
}

void AccessibilityBridge::UpdateScreenRects(
int32_t node_id,
SkMatrix44 parent_transform,
std::unordered_set<int32_t>* visited_nodes) {
auto it = nodes_.find(node_id);
if (it == nodes_.end()) {
FML_LOG(ERROR) << "UpdateScreenRects called on unknown node";
Copy link
Member

Choose a reason for hiding this comment

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

Aren't you missing an early return here?

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 - added.

}
auto& node = it->second;
const auto& current_transform = parent_transform * node.transform;

const auto& rect = node.rect;
FML_LOG(ERROR) << "nodeid: " << node_id;
SkMScalar quad[] = {
rect.left(), rect.top(), //
rect.right(), rect.top(), //
rect.right(), rect.bottom(), //
rect.left(), rect.bottom(), //
};
SkMScalar dst[4 * 4];
current_transform.map2(quad, 4, dst);
node.screen_rect.setLTRB(dst[0], dst[1], dst[8], dst[9]);
node.screen_rect.sort();
std::vector<SkVector4> points = {
Copy link
Member

Choose a reason for hiding this comment

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

This can all be done in a single call to SkMatrix44::map2 , then putting the resulting points in a SkRect and then calling SkRect::sort.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice. Done ... I think. Am I picking the right indices?

Copy link
Member

Choose a reason for hiding this comment

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

lol, I am confused too. But these look right.

current_transform * SkVector4(rect.left(), rect.top(), 0, 1),
current_transform * SkVector4(rect.right(), rect.top(), 0, 1),
current_transform * SkVector4(rect.right(), rect.bottom(), 0, 1),
current_transform * SkVector4(rect.left(), rect.bottom(), 0, 1),
};

visited_nodes->emplace(node_id);

for (uint32_t child_id : node.children_in_hit_test_order) {
if (visited_nodes->find(child_id) == visited_nodes->end()) {
UpdateScreenRects(child_id, current_transform, visited_nodes);
}
}
}

// |fuchsia::accessibility::semantics::SemanticListener|
void AccessibilityBridge::OnAccessibilityActionRequested(
uint32_t node_id,
Expand All @@ -239,7 +292,34 @@ void AccessibilityBridge::OnAccessibilityActionRequested(
void AccessibilityBridge::HitTest(
fuchsia::math::PointF local_point,
fuchsia::accessibility::semantics::SemanticListener::HitTestCallback
callback) {}
callback) {
auto hit_node_id = GetHitNode(kRootNodeId, local_point.x, local_point.y);
FML_DCHECK(hit_node_id.has_value());
fuchsia::accessibility::semantics::Hit hit;
hit.set_node_id(hit_node_id.value_or(kRootNodeId));
callback(std::move(hit));
}

std::optional<int32_t> AccessibilityBridge::GetHitNode(int32_t node_id,
float x,
float y) {
auto it = nodes_.find(node_id);
if (it == nodes_.end()) {
FML_LOG(ERROR) << "Attempted to hit test unkonwn node id: " << node_id;
return {};
}
auto const& node = it->second;
if (node.flags &
static_cast<int32_t>(flutter::SemanticsFlags::kIsHidden) || //
!node.screen_rect.contains(x, y)) {
return {};
}
auto hit = node_id;
for (int32_t child_id : node.children_in_hit_test_order) {
hit = GetHitNode(child_id, x, y).value_or(hit);
}
return hit;
}

// |fuchsia::accessibility::semantics::SemanticListener|
void AccessibilityBridge::OnSemanticsModeChanged(
Expand Down
47 changes: 39 additions & 8 deletions shell/platform/fuchsia/flutter/accessibility_bridge.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,24 @@ class AccessibilityBridge
// Notifies the bridge of a 'hover move' touch exploration event.
zx_status_t OnHoverMove(double x, double y);

// |fuchsia::accessibility::semantics::SemanticListener|
void HitTest(
fuchsia::math::PointF local_point,
fuchsia::accessibility::semantics::SemanticListener::HitTestCallback
callback) override;

private:
// Holds only the fields we need for hit testing.
// In particular, it adds a screen_rect field to flutter::SemanticsNode.
struct SemanticsNode {
Copy link
Member

Choose a reason for hiding this comment

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

Why not just reuse flutter::SemanticsNode from //flutter/lib/ui/semantics/semantics_node.h. It may have some redundant bits but having two of these is really hard to parse.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This started as an attempt to avoid holding onto more than we need, but I agree it's less helpful now. I'll change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah. The reason I can't do this is I need the screen_rect, which flutter::SemanticsNode does not have.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for clarifying.

int32_t id;
int32_t flags;
SkRect rect;
SkRect screen_rect;
SkMatrix44 transform;
std::vector<int32_t> children_in_hit_test_order;
};

AccessibilityBridge::Delegate& delegate_;

static constexpr int32_t kRootNodeId = 0;
Expand All @@ -95,8 +112,8 @@ class AccessibilityBridge
fuchsia::accessibility::semantics::SemanticTreePtr tree_ptr_;
bool semantics_enabled_;
// This is the cache of all nodes we've sent to Fuchsia's SemanticsManager.
// Assists with pruning unreachable nodes.
std::unordered_map<int32_t, std::vector<int32_t>> nodes_;
// Assists with pruning unreachable nodes and hit testing.
std::unordered_map<int32_t, SemanticsNode> nodes_;

// Derives the BoundingBox of a Flutter semantics node from its
// rect and elevation.
Expand Down Expand Up @@ -127,19 +144,33 @@ class AccessibilityBridge
// May result in a call to FuchsiaAccessibility::Commit().
void PruneUnreachableNodes();

// Updates the on-screen positions of accessibility elements,
// starting from the root element with an identity matrix.
//
// This should be called from Update.
void UpdateScreenRects();

// Updates the on-screen positions of accessibility elements, starting
// from node_id and using the specified transform.
//
// Update calls this via UpdateScreenRects().
void UpdateScreenRects(int32_t node_id,
SkMatrix44 parent_transform,
std::unordered_set<int32_t>* visited_nodes);

// Traverses the semantics tree to find the node_id hit by the given x,y
// point.
//
// Assumes that SemanticsNode::screen_rect is up to date.
std::optional<int32_t> GetHitNode(int32_t node_id, float x, float y);

// |fuchsia::accessibility::semantics::SemanticListener|
void OnAccessibilityActionRequested(
uint32_t node_id,
fuchsia::accessibility::semantics::Action action,
fuchsia::accessibility::semantics::SemanticListener::
OnAccessibilityActionRequestedCallback callback) override;

// |fuchsia::accessibility::semantics::SemanticListener|
void HitTest(
fuchsia::math::PointF local_point,
fuchsia::accessibility::semantics::SemanticListener::HitTestCallback
callback) override;

// |fuchsia::accessibility::semantics::SemanticListener|
void OnSemanticsModeChanged(bool enabled,
OnSemanticsModeChangedCallback callback) override;
Expand Down
70 changes: 70 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 @@ -90,10 +90,12 @@ TEST_F(AccessibilityBridgeTest, DeletesChildrenTransitively) {
flutter::SemanticsNode node1;
node1.id = 1;
node1.childrenInTraversalOrder = {2};
node1.childrenInHitTestOrder = {2};

flutter::SemanticsNode node0;
node0.id = 0;
node0.childrenInTraversalOrder = {1};
node0.childrenInHitTestOrder = {1};

accessibility_bridge_->AddSemanticsNodeUpdate({
{0, node0},
Expand All @@ -112,6 +114,7 @@ TEST_F(AccessibilityBridgeTest, DeletesChildrenTransitively) {

// Remove the children
node0.childrenInTraversalOrder.clear();
node0.childrenInHitTestOrder.clear();
accessibility_bridge_->AddSemanticsNodeUpdate({
{0, node0},
});
Expand Down Expand Up @@ -141,6 +144,7 @@ TEST_F(AccessibilityBridgeTest, TruncatesLargeLabel) {
std::string(fuchsia::accessibility::semantics::MAX_LABEL_SIZE + 1, '2');

node0.childrenInTraversalOrder = {1, 2};
node0.childrenInHitTestOrder = {1, 2};

accessibility_bridge_->AddSemanticsNodeUpdate({
{0, node0},
Expand Down Expand Up @@ -194,7 +198,9 @@ TEST_F(AccessibilityBridgeTest, SplitsLargeUpdates) {
std::string(fuchsia::accessibility::semantics::MAX_LABEL_SIZE, '4');

node0.childrenInTraversalOrder = {1, 2};
node0.childrenInHitTestOrder = {1, 2};
node1.childrenInTraversalOrder = {3, 4};
node1.childrenInHitTestOrder = {3, 4};

accessibility_bridge_->AddSemanticsNodeUpdate({
{0, node0},
Expand All @@ -219,6 +225,7 @@ TEST_F(AccessibilityBridgeTest, HandlesCycles) {
flutter::SemanticsNode node0;
node0.id = 0;
node0.childrenInTraversalOrder.push_back(0);
node0.childrenInHitTestOrder.push_back(0);
accessibility_bridge_->AddSemanticsNodeUpdate({
{0, node0},
});
Expand All @@ -231,9 +238,11 @@ TEST_F(AccessibilityBridgeTest, HandlesCycles) {
EXPECT_FALSE(semantics_manager_.UpdateOverflowed());

node0.childrenInTraversalOrder = {0, 1};
node0.childrenInHitTestOrder = {0, 1};
flutter::SemanticsNode node1;
node1.id = 1;
node1.childrenInTraversalOrder = {0};
node1.childrenInHitTestOrder = {0};
accessibility_bridge_->AddSemanticsNodeUpdate({
{0, node0},
{1, node1},
Expand All @@ -260,12 +269,14 @@ TEST_F(AccessibilityBridgeTest, BatchesLargeMessages) {
flutter::SemanticsNode node;
node.id = i;
node0.childrenInTraversalOrder.push_back(i);
node0.childrenInHitTestOrder.push_back(i);
for (int32_t j = 0; j < leaf_nodes; j++) {
flutter::SemanticsNode leaf_node;
int id = (i * child_nodes) + ((j + 1) * leaf_nodes);
leaf_node.id = id;
leaf_node.label = "A relatively simple label";
node.childrenInTraversalOrder.push_back(id);
node.childrenInHitTestOrder.push_back(id);
update.insert(std::make_pair(id, std::move(leaf_node)));
}
update.insert(std::make_pair(i, std::move(node)));
Expand All @@ -283,6 +294,7 @@ TEST_F(AccessibilityBridgeTest, BatchesLargeMessages) {

// Remove the children
node0.childrenInTraversalOrder.clear();
node0.childrenInHitTestOrder.clear();
accessibility_bridge_->AddSemanticsNodeUpdate({
{0, node0},
});
Expand All @@ -294,4 +306,62 @@ TEST_F(AccessibilityBridgeTest, BatchesLargeMessages) {
EXPECT_FALSE(semantics_manager_.DeleteOverflowed());
EXPECT_FALSE(semantics_manager_.UpdateOverflowed());
}

TEST_F(AccessibilityBridgeTest, HitTest) {
flutter::SemanticsNode node0;
node0.id = 0;
node0.rect.setLTRB(0, 0, 100, 100);

flutter::SemanticsNode node1;
node1.id = 1;
node1.rect.setLTRB(10, 10, 20, 20);

flutter::SemanticsNode node2;
node2.id = 2;
node2.rect.setLTRB(25, 10, 45, 20);

flutter::SemanticsNode node3;
node3.id = 3;
node3.rect.setLTRB(10, 25, 20, 45);

flutter::SemanticsNode node4;
node4.id = 4;
node4.rect.setLTRB(10, 10, 20, 20);
node4.transform.setTranslate(20, 20, 0);

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},
});
RunLoopUntilIdle();

uint32_t hit_node_id;
auto callback = [&hit_node_id](fuchsia::accessibility::semantics::Hit hit) {
EXPECT_TRUE(hit.has_node_id());
hit_node_id = hit.node_id();
};

// Nodes are:
// ----------
// | 1 2 |
// | 3 4 |
// ----------

accessibility_bridge_->HitTest({1, 1}, callback);
EXPECT_EQ(hit_node_id, 0u);
accessibility_bridge_->HitTest({15, 15}, callback);
EXPECT_EQ(hit_node_id, 1u);
accessibility_bridge_->HitTest({30, 15}, callback);
EXPECT_EQ(hit_node_id, 2u);
accessibility_bridge_->HitTest({15, 30}, callback);
EXPECT_EQ(hit_node_id, 3u);
accessibility_bridge_->HitTest({30, 30}, callback);
EXPECT_EQ(hit_node_id, 4u);
}
} // namespace flutter_runner_test