On Sat, 9 Nov 2024 07:43:43 GMT, Michael Strauß <mstra...@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.rushfo...@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
