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

8326712: Robot tests fail on XWayland #1490

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions buildSrc/linux.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -89,21 +89,22 @@ setupTools("linux_gtk3",
{ propFile ->
ByteArrayOutputStream results2 = new ByteArrayOutputStream();
exec {
commandLine("${toolchainDir}pkg-config", "--cflags", "gtk+-3.0", "gthread-2.0", "xtst")
commandLine("${toolchainDir}pkg-config", "--cflags", "gtk+-3.0", "gthread-2.0", "xtst", "gio-unix-2.0")
setStandardOutput(results2);
}
propFile << "cflagsGTK3=" << results2.toString().trim() << "\n";

ByteArrayOutputStream results4 = new ByteArrayOutputStream();
exec {
commandLine("${toolchainDir}pkg-config", "--libs", "gtk+-3.0", "gthread-2.0", "xtst")
commandLine("${toolchainDir}pkg-config", "--libs", "gtk+-3.0", "gthread-2.0", "xtst", "gio-unix-2.0")
setStandardOutput(results4);
}
propFile << "libsGTK3=" << results4.toString().trim() << "\n";

},
{ properties ->
def cflagsGTK3 = properties.getProperty("cflagsGTK3")
def cflagsGTK3 = properties.getProperty("cflagsGTK3") \
+ " -I${project(":graphics").projectDir}/src/main/native-glass/gtk/libpipewire/include"
def libsGTK3 = properties.getProperty("libsGTK3")
if (cflagsGTK3 && libsGTK3) {
gtk3CCFlags.addAll(cflagsGTK3.split(" "))
Expand Down Expand Up @@ -172,7 +173,7 @@ setupTools("linux_freetype_tools",
freetypeLinkFlags.addAll(libs.split(" "))
}
} else {
throw new IllegalStateException("Linux freetype packages not found.\nIf freetype pacakges are installed, please remove the build directory and try again.")
throw new IllegalStateException("Linux freetype packages not found.\nIf freetype packages are installed, please remove the build directory and try again.")
}
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
*/
package com.sun.glass.ui.gtk;

import com.sun.glass.ui.gtk.screencast.ScreencastHelper;
import javafx.scene.input.KeyCode;
import javafx.scene.input.MouseButton;
import javafx.scene.paint.Color;
Expand All @@ -32,8 +33,26 @@
import com.sun.glass.ui.GlassRobot;
import com.sun.glass.ui.Screen;

import java.security.AccessController;
import java.security.PrivilegedAction;

@SuppressWarnings("removal")
azvegint marked this conversation as resolved.
Show resolved Hide resolved
final class GtkRobot extends GlassRobot {

private static final String screenshotMethod;
private static final String METHOD_GTK = "gtk";
private static final String METHOD_SCREENCAST = "dbusScreencast";

static {
boolean isOnWayland = AccessController.doPrivileged((PrivilegedAction<Boolean>) () -> {
String waylandDisplay = System.getenv("WAYLAND_DISPLAY");
return waylandDisplay != null && !waylandDisplay.isBlank();
});

screenshotMethod = AccessController.doPrivileged((PrivilegedAction<String>) () ->
System.getProperty("fx.robot.screenshotMethod", isOnWayland ? METHOD_SCREENCAST : METHOD_GTK));
}

@Override
public void create() {
// no-op
Expand Down Expand Up @@ -115,7 +134,11 @@ public Color getPixelColor(double x, double y) {
x = (int) Math.floor((x + 0.5) * mainScreen.getPlatformScaleX());
y = (int) Math.floor((y + 0.5) * mainScreen.getPlatformScaleY());
int[] result = new int[1];
_getScreenCapture((int) x, (int) y, 1, 1, result);
if (METHOD_SCREENCAST.equals(screenshotMethod)) {
ScreencastHelper.getRGBPixels((int) x, (int) y, 1, 1, result);
} else {
_getScreenCapture((int) x, (int) y, 1, 1, result);
}
return GlassRobot.convertFromIntArgb(result[0]);
}

Expand All @@ -124,6 +147,10 @@ public Color getPixelColor(double x, double y) {
@Override
public void getScreenCapture(int x, int y, int width, int height, int[] data, boolean scaleToFit) {
Application.checkEventThread();
_getScreenCapture(x, y, width, height, data);
if (METHOD_SCREENCAST.equals(screenshotMethod)) {
ScreencastHelper.getRGBPixels(x, y, width, height, data);
} else {
_getScreenCapture(x, y, width, height, data);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

package com.sun.glass.ui.gtk.screencast;

import com.sun.glass.ui.Screen;

import com.sun.javafx.geom.Rectangle;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.List;
import java.util.Set;
import java.util.Timer;
import java.util.TimerTask;
import java.util.stream.IntStream;

/**
* Helper class for grabbing pixels from the screen using the
* <a href="https://flatpak.github.io/xdg-desktop-portal/#gdbus-org.freedesktop.portal.ScreenCast">
* org.freedesktop.portal.ScreenCast API</a>
*/

@SuppressWarnings("removal")
public class ScreencastHelper {

static final boolean SCREENCAST_DEBUG;
private static final boolean IS_NATIVE_LOADED;


private static final int ERROR = -1;
private static final int DENIED = -11;
private static final int OUT_OF_BOUNDS = -12;

private static final int DELAY_BEFORE_SESSION_CLOSE = 2000;

private static volatile TimerTask timerTask = null;
private static final Timer timerCloseSession
= new Timer("auto-close screencast session", true);


private ScreencastHelper() {
}

static {
SCREENCAST_DEBUG =
AccessController.doPrivileged((PrivilegedAction<Boolean>) () -> {
final String str =
System.getProperty("fx.robot.screenshotDebug", "false");
azvegint marked this conversation as resolved.
Show resolved Hide resolved
return "true".equalsIgnoreCase(str);
});

IS_NATIVE_LOADED = loadPipewire(SCREENCAST_DEBUG);
}

public static boolean isAvailable() {
return IS_NATIVE_LOADED;
}

private static native boolean loadPipewire(boolean screencastDebug);

private static native int getRGBPixelsImpl(
int x, int y, int width, int height,
int[] pixelArray,
int[] affectedScreensBoundsArray,
String token
);

//TODO copy from sun.java2d.pipe.Region.clipRound
public static int clipRound(final double coordinate) {
final double newv = coordinate - 0.5;
if (newv < Integer.MIN_VALUE) {
return Integer.MIN_VALUE;
}
if (newv > Integer.MAX_VALUE) {
return Integer.MAX_VALUE;
}
return (int) Math.ceil(newv);
}

private static List<Rectangle> getSystemScreensBounds() {
return Screen
.getScreens()
.stream()
.map(screen -> new Rectangle(
clipRound(screen.getPlatformX() * screen.getPlatformScaleX()),
clipRound(screen.getPlatformY() * screen.getPlatformScaleY()),
clipRound(screen.getPlatformWidth() * screen.getPlatformScaleX()),
clipRound(screen.getPlatformHeight() * screen.getPlatformScaleY())
))
.toList();
}

private static synchronized native void closeSession();

private static void timerCloseSessionRestart() {
if (timerTask != null) {
timerTask.cancel();
}

timerTask = new TimerTask() {
@Override
public void run() {
closeSession();
}
};

timerCloseSession.schedule(timerTask, DELAY_BEFORE_SESSION_CLOSE);
}

public static synchronized void getRGBPixels(
int x, int y, int width, int height, int[] pixelArray
) {
if (!IS_NATIVE_LOADED) return;

timerCloseSessionRestart();

Rectangle captureArea = new Rectangle(x, y, width, height);

List<Rectangle> affectedScreenBounds = getSystemScreensBounds()
.stream()
.filter(r -> !captureArea.intersection(r).isEmpty())
.toList();

if (SCREENCAST_DEBUG) {
System.out.printf("// getRGBPixels in %s, affectedScreenBounds %s\n",
captureArea, affectedScreenBounds);
}

if (affectedScreenBounds.isEmpty()) {
if (SCREENCAST_DEBUG) {
System.out.println("// getRGBPixels - requested area "
+ "outside of any screen");
}
return;
}

int retVal;
Set<TokenItem> tokensForRectangle =
TokenStorage.getTokens(affectedScreenBounds);

int[] affectedScreenBoundsArray = affectedScreenBounds
.stream()
.filter(r -> !captureArea.intersection(r).isEmpty())
.flatMapToInt(bounds -> IntStream.of(
bounds.x, bounds.y,
bounds.width, bounds.height
))
.toArray();

for (TokenItem tokenItem : tokensForRectangle) {
retVal = getRGBPixelsImpl(
x, y, width, height,
pixelArray,
affectedScreenBoundsArray,
tokenItem.token
);

if (retVal >= 0) { // we have received a screen data
return;
} else if (!checkReturnValue(retVal)) {
return;
} // else, try other tokens
}

// we do not have a saved token or it did not work,
// try without the token to show the system's permission request window
retVal = getRGBPixelsImpl(
x, y, width, height,
pixelArray,
affectedScreenBoundsArray,
null
);

checkReturnValue(retVal);
}

private static boolean checkReturnValue(int retVal) {
if (retVal == DENIED) {
// user explicitly denied the capture, no more tries.
throw new SecurityException(
"Screen Capture in the selected area was not allowed"
);
Copy link
Member Author

Choose a reason for hiding this comment

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

@kevinrushforth
I think this SecurityException should be removed, as it is not mentioned in the javadoc for the getPixelColor or getScreenCapture. And we also do not want to throw another runtime exception.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If the SecurityException is not thrown, how does the caller of getRGBPixels knows that the result will not be correct because of the missing permission? (as opposed to being incorrect due to a failure)

Copy link
Member Author

Choose a reason for hiding this comment

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

If the SecurityException is not thrown, how does the caller of getRGBPixels knows that the result will not be correct because of the missing permission? (as opposed to being incorrect due to a failure)

He won't know it. An indirect result of failure can be a black image.
The current documentation documentation specifies only IllegalStateException and NullPointerException.

Copy link
Member

Choose a reason for hiding this comment

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

Since SecurityException is deprecated, we should not throw that.

In a similar situation on macOS (user has not enabled screen capture), we return a blank (all black) image, so I think that's what we would do here.

Copy link
Member Author

@azvegint azvegint Jun 28, 2024

Choose a reason for hiding this comment

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

Updated to return a blank image if permission is not granted.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since SecurityException is deprecated, we should not throw that.

In a similar situation on macOS (user has not enabled screen capture), we return a blank (all black) image, so I think that's what we would do here.

No, SecurityException is NOT deprecated and there are no plans to deprecate it.

https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/SecurityException.html#%3Cinit%3E()

This kind of platform reason for a "SecurityException" is precisely why it isn't deprecated even though the Java SecurityManager is deprecated.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the correction about "SecurityException" not being deprecated.

I still think that for this PR, we should not throw an exception, since we A) don't specify it to do so, and B) don't do that on macOS in a nearly identical situation.

We could consider a future RFE to change the spec (and maybe the behavior).

} else if (retVal == ERROR) {
if (SCREENCAST_DEBUG) {
System.err.println("Screen capture failed.");
}
} else if (retVal == OUT_OF_BOUNDS) {
if (SCREENCAST_DEBUG) {
System.err.println(
"Token does not provide access to requested area.");
}
}
return retVal != ERROR;
}
}
Loading