review changes

mstr2 · Oct 16, 2024 · 2b40cf6 · 2b40cf6
1 parent e6680e6
commit 2b40cf6
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 27 deletions.
diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ImageStorage.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ImageStorage.java
@@ -46,6 +46,7 @@
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Map.Entry;
+import java.util.Optional;
 
 /**
  * A convenience class for simple image loading. Factories for creating loaders
@@ -178,7 +179,7 @@ public static enum ImageType {
      */
     private final HashMap<String, ImageLoaderFactory> loaderFactoriesByMimeSubtype;
     private final ImageLoaderFactory[] loaderFactories;
-    private final ImageLoaderFactory xImageLoaderFactory = tryGetXImageLoaderFactory();
+    private Optional<ImageLoaderFactory> xImageLoaderFactory;
     private int maxSignatureLength;
 
     private static final boolean isIOS = PlatformUtil.isIOS();
@@ -191,14 +192,6 @@ public static ImageStorage getInstance() {
         return InstanceHolder.INSTANCE;
     }
 
-    private static ImageLoaderFactory tryGetXImageLoaderFactory() {
-        try {
-            Class<?> factoryClass = Class.forName("com.sun.javafx.iio.javax.XImageLoaderFactory");
-            return (ImageLoaderFactory)factoryClass.getMethod("getInstance").invoke(null);
-        } catch (ReflectiveOperationException e) {
-            return null;
-        }
-    }
 
     public ImageStorage() {
         if (isIOS) {
@@ -423,9 +416,7 @@ public ImageFrame[] loadAll(String input, ImageLoadListener listener,
                     } else {
                         // If we don't have a built-in loader factory we try to find an ImageIO loader
                         // that can load the content of the data URI.
-                        ImageLoader imageLoader = xImageLoaderFactory != null
-                            ? xImageLoaderFactory.createImageLoader(new ByteArrayInputStream(dataUri.getData()))
-                            : null;
+                        ImageLoader imageLoader = tryCreateXImageLoader(new ByteArrayInputStream(dataUri.getData()));
 
                         if (imageLoader == null) {
                             throw new IllegalArgumentException(
@@ -553,9 +544,9 @@ private ImageLoader findImageLoader(InputStream stream, ImageLoadListener listen
         stream.mark(Integer.MAX_VALUE);
         ImageLoader loader = getLoaderBySignature(stream, listener);
 
-        if (loader == null && xImageLoaderFactory != null) {
+        if (loader == null) {
             stream.reset();
-            loader = xImageLoaderFactory.createImageLoader(stream);
+            loader = tryCreateXImageLoader(stream);
         }
 
         return loader;
@@ -609,4 +600,26 @@ private ImageLoader getLoaderBySignature(InputStream stream, ImageLoadListener l
         // not found
         return null;
     }
+
+    /**
+     * Tries to create an {@link com.sun.javafx.iio.javax.XImageLoader} for the specified input stream.
+     * This might fail in the future if the {@code java.desktop} module is not present on the module path.
+     * At present, this will not fail because JavaFX requires the {@code java.desktop} module.
+     */
+    private synchronized ImageLoader tryCreateXImageLoader(InputStream stream) throws IOException {
+        if (xImageLoaderFactory == null) {
+            try {
+                Class<?> factoryClass = Class.forName("com.sun.javafx.iio.javax.XImageLoaderFactory");
+                xImageLoaderFactory = Optional.of((ImageLoaderFactory)factoryClass.getMethod("getInstance").invoke(null));
+            } catch (ReflectiveOperationException e) {
+                xImageLoaderFactory = Optional.empty();
+            }
+        }
+
+        if (xImageLoaderFactory.isEmpty()) {
+            return null;
+        }
+
+        return xImageLoaderFactory.get().createImageLoader(stream);
+    }
 }
diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/iio/javax/XImageLoader.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/iio/javax/XImageLoader.java
@@ -209,25 +209,15 @@ private static ByteBuffer getByteBuffer(DataBuffer buffer) {
         byte[] data = byteBuffer.getData();
         int offset = byteBuffer.getOffset();
         int size = byteBuffer.getSize();
-
-        if (offset == 0 && size == data.length) {
-            return ByteBuffer.wrap(data);
-        }
-
-        return ByteBuffer.wrap(Arrays.copyOf(data, size - offset));
+        return ByteBuffer.wrap(data, offset, size);
     }
 
     private static IntBuffer getIntBuffer(DataBuffer buffer) {
         DataBufferInt byteBuffer = (DataBufferInt)buffer;
         int[] data = byteBuffer.getData();
         int offset = byteBuffer.getOffset();
         int size = byteBuffer.getSize();
-
-        if (offset == 0 && size == data.length) {
-            return IntBuffer.wrap(data);
-        }
-
-        return IntBuffer.wrap(Arrays.copyOf(data, size - offset));
+        return IntBuffer.wrap(data, offset, size);
     }
 
     private final class LoadListenerImpl implements IIOReadProgressListener, IIOReadWarningListener {

diff --git a/modules/javafx.graphics/src/main/java/com/sun/prism/Image.java b/modules/javafx.graphics/src/main/java/com/sun/prism/Image.java
@@ -222,7 +222,6 @@ public static Image convertImageFrame(ImageFrame frame) {
 
         // GRAY, RGB, BGRA_PRE, and INT_ARGB_PRE are directly supported by Prism.
         // We'll need to convert all other formats that we might encounter to one of the supported formats.
-        // TODO: 3D - need a way to handle pre versus non-Pre
         return switch (type) {
             case GRAY -> fromByteGrayData((ByteBuffer)frame.getImageData(), w, h, stride, ps);
             case RGB -> fromByteRgbData((ByteBuffer)frame.getImageData(), w, h, stride, ps);