On Thu, 2 Jul 2020 12:41:34 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> I tested this pull request on all of the following platforms: >> >> * JavaFX desktop platforms (*amd64* architecture) >> * Windows SDK on Windows 10 Pro Version 2004 >> * Mac OS X SDK on macOS 10.15.5 (Catalina) >> * Linux SDK on Ubuntu 16.04.6 LTS (Xenial Xerus) >> * JavaFX embedded platforms (*armhf* architecture) >> * armv6hf SDK (Monocle EPD) on Ubuntu 14.04.6 LTS (Trusty Tahr) >> * armv6hf SDK (Monocle VNC) on Ubuntu 20.04 LTS (Focal Fossa) >> >> This is a tricky timing problem on most platforms, so I used rather >> intrusive techniques to catch the error and verify >> it was fixed. The bug is easily visible only on the Monocle VNC platform. >> ### Desktop >> >> I tested the desktop platforms as follows: >> >> 1. I added `assert` statements and calls to `Thread.sleep` as shown by [this >> commit](https://github.com/jgneff/javafx-graphics/commit/ffc639e4) to catch >> the error. It looks like a lot of changes, >> but they simply call `sleep` to change the timing of the threads so that the >> JavaFX Application Thread is unable to >> keep up with the QuantumRenderer. The `assert` statements catch the error >> when the rendering thread starts looking for >> an unused buffer. >> All platforms printed many `InvalidMarkException` errors as the buffer >> position was modified while in use on the JavaFX >> application thread. The Linux and Windows platforms printed, "Exception >> in thread 'JavaFX Application Thread' >> java.nio.InvalidMarkException," while the macOS platform printed, >> "Exception in thread 'AppKit Thread' >> java.nio.InvalidMarkException." >> >> 2. I applied the fix to call `getBuffer` instead of `getPixels` in >> `QueuedPixelSource`. >> >> 3. After the fix, no errors were detected by the assertions on any of the >> platforms. >> >> ### Embedded >> >> I tested the embedded platforms as follows: >> >> 1. I added only `assert` statements to the `HeadlessScreen` and `EPDScreen` >> classes, as shown below. Both platforms >> printed many `InvalidMarkException` errors as the buffer position was >> modified while in use on the JavaFX application >> thread. 2. I applied the fix to call `getBuffer` instead of `getPixels` in >> `QueuedPixelSource`. >> >> 3. After the fix, no errors were detected by the assertions on either >> platform. >> >> #### `HeadlessScreen` and `EPDScreen` >> >> @Override >> public synchronized void uploadPixels(Buffer b, int x, int y, >> int width, int height, float alpha) { >> + assert b.mark() == b; >> pixels.composePixels(b, x, y, width, height, alpha); >> + assert b.reset() == b; >> } >> >> For the Monocle VNC platform, you can also simply connect and watch the bug >> corrupt the frames sent to the VNC client, >> as shown in my prior comment on this pull request. >> ### Other results >> >> I found some unexpected results, listed below. >> >> * JavaFX on Linux does not limit its frame rate to 60 Hz when using the >> hardware pipeline, reaching over 350 frames per >> second. >> >> * JavaFX on macOS ignores the system property `-Djavafx.animation.pulse=8` >> and runs at 60 Hz, even though it prints the >> message "Setting PULSE_DURATION to 8 hz." >> >> * JavaFX on macOS prints the error shown below when `Platform.exit` is >> called to end the application. The error also >> occurs on JavaFX 14.0.1 and 15-ea+6. The error does not occur when the >> window is closed manually using the mouse. >> >> Java has been detached already, but someone is still trying to use it at >> -[GlassViewDelegate dealloc]: >> /Users/john/src/jfx/modules/javafx.graphics/src/main/native-glass/mac/GlassViewDelegate.m:204 >> 0 libglass.dylib 0x000000010eb6c05d -[GlassViewDelegate dealloc] + 221 >> 1 libglass.dylib 0x000000010eb71af6 -[GlassView3D dealloc] + 246 >> 2 libobjc.A.dylib 0x00007fff66937054 >> _ZN19AutoreleasePoolPage12releaseUntilEPP11objc_object + 134 >> 3 libobjc.A.dylib 0x00007fff6691bdba objc_autoreleasePoolPop + 175 >> 4 CoreFoundation 0x00007fff2dad23c5 >> __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23 >> 5 CoreFoundation 0x00007fff2dad22f7 __CFRunLoopDoObservers + 457 >> 6 CoreFoundation 0x00007fff2dad1895 __CFRunLoopRun + 874 >> 7 CoreFoundation 0x00007fff2dad0ece CFRunLoopRunSpecific + 462 >> 8 libjli.dylib 0x000000010be945c1 CreateExecutionEnvironment + 399 >> 9 libjli.dylib 0x000000010be90752 JLI_Launch + 1354 >> 10 java 0x000000010be83ca1 main + 375 >> 11 libdyld.dylib 0x00007fff67acdcc9 start + 1 >> 12 ??? 0x000000000000000b 0x0 + 11 >> >> ### Test scripts >> >> Below are the scripts I used to run the tests. The scripts include the >> system property `-Djavafx.animation.pulse=8`, >> but I removed it when trying to recreate the bug with the added `assert` and >> `sleep` statements. >> #### linux.sh >> >> #!/bin/bash >> # Linux desktop >> $HOME/opt/jdk-14.0.1/bin/java -ea --show-version \ >> --module-path=$HOME/lib/javafx-sdk-dev/lib \ >> --add-modules=javafx.graphics \ >> -Dprism.order=sw -Djavafx.animation.pulse=8 \ >> -jar dist/epd-javafx.jar \ >> --width=800 --height=600 --pattern=3 --loops=0 >> >> #### macos.sh >> >> #!/bin/bash >> # macOS desktop >> $HOME/opt/jdk-14.0.1.jdk/Contents/Home/bin/java -ea --show-version \ >> --module-path=$HOME/lib/javafx-sdk-dev/lib \ >> --add-modules=javafx.graphics \ >> -Dprism.order=sw -Djavafx.animation.pulse=8 \ >> -jar dist/epd-javafx.jar \ >> --width=800 --height=600 --pattern=3 --loops=0 >> >> #### windows.sh >> >> #!/bin/bash >> # Windows desktop under Cygwin >> $HOME/opt/jdk-14.0.1/bin/java -ea --show-version \ >> --module-path=$(cygpath -m $HOME/lib/javafx-sdk-dev/lib) \ >> --add-modules=javafx.graphics \ >> -Dprism.order=sw -Djavafx.animation.pulse=8 \ >> -jar dist/epd-javafx.jar \ >> --width=800 --height=600 --pattern=3 --loops=0 >> >> #### windows.bat >> >> REM Windows desktop under Command Prompt >> C:\cygwin64\home\john\opt\jdk-14.0.1\bin\java -ea --show-version^ >> --module-path=C:/cygwin64/home/john/lib/javafx-sdk-dev/lib^ >> --add-modules=javafx.graphics^ >> -Dprism.order=sw -Djavafx.animation.pulse=8^ >> -jar dist\epd-javafx.jar^ >> --width=800 --height=600 --pattern=3 --loops=0 >> >> #### monocle-epd.sh >> >> Run with `sudo`. >> >> #!/bin/bash >> # Monocle EPD platform on Linux armhf >> $HOME/opt/jdk-14.0.1+7/bin/java -ea --show-version \ >> --module-path=$HOME/lib/armv6hf-sdk/lib \ >> --add-modules=javafx.graphics \ >> -Dglass.platform=Monocle -Dmonocle.platform=EPD -Dprism.order=sw \ >> -Dmonocle.input.18/0/0/0.maxX=800 -Dmonocle.input.18/0/0/0.maxY=600 \ >> -Dmonocle.epd.waveformMode=4 -jar dist/epd-javafx.jar \ >> --width=800 --height=600 --pattern=3 --loops=0 >> >> #### monocle-vnc.sh >> >> #!/bin/bash >> # Monocle VNC platform on Linux armhf >> /usr/lib/jvm/java-14-openjdk-armhf/bin/java -ea --show-version \ >> --module-path=$HOME/lib/armv6hf-sdk/lib \ >> --add-modules=javafx.graphics \ >> -Dprism.order=sw -Djavafx.animation.pulse=8 \ >> -Dglass.platform=Monocle -Dmonocle.platform=VNC \ >> -jar dist/epd-javafx.jar \ >> --width=1024 --height=600 --pattern=3 --loops=0 > >> Does this fix the years old Linux JavaFX buffer reset bug? > > Possibly. This is a race condition that can affect the use of > `UploadingPainter`, which is used by the SW pipeline. This fix might be a candidate for JavaFX 15, so I recommend to _not_ merge the master branch. If we don't spot anything of concern during the review, then we might ask you to retarget your PR to the `jfx15` branch. ------------- PR: https://git.openjdk.java.net/jfx/pull/255
