On Fri, 10 Jun 2022 09:39:48 GMT, Martin Desruisseaux <[email protected]> wrote:
> `AffineTransform.equals(Object)` and `hashCode()` break two contracts:
>
> * `A.equals(A)` returns `false` if at least one affine transform coefficient
> is NaN.
> * `A.equals(B)` should imply `A.hashCode() == B.hashCode()`, but it is not
> the case if a coefficient is zero with an opposite sign in A and B.
>
> This patch preserves the current behaviour regarding 0 (i.e. -0 is considered
> equal to +0) for backward compatibility reason. Instead the `hashCode()`
> method is updated for being consistent with `equals(Object)` behaviour.
Indeed, in this proposed patch +0 is considered equal to -0 mainly for
preserving current behaviour. The proposed new `AffineTransform.equals(Object)`
has a behaviour different than the old one only regarding NaN, which should be
rare in `AffineTransform`. So for practical purposes; the `equals` method would
be basically unchanged in this patch proposal.
But there is also an additional reason. It is because two `AffineTransform`
instances with the same coefficients except for the sign of zero will compute
the same points when a `transform(…)` method is invoked (except when a result
is zero, in which case the sign of that 0 may be different in some cases). So
those two transforms can be considered as equal for the purpose of transforming
points. There is a possibility that some existing codes rely on this equality
(which is not mathematically wrong I think), especially since negative zeros
appear easily in `AffineTransform`. Consider for example a translation on only
one axis (line break added for clarity):
AffineTransform[[1.0, 0.0, 2.0],
[0.0, 1.0, 0.0]]
A call to `AffineTransform.createInverse()` gives:
AffineTransform[[1.0, 0.0, -2.0],
[0.0, 1.0, -0.0]]
Both translation terms become negative (as expected), including zero. But
transforming points with that `AffineTransform` will give results strictly
identical (except for the sign of some 0 coordinate values) to an
`AffineTransform` created by `getTranslateInstance(-2, 0)`, so the current
behaviour of considering them as equal may not be wrong.
-------------
PR: https://git.openjdk.org/jdk/pull/9121