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

Reply via email to