RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix [v6]

[Alexey Ivanov]

On Fri, 5 Jan 2024 13:18:46 GMT, Renjith Kannath Pariyangad <rkannathpari at openjdk.org> wrote:

>> There are at least two similar copy/paste typos.
>> In the next line https://github.com/openjdk/jdk/blob/f73dbb985823d2d59bfcff8ba6951a0d9eb2cefc/src/java.desktop/share/classes/java/awt/image/ColorConvertOp.java#L771 we use the srcNumComp which is the number of components for the incoming source color space, but the loop should be done over iccSrcNumComp which is a number of components of the "intermidiate" source color space. Similar issue is in the next line but for the destination https://github.com/openjdk/jdk/blob/f73dbb985823d2d59bfcff8ba6951a0d9eb2cefc/src/java.desktop/share/classes/java/awt/image/ColorConvertOp.java#L785
>> 
>> It is possible that these compensate/affects each other and produce some non-completly broken result.
>> The test from the comment above start to pass after these will be fixed. 
>> 
>> Since we found a few bugs in this code path, it will be useful to check the code path in the nonICCBIFilter where the CSList is not null(the intermidiate transform is wrapper as well, as of now our test uses the built-in ColorSpace.getInstance(CS_sRGB))
>
> @mrserb for avoiding _ArrayIndexOutOfBoundsException_ I have modified `CS_GRAY `with `CS_LINEAR_RGB ` for making same number of channels. But the test passed irrespective of fix.
> Do you find any alternative way to fail the test with out fix ?

> There are at least two similar copy/paste typos. In the next line
> 
> https://github.com/openjdk/jdk/blob/f73dbb985823d2d59bfcff8ba6951a0d9eb2cefc/src/java.desktop/share/classes/java/awt/image/ColorConvertOp.java#L771
> we use the srcNumComp which is the number of components for the incoming source color space, but the loop should be done over iccSrcNumComp which is a number of components of the "intermidiate" source color space. Similar issue is in the next line but for the destination
> 
> https://github.com/openjdk/jdk/blob/f73dbb985823d2d59bfcff8ba6951a0d9eb2cefc/src/java.desktop/share/classes/java/awt/image/ColorConvertOp.java#L785
> 
> It is possible that these compensate/affects each other and produce some non-completly broken result. The test from the comment above start to pass after these will be fixed.

Indeed, these should be `iccSrcNumComp` and `iccDstNumComp` according to number of components in the allocated arrays. Modifying these lines make the test pass.

> Since we found a few bugs in this code path, it will be useful to check the code path in the nonICCBIFilter where the CSList is not null(the intermidiate transform is wrapper as well, as of now our test uses the built-in ColorSpace.getInstance(CS_sRGB))

Agree, another (unit) test is required which exercises other code paths.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16895#discussion_r1443068123