On Mon, 8 Jan 2024 13:53:21 GMT, Alexey Ivanov <[email protected]> wrote:

>>> I think if condition missed `not (!)` (original code return _true_ if match 
>>> and _false_ if docent, in Sergey sample `compareImages ` returns _true_ if 
>>> not match and _false_ if match but `if (compareImages(destTest, destGold))` 
>>> same. @mrserb hope this observation is correct.
>> 
>> I believe Sergey's code in the test is correct. There are two images: 
>> `destTest` and `destGold`. The former is produced using the wrapper class, 
>> the latter is produced using the ICC color space. The wrapper 
>> `TestColorSpace` forwards all the calls to the `ICC_ColorSpace` that it 
>> wraps, therefore the transformed images must be *equal* because the applied 
>> transforms are absolutely the same.
>> 
>> To avoid any confusion, I suggest renaming `compareImages` to 
>> `areImagesEqual` which leaves no ambiguity for its return value. I've 
>> updated the code in [commit 
>> `1788ef6`](https://github.com/aivanov-jdk/jdk/commit/1788ef69958cefef7c65fdf37607f0410b868aff).
>> 
>>> Thank to @mrserb and @aivanov-jdk for your time and investigation, this 
>>> helped to bring up few more issues. What do you suggest to cover these 
>>> cases ?
>> 
>> I'm not sure… We want the filter to take another path, there could be a list 
>> of filters applied, if I understand @mrserb correctly. Sergey could be able 
>> to provide a more detailed guidance.
>
>> To avoid any confusion, I suggest renaming `compareImages` to 
>> `areImagesEqual` which leaves no ambiguity for its return value. I've 
>> updated the code in [commit 
>> `1788ef6`](https://github.com/aivanov-jdk/jdk/commit/1788ef69958cefef7c65fdf37607f0410b868aff).
> 
> @Renjithkannath, you can merge the changes I made into your branch using the 
> following command:
> 
> 
> git pull https://github.com/aivanov-jdk/jdk.git 
> renjith/sergey-8316497-colorConvertOp
> 
> 
> The command will merge the changes from the remote branch into your currently 
> checked out branch.

@aivanov-jdk Thank you for sharing the updated code I have merged your changes 
from the branch and pushed in.

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

PR Comment: https://git.openjdk.org/jdk/pull/16895#issuecomment-1882425761

Reply via email to