Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 12 additions & 2 deletions packages/tool-server/src/tools/describe/platforms/android/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import type { Registry, ToolDependency } from "@argent/registry";
import type { DescribeTreeData } from "../../contract";
import { adbExecOutBinary, isAndroidTv } from "../../../../utils/adb";
import { resolveDevice } from "../../../../utils/device-info";
import { getAndroidScreenSize } from "../../../../utils/android-screen";
import {
getAndroidScreenSize,
orientScreenSize,
parseDumpRotation,
} from "../../../../utils/android-screen";
import { parseUiAutomatorDump } from "./uiautomator-parser";
import {
androidDevtoolsRef,
Expand Down Expand Up @@ -113,6 +117,12 @@ export async function describeAndroid(
}
);
}
const tree = parseUiAutomatorDump(raw, size.width, size.height);
// `wm size` is not rotation-aware, but the dump says which rotation it was
// taken at. Orienting the divisor here is what keeps a rotated device's frames
// in the same upright space the android-devtools path already produces — and
// stops the right-hand half of a landscape screen being pruned away as
// off-screen (#609).
const oriented = orientScreenSize(size, parseDumpRotation(raw));
const tree = parseUiAutomatorDump(raw, oriented.width, oriented.height);
return { tree, source: "uiautomator", hint };
}
32 changes: 29 additions & 3 deletions packages/tool-server/src/tools/screenshot-diff/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import path from "path";
import { z } from "zod";
import { FAILURE_CODES, FailureError } from "@argent/registry";
import type {
DeviceInfo,
FileInputSpec,
ServiceRef,
ToolContext,
Expand All @@ -14,6 +15,7 @@ import type {
import { simulatorServerRef, type SimulatorServerApi } from "../../blueprints/simulator-server";
import { resolveDevice } from "../../utils/device-info";
import { httpScreenshot } from "../../utils/simulator-client";
import { captureScreenshotUpright } from "../../utils/rotation-aware-capture";
import { requireArtifacts, type ArtifactHandle } from "../../artifacts";
import { diffPngFiles } from "./screenshot-diff";

Expand Down Expand Up @@ -48,7 +50,11 @@ const zodSchema = z
rotation: z
.enum(["Portrait", "LandscapeLeft", "LandscapeRight", "PortraitUpsideDown"])
.optional()
.describe("Orientation override for live baseline/current captures."),
.describe(
"Orientation override for live baseline/current captures. Rarely needed: an Android capture " +
"already follows the device's rotation. Setting it pins a fixed rotation, which can make a " +
"live capture disagree with a saved baseline taken at a different device rotation."
),
outputDir: z
.string()
.min(1)
Expand Down Expand Up @@ -197,6 +203,7 @@ async function resolveInputPaths(
const baselinePath = params.captureBaseline
? await captureLiveInput({
api: requireSimulatorServer(services),
device: resolveDevice(params.udid),
outputDir,
name: "baseline",
rotation: params.rotation,
Expand All @@ -208,6 +215,7 @@ async function resolveInputPaths(
const currentPath = params.captureCurrent
? await captureLiveInput({
api: requireSimulatorServer(services),
device: resolveDevice(params.udid),
outputDir,
name: "current",
rotation: params.rotation,
Expand Down Expand Up @@ -279,6 +287,10 @@ async function captureLiveInput(params: {
// Resolved and validated by requireSimulatorServer at the call site, so it is
// never undefined here.
api: SimulatorServerApi;
// Needed so a live capture picks up the device's rotation the same way the
// `screenshot` tool does. Without it a rotated-Android `captureCurrent` would
// come back sideways and diff at ~100% against an upright saved baseline.
device: DeviceInfo;
outputDir: string;
name: "baseline" | "current";
rotation?: Params["rotation"];
Expand All @@ -294,9 +306,23 @@ async function captureLiveInput(params: {
// baseline saved at any scale. Full-res is preserved wherever it works (iOS).
let capture: Awaited<ReturnType<CaptureScreenshot>>;
try {
capture = await params.captureScreenshot(params.api, params.rotation, params.signal, 1.0);
capture = await captureScreenshotUpright(
params.api,
params.device,
params.rotation,
params.signal,
1.0,
params.captureScreenshot
);
} catch {
capture = await params.captureScreenshot(params.api, params.rotation, params.signal);
capture = await captureScreenshotUpright(
params.api,
params.device,
params.rotation,
params.signal,
undefined,
params.captureScreenshot
);
}
const suffix = crypto.randomBytes(4).toString("hex");
const destination = path.join(params.outputDir, `${params.name}-${suffix}.live.png`);
Expand Down
13 changes: 10 additions & 3 deletions packages/tool-server/src/tools/screenshot/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import type { Registry, ToolCapability, ToolDefinition } from "@argent/registry"
import { simulatorServerRef, type SimulatorServerApi } from "../../blueprints/simulator-server";
import { chromiumCdpRef, type ChromiumCdpApi } from "../../blueprints/chromium-cdp";
import { resolveDevice } from "../../utils/device-info";
import { getScreenshotScale, httpScreenshot } from "../../utils/simulator-client";
import { getScreenshotScale } from "../../utils/simulator-client";
import { captureScreenshotUpright } from "../../utils/rotation-aware-capture";
import { isTvOsSimulator } from "../../utils/ios-devices";
import { simctlArgsForUdid } from "../../utils/ios-device-sets";
import { captureVegaScreenshotPng } from "../../utils/vega-screen";
Expand All @@ -25,7 +26,11 @@ const zodSchema = z.object({
.enum(["Portrait", "LandscapeLeft", "LandscapeRight", "PortraitUpsideDown"])
.optional()
.describe(
"Orientation override for the screenshot (rotates the captured image after Page.captureScreenshot on Chromium)."
"Orientation override. Rarely needed: on Android the capture already follows the device's " +
"current rotation, so it is upright without this. Setting it replaces that with a fixed " +
"rotation, which on a rotated device produces an image whose geometry no longer matches " +
"`describe` frames or gesture coordinates. On Chromium it rotates the captured image after " +
"Page.captureScreenshot."
),
scale: z
.number()
Expand Down Expand Up @@ -130,6 +135,7 @@ export function createScreenshotTool(registry: Registry): ToolDefinition<Params,
},
description: `Capture a screenshot of the device screen (iOS simulator, Android emulator, Apple TV simulator, Vega, or Chromium app). Returns { image }; the MCP adapter renders it as a visible image unless the caller passed includeImageInContext: false.
Use when you need a baseline image before an interaction or to inspect the current screen state after a delay.
On a rotated Android device the capture follows the device's rotation, so it comes back upright and its geometry matches \`describe\` frames and gesture coordinates. Do not pass \`rotation\` to correct a sideways image.
Fails if the simulator-server / emulator backend / Chromium CDP is not reachable for the given device.`,
alwaysLoad: true,
searchHint: "device simulator emulator chromium screen image capture baseline tvos apple tv",
Expand Down Expand Up @@ -177,8 +183,9 @@ Fails if the simulator-server / emulator backend / Chromium CDP is not reachable

const ref = simulatorServerRef(device);
const api = (await registry.resolveService(ref.urn, ref.options)) as SimulatorServerApi;
const { path: capturedPath } = await httpScreenshot(
const { path: capturedPath } = await captureScreenshotUpright(
api,
device,
params.rotation,
signal,
params.scale
Expand Down
52 changes: 48 additions & 4 deletions packages/tool-server/src/utils/android-screen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,18 @@ export interface AndroidScreenSize {
* `wm size` reports "Physical size: WxH\nOverride size: WxH"; the override
* wins when present (set by emulators and some system configs).
*
* IMPORTANT: this reports the *unrotated* size. Measured on a landscape Pixel_9
* (API 36) whose display really was 2424x1080: `wm size` still answered
* "Physical size: 1080x2424" with no Override line. So the returned size must be
* oriented by the caller — see `orientScreenSize` — before it is used as a
* divisor for rotated bounds. An earlier revision of this comment assumed the
* opposite and that assumption is what made the legacy describe path wrong on a
* rotated device (#609).
*
* NOT cached: a 5 s TTL would have served stale dimensions for several
* describes after a rotation (rotation completes in <500 ms), producing
* normalized frames with x>1 / width>1 because the screenW used for the
* divisor was pre-rotation. One extra `adb shell` per `describe` is cheap
* compared to the uiautomator dump exec-out it sits next to.
* describes after a rotation (rotation completes in <500 ms). One extra
* `adb shell` per `describe` is cheap compared to the uiautomator dump exec-out
* it sits next to.
*/
export async function getAndroidScreenSize(serial: string): Promise<AndroidScreenSize> {
const out = await adbShell(serial, "wm size", { timeoutMs: 5_000 });
Expand All @@ -45,3 +52,40 @@ export async function getAndroidScreenSize(serial: string): Promise<AndroidScree
}
return { width, height };
}

/**
* The surface rotation a uiautomator dump was taken at, read from the dump
* itself: `<hierarchy rotation="1">`.
*
* Taking it from the XML rather than asking the device again is deliberate. It
* costs no extra round-trip, it cannot disagree with the bounds it is used to
* normalize (a rotation between the two calls would), and — most importantly —
* the legacy path runs precisely when the android-devtools helper could not be
* reached, which is when the device is least likely to answer more adb probes.
*
* Returns null when absent, which keeps pre-rotation-attribute dumps working.
*/
export function parseDumpRotation(rawOutput: string): 0 | 1 | 2 | 3 | null {
const value = Number(/<hierarchy[^>]*\brotation="([0-3])"/.exec(rawOutput)?.[1]);
if (value === 0 || value === 1 || value === 2 || value === 3) return value;
return null;
}

/**
* Swap width and height when the device is on its side, so the result describes
* the display as it is currently laid out.
*
* uiautomator reports node bounds against the rotated display, so on a rotated
* device the unrotated `wm size` is the wrong divisor in both axes. The visible
* consequence was not merely squashed frames: `isVisibleRect` drops any node
* whose left edge is past the screen width, so with a 1080-wide divisor against
* a 2424-wide display, everything on the right-hand half of the screen
* disappeared from the tree entirely.
*/
export function orientScreenSize(
size: AndroidScreenSize,
rotation: 0 | 1 | 2 | 3 | null
): AndroidScreenSize {
if (rotation !== 1 && rotation !== 3) return size;
return { width: size.height, height: size.width };
}
147 changes: 147 additions & 0 deletions packages/tool-server/src/utils/device-orientation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
import { promises as fs } from "node:fs";
import { adbShell } from "./adb";

/**
* The orientation names the simulator-server screenshot API accepts.
*
* Read these as *compositing transforms*, not as physical orientations. The
* distinction matters: a report on #609 established that on iOS the same names
* are 180° inverted from the physical result, so nothing here may be derived
* from what the words mean.
*/
export type OrientationName =
| "Portrait"
| "LandscapeLeft"
| "LandscapeRight"
| "PortraitUpsideDown";

/** Android's surface rotation, as reported by the platform (`Surface.ROTATION_*`). */
export type SurfaceRotation = 0 | 1 | 2 | 3;

/**
* Surface rotation → the name that makes simulator-server hand back an upright
* capture.
*
* DERIVED BY MEASUREMENT, NOT BY NAME. `adb exec-out screencap` is
* rotation-aware, so it is ground truth; each candidate name was captured at
* full scale and scored against it as mean absolute difference over a 96×54
* grayscale grid, both as-is and rotated 180° to catch an inverted mapping:
*
* rotation 1 LandscapeLeft MAD 2.19 (vs 6.54 for its 180° twin)
* rotation 3 LandscapeRight MAD 2.19 (vs 6.34)
* rotation 2 PortraitUpsideDown MAD 2.67 (vs 13.13)
* rotation 0 Portrait identity — an unrotated capture is already upright
*
* Note this table is the INVERSE of simulator-server's own convention, which
* maps 90° → LandscapeRight (`src/device_controller.rs`) and rotates the frame
* at decode time from the scrcpy header's `display_orientation`
* (`src/device_controller/android_device/video.rs`,
* `src/media_handler/decoder/video_toolbox.rs`). We are compensating downstream
* for what that layer already did, so this constant encodes the behaviour of a
* particular simulator-server build rather than a property of Android.
*
* That is why a unit test pinning this table is not sufficient on its own: it
* could never notice simulator-server changing underneath us. `captureLooksUpright`
* below is the check that would, and it is applied to the real capture.
*/
export const SURFACE_ROTATION_TO_NAME: Readonly<Record<SurfaceRotation, OrientationName>> = {
0: "Portrait",
1: "LandscapeLeft",
2: "PortraitUpsideDown",
3: "LandscapeRight",
};

function isSurfaceRotation(value: number): value is SurfaceRotation {
return value === 0 || value === 1 || value === 2 || value === 3;
}

/**
* Read the device's current surface rotation.
*
* Returns `null` — never throws — when the rotation cannot be established. A
* failed query must degrade to "unknown", which callers treat as "behave
* exactly as before". It must never fall back to a guess: a wrong orientation
* produces a confidently wrong image, which is worse than the sideways one this
* is fixing.
*
* Two independent readings are tried because `dumpsys` output is not a stable
* API. `|| true` keeps a `grep` miss (exit 1) from making `adbShell` throw.
*/
export async function readAndroidSurfaceRotation(serial: string): Promise<SurfaceRotation | null> {
const probes: { cmd: string; pattern: RegExp }[] = [
{
cmd: "dumpsys display | grep mCurrentOrientation || true",
pattern: /mCurrentOrientation=([0-3])/,
},
{ cmd: "dumpsys window displays || true", pattern: /\bmRotation=([0-3])/ },
];

for (const { cmd, pattern } of probes) {
try {
const out = await adbShell(serial, cmd, { timeoutMs: 5_000 });
const value = Number(pattern.exec(out)?.[1]);
if (Number.isInteger(value) && isSurfaceRotation(value)) return value;
} catch {
// Try the next probe; exhausting them yields null.
}
}
return null;
}

/**
* The rotation to request for an upright capture, or `undefined` to send no
* rotation at all.
*
* `undefined` is returned for an unrotated device rather than `"Portrait"` so
* the request body is byte-identical to what it was before this existed — the
* overwhelmingly common case stays provably unchanged.
*/
export function captureRotationForSurface(
rotation: SurfaceRotation | null
): OrientationName | undefined {
if (rotation === null || rotation === 0) return undefined;
return SURFACE_ROTATION_TO_NAME[rotation];
}

/** Width and height from a PNG's IHDR chunk, or null if this isn't a PNG. */
export async function readPngSize(path: string): Promise<{ width: number; height: number } | null> {
let handle: Awaited<ReturnType<typeof fs.open>> | undefined;
try {
handle = await fs.open(path, "r");
const buf = Buffer.alloc(24);
const { bytesRead } = await handle.read(buf, 0, 24, 0);
if (bytesRead < 24) return null;
// 8-byte signature, then a length+type header, then IHDR's width/height.
if (buf.readUInt32BE(0) !== 0x89504e47) return null;
const width = buf.readUInt32BE(16);
const height = buf.readUInt32BE(20);
if (width <= 0 || height <= 0) return null;
return { width, height };
} catch {
return null;
} finally {
await handle?.close().catch(() => {});
}
}

/**
* Does a capture taken with `requested` actually have the shape that rotation
* implies?
*
* This is the guard that a table test cannot be. The mapping above compensates
* for a rotation simulator-server applies at decode time; if that layer changes,
* the compensation silently becomes a 180° error or a no-op. Comparing the
* delivered PNG's aspect against the orientation we asked for catches the case
* where the request did not do what this module assumes.
*
* Returns true when unknown — an unreadable PNG is not evidence of a problem.
*/
export function captureLooksUpright(
requested: OrientationName,
size: { width: number; height: number } | null
): boolean {
if (!size || size.width === size.height) return true;
const wantsLandscape = requested === "LandscapeLeft" || requested === "LandscapeRight";
const isLandscape = size.width > size.height;
return wantsLandscape === isLandscape;
}
Loading
Loading