RFR: 8306707: Support pluggable image loading via javax.imageio [v19]

[John Hendrikx]

On Sat, 9 Nov 2024 07:43:43 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> This PR is an improved version of #1093.
>> 
>> JavaFX can load BMP, GIF, PNG, and JPEG images with its built-in image loaders. It has been a long-standing request to support more image formats, most notably (but not limited to) SVG. However, adding more built-in image loaders is a significant effort not only in creating the functionality, but also in maintaining the additional dependencies.
>> 
>> This will probably not happen any time soon, so we are left with three alternatives:
>> 1. Accept the fact that JavaFX will never be able to load additional image formats.
>> 2. Create a public image loader API, and hope that developers in the JavaFX ecosystem will create image loader plugins.
>> 3. Leverage the existing Java Image I/O API.
>> 
>> From these options, I think we should simply support existing Java APIs; both because it is the shortest and most realistic path forward, but also because I don't think it is sensible to bifurcate pluggable image loading in the Java ecosystem.
>> 
>> Of course, Java Image I/O is a part of the `java.desktop` module, which as of now, all JavaFX applications require. However, it has been noted in the previous PR that we shouldn't lock JavaFX into the `java.desktop` dependency even further.
>> 
>> I've improved this PR to not permanently require the `java.desktop` dependency: if the module is present, then JavaFX will use Image I/O for image formats that it can't load with the built-in loaders; if the module is not present, only the built-in loaders are available.
>> 
>> I have prepared a small sample application that showcases how the feature can be used to load SVG images in a JavaFX application: https://github.com/mstr2/jfx-imageio-sample
>
> Michael Strauß has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 33 commits:
> 
>  - Merge branch 'master' into feature/ximageloader
>  - move automatically added imports
>  - rename test...javax -> test...java2d
>  - Merge branch 'master' into feature/ximageloader
>  - Merge branch 'master' into feature/ximageloader
>  - rename iio.javax -> iio.java2d
>  - revert doc change
>  - Merge branch 'master' into feature/ximageloader
>  - validate max image width/height
>  - catch NoClassDefFoundError
>    
>    Co-authored-by: Kevin Rushforth <kevin.rushforth at oracle.com>
>  - ... and 23 more: https://git.openjdk.org/jfx/compare/d0011b21...d8d2a9c1

This looks good, I will re-approve if you decide to apply my suggestion for the exception messages.

modules/javafx.graphics/src/main/java/com/sun/javafx/iio/common/ImageTools.java line 216:

> 214:     public static void validateMaxDimensions(double width, double height, double scaleFactor) {
> 215:         if (width * scaleFactor > Integer.MAX_VALUE) {
> 216:             throw new IllegalArgumentException("Image width exceeds maximum value");

You may want to include the values used to reach this conclusion here and with the other 2 exceptions; if these exceptions ever come up, that's the first thing a developer will likely want to know (which may be you if this is posted in an issue).

-------------

Marked as reviewed by jhendrikx (Reviewer).

PR Review: https://git.openjdk.org/jfx/pull/1593#pullrequestreview-2432927798
PR Review Comment: https://git.openjdk.org/jfx/pull/1593#discussion_r1840156132