RFR: 8307132: Cleanup the code of sun.java2d.cmm.lcms package
Sergey Bylokhov
serb at openjdk.org
Thu May 4 06:18:14 UTC 2023
On Wed, 3 May 2023 20:44:24 GMT, Phil Race <prr at openjdk.org> wrote:
>> The description from big to small:
>>
>> - Our code to handle the image types we support is too generic, the LCMSImageLayout class can handle, byte, short, int, and double types, and many various properties of the image layout. As a result, we pass a couple of good parameters to the LCMSImageLayout and have to use safeXX methods in it to calculate the final layout, and then validate it.
>> This patch moves the layout properties calculation to one place - the constructor of LCMSImageLayout, and from the outside of the class, we now pass only the data array and the number of components per pixel:
>> - The usage of Double type is removed, we do not use that type currently, and do not plan to support it in the future. Note that we support the float type, and I tried to implement it, but unfortunately, it is [intentionally ](https://github.com/mm2/Little-CMS/issues/356)slow. So will continue to use short type instead of float.
>> - Discussed a few times the `do { } while (false);` block is removed.
>
> src/java.desktop/share/native/liblcms/LCMS.c line 51:
>
>> 49: #define DT_BYTE sun_java2d_cmm_lcms_LCMSImageLayout_DT_BYTE
>> 50: #define DT_SHORT sun_java2d_cmm_lcms_LCMSImageLayout_DT_SHORT
>> 51: #define DT_INT sun_java2d_cmm_lcms_LCMSImageLayout_DT_INT
>
> I don't understand why this isn't a step backwards.
> It is overloading the "size" to mean the "type"
> How would you tell the difference between SHORT vs UNSIGNED SHORT - both 2 bytes,
> or INT vs FLOAT - both 4 bytes ?
> or LONG vs DOUBLE ..
> If we ever need to, then this all needs to be reverted.
> % cd java/lang
> % grep BYTES *java
> Byte.java: public static final int BYTES = SIZE / Byte.SIZE;
> Character.java: public static final int BYTES = SIZE / Byte.SIZE;
> Double.java: public static final int BYTES = SIZE / Byte.SIZE;
> Float.java: public static final int BYTES = SIZE / Byte.SIZE;
> Integer.java: public static final int BYTES = SIZE / Byte.SIZE;
> Long.java: public static final int BYTES = SIZE / Byte.SIZE;
> Short.java: public static final int BYTES = SIZE / Byte.SIZE;
right, the idea was to use one variable to select the supported type using its size.
Based on size we will switch what GetXXArrayElements to use. java right now supports these related functions:
GetByteArrayElements,
GetShortArrayElements,
GetIntArrayElements,
GetLongArrayElements,
GetFloatArrayElements,
GetDoubleArrayElements,
The images based on long format currently are not supported by java2d and lcms. And the "float" formats are really slow in LCMS as mentioned in the description.
>How would you tell the difference between SHORT vs UNSIGNED SHORT - both 2 bytes,
Both will use the same code path.
But I can switch this to using two different variables for type and size.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13732#discussion_r1184580685
More information about the client-libs-dev
mailing list