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

[John Hendrikx]

On Tue, 22 Oct 2024 05:51:04 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   scanline stride always measured in bytes
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/iio/javax/XImageLoader.java line 209:
> 
>> 207:                     : ImageStorage.ImageType.PALETTE;
>> 208: 
>> 209:                 var scanlineStride = switch(image.getSampleModel()) {
> 
> Just a general comment here, on the use of `var`; in my opinion, `var` helps the writer of the code, but almost never helps the reader of the code who now must read the RHS and manually determine what the type is.  As we should be aiming for maintainable code and code that's as easy to read as possible without having to guess (there's usually enough guessing involved already), I always find it hard how the use of `var` can be justified anywhere. 
> 
> Also in my experience, `var` complicates refactors as it will morph with the target type, making all refactor errors occur at the use location instead of the declaration site when `var` isn't used (ie. a refactor could indicate dozens of errors, while fixing 2 declarations could solve all of them).
> 
> So why make us guess that `scanlineStride` is an `int` here?  And why force us to read the right hand sides of `colorModel`, `palette` and `imageType`...

`scanlineStride` is useful for all the `ImageFrame` constructors, or do you think recomputing it (with the risk of it being different) is still the best course of action given that AWT supplies it and might be using some other alignment/padding?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1593#discussion_r1809992939