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

[Michael Strauß]

On Tue, 15 Oct 2024 22:47:38 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review changes
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/image/impl/IntBgr.java line 68:
> 
>> 66:             int b = (pixel >> 16) & 0xff;
>> 67:             return (255 << 24) | (r << 16) | (g << 8) | b;
>> 68:         }
> 
> Not sure if we optimizing here, but could eliminate two shifts like this:
> 
> Suggestion:
> 
>         public int getArgb(int[] arr, int offset) {
>             int pixel = arr[offset];
>             int r = pixel & 0xff;
>             int g = pixel & 0xff00;
>             int b = pixel & 0xff0000;
>             return (255 << 24) | (r << 16) | g | (b >> 16);
>         }

There are many such cases, and I'm not sure if it is really important to optimize this.

> modules/javafx.graphics/src/main/java/com/sun/javafx/image/impl/IntRgb.java line 43:
> 
>> 41:     public static final IntPixelAccessor accessor = Accessor.instance;
>> 42: 
>> 43:     public static IntToIntPixelConverter ToIntArgbPreConverter() {
> 
> This method is not following Java naming conventions:
> 
> Suggestion:
> 
>     public static IntToIntPixelConverter toIntArgbPreConverter() {

Yes, but again, the code style is kind of special here. All similar methods are named like that...

> modules/javafx.graphics/src/main/java/com/sun/prism/Image.java line 225:
> 
>> 223:         // GRAY, RGB, BGRA_PRE, and INT_ARGB_PRE are directly supported by Prism.
>> 224:         // We'll need to convert all other formats that we might encounter to one of the supported formats.
>> 225:         // TODO: 3D - need a way to handle pre versus non-Pre
> 
> Is this TODO still relevant?

Probably not. In any case, I don't know what the todo specifically wants us to do.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1593#discussion_r1803193449
PR Review Comment: https://git.openjdk.org/jfx/pull/1593#discussion_r1803195074
PR Review Comment: https://git.openjdk.org/jfx/pull/1593#discussion_r1803188704