On Fri, 5 Jan 2024 16:16:51 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:

>> @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.

These typos also fixed

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

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

Reply via email to