RFR: 8280982: [Wayland] [XWayland] java.awt.Robot taking screenshots [v8]

[Alexander Zvegintsev]

On Tue, 30 May 2023 19:33:20 GMT, Phil Race <prr at openjdk.org> wrote:

>> Alexander Zvegintsev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix macos build
>
> src/java.desktop/unix/classes/sun/awt/screencast/ScreencastHelper.java line 139:
> 
>> 137: 
>> 138:         for (TokenItem tokenItem : tokensForRectangle) {
>> 139:             retVal = getRGBPixelsImpl(
> 
> This can't be right. You surely want to check all cases of retVal not just the last one.

Updated.

> src/java.desktop/unix/classes/sun/awt/screencast/TokenStorage.java line 62:
> 
>> 60:     private static final String REL_NAME =
>> 61:             ".java/.robot/screencast-tokens.properties";
>> 62: 
> 
> .java is already hidden. Why make .robot hidden ?

Updated.

> src/java.desktop/unix/classes/sun/awt/screencast/TokenStorage.java line 150:
> 
>> 148:     }
>> 149: 
>> 150:     private static class WatcherThread extends Thread {
> 
> What is all this for ? Reading and writing a file doesn't need all this.

This is to work from multiple VM with the file.

We only read the file the first time we use `createScreenCapture` and when another VM externally modifies the file.

Alternatively we could read it before each use of `createScreenCapture`, but I wanted to avoid unnecessary reading and reduce the possible simultaneous writing and reading time.

> src/java.desktop/unix/classes/sun/awt/screencast/TokenStorage.java line 387:
> 
>> 385:         }
>> 386: 
>> 387:         try (OutputStream output = Files.newOutputStream(PROPS_PATH)) {
> 
> Note that this stream is un-buffered (slow)
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/nio/file/Files.html#newOutputStream(java.nio.file.Path,java.nio.file.OpenOption...)
> Maybe doesn't matter given how small the file is.

Moved to newBufferedWriter/Readed

> src/java.desktop/unix/native/libawt_xawt/awt/screencast_pipewire.c line 43:
> 
>> 41: 
>> 42: #define EXCEPTION_CHECK_DESCRIBE_CLEAR() if ((*env)->ExceptionCheck(env)) { \
>> 43:                                             (*env)->ExceptionDescribe(env); \
> 
> Describe already clears the exception
> https://docs.oracle.com/en/java/javase/17/docs/specs/jni/functions.html#exceptiondescribe

Updated.

> src/java.desktop/unix/native/libawt_xawt/awt/screencast_pipewire.c line 375:
> 
>> 373:     DEBUG_SCREENCAST("@@@ using screen %i\n", index);
>> 374:     if (index >= screenSpace.screenCount) {
>> 375:         DEBUG_SCREENCAST("⚠ Wrong index for screen\n", NULL);
> 
> I've seen this "! in a triangle" symbol in a quite a few places in the DEBUG messages.
> Looks like you got some non-ascii char in there.

Fixed.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13803#discussion_r1211860159
PR Review Comment: https://git.openjdk.org/jdk/pull/13803#discussion_r1211860753
PR Review Comment: https://git.openjdk.org/jdk/pull/13803#discussion_r1211727223
PR Review Comment: https://git.openjdk.org/jdk/pull/13803#discussion_r1211865131
PR Review Comment: https://git.openjdk.org/jdk/pull/13803#discussion_r1211864955
PR Review Comment: https://git.openjdk.org/jdk/pull/13803#discussion_r1211866052