On Tue, 15 Oct 2024 22:29:52 GMT, John Hendrikx <jhendr...@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 > > modules/javafx.graphics/src/main/java/com/sun/javafx/image/impl/BaseByteToByteConverter.java > line 296: > >> 294: { >> 295: srcscanbytes -= w * 3; >> 296: srcscanbytes -= w * 4; > > This really doesn't look right. Should it not be `dstscanbytes` on the 2nd > line, and also multiplied by `3` instead of `4`? > > It looks like this was incorrect in the original? Also nitpicking: I think it's bad form to modify parameter values. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1593#discussion_r1802105879
