sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1702110 - in /sis/branches/JDK8/core/sis-referencing/src: main/java/org/apache/sis/internal/referencing/j2d/ main/java/org/apache/sis/referencing/operation/matrix/ main/java/org/apache/sis/referencing/operation/transform/ test/java/org/apa...
Date Wed, 09 Sep 2015 21:01:23 GMT
Author: desruisseaux
Date: Wed Sep  9 21:01:22 2015
New Revision: 1702110

URL: http://svn.apache.org/r1702110
Log:
Fix an accuracy problem identified by the test added in the previous commit.
As a side effect of this fix, the internal AffineTransform2D class can no longer be mutable.

Document better why the GeodeticObjectParserTest.testMathTransform() method expects a shift
of 2000 metres in the test using the "Transverse Mercator (South Orientated)" projection,
since the meaning of the "False Northing" parameter in that projection is non-intuitive.

Modified:
    sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/AffineMatrix.java
    sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/AffineTransform2D.java
    sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/GeneralMatrix.java
    sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/Solver.java
    sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/IdentityTransform.java
    sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/io/wkt/GeodeticObjectParserTest.java
    sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/ConcatenatedTransformTest.java

Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/AffineMatrix.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/AffineMatrix.java?rev=1702110&r1=1702109&r2=1702110&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/AffineMatrix.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/AffineMatrix.java
[UTF-8] Wed Sep  9 21:01:22 2015
@@ -175,6 +175,7 @@ final class AffineMatrix implements Exte
      * Returns a copy of the matrix that user can modify.
      */
     @Override
+    @SuppressWarnings("CloneDoesntCallSuperClone")
     public final Matrix clone() {
         return Matrices.copy(this);
     }

Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/AffineTransform2D.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/AffineTransform2D.java?rev=1702110&r1=1702109&r2=1702110&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/AffineTransform2D.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/AffineTransform2D.java
[UTF-8] Wed Sep  9 21:01:22 2015
@@ -31,6 +31,7 @@ import org.apache.sis.referencing.operat
 import org.apache.sis.referencing.operation.matrix.Matrices;
 import org.apache.sis.referencing.operation.matrix.AffineTransforms2D;
 import org.apache.sis.referencing.operation.transform.LinearTransform;
+import org.apache.sis.internal.referencing.ExtendedPrecisionMatrix;
 import org.apache.sis.internal.referencing.provider.Affine;
 import org.apache.sis.io.wkt.Formatter;
 import org.apache.sis.util.LenientComparable;
@@ -70,9 +71,8 @@ public class AffineTransform2D extends I
      * set to a non-null value before an {@link AffineTransform2D} instance is published.</p>
      *
      * @see #getMatrix()
-     * @see #freeze()
      */
-    private AffineMatrix matrix;
+    private final AffineMatrix matrix;
 
     /**
      * The inverse transform. This field will be computed only when needed.
@@ -80,37 +80,14 @@ public class AffineTransform2D extends I
     private transient volatile AffineTransform2D inverse;
 
     /**
-     * Constructs a <strong>temporarily mutable</strong> identity affine transform.
-     * Callers shall initializing the affine transform to the desired final values,
-     * then invoke {@link #freeze()}.
-     */
-    public AffineTransform2D() {
-        super();
-    }
-
-    /**
-     * Constructs a new affine transform with the same coefficients than the specified transform.
-     *
-     * @param transform The affine transform to copy.
-     * @param mutable {@code true} if this affine transform needs to be <strong>temporarily</strong>
mutable.
-     *        If {@code true}, then caller shall invoke {@link #freeze()} after they completed
initialization.
-     */
-    public AffineTransform2D(final AffineTransform transform, final boolean mutable) {
-        super(transform);
-        if (!mutable) {
-            freeze();
-        }
-    }
-
-    /**
      * Constructs a new affine transform with the same coefficients than the specified transform.
      *
      * @param transform The affine transform to copy.
      */
     public AffineTransform2D(final AffineTransform transform) {
         super(transform);
-        forcePositiveZeros();
-        freeze();
+        forcePositiveZeros();   // Must be invoked before to set the 'matrix' value.
+        matrix = new AffineMatrix(this, null);
     }
 
     /**
@@ -123,7 +100,6 @@ public class AffineTransform2D extends I
               pz(elements[1]), pz(elements[4]),
               pz(elements[2]), pz(elements[5]));
         matrix = new AffineMatrix(this, elements);
-        // Do not call freeze(), as it was implied by above line.
     }
 
     /**
@@ -141,7 +117,7 @@ public class AffineTransform2D extends I
      */
     public AffineTransform2D(double m00, double m10, double m01, double m11, double m02,
double m12) {
         super(pz(m00), pz(m10), pz(m01), pz(m11), pz(m02), pz(m12));
-        freeze();
+        matrix = new AffineMatrix(this, null);
     }
 
     /**
@@ -167,15 +143,6 @@ public class AffineTransform2D extends I
     }
 
     /**
-     * Makes this {@code AffineTransform2D} immutable.
-     * This method shall be invoked exactly once.
-     */
-    public final void freeze() {
-        assert matrix == null;
-        matrix = new AffineMatrix(this, null);
-    }
-
-    /**
      * Throws an {@link UnsupportedOperationException} when a mutable method
      * is invoked, since {@code AffineTransform2D} must be immutable.
      *
@@ -327,15 +294,25 @@ public class AffineTransform2D extends I
                  * Is okay with the new memory model since Java 5 provided that the field
is
                  * declared volatile (Joshua Bloch, "Effective Java" second edition).
                  */
-                if (inverse == null) try {
-                    final AffineTransform2D work = new AffineTransform2D(this, true);
-                    work.invert();
-                    work.forcePositiveZeros();
-                    work.freeze();
+                if (inverse == null) {
+                    /*
+                     * In a previous version, we were using the Java2D code as below:
+                     *
+                     *     AffineTransform2D work = new AffineTransform2D(this, true);
+                     *     work.invert();
+                     *     work.forcePositiveZeros();
+                     *     work.freeze();
+                     *
+                     * Current version now uses the SIS code instead in order to get the
double-double precision.
+                     * It usually does not make a difference in the result of the matrix
inversion, when ignoring
+                     * the error terms.  But those error terms appear to be significant later,
when the result of
+                     * this matrix inversion is multiplied with other matrices: the double-double
accuracy allows
+                     * us to better detect the terms that are 0 or 1 after matrix concatenation.
+                     */
+                    final AffineTransform2D work = new AffineTransform2D(
+                            ((ExtendedPrecisionMatrix) Matrices.inverse(matrix)).getExtendedElements());
                     work.inverse = this;
                     inverse = work; // Set only on success.
-                } catch (java.awt.geom.NoninvertibleTransformException exception) {
-                    throw new NoninvertibleTransformException(exception.getLocalizedMessage(),
exception);
                 }
             }
         }
@@ -432,6 +409,7 @@ public class AffineTransform2D extends I
      * @return A modifiable copy of this affine transform.
      */
     @Override
+    @SuppressWarnings("CloneDoesntCallSuperClone")
     public AffineTransform clone() {
         return new AffineTransform(this);
     }

Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/GeneralMatrix.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/GeneralMatrix.java?rev=1702110&r1=1702109&r2=1702110&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/GeneralMatrix.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/GeneralMatrix.java
[UTF-8] Wed Sep  9 21:01:22 2015
@@ -634,6 +634,7 @@ class GeneralMatrix extends MatrixSIS im
      * {@inheritDoc}
      */
     @Override
+    @SuppressWarnings("CloneDoesntCallSuperClone")
     public MatrixSIS clone() {
         return new GeneralMatrix(this);
     }

Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/Solver.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/Solver.java?rev=1702110&r1=1702109&r2=1702110&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/Solver.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/matrix/Solver.java
[UTF-8] Wed Sep  9 21:01:22 2015
@@ -86,6 +86,7 @@ final class Solver implements Matrix { /
      * Returns {@code this} since this matrix is immutable.
      */
     @Override
+    @SuppressWarnings("CloneDoesntCallSuperClone")
     public Matrix clone() {
         return this;
     }

Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/IdentityTransform.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/IdentityTransform.java?rev=1702110&r1=1702109&r2=1702110&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/IdentityTransform.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/IdentityTransform.java
[UTF-8] Wed Sep  9 21:01:22 2015
@@ -87,11 +87,9 @@ final class IdentityTransform extends Ab
                 }
             }
             switch (dimension) {
-                default: candidate = new IdentityTransform(dimension); break;
-                case 1:  candidate = IdentityTransform1D.INSTANCE;     break;
-                case 2:  candidate = new AffineTransform2D();
-                         ((AffineTransform2D) candidate).freeze();
-                         break;
+                default: candidate = new IdentityTransform(dimension);        break;
+                case 1:  candidate = IdentityTransform1D.INSTANCE;            break;
+                case 2:  candidate = new AffineTransform2D(1, 0, 0, 1, 0, 0); break;
             }
             if (dimension < IDENTITIES.length) {
                 IDENTITIES[dimension] = candidate;

Modified: sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/io/wkt/GeodeticObjectParserTest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/io/wkt/GeodeticObjectParserTest.java?rev=1702110&r1=1702109&r2=1702110&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/io/wkt/GeodeticObjectParserTest.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/io/wkt/GeodeticObjectParserTest.java
[UTF-8] Wed Sep  9 21:01:22 2015
@@ -829,9 +829,13 @@ public final strictfp class GeodeticObje
     @Test
     @DependsOnMethod("testProjectedCRS")
     public void testMathTransform() throws ParseException, NoninvertibleTransformException
{
-        final double TOLERANCE = 1E-10;
         /*
-         * Tests "Transverse Mercator" (not south-oriented) with an axis oriented toward
south.
+         * Test "Transverse Mercator" (not south-oriented) with an axis oriented toward south.
+         * The 'south' transform is actually the usual Transverse Mercator projection, despite
+         * having axis oriented toward South.  Consequently the "False Northing" parameter
has
+         * the same meaning for those two CRS. Since we assigned the same False Northing
value,
+         * those two CRS have their "False origin" at the same location. This is why conversion
+         * from 'south' to 'north' introduce no translation, only a reversal of y axis.
          */
         ProjectedCRS north = parseTransverseMercator(false, false, 1000);
         assertEquals(AxisDirection.WEST,  north.getCoordinateSystem().getAxis(0).getDirection());
@@ -842,24 +846,27 @@ public final strictfp class GeodeticObje
         assertEquals(AxisDirection.SOUTH, south.getCoordinateSystem().getAxis(1).getDirection());
 
         Matrix matrix = conversion(north, south);
-        assertEquals("West direction should be unchanged. ",      +1, matrix.getElement(0,0),
TOLERANCE);
-        assertEquals("North-South direction should be reverted.", -1, matrix.getElement(1,1),
TOLERANCE);
-        assertEquals("No easting expected.",                       0, matrix.getElement(0,2),
TOLERANCE);
-        assertEquals("No northing expected.",                      0, matrix.getElement(1,2),
TOLERANCE);
-        assertDiagonalEquals(new double[] {+1, -1, 1}, true, matrix, TOLERANCE);
+        assertEquals("West direction should be unchanged. ",      +1, matrix.getElement(0,0),
STRICT);
+        assertEquals("North-South direction should be reverted.", -1, matrix.getElement(1,1),
STRICT);
+        assertEquals("No easting expected.",                       0, matrix.getElement(0,2),
STRICT);
+        assertEquals("No northing expected.",                      0, matrix.getElement(1,2),
STRICT);
+        assertDiagonalEquals(new double[] {+1, -1, 1}, true, matrix, STRICT);
         /*
-         * Tests "Transverse Mercator South Orientated".
-         * The "False Northing" parameter is actually interpreted as a "False Southing".
-         * It may sound surprising, but "South Orientated" projection is defined that way.
+         * Test "Transverse Mercator South Orientated". In this projection, the "False Northing"
parameter
+         * is actually a "False Southing". It may sound surprising, but "South Orientated"
projections are
+         * defined that way.  For converting from our CRS having a False Northing of 1000
to a CRS without
+         * False Northing or Southing, we must subtract 1000 from the axis which is oriented
toward North.
+         * This means adding 1000 if the axis is rather oriented toward South. Then we add
another 1000 m
+         * (the value specified in the line just below) toward South.
          */
-        south = parseTransverseMercator(true, true, 1000);
+        south = parseTransverseMercator(true, true, 1000);  // "False Southing" of 1000 metres.
         assertEquals(AxisDirection.WEST,  south.getCoordinateSystem().getAxis(0).getDirection());
         assertEquals(AxisDirection.SOUTH, south.getCoordinateSystem().getAxis(1).getDirection());
         matrix = conversion(north, south);
-        assertEquals("West direction should be unchanged. ",      +1, matrix.getElement(0,0),
TOLERANCE);
-        assertEquals("North-South direction should be reverted.", -1, matrix.getElement(1,1),
TOLERANCE);
-        assertEquals("No easting expected.",                       0, matrix.getElement(0,2),
TOLERANCE);
-        assertEquals("Northing expected.",                      2000, matrix.getElement(1,2),
TOLERANCE);
+        assertEquals("West direction should be unchanged. ",      +1, matrix.getElement(0,0),
STRICT);
+        assertEquals("North-South direction should be reverted.", -1, matrix.getElement(1,1),
STRICT);
+        assertEquals("No easting expected.",                       0, matrix.getElement(0,2),
STRICT);
+        assertEquals("Northing expected.",                      2000, matrix.getElement(1,2),
STRICT);
     }
 
     /**

Modified: sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/ConcatenatedTransformTest.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/ConcatenatedTransformTest.java?rev=1702110&r1=1702109&r2=1702110&view=diff
==============================================================================
--- sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/ConcatenatedTransformTest.java
[UTF-8] (original)
+++ sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/ConcatenatedTransformTest.java
[UTF-8] Wed Sep  9 21:01:22 2015
@@ -44,13 +44,8 @@ public final strictfp class Concatenated
      */
     @Test
     public void testDirect2D() throws TransformException {
-        final AffineTransform2D first = new AffineTransform2D();
-        first.translate(2,4);
-        first.freeze();
-
-        final AffineTransform2D second = new AffineTransform2D();
-        second.translate(0.25, 0.75);
-        second.freeze();
+        final AffineTransform2D first  = new AffineTransform2D(1, 0, 0, 1, 2.00, 4.00); 
  // translate(2, 4)
+        final AffineTransform2D second = new AffineTransform2D(1, 0, 0, 1, 0.25, 0.75); 
  // translate(0.25, 0.75);
 
         // Direct for 2D case.
         tolerance = 1E-10;
@@ -93,9 +88,7 @@ public final strictfp class Concatenated
     public void testGeneric() throws TransformException {
         final MathTransform first = null; //MathTransforms.dimensionFilter(4, new int[] {1,3});
 
-        final AffineTransform2D second = new AffineTransform2D();
-        second.scale(0.5, 0.25);
-        second.freeze();
+        final AffineTransform2D second = new AffineTransform2D(0.5, 0, 0, 0.25, 0, 0);  //
scale(0.5, 0.25);
 
         transform = new ConcatenatedTransform(first, second);
         isInverseTransformSupported = false;



Mime
View raw message