feat: add fromSurface option

[LeviPesin]

What kind of change does this PR introduce?

Feature.

Did you add tests for your changes?

No, I am unsure how to do that. Also, it seems like there is no test for the captureBeyondViewport option.

If relevant, did you update the documentation?

Yes.

Summary

This PR adds the fromSurface option to the Page.screenshot method.

This option can e.g. be used to properly screenshot WebGPU (see mrdoob/three.js#24109 for a discussion).

Does this PR introduce a breaking change?

No.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[munrocket]

This is great.

[jrandolf]

Thanks for the PR. Please provides tests for this.

[LeviPesin]

Added the test.
As you can see, the screenshot is not 500x500 - this is because fromSurface: false mode ignores viewport, but not browser window's bounds, but they are not set by the Puppeteer - this is exactly #1910.
I can make a PR solving it after this PR is merged (my workaround requires capturing fromSurface: false screenshot) or can include it in this PR.

[LeviPesin]

Added the test.
As you can see, the screenshot is not 500x500 - this is because fromSurface: false mode ignores viewport, but not browser window's bounds, but they are not set by the Puppeteer - this is exactly #1910.
I can make a PR solving it after this PR is merged (my workaround requires capturing fromSurface: false screenshot) or can include it in this PR.

@jrandolf Should I include the fix in this PR or in a separate PR?

I'm afraid that because my workaround requires getting width and height from screenshot, an image-parsing library should be added - e.g. either jpeg-js or pngjs should be moved from devDependencies to dependencies... Would that be OK?

[OrKoN]

I think we would not want to bring an image parsing library into the list of our prod dependencies. Could you please describe the latest problem you are facing? IIUC, the screenshot is bigger than the current viewport because setting the viewport does not change the window bounds?

P.S. also if you could rebase once more, I would run the test on the bots

[LeviPesin]

Could you please describe the latest problem you are facing? IIUC, the screenshot is bigger than the current viewport because setting the viewport does not change the window bounds?

The problem is that capturing Chromium screenshot in fromSurface: false mode ignores the page viewport, but does not ignore window bounds. And the Puppeteer does not change window bounds in the headful mode because of #1910.

P.S. also if you could rebase once more, I would run the test on the bots

Done.

[OrKoN]

The problem is that capturing Chromium screenshot in fromSurface: false mode ignores the page viewport, but does not ignore window bounds. And the Puppeteer does not change window bounds in the headful mode because of #1910.

(just for me to understand it) so once we capture a screenshot with fromSurface=false we could crop it to the current viewport dimensions and that would give us a correct screenshot of the viewport?

[LeviPesin]

In my workaround Browser.setWindowBounds is used to first set the "full" window bounds to the viewport, then a fromSurface: false-screenshot is made to find the "true" window bounds ("full" bounds minus UI), and then Browser.setWindowBounds is again used to update the window bounds.
I used a fromSurface: false-screenshot to find true bounds because it seems that other methods of getting bounds do not work (like document.body.clientWidth). I will recheck it, maybe there is another method...

[LeviPesin]

Checked the following methods of getting true bounds:

• Getting width and height of fromSurface: true-screenshot - works,
• Getting width and height of fromSurface: false-screenshot - does not work,
• Getting document.body.clientWidth and document.body.clientHeight - does not work,
• Getting document.body.getBoundingClientRect() - does not work.

So the capturing of the screenshot appears to be the only option.

[OrKoN]

Does the method Browser.getWindowBounds in CDP returns anything relevant for this issue? (no familiar with this method so not sure what it actually return)

[LeviPesin]

It also returns full window bounds (i.e. with UI).

[OrKoN]

Thanks for the PR! Let's land it and we can see if we find the workaround for the original issue.

[LeviPesin]

1. Screenshots
   Page.screenshot
   should work in "fromSurface: false" mode:
   Error: Sizes differ: expected image 929px X 887px, but got 1050px X 886px.
   at compareImages (test/src/golden-utils.ts:68:11)
   at compare (test/src/golden-utils.ts:145:18)
   at Object.toBeGolden (test/src/utils.ts:32:29)
   at EXTERNAL_MATCHER_TRAP (node_modules/expect/build/index.js:380:30)
   at Object.throwingMatcher [as toBeGolden] (node_modules/expect/build/index.js:381:15)
   at Context. (test/src/screenshot.spec.ts:200:26)
   at processTicksAndRejections (node:internal/process/task_queues:95:5)

It seems that the test does not even work because of different UI sizes on different platforms, I think? Not sure...

Also not sure about chrome-headless mode (does the options work in it or only in true headful), will investigate...

[LeviPesin]

It seems that the test does not even work because of different UI sizes on different platforms, I think? Not sure...

Yes, exactly - it works perfectly for me on Windows.

[LeviPesin]

Also not sure about chrome-headless mode (does the options work in it or only in true headful), will investigate...

It does not support the option. So everything in this PR seems to be correct :-)

[LeviPesin]

@OrKoN So is this PR going to be merged? I can temporary disable the test until the decision of what to do with #1910 will be made.

[OrKoN]

@LeviPesin let's land the PR but update the test to just verify that a screenshot is taken. The dimensions of the screenshot are not really under control of Puppeteer so I think the solution to that can be found later.

[LeviPesin]

Done.

[OrKoN]

let's also remove the golden image file!

[LeviPesin]

Done.