feat: add Rust-side webview screenshot API#1674
Conversation
Package Changes Through bb37d12There are 1 changes which include wry with patch Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
FabianLars
left a comment
FabianLars
left a comment
There was a problem hiding this comment.
just some comments on macos, windows and linux look good at first glance and seem to match the other PRs and https://github.com/Fractal-Tess/tauri-plugin-snapshot (which i helped with back then so take that with a grain of salt)
| #[cfg(target_os = "macos")] | ||
| unsafe { | ||
| let config = WKSnapshotConfiguration::new(self.mtm); | ||
| config.setAfterScreenUpdates(true); |
There was a problem hiding this comment.
this is the default https://developer.apple.com/documentation/webkit/wksnapshotconfiguration/afterscreenupdates?language=objc so i think we can remove this and give takeSnapshotWithConfiguration None for the config?
There was a problem hiding this comment.
This ought to be resolved
| let config = WKSnapshotConfiguration::new(self.mtm); | ||
| config.setAfterScreenUpdates(true); | ||
|
|
||
| let callback = block2::RcBlock::new(move |image: *mut NSImage, _err: *mut NSError| { |
There was a problem hiding this comment.
should we not check for an error first?
| let Some(image) = image.as_ref() else { | ||
| handler(Err(Error::NilScreenshot)); | ||
| return; | ||
| }; | ||
|
|
||
| let Some(tiff) = image.TIFFRepresentation() else { | ||
| handler(Err(Error::NilScreenshot)); | ||
| return; | ||
| }; | ||
|
|
||
| let Some(bitmap) = NSBitmapImageRep::imageRepWithData(&tiff) else { | ||
| handler(Err(Error::NilScreenshot)); | ||
| return; | ||
| }; | ||
|
|
||
| let props: Retained<NSDictionary<NSBitmapImageRepPropertyKey, AnyObject>> = | ||
| NSDictionary::new(); | ||
| let png = bitmap.representationUsingType_properties(NSBitmapImageFileType::PNG, &props); | ||
| match png { | ||
| Some(data) => handler(Ok(data.to_vec())), | ||
| None => handler(Err(Error::NilScreenshot)), | ||
| } | ||
| }); |
There was a problem hiding this comment.
this is very different from all other implementations i saw on the internet (see the other 2 prs for how it usually looks like). Do you have some source for this perhaps? Also, did you actually test this on macOS?
Looks like people usually have issues with scaling with retina somethingsomething
There was a problem hiding this comment.
I did test this on macOS Sequoia 15.6, but it's entirely possible that I missed bugs if there are issues with scaling at high DPIs. Implementation is now closer to the other open PRs.
| #[cfg(target_os = "ios")] | ||
| { | ||
| // Unsupported | ||
| let _ = handler; | ||
| } |
There was a problem hiding this comment.
we should add this before merging but i think i can take this on if you can't do mobile :)
There was a problem hiding this comment.
I can probably try my hand but I can't promise anything good - will update if I have issues.
| /// | ||
| /// - **Linux / macOS / Windows**: Implemented (visible region only). | ||
| /// - **Android / iOS**: Not supported; `handler` will never be called. | ||
| pub fn screenshot<F>(&self, handler: F) -> Result<()> |
There was a problem hiding this comment.
would be so awesome if we could abstract away the handler but i guess that's more something for crate consumers like tauri
|
Thanks for the review. I'll address these tomorrow when I wake up. |
|
Colleague of @josh-richardson's here. Want to add that I've tested the latest commit on both:
Screenshot is 1:1 with the window I see on my screen, so can confirm working on both Retina and non-Retina displays. |
3 participants
Related to #1358. Prior art: #266, #1258. I'd really like to be able to take screenshots of my webviews in my Wry app.
Adds a Rust-side screenshot API for
WebView:WebView::screenshot(handler)which returns visible-region screenshots as PNG bytes. Implemented for and tested on:WEBKIT_DISABLE_DMABUF_RENDERER=1))I've also added
examples/screenshot_smoke.rs(inline HTML, waits for page load + 1s, writesscreenshot.png). I suspect this should probably be removed before this is merged, I just wanted an easy way of testing cross platform.Scope is intentionally limited to visible-region screenshots only (no full-document capture, afaict APIs to do this cross platform are not mature / well standardized yet).
On Linux I'm using webkit2gtk snapshot API and encodes PNGs via GTK/GDK Pixbuf, so it does not introduce a cairo dependency. On macOS, it uses WKWebView’s native snapshot API rather than OS/screen capture APIs, which avoids the screen-recording permission requirement that affected the older approach.