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

Renjith Kannath Pariyangad rkannathpari at openjdk.org
Fri Jan 5 13:21:29 UTC 2024


On Fri, 5 Jan 2024 03:32:10 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>>> Still, the current test catches the typo in the code srcColorSpace → dstColorSpace that's now replaced. From this point view, the stated bug is fixed.
>> 
>> But the implementation of TestColorSpacewe is wrong, and it may not work as expected after any other future updates.
>
> 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 ?

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

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


More information about the client-libs-dev mailing list