From 31918b8636d7e9bda3cdb5c4db617a39eae21a4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Tue, 13 Oct 2026 02:31:07 +0200 Subject: [PATCH] UI: Arrow key focus moves by beam and edge distance Replaces the center-to-center scoring, which skipped a wide view between narrow ones, since its center is far away even when its edge is right there. Now it works like Android's FocusFinder: only views in the direction of the move are candidates, ones in the origin's beam beat ones off to the side (going up or down, unless those are in a nearer row), and otherwise the nearest wins, with distance along the move weighted 13 times distance across it. Unlike Android, the distance across is to the candidate's nearest edge rather than its center. First/last and page moves keep their scoring. Co-Authored-By: Claude Opus 5.5 --- Common/UI/ViewGroup.cpp | 234 ++++++++++++++++++++++++---------------- Common/UI/ViewGroup.h | 5 + unittest/UnitTest.cpp | 89 +++++++++++++++ 3 files changed, 235 insertions(+), 93 deletions(-) diff --git a/Common/UI/ViewGroup.cpp b/Common/UI/ViewGroup.cpp index c3b4edde97..6835ece25d 100644 --- a/Common/UI/ViewGroup.cpp +++ b/Common/UI/ViewGroup.cpp @@ -284,43 +284,129 @@ bool ViewGroup::SubviewFocused(View *queryView) { return false; } -// Returns the percentage the smaller one overlaps the bigger one. -static float HorizontalOverlap(const Bounds &a, const Bounds &b) { - if (a.x2() < b.x || b.x2() < a.x) - return 0.0f; - // okay they do overlap. Let's clip. - float maxMin = std::max(a.x, b.x); - float minMax = std::min(a.x2(), b.x2()); - float minW = std::min(a.w, b.w); - float overlap = minMax - maxMin; - if (overlap < 0.0f || minW <= 0.0f) - return 0.0f; - else - return std::min(1.0f, overlap / minW); +// Directional focus moves, the way Android's FocusFinder (and the W3C spatial navigation draft) +// do them. Only views that lie in the direction of the move are candidates. Of those, one that the +// origin's "beam" (its extent across the direction of the move) hits beats one off to the side, +// and otherwise the nearest wins, with distance along the move counting far more than distance +// across it. Measuring to edges rather than centers is what keeps a wide view sitting between +// two narrow ones from being skipped. + +static bool IsInDirection(const Bounds &origin, const Bounds &dest, FocusMove direction) { + switch (direction) { + case FocusMove::LEFT: + return (origin.x2() > dest.x2() || origin.x >= dest.x2()) && origin.x > dest.x; + case FocusMove::RIGHT: + return (origin.x < dest.x || origin.x2() <= dest.x) && origin.x2() < dest.x2(); + case FocusMove::UP: + return (origin.y2() > dest.y2() || origin.y >= dest.y2()) && origin.y > dest.y; + case FocusMove::DOWN: + return (origin.y < dest.y || origin.y2() <= dest.y) && origin.y2() < dest.y2(); + default: + return false; + } } -// Returns the percentage the smaller one overlaps the bigger one. -static float VerticalOverlap(const Bounds &a, const Bounds &b) { - if (a.y2() < b.y || b.y2() < a.y) - return 0.0f; - // okay they do overlap. Let's clip. - float maxMin = std::max(a.y, b.y); - float minMax = std::min(a.y2(), b.y2()); - float minH = std::min(a.h, b.h); - float overlap = minMax - maxMin; - if (overlap < 0.0f || minH <= 0.0f) - return 0.0f; - else - return std::min(1.0f, overlap / minH); +// Whether dest is past the origin's leading edge entirely. +static bool IsEntirelyInDirection(const Bounds &origin, const Bounds &dest, FocusMove direction) { + switch (direction) { + case FocusMove::LEFT: return origin.x >= dest.x2(); + case FocusMove::RIGHT: return origin.x2() <= dest.x; + case FocusMove::UP: return origin.y >= dest.y2(); + case FocusMove::DOWN: return origin.y2() <= dest.y; + default: return false; + } +} + +static bool InBeam(const Bounds &origin, const Bounds &dest, FocusMove direction) { + if (direction == FocusMove::LEFT || direction == FocusMove::RIGHT) { + return dest.y2() > origin.y && dest.y < origin.y2(); + } else { + return dest.x2() > origin.x && dest.x < origin.x2(); + } +} + +// From the origin's leading edge to the near edge of dest. +static float MajorAxisDistance(const Bounds &origin, const Bounds &dest, FocusMove direction) { + switch (direction) { + case FocusMove::LEFT: return std::max(0.0f, origin.x - dest.x2()); + case FocusMove::RIGHT: return std::max(0.0f, dest.x - origin.x2()); + case FocusMove::UP: return std::max(0.0f, origin.y - dest.y2()); + case FocusMove::DOWN: return std::max(0.0f, dest.y - origin.y2()); + default: return 0.0f; + } +} + +// From the origin's leading edge to the far edge of dest. +static float MajorAxisDistanceToFarEdge(const Bounds &origin, const Bounds &dest, FocusMove direction) { + switch (direction) { + case FocusMove::LEFT: return std::max(1.0f, origin.x - dest.x); + case FocusMove::RIGHT: return std::max(1.0f, dest.x2() - origin.x2()); + case FocusMove::UP: return std::max(1.0f, origin.y - dest.y); + case FocusMove::DOWN: return std::max(1.0f, dest.y2() - origin.y2()); + default: return 1.0f; + } +} + +// Across the move, from the origin's focus position (normally its center) to the nearest part of +// dest, so zero if dest is straight ahead. Android measures to dest's center instead, which lets a +// narrow view further on beat a wide one that's right there. +static float MinorAxisDistance(const Point2D &originPos, const Bounds &dest, FocusMove direction) { + if (direction == FocusMove::LEFT || direction == FocusMove::RIGHT) { + return std::max(0.0f, std::max(dest.y - originPos.y, originPos.y - dest.y2())); + } else { + return std::max(0.0f, std::max(dest.x - originPos.x, originPos.x - dest.x2())); + } +} + +// Whether a, which is in the beam, beats b, which isn't. +static bool BeamBeats(const Bounds &origin, const Bounds &a, const Bounds &b, FocusMove direction) { + const bool aInBeam = InBeam(origin, a, direction); + const bool bInBeam = InBeam(origin, b, direction); + if (bInBeam || !aInBeam) { + return false; + } + // b overlaps the origin along the move, so it's only partly in that direction. + if (!IsEntirelyInDirection(origin, b, direction)) { + return true; + } + // Sideways moves always prefer the beam: things in a row line up. + if (direction == FocusMove::LEFT || direction == FocusMove::RIGHT) { + return true; + } + // Up and down, something off to the side in the next row beats something in the beam further on. + return MajorAxisDistance(origin, a, direction) < MajorAxisDistanceToFarEdge(origin, b, direction); +} + +static float WeightedDistance(const Point2D &originPos, const Bounds &origin, const Bounds &dest, FocusMove direction) { + const float major = MajorAxisDistance(origin, dest, direction); + const float minor = MinorAxisDistance(originPos, dest, direction); + // Android's weighting, which makes being straight ahead count for a lot. + return 13.0f * major * major + minor * minor; +} + +bool IsBetterFocusCandidate(const Bounds &origin, const Point2D &originPos, const Bounds &candidate, const Bounds *best, FocusMove direction) { + if (!IsInDirection(origin, candidate, direction)) { + return false; + } + if (!best) { + return true; + } + if (BeamBeats(origin, candidate, *best, direction)) { + return true; + } + if (BeamBeats(origin, *best, candidate, direction)) { + return false; + } + return WeightedDistance(originPos, origin, candidate, direction) < WeightedDistance(originPos, origin, *best, direction); +} + +static bool CanTakeFocus(const View *view) { + return view->CanBeFocused() && view->IsEnabled() && view->GetVisibility() == V_VISIBLE; } float GetTargetScore(const Point2D &originPos, int originIndex, const View *origin, const View *destination, FocusMove direction) { // Skip labels and things like that. - if (!destination->CanBeFocused()) - return 0.0f; - if (destination->IsEnabled() == false) - return 0.0f; - if (destination->GetVisibility() != V_VISIBLE) + if (!CanTakeFocus(destination)) return 0.0f; Point2D destPos = destination->GetFocusPosition(Opposite(direction)); @@ -329,52 +415,10 @@ float GetTargetScore(const Point2D &originPos, int originIndex, const View *orig float dy = destPos.y - originPos.y; const float distance = std::max(sqrtf(dx*dx + dy*dy), 0.001f); - float overlap = 0.0f; const float dirX = dx / distance; const float dirY = dy / distance; - bool wrongDirection = false; - bool vertical = false; - const float horizOverlap = HorizontalOverlap(origin->GetBounds(), destination->GetBounds()); - const float vertOverlap = VerticalOverlap(origin->GetBounds(), destination->GetBounds()); - if (horizOverlap == 1.0f && vertOverlap == 1.0f) { - if (direction != FocusMove::PREV_PAGE && direction != FocusMove::NEXT_PAGE) { - // INFO_LOG(Log::UI, "Contain overlap: %s, %s", origin->Tag().c_str(), destination->Tag().c_str()); - return 0.0f; - } - } - float originSize = 0.0f; switch (direction) { - case FocusMove::LEFT: - overlap = vertOverlap; - originSize = origin->GetBounds().w; - if (dirX > 0.0f) { - wrongDirection = true; - } - break; - case FocusMove::UP: - overlap = horizOverlap; - originSize = origin->GetBounds().h; - if (dirY > 0.0f) { - wrongDirection = true; - } - vertical = true; - break; - case FocusMove::RIGHT: - overlap = vertOverlap; - originSize = origin->GetBounds().w; - if (dirX < 0.0f) { - wrongDirection = true; - } - break; - case FocusMove::DOWN: - overlap = horizOverlap; - originSize = origin->GetBounds().h; - if (dirY < 0.0f) { - wrongDirection = true; - } - vertical = true; - break; case FocusMove::FIRST: if (originIndex == -1) return 0.0f; @@ -391,23 +435,11 @@ float GetTargetScore(const Point2D &originPos, int originIndex, const View *orig return distance; case FocusMove::PREV_PAGE: case FocusMove::NEXT_PAGE: - // Not always, but let's go with the bonus on height. - vertical = true; - break; - case FocusMove::PREV: - case FocusMove::NEXT: + return 10.0f / std::max(1.0f, distance); + default: + // The arrow directions go through IsBetterFocusCandidate. ERROR_LOG(Log::UI, "Invalid focus direction"); - break; - } - - // At large distances, ignore overlap. - if (distance > 2.0 * originSize) - overlap = 0.0f; - - if (wrongDirection) { return 0.0f; - } else { - return 10.0f / std::max(1.0f, distance) + overlap * 2.0; } } @@ -439,6 +471,27 @@ NeighborResult ViewGroup::FindNeighbor(View *view, FocusMove direction, Neighbor case FocusMove::LEFT: case FocusMove::RIGHT: case FocusMove::DOWN: + { + // Candidates are compared with the best so far rather than scored, so score stays unused. + const Bounds &origin = view->GetBounds(); + const Point2D originPos = view->GetFocusPosition(direction); + for (View *candidate : views_) { + if (candidate == view || !CanTakeFocus(candidate)) + continue; + const Bounds *best = result.view ? &result.view->GetBounds() : nullptr; + if (IsBetterFocusCandidate(origin, originPos, candidate->GetBounds(), best, direction)) { + result.view = candidate; + } + } + + // Then see if any of the children contain any better candidates. + for (View *v : views_) { + if (v->IsViewGroup()) { + result = static_cast(v)->FindNeighbor(view, direction, result); + } + } + return result; + } case FocusMove::FIRST: case FocusMove::LAST: { @@ -462,11 +515,6 @@ NeighborResult ViewGroup::FindNeighbor(View *view, FocusMove direction, Neighbor result = vg->FindNeighbor(view, direction, result); } } - - // Boost neighbors with the same parent - if (num != -1) { - //result.score += 100.0f; - } return result; } case FocusMove::PREV_PAGE: diff --git a/Common/UI/ViewGroup.h b/Common/UI/ViewGroup.h index 8e8534ae2f..7ed8258e8f 100644 --- a/Common/UI/ViewGroup.h +++ b/Common/UI/ViewGroup.h @@ -16,6 +16,11 @@ namespace UI { class AnchorTranslateTween; class ScrollView; +// For the arrow directions: whether candidate is a better place to move the focus to from origin +// than best (null if there's nothing yet). originPos is where the move starts across the +// direction of the move, normally origin's center. +bool IsBetterFocusCandidate(const Bounds &origin, const Point2D &originPos, const Bounds &candidate, const Bounds *best, FocusMove direction); + struct NeighborResult { NeighborResult() : view(nullptr), score(0) {} NeighborResult(View *v, float s) : view(v), score(s) {} diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index fd25357f62..e2fece6a00 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -3253,6 +3253,94 @@ bool TestUITabOrder() { return true; } +// Arrow key focus moves go by geometry, so these place views by hand rather than running layout. +bool TestUIFocusNavigation() { + using namespace UI; + + auto add = [](LinearLayout *parent, float x, float y, float w, float h) { + Choice *choice = parent->Add(new Choice("")); + choice->SetBounds(Bounds(x, y, w, h)); + return choice; + }; + auto move = [](ViewGroup *root, View *from, FocusMove dir) { + return root->FindNeighbor(from, dir, NeighborResult()).view; + }; + + { + // A wide button between rows of narrow ones. Its center is far from the narrow ones at the + // ends, but its edge is right there, so it mustn't be skipped. + LinearLayout root(ORIENT_VERTICAL); + Choice *topLeft = add(&root, 0, 0, 100, 50); + Choice *topRight = add(&root, 500, 0, 100, 50); + Choice *wide = add(&root, 0, 60, 600, 50); + Choice *bottomLeft = add(&root, 0, 120, 100, 50); + Choice *bottomRight = add(&root, 500, 120, 100, 50); + EXPECT_TRUE(move(&root, topRight, FocusMove::DOWN) == wide); + EXPECT_TRUE(move(&root, topLeft, FocusMove::DOWN) == wide); + EXPECT_TRUE(move(&root, bottomRight, FocusMove::UP) == wide); + EXPECT_TRUE(move(&root, bottomLeft, FocusMove::UP) == wide); + // From the wide one, the one nearest its center across the move. + EXPECT_TRUE(move(&root, bottomLeft, FocusMove::RIGHT) == bottomRight); + EXPECT_TRUE(move(&root, wide, FocusMove::LEFT) == nullptr); + } + + { + // A row: left and right go to the neighbors, and nothing is up or down. + LinearLayout root(ORIENT_HORIZONTAL); + Choice *a = add(&root, 0, 0, 100, 50); + Choice *b = add(&root, 110, 0, 100, 50); + Choice *c = add(&root, 220, 0, 100, 50); + EXPECT_TRUE(move(&root, b, FocusMove::RIGHT) == c); + EXPECT_TRUE(move(&root, b, FocusMove::LEFT) == a); + EXPECT_TRUE(move(&root, a, FocusMove::LEFT) == nullptr); + EXPECT_TRUE(move(&root, b, FocusMove::UP) == nullptr); + EXPECT_TRUE(move(&root, b, FocusMove::DOWN) == nullptr); + } + + { + // Going down, the next row wins even off to the side, over something in line much further on. + LinearLayout root(ORIENT_VERTICAL); + Choice *origin = add(&root, 0, 0, 100, 50); + Choice *nextRowAside = add(&root, 150, 60, 100, 50); + add(&root, 0, 500, 100, 50); + EXPECT_TRUE(move(&root, origin, FocusMove::DOWN) == nextRowAside); + } + + { + // Sideways, things in line win however far away they are. + LinearLayout root(ORIENT_HORIZONTAL); + Choice *origin = add(&root, 0, 0, 50, 100); + add(&root, 60, 150, 50, 100); + Choice *inLine = add(&root, 500, 0, 50, 100); + EXPECT_TRUE(move(&root, origin, FocusMove::RIGHT) == inLine); + } + + { + // Rows shifted against each other: down goes to the one below the origin's center. + LinearLayout root(ORIENT_VERTICAL); + Choice *origin = add(&root, 100, 0, 100, 50); + add(&root, 20, 60, 100, 50); + Choice *underCenter = add(&root, 130, 60, 100, 50); + EXPECT_TRUE(move(&root, origin, FocusMove::DOWN) == underCenter); + } + + { + // Things that can't take focus are passed over, also inside nested groups. + LinearLayout root(ORIENT_VERTICAL); + Choice *origin = add(&root, 0, 0, 100, 50); + Choice *disabled = add(&root, 0, 60, 100, 50); + disabled->SetEnabled(false); + LinearLayout *inner = root.Add(new LinearLayout(ORIENT_VERTICAL)); + Choice *hidden = add(inner, 0, 120, 100, 50); + hidden->SetVisibility(V_INVISIBLE); + Choice *last = add(inner, 0, 180, 100, 50); + EXPECT_TRUE(move(&root, origin, FocusMove::DOWN) == last); + EXPECT_TRUE(move(&root, last, FocusMove::UP) == origin); + } + + return true; +} + bool TestTextureReplacer(); static bool TestQueueBuf() { @@ -3395,6 +3483,7 @@ TestItem availableTests[] = { TEST_ITEM(Demangle), TEST_ITEM(TextureReplacer), TEST_ITEM(UITabOrder), + TEST_ITEM(UIFocusNavigation), TEST_ITEM(FatShortNames), TEST_ITEM(GEDumpGameID), TEST_ITEM(QueueBuf),