Modify

Opened 3 months ago

Closed 2 months ago

Last modified 2 months 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 months ago.
Initial patch
18925.1.patch (5.2 KB) - added by taylor.smock 3 months ago.
Ask user to disable plugin if JavaFX is missing
18925.2.patch (5.9 KB) - added by taylor.smock 2 months 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 months ago.
Java 11+
18925.4.patch (3.2 KB) - added by taylor.smock 2 months ago.
Completely remove "UnsupportedLookAndFeelException"
18925.5.patch (3.2 KB) - added by taylor.smock 2 months ago.
Oracle removed their built-in JavaFX in Java 11.
18925.6.patch (3.2 KB) - added by taylor.smock 2 months ago.
Use PlatformManager
18925.7.patch (4.0 KB) - added by taylor.smock 2 months ago.
Add missing files

Download all attachments as: .zip

Change History (26)

Changed 3 months ago by taylor.smock

Attachment: 18925.patch added

Initial patch

comment:1 Changed 3 months ago by Don-vip

Keywords: java8 added; java 8 removed

comment:2 Changed 3 months ago by Don-vip

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

comment:3 Changed 3 months 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 3 months 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 months 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 months 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 months 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 months ago by Don-vip (previous) (diff)

Changed 2 months ago by taylor.smock

Attachment: 18925.2.patch added

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

Changed 2 months ago by taylor.smock

Attachment: 18925.3.patch added

Java 11+

Changed 2 months ago by taylor.smock

Attachment: 18925.4.patch added

Completely remove "UnsupportedLookAndFeelException"

comment:7 Changed 2 months 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 months 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 months ago by taylor.smock

Attachment: 18925.5.patch added

Oracle removed their built-in JavaFX in Java 11.

comment:9 Changed 2 months 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 months 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 months ago by taylor.smock

Attachment: 18925.6.patch added

Use PlatformManager

comment:11 in reply to:  10 Changed 2 months 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 months 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.

Version 0, edited 2 months ago by Don-vip (next)

comment:13 in reply to:  12 Changed 2 months 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 months ago by taylor.smock (previous) (diff)

Changed 2 months ago by taylor.smock

Attachment: 18925.7.patch added

Add missing files

comment:14 Changed 2 months 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 months ago by Don-vip (previous) (diff)

comment:15 Changed 2 months ago by Don-vip

Damn, the JavaFX 14 upgrade broke the Jenkins build

comment:16 Changed 2 months 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 months 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 months 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.