fix: respect screenshot bounds on HiDPI displays (#2536)

Fixes #2531

## What changed

- factor the page's actual `window.devicePixelRatio` into screenshot
downscaling
- keep CDP clip dimensions in CSS pixels while bounding the resulting
bitmap dimensions
- add a regression test for the real `defaultViewport: null` path with a
forced 2x device scale factor

## Why

Screenshot source boxes and CDP clip dimensions are expressed in CSS
pixels, but the returned bitmap is scaled by the page's device pixel
ratio. The previous calculation compared the CSS width directly with
`screenshotMaxWidth`, so a 2x page could return an image twice the
configured bound.

The new calculation derives the clip scale from `box ×
devicePixelRatio`, making the limit apply to the image pixels sent to
the model.

## Validation

- `npm run check-format`
- `npm run build`
- `npm run test -- tests/tools/screenshot.test.ts
--test-name-pattern='honors screenshotMaxWidth at device scale factors
above 1'`
- `npm run test -- tests/tools/screenshot.test.ts
--test-skip-pattern='with full page resulting in a large screenshot'`

The new regression fails on `main` with a 200px-wide image and passes
with an exact 100px result after this change. I also attempted the full
suite with retries; this local environment still times out in unrelated
daemon/extension E2E tests and hits the existing
`Page.captureScreenshot: Page is too large` case, which reproduces on
pristine `main`.
This commit is contained in:
Shixi Li
2026-08-21 19:12:22 +00:00
committed by GitHub
parent 2ce42f8738
commit ebf58f2f4a
2 changed files with 86 additions and 10 deletions
+47 -10
View File
@@ -17,14 +17,24 @@ import {definePageTool} from './ToolDefinition.js';
type ScreenshotFormat = 'png' | 'jpeg' | 'webp';
type SourceBox = BoundingBox & {
devicePixelRatio: number;
};
async function getSourceBox(
page: Page,
element: ElementHandle | undefined,
fullPage: boolean,
): Promise<BoundingBox | undefined> {
): Promise<SourceBox | undefined> {
if (element) {
const box = await element.boundingBox();
return box ?? undefined;
const viewport = page.viewport();
const [box, devicePixelRatio] = await Promise.all([
element.boundingBox(),
viewport
? (viewport.deviceScaleFactor ?? 1)
: page.evaluate(() => window.devicePixelRatio),
]);
return box ? {...box, devicePixelRatio} : undefined;
}
if (fullPage) {
const dims = await page.evaluate(() => ({
@@ -36,15 +46,28 @@ async function getSourceBox(
document.documentElement.scrollHeight,
document.body?.scrollHeight ?? 0,
),
devicePixelRatio: window.devicePixelRatio,
}));
if (dims.width <= 0 || dims.height <= 0) {
return undefined;
}
return {x: 0, y: 0, width: dims.width, height: dims.height};
return {
x: 0,
y: 0,
width: dims.width,
height: dims.height,
devicePixelRatio: dims.devicePixelRatio,
};
}
const viewport = page.viewport();
if (viewport) {
return {x: 0, y: 0, width: viewport.width, height: viewport.height};
return {
x: 0,
y: 0,
width: viewport.width,
height: viewport.height,
devicePixelRatio: viewport.deviceScaleFactor ?? 1,
};
}
// The browser is launched and connected with `defaultViewport: null`, so
// `page.viewport()` stays null until something emulates one. Fall back to the
@@ -52,28 +75,42 @@ async function getSourceBox(
const dims = await page.evaluate(() => ({
width: window.innerWidth,
height: window.innerHeight,
devicePixelRatio: window.devicePixelRatio,
}));
if (dims.width <= 0 || dims.height <= 0) {
return undefined;
}
return {x: 0, y: 0, width: dims.width, height: dims.height};
return {
x: 0,
y: 0,
width: dims.width,
height: dims.height,
devicePixelRatio: dims.devicePixelRatio,
};
}
function computeDownscaleClip(
box: BoundingBox,
box: SourceBox,
maxWidth: number | undefined,
maxHeight: number | undefined,
): ScreenshotClip | undefined {
const widthScale =
maxWidth !== undefined ? Math.min(1, maxWidth / box.width) : 1;
maxWidth !== undefined
? Math.min(1, maxWidth / (box.width * box.devicePixelRatio))
: 1;
const heightScale =
maxHeight !== undefined ? Math.min(1, maxHeight / box.height) : 1;
maxHeight !== undefined
? Math.min(1, maxHeight / (box.height * box.devicePixelRatio))
: 1;
const scale = Math.min(widthScale, heightScale);
if (scale >= 1) {
return undefined;
}
// Skip degenerate sub-pixel results.
if (Math.round(box.width * scale) < 1 || Math.round(box.height * scale) < 1) {
if (
Math.round(box.width * box.devicePixelRatio * scale) < 1 ||
Math.round(box.height * box.devicePixelRatio * scale) < 1
) {
return undefined;
}
return {
+39
View File
@@ -403,6 +403,45 @@ describe('screenshot', () => {
});
});
it('honors screenshotMaxWidth at device scale factors above 1', async () => {
const tool = screenshot({
screenshotMaxWidth: 100,
} as ParsedArguments);
await withMcpContext(
async (response, context) => {
const page = context.getSelectedMcpPage().pptrPage;
assert.equal(page.viewport(), null);
await page.setContent(
html`<div style="width:100vw;height:100vh;background:red"></div>`,
);
const source = await page.evaluate(() => ({
width: window.innerWidth,
height: window.innerHeight,
devicePixelRatio: window.devicePixelRatio,
}));
assert.equal(source.devicePixelRatio, 2);
await tool.handler(
{params: {format: 'png'}, page: context.getSelectedMcpPage()},
response,
context,
);
assert.equal(response.images.length, 1);
const buf = Buffer.from(response.images[0].data, 'base64');
assert.equal(pngWidth(buf), 100);
const expectedHeight = Math.round(
source.height * (100 / source.width),
);
assert.ok(
Math.abs(pngHeight(buf) - expectedHeight) <= 1,
`expected height ~${expectedHeight}, got ${pngHeight(buf)}`,
);
},
{args: ['--force-device-scale-factor=2']},
);
});
it('downscales viewport screenshot when no viewport is emulated', async () => {
const tool = screenshot({
screenshotMaxWidth: 100,