Skip to content

Introduce simple API to take a screenshot #2104

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

HannesWell
Copy link
Member

Currently it's quite cumbersome to take a screenshot from a Control or a Display. This proposes a convenience API to simplify this task.

When fully used within SWT it significantly reduces the calls to the to be deprecated Image constructor:

Together with #2103, it becomes very simple to take and save a screenshot.

I'm not yet sure where such a method is added best. Currently it's a new static method in the general SWT class. Alternativly it could be a static 'factory' like method in the Image/ImageData class or it could be in a new utility class or somewhere else?

Copy link
Contributor

github-actions bot commented May 5, 2025

Test Results

   539 files  ±0     539 suites  ±0   30m 3s ⏱️ -45s
 4 339 tests ±0   4 323 ✅ ±0   15 💤 ±0  1 ❌ ±0 
16 606 runs  ±0  16 468 ✅ ±0  137 💤 ±0  1 ❌ ±0 

For more details on these failures, see this check.

Results for commit 15c78c2. ± Comparison against base commit d1859ce.

@laeubi
Copy link
Contributor

laeubi commented May 6, 2025

What about a class Screenshot and then have Screenshot.of(Display ) (because you actually capture always from a Display, e.g. if controls overlap one would still not get a control) and then return anScreenshot instance that has methods to get the Image / ImageData (just in case someone want to operate on the Image itself) and then have Screenshot#take(Control) as convenience to call parameter or a Screenshot.take(Recangle) or just call Screenshot.take() for the full display, that would be much more flexible than a possible growing set of static methods.

@ptziegler
Copy link
Contributor

Wouldn't it make more sense to have this be a property of the Control, rather than moving this to the SWT class? Eventually, this functionality should probably also be supported for Widgets in general and not just Controls, which doesn't really work with a static method.

Image image = new Image(display, bounds);
GC gc = new GC(source);
try {
gc.copyArea(image, 0, 0);
Copy link
Contributor

@ptziegler ptziegler May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the source is invisible, this will capture the area underneath the widget. This should then either return null or a blank image.

@laeubi
Copy link
Contributor

laeubi commented May 6, 2025

Wouldn't it make more sense to have this be a property of the Control, rather than moving this to the SWT class? Eventually, this functionality should probably also be supported for Widgets in general and not just Controls, which doesn't really work with a static method.

As mentioned one can not take a screenshot of a Control, this is only used to make a screendump of the area where the control is currently located. To capture a control one can create a GC and "draw" the control then there.

@ptziegler
Copy link
Contributor

ptziegler commented May 6, 2025

As mentioned one can not take a screenshot of a Control, this is only used to make a screendump of the area where the control is currently located. To capture a control one can create a GC and "draw" the control then there.

But in that case I don't really see the benefit for the user. Keeping an internal Screenshot class is fine, but I don't think it's a good idea to make this public API. Especially when capturing the display is something that's prohibited on both GTK4 and Wayland anyway... (#673)

@HannesWell
Copy link
Member Author

What about a class Screenshot and then have Screenshot.of(Display ) (because you actually capture always from a Display, e.g. if controls overlap one would still not get a control) and then return anScreenshot instance that has methods to get the Image / ImageData (just in case someone want to operate on the Image itself) and then have Screenshot#take(Control) as convenience to call parameter or a Screenshot.take(Recangle) or just call Screenshot.take() for the full display, that would be much more flexible than a possible growing set of static methods.

As written in my initial message, I appreciate any suggestion where this new functionality is discoverable and flexibly usable. So if it makes sense to have various configuration options a new Screenshot class sounds good.

As mentioned one can not take a screenshot of a Control, this is only used to make a screendump of the area where the control is currently located. To capture a control one can create a GC and "draw" the control then there.

I see that there is also a method Control.print():

Can you tell if that behaves differently and e.g. allows to record the surface of that Control even if it's not visible respectively not supported to capture the entire display?

But in that case I don't really see the benefit for the user. Keeping an internal Screenshot class is fine, but I don't think it's a good idea to make this public API. Especially when capturing the display is something that's prohibited on both GTK4 and Wayland anyway... (#673)

That's kind of funny, Windows offers a 'feature' where it take screenshots automatically all the time (https://support.microsoft.com/de-de/windows/mit-recall-ihre-schritte-zur%C3%BCckverfolgen-aa03f8a0-a78b-4b3e-b0a1-2eb8ac48701c, article is in Germany, I didn't found the English version quickly).
But back to topic, is it supported on GTK4/Wayland to capture at least parts of the screen?

But even if it's not supported at all respectively impossible on some OS/WS combinations an API could still help users, if it's stated that it's an optional operation and if it fails cleanly with an exception if one is really in that situation. Then clients would at least not use some random snippets that don't work all the time and not fail cleanly, but just produce unexpected result.

As this topic doesn't seem to be that easy (thank you for all that input), I think it's even more important to help users and provide the 'best', most robust solutions possible.

@ptziegler
Copy link
Contributor

ptziegler commented May 7, 2025

But back to topic, is it supported on GTK4/Wayland to capture at least parts of the screen?

Screen capture is explicitly unsupported in Wayland for the sake of "security":

"By design, Wayland is a lot more secure than X11 and does not allow one application to capture the content of other applications' windows"

Or in short: This has been pushed down to the DE. So you can still take screenshots with Wayland, but now there's a Gnome way to do it, a KDE way to do it and so on... Not really something that can be integrated in SWT.

I think the "official" way is via the xdg-desktop-portal but I've never worked with it and you'd be yet again at the mercy of the DE to support this portal...

With GTK4, the GdkWindow has been made internal, so the required API is no longer available. See

} else if (data.drawable != 0) {
if (!GTK.GTK4) GDK.gdk_cairo_set_source_window(cairo, data.drawable, 0, 0);
} else {

You have the GtkSnapshot as replacement but which only works for widgets.

To summarize: Screen capture in Linux sucks.

But even if it's not supported at all respectively impossible on some OS/WS combinations an API could still help users, if it's stated that it's an optional operation and if it fails cleanly with an exception if one is really in that situation. Then clients would at least not use some random snippets that don't work all the time and not fail cleanly, but just produce unexpected result.

Which in the end would make (parts of) SWT no longer cross-platform compatible...

@ptziegler
Copy link
Contributor

To add to my previous comment: It might still be possible to capture the SWT widgets in GTK4/Wayland, which I believe is exactly what print(...) is doing.

@mickaelistria
Copy link
Contributor

Have you evaluated whether org.eclipse.ui.workbench.texteditor.tests.ScreenshotTest.takeScreenshot(...) can be reused?

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

Successfully merging this pull request may close these issues.

4 participants