RFR: 8290973: In AffineTransform, equals(Object) is inconsistent with hashCode()

[Joe Darcy]

On Thu, 4 Aug 2022 22:21:19 GMT, Phil Race <prr at openjdk.org> wrote:

> I've run all tests we have and not too surprised they pass since the affected values probably aren't being exercised.
> 
> However I've been reading the docs about NaN and +/-0 equivalence at https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Double.html since if we change anything it probably ought to align with what ever says there.
> 
> Since I recalled that NaN == NaN will always return false, it seemed what you have was not consistent with that. Whilst this is true it seems Double.equals() does the opposite .. so OK .. although it surprised me.
> 
> But then the docs say +0 and -0 should not be equal whereas for backwards compatibility you say you extended them being treated as equal from equals() into hashCode() ..
> 
> Not sure why only in this case was the compatibility important.
> 
> I'd like to hear what @jddarcy (Joe Darcy) thinks about all of this.

For the equals contract, members of an equivalence class should all be equal to each other, meaning they are substitutable for each other in some sense. Different equivalence classes can be defined for the same set of values, one notable example being == (object identity) versus calling .equals().

>From my reading of AffineTransform, it should be an acceptable substitution to use -0.0 rather +0.0. As noted a NaN value is "the same" as another NaN in the sense being tested in this method.

One way to express this would be to define

private static boolean equiv(double a, double b) {
    return Double.compare(a + 0.0, b + 0.0);
}

Adding 0.0 turn -0.0 into +0.0 and has no other effect. Double.compare treats NaN inputs as the same. This could then do pair-wise comparison of the various components, 

return equiv(m00, a.m00) && equiv(m01, a.m01) && ...

If this is the equals function, than the hash can be a combination of

Double.doubleToLongBits(m01 +0.0) ...

using adding 0.0 adding to remove the case of -0.0. As is already being done, using doubleToLongBits instead of doubleToRawLongBits is important so that all NaN values get mapped the same.

A different equals and hashCode could be constructed if all 90 degree rotations (of the same length) were considered the same, but that would require more work to construct.

HTH

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

PR: https://git.openjdk.org/jdk/pull/9121