I'm back, and given that the review will take some time anyway, I will sponsor this once the review is complete. I see that Guru uploaded the patch for you (thanks, Guru) so I can test it next week.

-- Kevin


Alexander Nyssen wrote:
It seems I was unsuccessful again. If somebody would be willing to sponsor this 
contribution while Kevin is away (or at least update the patch provided for 
JDK-8088147), I could mail the patch privately.

Regards,
Alexander

Am 11.07.2016 um 19:59 schrieb Alexander Nyssen <alexan...@nyssen.org>:

It seems my attachment has again been ‚consumed‘ by the list. Trying again with 
an archive containing the patch file.

Regards,
Alexander


Am 08.07.2016 um 23:28 schrieb Alexander Nyssen <alexan...@nyssen.org>:

Hi Kevin, all, attached please find a revised patch, where I have added the required export to PlatformImpl and have fixed the build script, so it does no longer break when being executed in jigsaw mode.
As swt is no named module yet, I have disabled the JUnit test in jigsaw mode 
for now. We would probably have to set up swt as a named module to enable this, 
and would further have to make swt-debug.jar available as an automated module 
on its module path. But this is a different problem.

Regards,
Alexander


Am 30.06.2016 um 12:14 schrieb Alexander Nyssen <alexan...@nyssen.org>:

Hi Kevin,

Am 30.06.2016 um 01:20 schrieb Kevin Rushforth <kevin.rushfo...@oracle.com>:

Hi Alexander,

I attached the patch to the bug:

https://bugs.openjdk.java.net/browse/JDK-8088147

If I build it and run the manual test in "legacy" mode, meaning run everything 
with 9+109 and the legacy jfxrt.jar file, then it runs and the cursor now changes. So 
this looks like a good starting point for a fix.
Fine.

I tried building and running this in Jigsaw mode (building with jdk-9+109, but 
running the tests with a more recent JDK that includes modularization support), 
and noticed two problems right away that must be addressed:

1. The unit tests for SWT are missing some of the jigsaw test tasks so the 
build fails right away with an exception from gradle:

Task with name 'jigsawTestsLinux' not found in project ':swt'.
Wiring up SWT-based tests to our unit test harness will take a bit more work 
than what you have provided (not even counting the Mac issue, which could be 
handled by excluding the test on Mac). In the short term, relying on manual 
tests for this fix might be best.

I did not execute the tests in jigsaw mode yet, because other tests failed in 
this mode, too (as indicated in an earlier discussion). I will try to set 
things up in a virtual machine with Windows and/or Linux so I can work on the 
Gradle tests without having to deal with the Mac issue. The test harness will 
IMHO also be required for other contributions, and it would of course be fine 
if the automated test, I included in this patch, could be executed as well.

2. You have introduced a dependency on a new internal package, 
com.sun.javafx.tk. If this is required in order to implement the fix, then you 
will need to add this package to the list of packages exports to javafx.swt in 
PlatformImpl; otherwise the following exception is thrown at runtime:

Exception in thread "JavaFX Application Thread" java.lang.IllegalAccessError: 
class javafx.embed.swt.SWTCursors (in module javafx.swt) cannot access class 
com.sun.javafx.tk.Toolkit (in module javafx.graphics) because module javafx.graphics does 
not export com.sun.javafx.tk to module javafx.swt
This dependency is required unless there is public API to convert a platform image (which is provided by the image cursor frame) to an image. To me, Image image = Toolkit.getImageAccessor().fromPlatformImage(cursorFrame.getPlatformImage());
seemed to be the way to go. I will thus add the respective export in a revised 
patch.

I won't have time to sponsor this change until the second half of July, but if 
others have time, the review can proceed and I'll pick it back up then if it is 
in good enough shape to run.
Especially setting up the SWT test harness will be kind of a blocker for 
succeeding contributions, so it would be nice if somebody could step in.

Regards,
Alexander

-- Kevin


Kevin Rushforth wrote:
Hi Alexander,

It looks like your patch was stripped out by the mailing list server.

Can you please send me the patch offline, as a zip file (so line endings are 
preserved across different systems), and I will unzip it and attach it to the 
bug report.

-- Kevin


Alexander Nyssen wrote:
Hi,

I have worked on a first contribution related to JDK-8088147. Attached please 
find a patch (created in extended Git format) that comprises the related 
changes. I have augmented the implementation of javafx.embed.swt.SWTCursors to 
handle the image cursor case. I further adjusted javafx.scene.Scene to update 
the cursor frame (in addition to the cursor) within synchronizeSceneProperties, 
so the cursor is not cleared in the first pulse succeeding the cursor property 
change.
I have added an automated JUnit test (SWTCursorsTest) to the swt module, as 
well as a manual test (SWTImageCursorTest) to the systemTests module, with 
which the proper behavior can be verified. As no tests for SWT existed so far, 
I updated the build.gradle and gradle.properties files to support an SWT_TEST 
option, which allows to handle them similar to Swing tests. I also added the 
respective SWT dependency to the systemTests module. Please note that the JUnit 
test can currently not be executed using Gradle on the Mac (where the manual 
test currently is the single option; the automated tests are disabled), because 
there SWT-based tests require the -XstartOnFirstThread option that is currently 
not supported by the Gradle test runner (see 
https://issues.gradle.org/browse/GRADLE-3290 for details). We would have to use 
an ant task as a workaround.

Regards,
Alexander




Reply via email to