Skip to content

fix(commands): orientation-aware coords for both rotated devices and letterboxed landscape-only apps (#5) - #44

Merged
cameroncooke merged 5 commits into
cameroncooke:mainfrom
Nitewriter:fix/issue-5-orientation-aware-tap-coords
May 10, 2026
Merged

cameroncooke merged 5 commits into
cameroncooke:mainfrom
Nitewriter:fix/issue-5-orientation-aware-tap-coords

Conversation

@Nitewriter

@Nitewriter Nitewriter commented Apr 26, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #5.

Root cause

FBSimulatorHIDEvent operates in physical portrait coordinate space. The accessibility tree reports element positions in the logical UI orientation the app presents. This PR handles both landscape cases:

  1. Rotated hardware — device is rotated to landscape. Rotation math remaps logical → physical.
  2. Letterboxed landscape-only app — hardware is portrait, but UISupportedInterfaceOrientations declares only landscape orientations. iOS scales + centers the landscape UI inside the portrait viewport. Scale + offset math applies.

Both cases produce an identical landscape-shaped AX application frame, so the AX tree alone cannot distinguish them.

Detection

On a landscape AX frame, AXe probes the screenshot pixel dimensions via xcrun simctl io <udid> screenshot - (reads only the first 24 bytes — the PNG IHDR header — no disk I/O). If the screenshot is taller than wide while the AX frame is wider than tall → letterbox case. If both are landscape-shaped → rotated device case.

What changed

Sources/AXe/Utilities/OrientationAwareCoordinates.swift

  • CoordinateMapping enum — three cases: .passthrough (portrait), .rotation(SimulatorOrientation, portraitSize:) (rotated hardware), .letterbox(scale:offsetX:offsetY:) (portrait hardware, landscape-only app).
  • detectMapping(for:orientationOverride:logger:) — unified detection entry point. Fetches AX frame; if landscape-shaped, probes screenshot dimensions to select rotation vs letterbox path.
  • screenshotPixelDimensions(for:logger:) — reads PNG IHDR from simctl stdout (first 24 bytes). Fast and side-effect-free.
  • letterboxToPhysical(point:scale:offsetX:offsetY:) — pure scale + offset math.
  • translate and translateBatch dispatch through detectMapping; public API is unchanged.
  • detectOrientation preserved for callers that need it directly, with a doc note pointing to detectMapping.

Letterbox math

Given logical size from AX frame and physical portrait viewport dimensions:

scale   = min(physW / logW, physH / logH)
offsetX = (physW - logW * scale) / 2
offsetY = (physH - logH * scale) / 2

physical.x = offsetX + logical.x * scale
physical.y = offsetY + logical.y * scale

Rotation math

Orientation Physical x Physical y
portrait lx ly
portraitUpsideDown pw - lx ph - ly
landscape (home right) ly ph - lx
landscapeFlipped (home left) pw - ly lx

Where pw = portrait short side, ph = portrait long side.

Empirical evidence — letterbox case

Setup: JAMDOG iPad Pro 13" M5 Simulator (iOS 26.4.1), BeatSmart app with INFOPLIST_KEY_UISupportedInterfaceOrientations_iPad = "UIInterfaceOrientationLandscapeLeft UIInterfaceOrientationLandscapeRight", device never rotated via Hardware menu.

  • AXe describe-ui reports AXApplication frame: 1376×1032 (logical landscape).
  • axe screenshot saves a 2064×2752 PNG (portrait pixel buffer) → screenshot is portrait-shaped, AX frame is landscape-shaped → letterbox case detected.
  • Parameters: scale = min(1032/1376, 1376/1032) = 0.75, offsetX = 0, offsetY = 301.
  • Logical (86, 269) → physical (64.5, 502.75) → confirmed hit: tap switched Lobby from 4-player to 8-player fixture.

Note on --landscape-flipped

The flag is preserved for the rotated device case. It has no effect when the letterbox case is detected (both landscape orientations produce the same scale+offset math; there is no rotation ambiguity). Callers do not need to know which case applies.

Tests

Tests/OrientationAwareCoordinatesTests.swift — 34 unit tests total (22 existing + 12 new):

New: LetterboxCoordinatesTests

  • Scale derivation: verifies 0.75 for iPad Pro 13" M5
  • Offset derivation: verifies offsetX=0, offsetY=301
  • Empirical regression: logical (86, 269) → physical (64.5, 502.75)
  • Parameterized corner mapping (4 cases)
  • Bounds check (6 sample points including the empirical point)
  • Determinism
  • Detection signal: portrait screenshot + landscape AX frame → letterbox trigger
  • Detection signal: landscape screenshot + landscape AX frame → rotation trigger

All 121 tests pass (swift test). E2E suite skipped (requires simulator + AXE_E2E=1).

Credit

Thanks to @paul-foreflight for the Python reference implementation in the issue comments, which confirmed the original rotation math and indirectly highlighted the letterbox case as a separate gap.

Nitewriter and others added 2 commits April 26, 2026 14:23
…orientations (cameroncooke#5)

FBSimulatorHIDEvent operates in physical portrait space but the accessibility
tree reports element positions in the logical UI orientation. On a rotated
simulator, tap/touch/swipe coordinates were dispatched without correction,
causing events to land in the wrong location.

Changes:
- Add OrientationAwareCoordinates.swift with rotation math for all four
  orientations (portrait, portraitUpsideDown, landscape, landscapeFlipped)
- Detect orientation from AX application frame (width > height = landscape)
- Apply translate() in Tap, Touch, and Swipe before HID dispatch
- Use translateBatch() in Swipe to translate start/end in a single AX fetch
- Add --landscape-flipped flag to tap/touch/swipe for the CCW landscape
  variant, which cannot be distinguished from landscape via the AX tree alone
- Add OrientationAwareCoordinatesTests.swift: 22 unit tests covering all four
  orientations, parameterized corner mapping, bounds checking, and the
  reference point from paul-foreflight's Python workaround (logical 1174,428
  → physical 428,202 for iPad Pro 13")

Math verified against paul-foreflight's Python workaround in issue comments.
Thanks to @paul-foreflight for the reference implementation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… case)

Two landscape cases produce an identical AX application frame (width > height):
  (a) Hardware rotated to landscape — rotation math (existing).
  (b) Hardware portrait, app declares landscape-only orientations — iOS
      scales + letterboxes the content. Scale+offset math (new).

Detection: take a quick screenshot via `xcrun simctl io <udid> screenshot -`
and read the PNG IHDR dimensions (first 24 bytes, no disk I/O). If the
screenshot is portrait-shaped (taller than wide) while the AX frame is
landscape-shaped → letterbox case. Otherwise → rotated device case.

New additions to OrientationAwareCoordinates.swift:
  - `CoordinateMapping` enum: `.passthrough`, `.rotation(...)`, `.letterbox(...)`
  - `detectMapping(for:orientationOverride:logger:)` — unified detection entry
    point that probes both AX frame and screenshot dimensions.
  - `screenshotPixelDimensions(for:logger:)` — reads PNG IHDR via simctl stdout;
    only runs when the AX frame is already landscape-shaped.
  - `letterboxToPhysical(point:scale:offsetX:offsetY:)` — pure scale+offset math.
  - `translate` and `translateBatch` now dispatch through `detectMapping`; public
    API unchanged.

Letterbox math (iPad Pro 13" M5 example):
  logical (AX frame): 1376×1032 pts
  physical (portrait): 1032×1376 pts
  scale = min(1032/1376, 1376/1032) = 0.75
  offsetX = (1032 - 1376*0.75) / 2 = 0
  offsetY = (1376 - 1032*0.75) / 2 = 301
  → logical (86, 269) → physical (64.5, 502.75)

Empirically verified on JAMDOG iPad Pro 13" M5 / iOS 26.4.1 with BeatSmart
(landscape-only, device never rotated). Tap at computed physical coords switched
Lobby from 4-player to 8-player fixture.

`--landscape-flipped` flag is preserved for the rotated case. It has no effect
when the letterbox case is detected (no rotation ambiguity in that path).

New tests in LetterboxCoordinatesTests (13 tests):
  - Scale/offset parameter derivation for iPad Pro 13" M5
  - Empirical regression: logical (86, 269) → physical (64.5, 502.75)
  - Parameterized corner mapping (4 cases)
  - Bounds check across sample inputs
  - Determinism
  - Detection signal assertions (both letterbox and rotation triggers)

All 121 tests pass (`swift test`).
@Nitewriter Nitewriter changed the title fix(commands): translate logical coords to physical for non-portrait orientations (#5) fix(commands): orientation-aware coords for both rotated devices and letterboxed landscape-only apps (#5) Apr 26, 2026
@Nitewriter
Nitewriter marked this pull request as ready for review April 26, 2026 23:43
@coderabbitai

coderabbitai Bot commented Apr 26, 2026

Copy link
Copy Markdown

Walkthrough

The pull request adds coordinate translation functionality for iOS simulator interactions across different orientations. A new OrientationAwareCoordinates utility module detects the simulator's current orientation and maps logical accessibility-tree coordinates to physical device coordinates. The Tap, Touch, and Swipe commands are updated with a --landscape-flipped CLI flag to optionally override orientation detection. Comprehensive tests validate the coordinate mapping logic for various rotation and letterboxing scenarios. The changelog documents the fix for orientation-aware coordinate handling.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 58.54% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and specifically summarises the main change: fixing orientation-aware coordinates for both rotated devices and letterboxed landscape-only apps, directly addressing the core objective.
Description check ✅ Passed The PR description comprehensively documents the root cause, detection mechanism, implementation details, coordinate mapping formulas, empirical evidence, and test coverage, all directly related to the changeset.
Linked Issues check ✅ Passed The PR fully addresses issue #5 by implementing coordinate translation for both rotated devices (with rotation math applied via OrientationAwareCoordinates) and letterboxed landscape-only apps (with scale+offset detection), ensuring taps land correctly in all orientations.
Out of Scope Changes check ✅ Passed All changes are directly scoped to coordinate mapping logic: new OrientationAwareCoordinates utility, --landscape-flipped CLI flag additions to tap/touch/swipe commands, corresponding tests, and CHANGELOG updates. No unrelated modifications present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Sources/AXe/Commands/Tap.swift (1)

166-166: ⚠️ Potential issue | 🟡 Minor

Success message reports physical coordinates after translation — likely confusing.

Previously this message echoed what the user typed (or the resolved element centre). After this change it prints the translated resolvedPoint, which in landscape/letterbox cases will not match the user-supplied -x/-y (e.g. axe tap -x 1174 -y 428 on rotated iPad will print ✓ Tap at (428.0, 202.0) completed successfully). This contradicts logger.info().log("Tapping at \(resolvedDescription)") on Line 138 which still uses the logical point, and is inconsistent with Touch/Swipe which keep user-facing output in logical space.

💡 Proposed fix
-        print("✓ Tap at (\(resolvedPoint.x), \(resolvedPoint.y)) completed successfully")
+        print("✓ Tap at \(resolvedDescription) completed successfully")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/AXe/Commands/Tap.swift` at line 166, The success message prints the
translated physical coordinates (resolvedPoint) which can differ from the
user-supplied/logical point; update the success print in Tap (replace the print
using resolvedPoint) to report the logical/user-facing coordinates used earlier
(use resolvedDescription or the original logical point) so it matches
logger.info().log("Tapping at \(resolvedDescription)") and is consistent with
Touch/Swipe output.
🧹 Nitpick comments (7)
Sources/AXe/Utilities/OrientationAwareCoordinates.swift (4)

266-319: Consider a typed Point (or CGPoint) instead of unlabelled tuples throughout the API.

The public API (translate, translateBatch, translateToPhysical, letterboxToPhysical) all traffic in (x: Double, y: Double) tuples. While Swift permits this, it scales poorly: tuple types can't be extended, can't conform to protocols (e.g. Equatable for tests, CustomStringConvertible for logging), and force every call site to re-spell (x:, y:). CoreGraphics.CGPoint is already imported transitively and is the idiomatic choice for 2D points on Apple platforms; alternatively a small struct LogicalPoint / struct PhysicalPoint would also encode the intent in the type system and prevent passing a logical point into an HID API by accident.

Not blocking — purely a maintainability nudge for a future revision.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/AXe/Utilities/OrientationAwareCoordinates.swift` around lines 266 -
319, Change the point representation from unlabeled (x: Double, y: Double)
tuples to a concrete point type across the public API: update function
signatures and bodies for translate, translateBatch, translateToPhysical, and
letterboxToPhysical to accept and return CoreGraphics.CGPoint (or define small
LogicalPoint/PhysicalPoint structs if you need stronger typing), update all
internal uses to .x/.y (or struct properties), and adjust callers/tests
accordingly; if you choose custom structs, make them conform to
Equatable/CustomStringConvertible to preserve testability and logging.

165-171: Avoid the force-unwrap by shadowing the optional.

After the screenshotDims == nil check, the else branch force-unwraps screenshotDims! twice. Re-binding the optional makes it harder for a future refactor to introduce a crash and reads more idiomatically.

♻️ Proposed fix
-        // Screenshot is landscape-shaped (or unavailable) — hardware is rotated.
-        if screenshotDims == nil {
-            logger.info().log("Screenshot probe unavailable; assuming rotated hardware")
-        } else {
-            logger.info().log(
-                "Screenshot \(screenshotDims!.width)×\(screenshotDims!.height)px is landscape-shaped; using rotation mapping"
-            )
-        }
+        // Screenshot is landscape-shaped (or unavailable) — hardware is rotated.
+        if let dims = screenshotDims {
+            logger.info().log(
+                "Screenshot \(dims.width)×\(dims.height)px is landscape-shaped; using rotation mapping"
+            )
+        } else {
+            logger.info().log("Screenshot probe unavailable; assuming rotated hardware")
+        }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/AXe/Utilities/OrientationAwareCoordinates.swift` around lines 165 -
171, The else branch force-unwraps screenshotDims (used twice) which can be
avoided by shadowing the optional; replace the current nil check and else with
an if let binding (e.g., if let screenshotDims = screenshotDims { ... } else {
... }) or guard let screenshotDims = screenshotDims else {
logger.info().log("Screenshot probe unavailable; assuming rotated hardware");
return } and then use screenshotDims.width/height in the logger.info().log call
(reference: screenshotDims and logger.info().log in
OrientationAwareCoordinates.swift).

222-231: availableData on the main actor blocks the executor; also no waitUntilExit.

Two related concerns:

  1. OrientationAwareCoordinates is @MainActor, so screenshotPixelDimensions runs on the main actor. FileHandle.availableData is documented to block until data arrives or the channel closes; for the duration of the simctl screenshot (typically 100–500 ms) the entire main actor is unavailable. For a CLI process this is functionally tolerable but the @MainActor annotation on a stateless utility offers no benefit — consider removing it (or running this probe in Task.detached).

  2. After reading the IHDR you process.terminate() but never waitUntilExit(), leaving the child as a transient zombie until the parent reaps it. On a long-lived batch invocation the simctl children accumulate.

♻️ Proposed fix
-        process.terminate()
+        process.terminate()
+        process.waitUntilExit()

…and consider dropping @MainActor from OrientationAwareCoordinates (Line 86) since none of the methods touch main-actor-isolated state.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/AXe/Utilities/OrientationAwareCoordinates.swift` around lines 222 -
231, screenshotPixelDimensions is blocking the main actor by calling
FileHandle.availableData and also leaves child processes as zombies because
process.waitUntilExit() is never called; update OrientationAwareCoordinates by
removing the `@MainActor` annotation (or run the probe inside Task.detached) so
the work runs off the main actor, replace the availableData loop in
screenshotPixelDimensions with non-blocking reads (e.g.,
FileHandle.read(upToCount:) in a loop until 24 bytes or EOF) or perform the
blocking reads inside the detached Task, and after terminating the Process call
process.waitUntilExit() (or await process.termination if using async APIs) to
ensure the child is reaped; reference symbols: OrientationAwareCoordinates,
screenshotPixelDimensions, FileHandle.availableData, process.terminate(),
process.waitUntilExit().

442-487: Consider removing detectOrientation or making it private.

The method has no in-tree callers. Its doc comment marks it as "legacy — preserved for callers that need it directly", but the code already migrated Tap/Touch/Swipe to use translate/translateBatch, and a fresh module has no external consumers. Since detectMapping already handles the same AX-frame-parsing logic, keeping this public method extends the API surface unnecessarily.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/AXe/Utilities/OrientationAwareCoordinates.swift` around lines 442 -
487, The detectOrientation(for:orientationOverride:logger:) method is unused
in-tree and duplicates logic in detectMapping; either remove it or reduce API
surface by making it private: locate the static func detectOrientation(...) and
either delete it (ensuring no external callers exist) or change its visibility
to private, while keeping applicationFrame(...) and
AccessibilityFetcher.fetchAccessibilityElements(...) usage and preserving the
documentation if you keep it private; verify Tap/Touch/Swipe were migrated to
translate/translateBatch and run tests/build to ensure no callers remain.
Tests/OrientationAwareCoordinatesTests.swift (2)

450-482: Detection-signal tests don't exercise the production detection path.

letterboxDetectionSignal and rotationDetectionSignal only assert the boolean conditions a reader expects to be true; they don't call into OrientationAwareCoordinates.detectMapping. If the production detection inverts a comparison (e.g. uses >= instead of >, or swaps width/height), these tests will still pass. Consider extracting the decision predicate from detectMapping (or making it injectable) so a real call can be unit-tested with mocked AX frame and screenshot dimensions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Tests/OrientationAwareCoordinatesTests.swift` around lines 450 - 482, Tests
letterboxDetectionSignal and rotationDetectionSignal only check local booleans
and never exercise the production logic; update them to call
OrientationAwareCoordinates.detectMapping (or extract/inject its decision
predicate) with the constructed screenshotWidth/screenshotHeight and
axFrameWidth/axFrameHeight values and assert the returned mapping is the
expected case (letterbox for the first test, rotation for the second). If
detectMapping is not easily callable, refactor OrientationAwareCoordinates to
expose the comparison predicate (e.g., isScreenshotPortrait, isAxFrameLandscape)
or accept an injectable comparator, then use that API from the two tests to
verify the actual detection logic rather than duplicating the boolean
conditions.

14-17: Consider testing the production code directly via @testable import.

Duplicating translateToPhysical, letterboxToPhysical, and the letterbox parameter math in the test target means a bug introduced into production can ship while tests still pass — the tests pin down the expected formulas, not the actual code under review. Swift packages can expose executable-target symbols to tests via @testable import AXe (with internal visibility on the helpers), which would let these tests bind to the real OrientationAwareCoordinates and remove the "must stay in sync" footgun. Worth a follow-up if package layout permits it.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Tests/OrientationAwareCoordinatesTests.swift` around lines 14 - 17, Replace
the duplicated math in the test target with direct tests against the production
implementation by using `@testable` import AXe (so tests can access internal
helpers) and remove the local copies of translateToPhysical, letterboxToPhysical
and the letterbox math; update Tests/OrientationAwareCoordinatesTests.swift to
call OrientationAwareCoordinates.translateToPhysical and
OrientationAwareCoordinates.letterboxToPhysical (or their internal equivalents)
from the AXe module instead of the duplicated functions so the tests exercise
the real production code.
Sources/AXe/Commands/Swipe.swift (1)

132-133: Consider whether delta needs scaling under letterbox/rotation.

delta (default 50pt) is the inter-touch step distance for the swipe. It is supplied in logical (user) units but is forwarded as-is to FBSimulatorHIDEvent.swipe, which interprets it in physical portrait space. Under letterbox (scale ≠ 1.0) this changes the effective step density of the swipe versus what a user would reason about from the on-screen logical distance. Not a correctness bug, but worth confirming the desired semantics — and documenting if delta is intentionally treated as physical.

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Sources/AXe/Utilities/OrientationAwareCoordinates.swift`:
- Around line 109-192: The detectMapping function currently ignores an
orientationOverride in the letterbox branch without informing the user; inside
the letterbox handling block in detectMapping(for:orientationOverride:logger:),
check if orientationOverride is non-nil and, if so, emit a single logger.info()
log that the provided orientation override is being ignored for the letterbox
case (include the override.rawValue for clarity) before returning .letterbox;
this uses the existing symbols detectMapping, orientationOverride, and
.letterbox to locate where to add the log.
- Around line 211-231: The code creates an undrained stderr pipe
(process.standardError = Pipe()) which can block the child when stderr fills;
change the suppression to redirect stderr to /dev/null instead of a Pipe so
there's no read end to fill. Locate the block using the Process instance named
process (and the assignment process.standardError = Pipe()) in
OrientationAwareCoordinates.swift and replace that assignment with a redirection
to the system null device (e.g., use FileHandle.nullDevice or open /dev/null) so
stderr is discarded without creating a blocking pipe.

---

Outside diff comments:
In `@Sources/AXe/Commands/Tap.swift`:
- Line 166: The success message prints the translated physical coordinates
(resolvedPoint) which can differ from the user-supplied/logical point; update
the success print in Tap (replace the print using resolvedPoint) to report the
logical/user-facing coordinates used earlier (use resolvedDescription or the
original logical point) so it matches logger.info().log("Tapping at
\(resolvedDescription)") and is consistent with Touch/Swipe output.

---

Nitpick comments:
In `@Sources/AXe/Utilities/OrientationAwareCoordinates.swift`:
- Around line 266-319: Change the point representation from unlabeled (x:
Double, y: Double) tuples to a concrete point type across the public API: update
function signatures and bodies for translate, translateBatch,
translateToPhysical, and letterboxToPhysical to accept and return
CoreGraphics.CGPoint (or define small LogicalPoint/PhysicalPoint structs if you
need stronger typing), update all internal uses to .x/.y (or struct properties),
and adjust callers/tests accordingly; if you choose custom structs, make them
conform to Equatable/CustomStringConvertible to preserve testability and
logging.
- Around line 165-171: The else branch force-unwraps screenshotDims (used twice)
which can be avoided by shadowing the optional; replace the current nil check
and else with an if let binding (e.g., if let screenshotDims = screenshotDims {
... } else { ... }) or guard let screenshotDims = screenshotDims else {
logger.info().log("Screenshot probe unavailable; assuming rotated hardware");
return } and then use screenshotDims.width/height in the logger.info().log call
(reference: screenshotDims and logger.info().log in
OrientationAwareCoordinates.swift).
- Around line 222-231: screenshotPixelDimensions is blocking the main actor by
calling FileHandle.availableData and also leaves child processes as zombies
because process.waitUntilExit() is never called; update
OrientationAwareCoordinates by removing the `@MainActor` annotation (or run the
probe inside Task.detached) so the work runs off the main actor, replace the
availableData loop in screenshotPixelDimensions with non-blocking reads (e.g.,
FileHandle.read(upToCount:) in a loop until 24 bytes or EOF) or perform the
blocking reads inside the detached Task, and after terminating the Process call
process.waitUntilExit() (or await process.termination if using async APIs) to
ensure the child is reaped; reference symbols: OrientationAwareCoordinates,
screenshotPixelDimensions, FileHandle.availableData, process.terminate(),
process.waitUntilExit().
- Around line 442-487: The detectOrientation(for:orientationOverride:logger:)
method is unused in-tree and duplicates logic in detectMapping; either remove it
or reduce API surface by making it private: locate the static func
detectOrientation(...) and either delete it (ensuring no external callers exist)
or change its visibility to private, while keeping applicationFrame(...) and
AccessibilityFetcher.fetchAccessibilityElements(...) usage and preserving the
documentation if you keep it private; verify Tap/Touch/Swipe were migrated to
translate/translateBatch and run tests/build to ensure no callers remain.

In `@Tests/OrientationAwareCoordinatesTests.swift`:
- Around line 450-482: Tests letterboxDetectionSignal and
rotationDetectionSignal only check local booleans and never exercise the
production logic; update them to call OrientationAwareCoordinates.detectMapping
(or extract/inject its decision predicate) with the constructed
screenshotWidth/screenshotHeight and axFrameWidth/axFrameHeight values and
assert the returned mapping is the expected case (letterbox for the first test,
rotation for the second). If detectMapping is not easily callable, refactor
OrientationAwareCoordinates to expose the comparison predicate (e.g.,
isScreenshotPortrait, isAxFrameLandscape) or accept an injectable comparator,
then use that API from the two tests to verify the actual detection logic rather
than duplicating the boolean conditions.
- Around line 14-17: Replace the duplicated math in the test target with direct
tests against the production implementation by using `@testable` import AXe (so
tests can access internal helpers) and remove the local copies of
translateToPhysical, letterboxToPhysical and the letterbox math; update
Tests/OrientationAwareCoordinatesTests.swift to call
OrientationAwareCoordinates.translateToPhysical and
OrientationAwareCoordinates.letterboxToPhysical (or their internal equivalents)
from the AXe module instead of the duplicated functions so the tests exercise
the real production code.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a46f190f-f98e-4f8b-9723-ab1237d03b8d

📥 Commits

Reviewing files that changed from the base of the PR and between 550b3ab and 06f0d8f.

📒 Files selected for processing (6)
  • CHANGELOG.md
  • Sources/AXe/Commands/Swipe.swift
  • Sources/AXe/Commands/Tap.swift
  • Sources/AXe/Commands/Touch.swift
  • Sources/AXe/Utilities/OrientationAwareCoordinates.swift
  • Tests/OrientationAwareCoordinatesTests.swift

Comment on lines +109 to +192
static func detectMapping(
for simulatorUDID: String,
orientationOverride: SimulatorOrientation? = nil,
logger: AxeLogger
) async throws -> CoordinateMapping {
let roots = try await AccessibilityFetcher.fetchAccessibilityElements(
for: simulatorUDID,
logger: logger
)

guard let appFrame = applicationFrame(from: roots) else {
logger.info().log("Could not read application frame; assuming portrait passthrough")
return .passthrough
}

guard appFrame.width > appFrame.height else {
// Portrait-shaped AX frame — no translation needed.
logger.info().log(
"AX frame \(Int(appFrame.width))×\(Int(appFrame.height)) is portrait; passthrough"
)
return .passthrough
}

// Landscape-shaped AX frame. Distinguish rotated hardware from letterboxed app.
logger.info().log(
"AX frame \(Int(appFrame.width))×\(Int(appFrame.height)) is landscape; probing screenshot dimensions"
)

let screenshotDims = await screenshotPixelDimensions(for: simulatorUDID, logger: logger)

if let dims = screenshotDims, dims.height > dims.width {
// Screenshot is portrait-shaped despite a landscape AX frame →
// hardware is portrait, app is landscape-only: letterbox case.
logger.info().log(
"Screenshot \(dims.width)×\(dims.height)px is portrait-shaped; using letterbox mapping"
)

// Physical viewport in points: short side = portrait width, long side = portrait height.
// Derived from the landscape AX frame by swapping axes (same as portraitDimensions).
let physW = appFrame.height // portrait width = landscape logical height
let physH = appFrame.width // portrait height = landscape logical width
let logW = appFrame.width // logical landscape width
let logH = appFrame.height // logical landscape height

// Uniform scale to fit logical content into physical viewport.
let scale = min(physW / logW, physH / logH)
let offsetX = (physW - logW * scale) / 2
let offsetY = (physH - logH * scale) / 2

logger.info().log(
String(format: "Letterbox: scale=%.4f offsetX=%.1f offsetY=%.1f", scale, offsetX, offsetY)
)
return .letterbox(scale: scale, offsetX: offsetX, offsetY: offsetY)
}

// Screenshot is landscape-shaped (or unavailable) — hardware is rotated.
if screenshotDims == nil {
logger.info().log("Screenshot probe unavailable; assuming rotated hardware")
} else {
logger.info().log(
"Screenshot \(screenshotDims!.width)×\(screenshotDims!.height)px is landscape-shaped; using rotation mapping"
)
}

let orientation: SimulatorOrientation
if let override = orientationOverride {
logger.info().log("Using explicit orientation override: \(override.rawValue)")
orientation = override
} else {
// Default: landscape home-right (most common iOS landscape orientation).
// Pass `--landscape-flipped` when the device is rotated counter-clockwise.
orientation = .landscape
logger.info().log("Defaulting to landscape (home right) orientation")
}

guard let portraitSize = portraitDimensions(from: roots, orientation: orientation) else {
logger.info().log(
"Could not determine portrait dimensions; falling back to passthrough"
)
return .passthrough
}

return .rotation(orientation, portraitSize: portraitSize)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Behavioural ambiguity: rotated device + landscape-flipped letterboxing isn't representable.

The detection logic treats "screenshot portrait + AX landscape" exclusively as the letterbox case and explicitly ignores orientationOverride there (Line 104: "Has no effect in the letterbox case"). That's correct for portrait hardware running a landscape-only app. However, the help text on --landscape-flipped (in Tap.swift/Touch.swift/Swipe.swift) doesn't tell users it is silently ignored under letterbox; users debugging an off-by-rotation tap may try it and observe no change.

Either surface a single-line logger.info when the override is supplied but discarded under letterbox, or add a brief note to the CLI help text that the flag only applies to rotated hardware.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/AXe/Utilities/OrientationAwareCoordinates.swift` around lines 109 -
192, The detectMapping function currently ignores an orientationOverride in the
letterbox branch without informing the user; inside the letterbox handling block
in detectMapping(for:orientationOverride:logger:), check if orientationOverride
is non-nil and, if so, emit a single logger.info() log that the provided
orientation override is being ignored for the letterbox case (include the
override.rawValue for clarity) before returning .letterbox; this uses the
existing symbols detectMapping, orientationOverride, and .letterbox to locate
where to add the log.

Read SimulatorKit private UI orientation before translating coordinate-based
HID events so callers can use describe-ui coordinates directly in landscape.
Fail clearly when rotated orientation cannot be determined instead of guessing
a landscape direction.

Add a playground fixture and E2E coverage for tap, swipe, touch, and batch
coordinate translation. Move pure coordinate math into AXeCore so production
and tests exercise the same implementation.

Fixes cameroncooke#5
Co-Authored-By: OpenAI Codex <noreply@openai.com>
@cameroncooke

Copy link
Copy Markdown
Owner

@cursor review

Route simctl screenshot probe output to the null device and wait for
process completion asynchronously. This prevents undrained process pipes
or main-actor sleeps from blocking orientation detection.

Refs cameroncooke#44
Co-Authored-By: OpenAI Codex <noreply@openai.com>
When SimulatorKit reports portrait but the accessibility frame is
landscape-shaped, continue into the screenshot-based letterbox path
instead of treating coordinates as passthrough.

Refs cameroncooke#44
Co-Authored-By: OpenAI Codex <noreply@openai.com>
@cameroncooke
cameroncooke merged commit cf2c344 into cameroncooke:main May 10, 2026
4 checks passed
cameroncooke added a commit that referenced this pull request May 10, 2026
Route simctl screenshot probe output to the null device and wait for
process completion asynchronously. This prevents undrained process pipes
or main-actor sleeps from blocking orientation detection.

Refs #44
Co-Authored-By: OpenAI Codex <noreply@openai.com>
@cameroncooke

Copy link
Copy Markdown
Owner

@Nitewriter thank you for the contribution here. This was a really valuable fix and a great catch: AXe was treating coordinates from the accessibility tree as if they always matched the simulator HID coordinate space, which breaks once the simulator/app is in landscape.

Your PR provided the core direction for solving that by translating logical accessibility coordinates back into the physical coordinate space expected by the simulator. I made a few follow-up changes on top to harden it before merge:

  • made orientation detection automatic so users do not need to pass a manual landscape flag;
  • handled both rotated simulators and portrait hardware running landscape-only apps, since those need different coordinate mappings;
  • made ambiguous rotated-landscape cases fail clearly instead of guessing and risking a bad tap;
  • wired the fix through tap, swipe, touch, and batch so the behavior is consistent;
  • added playground/e2e coverage where untranslated portrait coordinates deliberately miss, proving the translation is doing real work;
  • addressed review feedback around private orientation probing, screenshot subprocess handling, and docs.

Really appreciate you digging into this. The follow-up commits are mostly hardening and test coverage — the important insight and initial fix came from your PR.

@Nitewriter

Copy link
Copy Markdown
Contributor Author

@cameroncooke thank you for such a useful tool, glad I could help out. also, thank you for getting it across the finish line. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tap coordinates are incorrect after device rotation

2 participants