Hi Jim,
Thanks for your valuable inputs. I have made recommended changes.
Please find updated webrev for review:
http://cr.openjdk.java.net/~jdv/7107905/webrev.08/
Thanks,
Jay
-----Original Message-----
From: Jim Graham
Sent: Friday, May 27, 2016 11:52 PM
To: Jayathirth D V; Philip Race
Cc: 2d-dev@openjdk.java.net
Subject: Re: Review Request for JDK-7107905: ColorModel subclasses are missing
hashCode() or equals() or both methods
I think that makes sense. If it has no type-specific information to compare,
then it doesn't need to override them...
...jim
On 05/27/2016 05:32 AM, Jayathirth D V wrote:
> Hi Jim,
>
> Since we will be moving comparison for ColorSpace and transferType to CM, can
> we remove equals() & hashCode() methods in CCM?
> For PCM and ICM we have unique variables which we can compare, there will be
> no problem.
>
> Thanks,
> Jay
>
> -----Original Message-----
> From: Jim Graham
> Sent: Friday, May 27, 2016 2:18 PM
> To: Jayathirth D V; Philip Race
> Cc: 2d-dev@openjdk.java.net
> Subject: Re: Review Request for JDK-7107905: ColorModel subclasses are
> missing hashCode() or equals() or both methods
>
> Hi Jay,
>
> .equals() should not be comparing any fields that are computed from other
> fields - generally fields that come directly from a constructor argument tend
> to be the fields to compare. Internal state variables (such as "isInited"
> and the like) should never be compared. It should be comparing fields that
> affect the interpretation of the color. To that end...
>
> In CM, add comparison/hash of:
> ColorSpace
> transferType
>
> In CCM, do not compare or hash:
> needScaleInit (internal state for lazy ScaleInit)
> noUnnorm (computed from other constructor arguments)
> nonStdScale (computed from other constructor arguments)
> signed (computed from other constructor arguments)
> transferType (tested in CM)
> (basically, CCM doesn't have anything to compares as all of its
> significant values are compared in the super class)
>
> ...jim
>
> On 05/27/2016 01:02 AM, Jayathirth D V wrote:
>> Hi,
>>
>> Gentle reminder.
>> Please review updated fix:
>> http://cr.openjdk.java.net/~jdv/7107905/webrev.07/
>>
>> Thanks,
>> Jay
>>
>> -----Original Message-----
>> From: Jayathirth D V
>> Sent: Thursday, May 19, 2016 6:43 PM
>> To: Philip Race; Jim Graham
>> Cc: 2d-dev@openjdk.java.net
>> Subject: Review Request for JDK-7107905: ColorModel subclasses are
>> missing hashCode() or equals() or both methods
>>
>> Hi,
>>
>> Previously for this bug we were making changes related only to
>> IndexColorModel. Since we are expanding to include hashCode() or equals()
>> method from PackedColorModel and ComponentColorModel, I have created single
>> webrev for review under the same bug id.
>>
>> Now the "getclass()==" check is present in base class ColorModel and each
>> subclass of ColorModel have their own equality check for properties.
>>
>> Details:
>> Bug : https://bugs.openjdk.java.net/browse/JDK-7107905
>> Updated Webrev : http://cr.openjdk.java.net/~jdv/7107905/webrev.07/
>>
>> Please review the changes at your convenience.
>>
>> Thanks,
>> Jay
>>
