mirror of
https://github.com/ChromeDevTools/chrome-devtools-mcp.git
synced 2026-09-14 19:45:30 +08:00
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:
+47
-10
@@ -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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user