From ca76da403b61b4109486f1cd698f5fd01e15d1a6 Mon Sep 17 00:00:00 2001
From: Ben Fry <ben@fathom.info>
Date: Tue, 22 Jun 2021 11:39:13 -0400
Subject: [PATCH] major updates to JavaFX and modules handling

---
 java/libraries/javafx/build.xml               | 19 ++---
 .../src/processing/javafx/PGraphicsFX2D.java  |  2 +-
 java/src/processing/mode/java/JavaBuild.java  | 72 ++++++++++++++-----
 .../processing/mode/java/runner/Runner.java   | 35 ++++-----
 todo.txt                                      | 25 ++++---
 5 files changed, 97 insertions(+), 56 deletions(-)

diff --git a/java/libraries/javafx/build.xml b/java/libraries/javafx/build.xml
index b9cc4519b..7640c77f0 100644
--- a/java/libraries/javafx/build.xml
+++ b/java/libraries/javafx/build.xml
@@ -95,7 +95,8 @@
   <!-- extract the .jar files from the first javafx we run across -->
   <!-- <target name="unzip-gluon-jars" unless="${javafx.jars.exist}"> -->
   <target name="unzip-gluon-jars">
-    <echo message="Extracting jars from ${gluon.base}.zip" />
+    <property name="modules.path" value="${platform.path}/modules" />
+    <echo message="Extracting jars from ${gluon.base}.zip to ${modules.path}" />
     <!-- should javafx.properties be copyed? is it used for anything? [fry 210620] -->
 
     <!-- https://ant.apache.org/manual/Tasks/unzip.html -->
@@ -103,7 +104,7 @@
     <!-- !#($*#! the builds have *slightly* different classes in each release
          (WinPlatformFactory not in macOS .jar... FFS it 1100 bytes of glue code)
          So the .jar files go into the native subdirectories as well. -->
-    <unzip dest="${natives.path}" src="${gluon.base}.zip" overwrite="true">
+    <unzip dest="${modules.path}" src="${gluon.base}.zip" overwrite="true">
       <patternset>
         <include name="**/*.jar" />
         <!-- These two aren't supported/used -->
@@ -117,9 +118,9 @@
   </target>
 
   <target name="unzip-gluon-natives">
-    <echo message="Extracting ${gluon.base}.zip to ${natives.path}" />
+    <echo message="Extracting native libs from ${gluon.base}.zip to ${platform.path}" />
 
-    <unzip dest="${natives.path}" src="${gluon.base}.zip" overwrite="true">
+    <unzip dest="${platform.path}" src="${gluon.base}.zip" overwrite="true">
       <patternset>
         <include name="**/*.dll" />
         <include name="**/*.dylib" />
@@ -155,17 +156,17 @@
     <!-- javafx-${gluon.version}-sdk-${gluon.platform} -->
     <antcall target="retrieve-gluon">
       <param name="gluon.base" value="javafx-${gluon.version}-sdk-mac" />
-      <param name="natives.path" value="${library.path}/macosx" />
+      <param name="platform.path" value="${library.path}/macosx" />
     </antcall>
 
     <antcall target="retrieve-gluon">
       <param name="gluon.base" value="javafx-${gluon.version}-sdk-windows" />
-      <param name="natives.path" value="${library.path}/windows64" />
+      <param name="platform.path" value="${library.path}/windows64" />
     </antcall>
 
     <antcall target="retrieve-gluon">
       <param name="gluon.base" value="javafx-${gluon.version}-sdk-linux" />
-      <param name="natives.path" value="${library.path}/linux64" />
+      <param name="platform.path" value="${library.path}/linux64" />
     </antcall>
   </target>
 
@@ -361,8 +362,8 @@
     </condition>
     -->
 
-    <!-- just pick a platform; this should be sufficient for building -->
-    <property name="javafx.jar.path" value="library/macosx" />
+    <!-- just pick a platform; any should be sufficient for building -->
+    <property name="javafx.jar.path" value="library/macosx/modules" />
 
     <mkdir dir="bin" />
     <javac source="11" target="11"
diff --git a/java/libraries/javafx/src/processing/javafx/PGraphicsFX2D.java b/java/libraries/javafx/src/processing/javafx/PGraphicsFX2D.java
index 3c18a61ab..4e73a1709 100644
--- a/java/libraries/javafx/src/processing/javafx/PGraphicsFX2D.java
+++ b/java/libraries/javafx/src/processing/javafx/PGraphicsFX2D.java
@@ -1570,7 +1570,7 @@ public class PGraphicsFX2D extends PGraphics {
     PFont.Glyph glyph = textFont.getGlyph(ch);
     if (glyph != null) {
       if (textMode == MODEL) {
-        float bitmapSize = (float) textFont.getSize();
+        float bitmapSize = textFont.getSize();
         float high = glyph.height / bitmapSize;
         float wide  = glyph.width / bitmapSize;
         float leftExtent = glyph.leftExtent / bitmapSize;
diff --git a/java/src/processing/mode/java/JavaBuild.java b/java/src/processing/mode/java/JavaBuild.java
index 6b37b79f2..643de9ace 100644
--- a/java/src/processing/mode/java/JavaBuild.java
+++ b/java/src/processing/mode/java/JavaBuild.java
@@ -281,7 +281,11 @@ public class JavaBuild {
       if (library != null) {
         if (!importedLibraries.contains(library)) {
           importedLibraries.add(library);
-          classPath += library.getClassPath();
+          // don't add the JavaFX libraries to the classpath
+          // https://github.com/processing/processing4/issues/212
+          if (!library.getName().equals("JavaFX")) {
+            classPath += library.getClassPath();
+          }
           javaLibraryPath += File.pathSeparator + library.getNativePath();
         }
       } else {
@@ -440,13 +444,13 @@ public class JavaBuild {
   }
 
 
-  /** Returns the dummy "module" path so that JavaFX doesn't complain. */
-  public String getModulePath() {
-    // Just set this to the main core/library directory to pick up JavaFX
-    //return mode.getCoreLibrary().getLibraryPath();
-    File folder = new File(mode.getFolder(), "libraries/javafx/library");
-    return folder.getAbsolutePath();
-  }
+//  /** Returns the dummy "module" path so that JavaFX doesn't complain. */
+//  public String getModulePath() {
+//    // Just set this to the main core/library directory to pick up JavaFX
+//    //return mode.getCoreLibrary().getLibraryPath();
+//    File folder = new File(mode.getFolder(), "libraries/javafx/library");
+//    return folder.getAbsolutePath();
+//  }
 
 
   /** Return the java.library.path for this sketch (for all the native DLLs etc). */
@@ -831,6 +835,14 @@ public class JavaBuild {
       runOptions.append("-Djava.library.path=\"%EXEDIR%\\lib\"");
     }
 
+    Library javafx = findJavaFX();
+    if (javafx != null) {
+      String modulePath = exportPlatform == PConstants.MACOS ?
+        "$APP_ROOT/Contents/Java/modules" : "lib/modules";
+      for (String opt : getArgsJavaFX(modulePath)) {
+        runOptions.append(opt);
+      }
+    }
 
     /// macosx: write out Info.plist (template for classpath, etc)
 
@@ -843,6 +855,7 @@ public class JavaBuild {
         runOptionsXML.append('\n');
       }
 
+
       String PLIST_TEMPLATE = "Info.plist.tmpl";
       File plistTemplate = new File(sketch.getFolder(), PLIST_TEMPLATE);
       if (!plistTemplate.exists()) {
@@ -935,16 +948,13 @@ public class JavaBuild {
       for (String opt : runOptions) {
         jre.addChild("opt").setContent(opt);
       }
-
-      final String[] fxOptions = new String[]{
-        "--module-path=" + getModulePath(),
-        "--add-modules=javafx.base,javafx.graphics,javafx.swing",
-        "--add-exports=javafx.graphics/com.sun.javafx.geom=ALL-UNNAMED",
-        "--add-exports=javafx.graphics/com.sun.glass.ui=ALL-UNNAMED"
-      };
-      for (String opt : fxOptions) {
-        jre.addChild("opt").setContent(opt);
+      /*
+      if (javafx != null) {
+        for (String opt : getArgsJavaFX("lib")) {
+          jre.addChild("opt").setContent(opt);
+        }
       }
+      */
 
       config.save(configFile);
       project.save(buildFile);
@@ -1021,6 +1031,34 @@ public class JavaBuild {
   }
 
 
+  // This is a workaround until a more complete solution is found.
+  public Library findJavaFX() {
+    for (Library library : getImportedLibraries()) {
+      if (library.getName().equals("JavaFX")) {
+        return library;
+      }
+    }
+    return null;
+  }
+
+
+  static public String[] getArgsJavaFX(String modulePath) {
+    return new String[] {
+      "--module-path", modulePath,
+
+      // Full list of modules, let's not commit to all of these unless
+      // a compelling argument is made or a reason presents itself.
+      //"javafx.base,javafx.controls,javafx.fxml,javafx.graphics,javafx.media,javafx.swing,javafx.web"
+      "--add-modules", "javafx.base,javafx.graphics,javafx.swing",
+
+      // TODO Presumably, this is only because com.sun.* classes are being used?
+      // https://github.com/processing/processing4/issues/208
+      "--add-exports", "javafx.graphics/com.sun.javafx.geom=ALL-UNNAMED",
+      "--add-exports", "javafx.graphics/com.sun.glass.ui=ALL-UNNAMED"
+    };
+  }
+
+
   static Boolean xcodeInstalled;
 
   static protected boolean isXcodeInstalled() {
diff --git a/java/src/processing/mode/java/runner/Runner.java b/java/src/processing/mode/java/runner/Runner.java
index 995d948f5..02417cef7 100644
--- a/java/src/processing/mode/java/runner/Runner.java
+++ b/java/src/processing/mode/java/runner/Runner.java
@@ -205,7 +205,7 @@ public class Runner implements MessageConsumer {
 
   /**
    * Additional access to the virtual machine. TODO: may not be needed
-   * @return debugge VM or null if not running
+   * @return debugger VM or null if not running
    */
   public VirtualMachine vm() {
     return vm;
@@ -367,34 +367,29 @@ public class Runner implements MessageConsumer {
     // librariesClassPath will always have sep char prepended
     String javaLibraryPath = build.getJavaLibraryPath();
 
-    String javaLibraryPathParam = "-Djava.library.path=" +
-                  javaLibraryPath +
-                  File.pathSeparator +
-                  System.getProperty("java.library.path");
+    String javaLibraryPathParam =
+      "-Djava.library.path=" + javaLibraryPath +
+      File.pathSeparator +
+      System.getProperty("java.library.path");
 
     params.append(javaLibraryPathParam);
 
-    // TODO this should only happen with sketches using the JavaFX library
-    // https://github.com/processing/processing4/issues/209
-    params.append("--module-path");
-    params.append(build.getModulePath());
-    params.append("--add-modules");
-    //params.append("javafx.base,javafx.controls,javafx.fxml,javafx.graphics,javafx.media,javafx.swing,javafx.web");
-    params.append("javafx.base,javafx.graphics,javafx.swing");
-    // TODO Presumably, we need to move away from com.sun.* classes?
-    // https://github.com/processing/processing4/issues/208
-    params.append("--add-exports");
-    params.append("javafx.graphics/com.sun.javafx.geom=ALL-UNNAMED");
-    params.append("--add-exports");
-    params.append("javafx.graphics/com.sun.glass.ui=ALL-UNNAMED");
+    Library javafx = build.findJavaFX();
+    if (javafx != null) {
+      // The .jar files are in the same directory as the natives for windows64 et al,
+      // because the .jar files are ever-so-slightly different per-platform.
+      File modulesFolder = new File(javafx.getNativePath(), "modules");
+      for (String arg : JavaBuild.getArgsJavaFX(modulesFolder.getAbsolutePath())) {
+        params.append(arg);
+      }
+    }
 
     params.append("-cp");
     params.append(build.getClassPath());
 
     // enable assertions
-    // http://dev.processing.org/bugs/show_bug.cgi?id=1188
+    // http://processing.org/bugs/bugzilla/1188.html
     params.append("-ea");
-    //PApplet.println(PApplet.split(sketch.classPath, ':'));
 
     return params;
   }
diff --git a/todo.txt b/todo.txt
index b0347aa6e..8845f6840 100755
--- a/todo.txt
+++ b/todo.txt
@@ -56,9 +56,15 @@ X Only specify --modules-path when running JavaFX apps
 X   https://github.com/processing/processing4/issues/209
 _ lots more cleanup to do in javafx/build.xml
 
+major font cleanup
+X JDK fonts have been removed; fonts folder too? (not seeing it in the JDK)
+X   https://www.oracle.com/java/technologies/javase/11-relnote-issues.html#JDK-8191522
+X more reliable loading of default mono fonts
+X processing.mono used in preferences.txt
+X remove Preferences.getFont(), deal with incorrect usage
+X   getFont("editor.font") was returning getFont("editor.font.size")
+
 
-_ "Could not run" "For more information, read revisions.txt and Help → Troubleshooting."
-_   need to drop revisions.txt here and just reference Troubleshooting
 
 for next release
 _ debug JavaFX and Export to Application on Windows
@@ -66,9 +72,17 @@ _   this was working on Saturday, now broken after the move to a separate librar
 _   now "Art Station" is broken because it actually creates an FX2D sketch
 _     (and FX2D isn't on the classpath by default)
 _ debug JavaFX and Export to Application on Linux
+_ fix modules path warning for Tools in the PDE
+_   "WARNING: Unsupported JavaFX configuration" when running Tools that use JavaFX
+_   we were probably spared the warnings because the older JARs were hanging around
+_ update modules path when exporting application
+_ automatically import JavaFX if FX2D is in sketch? or tell user?
 _ Code completion not working
 _   https://github.com/processing/processing4/issues/177
 _   confirmed not working from Sam's repo either
+_ "Could not run" "For more information, read revisions.txt and Help → Troubleshooting."
+_   need to drop revisions.txt here and just reference Troubleshooting
+
 
 _ replace bug numbers
 _   http://dev.processing.org/bugs/show_bug.cgi?id=1188
@@ -80,13 +94,6 @@ _ macosx vs macosx64 in JavaFX
 _   the latter is making the export fail because it won't embed a Java VM
 _   may be because it's exporting twice and overwriting? or 64 takes precedence?
 
-_ fix modules path warning for Tools in the PDE
-_   "WARNING: Unsupported JavaFX configuration" when running Tools that use JavaFX
-_   we were probably spared the warnings because the older JARs were hanging around
-
-_ update modules path when exporting application
-
-_ automatically import JavaFX if FX2D is in sketch? or tell user?
 
 _ Remove usage of com.sun.* in JavaFX library
 _   https://github.com/processing/processing4/issues/208