From f437aaf359e3d7a9f46778aadaa9e8e90ef252b1 Mon Sep 17 00:00:00 2001
From: Chris Waldon <christopher.waldon.dev@gmail.com>
Date: Wed, 23 Aug 2023 14:21:54 -0400
Subject: [PATCH] io/router: fix semantic area traversal

This commit updates the logic behind SemanticAt to use the same hit area
traversal as normal event routing, which should result in more accurate
results for screen readers trying to resolve widgets that might be partially
obscured by non-semantic content.

While here, I realized that the iteration of hit areas needed to stop at
the first matching semantic area, and I added that capability and updated
the ActionAt logic to leverage it as well.

Signed-off-by: Chris Waldon <christopher.waldon.dev@gmail.com>
---
 io/router/pointer.go      | 34 +++++++++++++++++++---------------
 io/router/pointer_test.go | 13 +++++++++++++
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/io/router/pointer.go b/io/router/pointer.go
index 1e92483a..dd3269b0 100644
--- a/io/router/pointer.go
+++ b/io/router/pointer.go
@@ -433,38 +433,39 @@ func (q *pointerQueue) semanticIDFor(content semanticContent) SemanticID {
 }
 
 func (q *pointerQueue) ActionAt(pos f32.Point) (action system.Action, hasAction bool) {
-	q.hitTest(pos, func(n *hitNode) {
+	q.hitTest(pos, func(n *hitNode) bool {
 		area := q.areas[n.area]
 		if area.action != 0 {
 			action = area.action
 			hasAction = true
+			return false
 		}
+		return true
 	})
 	return action, hasAction
 }
 
-func (q *pointerQueue) SemanticAt(pos f32.Point) (SemanticID, bool) {
+func (q *pointerQueue) SemanticAt(pos f32.Point) (semID SemanticID, hasSemID bool) {
 	q.assignSemIDs()
-	for i := len(q.hitTree) - 1; i >= 0; i-- {
-		n := &q.hitTree[i]
-		hit, _ := q.hit(n.area, pos)
-		if !hit {
-			continue
-		}
+	q.hitTest(pos, func(n *hitNode) bool {
 		area := q.areas[n.area]
 		if area.semantic.id != 0 {
-			return area.semantic.id, true
+			semID = area.semantic.id
+			hasSemID = true
+			return false
 		}
-	}
-	return 0, false
+		return true
+	})
+	return semID, hasSemID
 }
 
 // hitTest searches the hit tree for nodes matching pos. Any node matching pos will
 // have the onNode func invoked on it to allow the caller to extract whatever information
-// is necessary for further processing. Providing this algorithm in this generic way
+// is necessary for further processing. onNode may return false to terminate the walk of
+// the hit tree, or true to continue. Providing this algorithm in this generic way
 // allows normal event routing and system action event routing to share the same traversal
 // logic even though they are interested in different aspects of hit nodes.
-func (q *pointerQueue) hitTest(pos f32.Point, onNode func(*hitNode)) pointer.Cursor {
+func (q *pointerQueue) hitTest(pos f32.Point, onNode func(*hitNode) bool) pointer.Cursor {
 	// Track whether we're passing through hits.
 	pass := true
 	idx := len(q.hitTree) - 1
@@ -485,19 +486,22 @@ func (q *pointerQueue) hitTest(pos f32.Point, onNode func(*hitNode)) pointer.Cur
 		} else {
 			idx = n.next
 		}
-		onNode(n)
+		if !onNode(n) {
+			break
+		}
 	}
 	return cursor
 }
 
 func (q *pointerQueue) opHit(pos f32.Point) ([]event.Tag, pointer.Cursor) {
 	hits := q.scratch[:0]
-	cursor := q.hitTest(pos, func(n *hitNode) {
+	cursor := q.hitTest(pos, func(n *hitNode) bool {
 		if n.tag != nil {
 			if _, exists := q.handlers[n.tag]; exists {
 				hits = addHandler(hits, n.tag)
 			}
 		}
+		return true
 	})
 	q.scratch = hits[:0]
 	return hits, cursor
diff --git a/io/router/pointer_test.go b/io/router/pointer_test.go
index 46516c58..a8b0476c 100644
--- a/io/router/pointer_test.go
+++ b/io/router/pointer_test.go
@@ -244,6 +244,19 @@ func TestPointerSystemAction(t *testing.T) {
 		r.Frame(&ops)
 		assertActionAt(t, r, f32.Pt(50, 50), system.ActionMove)
 	})
+	t.Run("uses topmost action op", func(t *testing.T) {
+		var ops op.Ops
+		r1 := clip.Rect(image.Rect(0, 0, 100, 100)).Push(&ops)
+		system.ActionInputOp(system.ActionMove).Add(&ops)
+		r2 := clip.Rect(image.Rect(0, 0, 100, 100)).Push(&ops)
+		system.ActionInputOp(system.ActionClose).Add(&ops)
+		r2.Pop()
+		r1.Pop()
+
+		var r Router
+		r.Frame(&ops)
+		assertActionAt(t, r, f32.Pt(50, 50), system.ActionClose)
+	})
 }
 
 func TestPointerPriority(t *testing.T) {