[OpenJDK 2D-Dev] Review Request for JDK-7107905: equals() method in IndexColorModel doesnt exist and it relies on ColorModel.equals() which is not strict

[Jim Graham]

On 4/12/2016 12:59 PM, Phil Race wrote:
> I am catching up on email here, and "+1" but a couple of comments
>
> - I suppose that we can't learn anything useful from
> "cm.validbits.equals(this.validbits)"
> since only the bits up to "map_size" should be tested ?

Perhaps if the constructors truncated it at the required size we could 
use equals().  I'm not sure that is worthwhile, given how rarely it is 
used.  I think it is used by default on some platforms (Windows?  X11?) 
if they have an 8-bit screen with a sparse colormap, but that should be 
pretty rare these days since nearly everything we use should be 24-bits 
these days.

I have a better idea, though.

But, since the field is assigned the original supplied value from the 
constructor, then the likelihood that a non-null value will be == 
identical to another colormap is likely larger than normal, perhaps we 
can make it faster by checking for == and then slipping into the faster 
test that simply compares the rgb[] values?  Something like:

if (validBits == cm.validBits) {
     compare rgb[] entries
} else if (validBits == null || cm.validBits == null) {
     return false;
} else {
     bigger loop that compares rgb[] and validBits() values
}

Note that if the two fields are == and non-null, then the entries in the 
rgb[] array for indices that are non-valid should have zeros in them 
because of the way that the colormap data is copied internally, so the 
rgb[] comparisons should be valid without checking the associated bits.

Potentially we could also use:

boolean fulltest;
if (validBits == cm.validBits) {
     fulltest = false;
} else if (validBits == null || cm.validBits == null) {
     return false;
} else if (validBits.equals(cm.validBits)) {
     fulltest = false;
} else {
     fulltest = true;
}

if (fulltest) {
     compare both rgb[] and validBits() values
} else {
     compare rgb[] entries alone
}

In this case we are using validBits.equals() to reduce the amount of 
testing in the loop, but are not basing a conclusion on whether the 
result of the comparison will be true or false.  equals() implies we 
don't need to compare its values bit by bit, but !equals() doesn't mean 
that the ICMs aren't equals()...

			...jim