[OpenJDK 2D-Dev] Patch submission: javadoc typos, javac warnings, AffineTransform.hashCode() and others

[Martin Desruisseaux]

Hello all

This is my first post on this mailing list. To introduce myself, I'm a developer 
of the open source GeoAPI (http://www.geoapi.org) and Geotoolkit.org 
(http://www.geotoolkit.org) projects. We use Java2D for rendering maps, but also 
Image I/O for raster data and we make extensive use of AffineTransform for 
calculation purpose (as a step in a chain of operations performing map projections).

I published here a first set of proposed patches: http://webrev.geomatys.com/

I think most of them are minors. The most questionable one is probably the patch 
for the AffineTransform equals/hashCode methods. The intend is to make 
AffineTransform fully compliant with the contract described in 
Object.equals(Object) and Object.hashCode() javadoc, in particular:

  * Object.equals is reflexive: for any non-null reference value x, x.equals(x)
    should return true.
  * If two objects are equal according to the equals(Object) method, then
    calling the hashCode method on each of the two objects must produce the same
    integer result.


The current AffineTransform implementation breaks those contracts in some 
particular cases. The reflexivity contract is broken if at least one coefficient 
is NaN. The hashCode() contract is broken if an affine transform contains 
positive zeros while the other affine transform contains negative zeros.

In core JDK, java.lang.Double.equals(Object) does not use the == operation for 
comparing floating point values, but instead uses doubleToLongBits(double). This 
is probably the safest way to ensure strict compliance with Object contract. 
However in order to preserve compatibility with the current AffineTransform 
behavior at least for non-NaN values, the proposed patch uses a slightly 
modified version of doubleToLongBits(double), which consider negative zero as 
equals to positive zero. Note that the JDK doubleToLongBits(double) method 
already collapses all NaN values into a canonical value, so the proposed patch 
could be seen as an extension of the later where the two possible zero values 
are also collapsed into a canonical one.

Note that negative zeros actually happens quite often on the translateY (m12) 
coefficient with geospatial data, because the scaleY (m11) coefficient usually 
have a negative value when the data to display have y ordinate values increasing 
upward rather than downward.

The purpose of this patch is to make AffineTransform suitable for use as keys in 
HashMap or as element in collections. In current implementation, an 
AffineTransform added into a collection can never be removed by the 
remove(Object) method if the collection implementation uses only the 
equals(Object) method and the AffineTransform contains NaN values. Other methods 
like contains(Object) have similar issues.

Regards,

     Martin Desruisseaux

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20111109/692022c5/attachment.html>