Skip to content
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

Screenshot domain/command #120

Closed
foolip opened this issue Jul 19, 2021 · 8 comments
Closed

Screenshot domain/command #120

foolip opened this issue Jul 19, 2021 · 8 comments

Comments

@foolip
Copy link
Member

foolip commented Jul 19, 2021

Prior art:

Something like a screenshot.takeScreenshot command with these parameters:

  • context: the browsing context id
  • element: an element id (or a clip rectangle?)
  • format: PNG, JPEG, etc.
  • quality: Compression quality
  • fullPage: Something like CDP's captureBeyondViewport
@jgraham
Copy link
Member

jgraham commented Jul 19, 2021

Yes, that looks reasonable to me. I think this should be pretty close to the existing WebDriver spec since there's nothing async happening, but obviously with a more CDPish flavour to the JSON data. Note that the minimum viable thing here probably only allows PNG and so doesn't have a quality parameter (but supporting JPEG probably isn't very hard, and it seems at least Puppeteer exposes that to users).

One challenge is that the fullPage option may not be trivial to specify; I think we skipped it for WebDriver Classic but in practice implementations have it, although I don't know they all work the same The typical kind of problem is "what if you haven't already rendered the whole page, but just some tiles. Then you might need to simulate scrolling to render the whole page, but what about position:fixed headers, or content that changes duirng scroll". So really you want "render the entire current layout tree as if the viewport length was unbounded", but that might not actually be a thing that's supported in engines.

@foolip
Copy link
Member Author

foolip commented Jul 19, 2021

Yes, fullPage is non-trivial, and I know that @sadym-chromium has been working on bugs related to it, precisely because of optimizations for things that aren't currently in view.

For the response, this is a place where having a binary WebSocket message and not having to encode it in JSON would reduce the size, but I don't think we should do that right away, or at least we should support JSON-only transport.

@whimboo
Copy link
Contributor

whimboo commented Jul 19, 2021

For the WebDriver HTTP fullpage option there is w3c/webdriver#1536, but we somehow dropped activity on it. Shall we continue there too beside BiDi, or close it and have it for BiDi only?

Full screenshots should work with Firefox these days, at least it was when I implemented the screenshot feature for CDP. But yes, that's just for Firefox and I don't know how other browsers behave.

Something else we might want to take into account is the device scale factor, which would come in via a viewPort parameter as @foolip already mentioned. In WebDriver HTTP we do not support it, and as such produces kinda large screenshots on high-resolution displays.

@foolip
Copy link
Member Author

foolip commented Jul 20, 2021

@whimboo for the device pixel ratio, I think that's something that should be possible to configure either when starting the session or per browsing context. In CDP, I think https://chromedevtools.github.io/devtools-protocol/tot/Emulation/#method-setPageScaleFactor is the command for it and I assume screenshots are then simply at the higher resolution. Or do you think it should be possible to downscale when taking a screenshot if the DPR is greater than 1?

@whimboo
Copy link
Contributor

whimboo commented Jul 20, 2021

I think right now it's still https://chromedevtools.github.io/devtools-protocol/tot/Emulation/#method-setDeviceMetricsOverride. setPageScaleFactor is experimental, so not sure if it might replace it at some point?

From the perspective of automating screenshot generation it's annoying right now that I have to resize the screenshot afterward (including quality loss) to get it to the regular size. The relevant GFX related code in browsers should do that way better. Further it would also allow us to actually compare against reference images, which is currently not possible (not saying that this is always a good idea).

@foolip
Copy link
Member Author

foolip commented Jul 20, 2021

It looks like Puppeteer uses Emulation.setDeviceMetricsOverride, so that's probably the most relevant to look at.

For the downscaling, don't some image format have metadata for saying what the display size should be independent of pixel size? Might that be something to investigate, if the main problem is that they're too big when one manually looks at them?

@whimboo
Copy link
Contributor

whimboo commented Jul 20, 2021

I cannot say, without having to dive into this more deeply. But right now I don't have the time for it.

@whimboo
Copy link
Contributor

whimboo commented Mar 15, 2023

Something like a screenshot.takeScreenshot command with these parameters:

This got initially landed as browsingContext.captureScreenshot. As such no new module is needed.

* `context`: the browsing context id

Support for the context parameter already exists.

* `element`: an element id (or a clip rectangle?)

Handled by #229

* `format`: PNG, JPEG, etc.
* `quality`: Compression quality

Handled by #383

* `fullPage`: Something like CDP's `captureBeyondViewport`

Handled by #384

I think that it will be best to close this issue given that we have individual ones for the remaining different parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants