Modify

Opened 3 weeks ago

Closed 2 weeks ago

Last modified 2 weeks ago

#18925 closed enhancement (fixed)

[WIP PATCH] JavaFX on Java 8

Reported by: taylor.smock Owned by: Don-vip
Priority: normal Milestone:
Component: Plugin javafx Version:
Keywords: java8 Cc:

Description

I know that only the "Oracle" java distributions have openjfx on Java 8 (linux distributions do have an openjfx package for Java 8).

I'm working on this since I implemented a date picker in Mapillary, and that requires JavaFX, and I don't want to unnecessarily force people off of Oracle Webstart just yet (see #17858).

Attachments (8)

18925.patch (5.4 KB) - added by taylor.smock 3 weeks ago.
Initial patch
18925.1.patch (5.2 KB) - added by taylor.smock 2 weeks ago.
Ask user to disable plugin if JavaFX is missing
18925.2.patch (5.9 KB) - added by taylor.smock 2 weeks ago.
Extend message given to users to indicate how they can fix the issue
18925.3.patch (5.9 KB) - added by taylor.smock 2 weeks ago.
Java 11+
18925.4.patch (3.2 KB) - added by taylor.smock 2 weeks ago.
Completely remove "UnsupportedLookAndFeelException"
18925.5.patch (3.2 KB) - added by taylor.smock 2 weeks ago.
Oracle removed their built-in JavaFX in Java 11.
18925.6.patch (3.2 KB) - added by taylor.smock 2 weeks ago.
Use PlatformManager
18925.7.patch (4.0 KB) - added by taylor.smock 2 weeks ago.
Add missing files

Download all attachments as: .zip

Change History (26)

Changed 3 weeks ago by taylor.smock

Attachment: 18925.patch added

Initial patch

comment:1 Changed 3 weeks ago by Don-vip

Keywords: java8 added; java 8 removed

comment:2 Changed 3 weeks ago by Don-vip

Please don't throw UnsupportedLookAndFeelException when JavaFX is not here. Just log an error.

comment:3 Changed 2 weeks ago by taylor.smock

Replying to Don-vip:

Please don't throw UnsupportedLookAndFeelException when JavaFX is not here. Just log an error.

I was trying to figure out the best way to indicate (to the user) that things might not work properly, and that they should either update Java to use OpenJFX 11+, or use Oracle Java, or to install the Linux distribution's openjfx package. I was (mostly) using UnsupportedLookAndFeelException as a temporary measure.

I think I found a better way (see the next patch), but the verbiage could definitely use a bit of help.

Changed 2 weeks ago by taylor.smock

Attachment: 18925.1.patch added

Ask user to disable plugin if JavaFX is missing

comment:4 in reply to:  3 Changed 2 weeks ago by Don-vip

Replying to taylor.smock:

I was trying to figure out the best way to indicate (to the user) that things might not work properly, and that they should either update Java to use OpenJFX 11+, or use Oracle Java, or to install the Linux distribution's openjfx package.

If you display a message, please only list the first and last option. I don't want to promote Oracle in any way, after all the mess they created.

comment:5 Changed 2 weeks ago by taylor.smock

Right now, I'm telling the user "JavaFX is not available" with PluginHandler.confirmDisablePlugin. I think that is (relatively) clear, but I'm not a standard end user.

If I were to expand the message, maybe something like "JavaFX is not available." + (Platform == "linux" ? "Please install OpenJFX through your package manager." : "Please update to Java 11");

comment:6 in reply to:  5 Changed 2 weeks ago by Don-vip

Replying to taylor.smock:

If I were to expand the message, maybe something like "JavaFX is not available." + (Platform == "linux" ? "Please install OpenJFX through your package manager." : "Please update to Java 11");

Perfect! Technically, "11+"

Last edited 2 weeks ago by Don-vip (previous) (diff)

Changed 2 weeks ago by taylor.smock

Attachment: 18925.2.patch added

Extend message given to users to indicate how they can fix the issue

Changed 2 weeks ago by taylor.smock

Attachment: 18925.3.patch added

Java 11+

Changed 2 weeks ago by taylor.smock

Attachment: 18925.4.patch added

Completely remove "UnsupportedLookAndFeelException"

comment:7 Changed 2 weeks ago by Don-vip

Not sure to understand the logic here:

boolean isJavaFx = Utils.getSystemProperty("javafx.runtime.version") != null || Arrays.asList("Oracle Corporation").contains(Utils.getSystemProperty("java.vendor"));

If you run Oracle Java 11+, you don't have JavaFX included anymore.

comment:8 Changed 2 weeks ago by taylor.smock

I added that since (on my system), Oracle Java 8 doesn't report javafx.runtime.version. Even though it should. I'll upload a patch that fixes that.

Changed 2 weeks ago by taylor.smock

Attachment: 18925.5.patch added

Oracle removed their built-in JavaFX in Java 11.

comment:9 Changed 2 weeks ago by Don-vip

Thanks. Final comment (sorry to not have done all of them in a single pass):

String os = Utils.getSystemProperty("os.name").toLowerCase();
if (Arrays.asList("windows", "mac os").contains(os)) {

please rather use

if (PlatformManager.isPlatformWindows() || PlatformManager.isPlatformOsx()) {

comment:10 Changed 2 weeks ago by taylor.smock

No problem. I thought there was a method somewhere in Utils, but I didn't see it.

I've been somewhat busy trying to figure out how to get Mapillary to load ~1600 icons without killing JOSM (VisualVM indicated that was the major time sink for loading my experimental Mapillary branch -- like 60s time sink).

This has provided an opportunity for me to think about something else for brief periods of time, which has given me an opportunity to come back to my 1600 icon issue with a different mindset.

Changed 2 weeks ago by taylor.smock

Attachment: 18925.6.patch added

Use PlatformManager

comment:11 in reply to:  10 Changed 2 weeks ago by Don-vip

Replying to taylor.smock:

I've been somewhat busy trying to figure out how to get Mapillary to load ~1600 icons without killing JOSM (VisualVM indicated that was the major time sink for loading my experimental Mapillary branch -- like 60s time sink).

This has provided an opportunity for me to think about something else for brief periods of time, which has given me an opportunity to come back to my 1600 icon issue with a different mindset.

:)

comment:12 Changed 2 weeks ago by Don-vip

I guess you also set the plugin.minimum.java.version in build.xml to 8? Otherwise I don't see how the plugin could work with Java 8, as JOSM would not load the plugin.

EDIT: or simply removed the property.

Last edited 2 weeks ago by Don-vip (previous) (diff)

comment:13 in reply to:  12 Changed 2 weeks ago by taylor.smock

Replying to Don-vip:

I guess you also set the plugin.minimum.java.version in build.xml to 8? Otherwise I don't see how the plugin could work with Java 8, as JOSM would not load the plugin.

EDIT: or simply removed the property.

It looks like I just removed the property.
EDIT: I thought I modified the .project file, but it looks like I accidentally added an ivy nature when I shouldn't have (I was avoiding running a standalone svn diff since I have the differences for #18747 in the same directory).

Last edited 2 weeks ago by taylor.smock (previous) (diff)

Changed 2 weeks ago by taylor.smock

Attachment: 18925.7.patch added

Add missing files

comment:14 Changed 2 weeks ago by Don-vip

Resolution: fixed
Status: newclosed

Fixed in [o35371:35373]. I also upgraded to JavaFX 14, as it remains compatible with Java 11.

Last edited 2 weeks ago by Don-vip (previous) (diff)

comment:15 Changed 2 weeks ago by Don-vip

Damn, the JavaFX 14 upgrade broke the Jenkins build

comment:16 Changed 2 weeks ago by taylor.smock

You have to worry about the side FX of upgrading...

Sorry, I can't help myself.

On a more serious note, I assume you fixed it with [o35374]. Does that mean that Mapillary/Microsoft Streetside/JavaFX are going to be built (by default) with Java 11, which means they will not work with Java 8?

If so, should I set the source/target for them in the build.xml to 1.8?

The major issue with that approach is that javadocs don't get generated, so the build fails. Its easy enough to "work around" by making the directory its looking for, but I don't believe javadocs are actually generated.

comment:17 in reply to:  16 ; Changed 2 weeks ago by Don-vip

Replying to taylor.smock:

ODoes that mean that Mapillary/Microsoft Streetside/JavaFX are going to be built (by default) with Java 11, which means they will not work with Java 8?

Built with Java 11, but in 1.8 mode. I looked at the .class files and it seems to be binary compatible with 8. Can you please check?

If so, should I set the source/target for them in the build.xml to 1.8?

It should already be the case, as it is defined in build-common.xml.

The major issue with that approach is that javadocs don't get generated, so the build fails. Its easy enough to "work around" by making the directory its looking for, but I don't believe javadocs are actually generated.

With Java 8 or 11?

comment:18 in reply to:  17 Changed 2 weeks ago by taylor.smock

Replying to Don-vip:

Built with Java 11, but in 1.8 mode. I looked at the .class files and it seems to be binary compatible with 8. Can you please check?

Looks like it -- I built with Java 11, then ran with Java 8.

It should already be the case, as it is defined in build-common.xml.

With <property name="java.lang.version" value="1.8" />, presumably. I've never seen this before, but it looks like it works. :)

With Java 8 or 11?

Java 11, when I have the following in the build.xml:

<property name="ant.build.javac.source" value="1.8"/>
<property name="ant.build.javac.target" value="1.8"/>

(technically, I can get away with just one, but I do both for clarity).

I think I started using that when I first starting making plugins for JOSM, when I had an issue with the class version being 55 when the JVM supported 52 (I was including ../build-common.xml at the time, IIRC).

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Don-vip.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.