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

[Alexey Ivanov]

On Fri, 12 Jan 2024 09:53:46 GMT, Renjith Kannath Pariyangad <rkannathpari at openjdk.org> wrote:

>> Hi Reviewers, 
>> There was a typo for color conversion instead of dstColorSpace function srcColorSpace was used. Please review and let me know your suggestions if any. 
>> 
>> Renjith.
>
> Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Included wrapper check

Changes requested by aivanov (Reviewer).

test/jdk/java/awt/color/NonICCFilterTest.java line 140:

> 138: 
> 139:         ColorConvertOp gold = new ColorConvertOp(mid, null);
> 140:         gold.filter(srcGold, destGold);

Shouldn't both filter use `gold` instead?

Then later, you create `test` with a wrapper instead?

test/jdk/java/awt/color/NonICCFilterTest.java line 151:

> 149: 
> 150:         gold = new ColorConvertOp(mid, null);
> 151:         gold.filter(srcGold, destGold);

Should both filter use the same instance of `ColorConvertOp` where the first one is `gold` and is applied to both `gold-` and `test-` images, then the next instance `ColorConvertOp` is a `test` with the wrapper which is again applied to both images?

Wouldn't it be clearer?

Suggestion:

        ColorConvertOp gold = new ColorConvertOp(createCS(ColorSpaceSelector.PYCC), null);
        gold.filter(srcTest, destTest);
        gold.filter(srcGold, destGold);

        if (!areImagesEqual(destTest, destGold)) {
            throw new RuntimeException("ICC test failed");
        }

        ColorConvertOp test = new ColorConvertOp(createCS(ColorSpaceSelector.WRAPPED_PYCC), null);
        test.filter(srcTest, destTest);
        test.filter(srcGold, destGold);


Currently, we have `test` and `gold` which use the same `ColorSpace` which could be plain or wrapper for both cases. It looks confusing to me.

What do you think, @mrserb?

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

PR Review: https://git.openjdk.org/jdk/pull/16895#pullrequestreview-1819034007
PR Review Comment: https://git.openjdk.org/jdk/pull/16895#discussion_r1450918862
PR Review Comment: https://git.openjdk.org/jdk/pull/16895#discussion_r1450923724