-
Notifications
You must be signed in to change notification settings - Fork 77
fix: compute pixel ratio based on image size in case of oopif #1332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: testplane@8
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,22 +12,15 @@ module.exports = class ScreenShooter { | |
| } | ||
|
|
||
| async capture(page, opts = {}) { | ||
| const { | ||
| allowViewportOverflow, | ||
| compositeImage, | ||
| screenshotDelay, | ||
| selectorToScroll, | ||
| preferredPixelRatio, | ||
| reprepareScreenshot, | ||
| } = opts; | ||
| const { allowViewportOverflow, compositeImage, screenshotDelay, selectorToScroll, reprepareScreenshot } = opts; | ||
| const viewportOpts = { allowViewportOverflow, compositeImage }; | ||
| const cropImageOpts = { screenshotDelay, compositeImage, selectorToScroll }; | ||
|
|
||
| const capturedImage = await this._browser.captureViewportImage(page, screenshotDelay); | ||
| if (reprepareScreenshot) { | ||
| const currentPixelRatio = await this._browser.evalScript("window.devicePixelRatio"); | ||
| const currentPixelRatio = getPixelRatioFromImage(capturedImage.uncroppedSize, page); | ||
|
|
||
| if (currentPixelRatio !== (preferredPixelRatio ?? page.pixelRatio)) { | ||
| if (currentPixelRatio !== undefined) { | ||
| Object.assign(page, await reprepareScreenshot(currentPixelRatio)); | ||
| delete opts.preferredPixelRatio; | ||
| delete opts.reprepareScreenshot; | ||
|
|
@@ -67,3 +60,27 @@ module.exports = class ScreenShooter { | |
| await viewport.extendBy(physicalScrollHeight, newImage); | ||
| } | ||
| }; | ||
|
|
||
| function getPixelRatioFromImage(imageSize, page) { | ||
| // Allow rounding differences between CSS geometry and the captured bitmap. | ||
| if ( | ||
| Math.abs(imageSize.width - page.viewport.width) <= 1 && | ||
| Math.abs(imageSize.height - page.viewport.height) <= 1 | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| const pixelRatio = imageSize.width / page.viewportSizeInCss.width; | ||
| if (Math.abs(imageSize.height - page.viewportSizeInCss.height * pixelRatio) > 1) { | ||
| throw new Error("Screenshot dimensions do not match the viewport at a consistent pixel ratio"); | ||
|
Comment on lines
+73
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the actual screenshot DPR is fractional and differs from the prepared estimate, bitmap width and height are rounded independently, so deriving an exact ratio from width and allowing only one pixel of height error rejects valid images. For example, the repository's Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| const roundedPixelRatio = Math.round(pixelRatio); | ||
| const epsilon = 0.001; | ||
|
|
||
| return roundedPixelRatio > 0 && | ||
| pixelRatio >= roundedPixelRatio - epsilon && | ||
| pixelRatio <= roundedPixelRatio + epsilon | ||
| ? roundedPixelRatio | ||
| : pixelRatio; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When pixel-ratio validation runs with
screenshotMode: "fullpage"(orautodetects a full-page bitmap),capturedImage.uncroppedSizecontains the entire document becauseCamera.captureViewportImagerecords it before cropping. Dividing that width by the CSS viewport width therefore interprets document extent as DPR; depending on the document aspect ratio, this either throws at the following check or reparses the page with a grossly inflated ratio, breakingassertViewfor this supported configuration. The calculation must use dimensions corresponding to the captured image or retain whether the source bitmap was full-page.Useful? React with 👍 / 👎.