sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] branch geoapi-4.0 updated: Add more tests for PassThroughTransform. This forced us to revisit the 'transform' implementation for overlapping arrays. This commit may make https://issues.apache.org/jira/browse/SIS-318 irrelevant.
Date Thu, 29 Nov 2018 10:31:30 GMT
This is an automated email from the ASF dual-hosted git repository.

desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git


The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
     new 48dbf45  Add more tests for PassThroughTransform. This forced us to revisit the 'transform'
implementation for overlapping arrays. This commit may make https://issues.apache.org/jira/browse/SIS-318
irrelevant.
48dbf45 is described below

commit 48dbf45951a41883e4f76353bce913656f1350d6
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Thu Nov 29 11:30:27 2018 +0100

    Add more tests for PassThroughTransform. This forced us to revisit the 'transform' implementation
for overlapping arrays.
    This commit may make https://issues.apache.org/jira/browse/SIS-318 irrelevant.
---
 .../operation/transform/PassThroughTransform.java  | 199 ++++++++++++---------
 .../transform/PassThroughTransformTest.java        |  91 ++++++----
 .../operation/transform/PseudoTransform.java       |   2 +-
 3 files changed, 176 insertions(+), 116 deletions(-)

diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/PassThroughTransform.java
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/PassThroughTransform.java
index 5fe2ec1..148d155 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/PassThroughTransform.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/PassThroughTransform.java
@@ -277,65 +277,130 @@ public class PassThroughTransform extends AbstractMathTransform implements
Seria
     }
 
     /**
-     * Transforms an array of points with overlapping source and target.
+     * Creates a new array of the same kind than the given array.
+     * This is used for creating {@code float[]} or {@code double[]} arrays.
+     */
+    private static Object newArray(final Object array, final int length) {
+        return Array.newInstance(array.getClass().getComponentType(), length);
+    }
+
+    /**
+     * Transforms an array of points with potentially overlapping source and target.
      *
-     * @param  points  the point to transform, as a {@code float[]} or {@code double[]} array.
+     * @param  srcPts  the point to transform, as a {@code float[]} or {@code double[]} array.
      * @param  srcOff  the offset to the point to be transformed in the array.
+     * @param  dstPts  where to store the transformed points, as an array of same type than
{@code srcPts}.
      * @param  dstOff  the offset to the location of the transformed point that is stored
in the destination array.
      * @param  numPts  number of points to transform.
-     * @return {@code null} on success, or a copy of a range from {@code points} array otherwise.
In the later case, callers
-     *          have to perform the transformation itself but can rely on the returned copy
to not overlap the original array.
      */
     @SuppressWarnings("SuspiciousSystemArraycopy")
-    private Object transformOverlapping(final Object points, int srcOff, int dstOff, int
numPts) throws TransformException {
-        final int subDimSource = subTransform.getSourceDimensions();
-        final int subDimTarget = subTransform.getTargetDimensions();
-        int srcStep = numTrailingOrdinates;
-        int dstStep = numTrailingOrdinates;
-        final int add = firstAffectedOrdinate + numTrailingOrdinates;
-        final int dimSource = subDimSource + add;
-        final int dimTarget = subDimTarget + add;
-        switch (IterationStrategy.suggest(srcOff, dimSource, dstOff, dimTarget, numPts))
{
-            case ASCENDING: {
-                break;
-            }
-            case DESCENDING: {
-                srcOff += (numPts - 1) * dimSource;
-                dstOff += (numPts - 1) * dimTarget;
-                srcStep -= 2*dimSource;
-                dstStep -= 2*dimTarget;
-                break;
-            }
-            default: {
-                final int length = numPts * dimSource;
-                final Object buffer = Array.newInstance(points.getClass().getComponentType(),
length);
-                System.arraycopy(points, srcOff, buffer, 0, length);
-                return buffer;
+    private void apply(final Object srcPts, final int srcOff,
+                       final Object dstPts, int dstOff, int numPts) throws TransformException
+    {
+        if (numPts <= 0) return;
+        final int subDimSource   = subTransform.getSourceDimensions();
+        final int subDimTarget   = subTransform.getTargetDimensions();
+        final int numPassThrough = firstAffectedOrdinate + numTrailingOrdinates;
+        final int dimSource      = subDimSource + numPassThrough;
+        final int dimTarget      = subDimTarget + numPassThrough;
+        /*
+         * Copy the pass-through coordinates (both before and after the sub-transform) into
the 'pasPts'
+         * temporary array. This will allow us to compact the coordinates to give to the
sub-transform,
+         * so we can process them in a single 'transform' method call. We do that also for
avoiding tricky
+         * issues with overlapping regions, because coordinate tuples are not processed automically
the
+         * way 'IterationStrategy' expects.
+         */
+        final Object pasPts;
+        {
+            pasPts = newArray(srcPts, numPassThrough * numPts);
+            System.arraycopy(srcPts, srcOff, pasPts, 0, firstAffectedOrdinate);
+            int pasOff = firstAffectedOrdinate;
+            int srcCpk = srcOff + pasOff + subDimSource;            // "Cpk" stands for "cherry-pick".
+            int n = numPts - 1;
+            while (--n >= 0) {
+                System.arraycopy(srcPts, srcCpk, pasPts, pasOff, numPassThrough);
+                pasOff += numPassThrough;
+                srcCpk += dimSource;
             }
+            System.arraycopy(srcPts, srcCpk, pasPts, pasOff, numTrailingOrdinates);
         }
-        final boolean copyTrailingFirst = subDimSource < subDimTarget;
-        while (--numPts >= 0) {
-            System.arraycopy(points, srcOff, points, dstOff, firstAffectedOrdinate);
-            srcOff += firstAffectedOrdinate;
-            dstOff += firstAffectedOrdinate;
-            if (copyTrailingFirst) {
-                System.arraycopy(points, srcOff + subDimSource,
-                                 points, dstOff + subDimTarget, numTrailingOrdinates);
-            }
-            if (points instanceof double[]) {
-                subTransform.transform((double[]) points, srcOff, (double[]) points, dstOff,
1);
+        /*
+         * Copy in a compact array the coordinates to be given to the sub-transform.
+         * We do the compaction in the destination array (if it is large enough) for
+         * avoiding the need to create a temporary buffer. We can do that only after
+         * all pass-through coordinates have been copied by above loop, for avoiding
+         * to overwrite values if the source and destination array regions overlap.
+         */
+        Object subPts = dstPts;
+        {
+            int subOff = dstOff;
+            int srcCpk = srcOff + firstAffectedOrdinate;    // "Cpk" stands for "cherry-pick".
+            int srcInc = dimSource;
+            int dstInc = subDimSource;
+            final IterationStrategy strategy;
+            if (subDimSource > subDimTarget + numPassThrough) {
+                // If the destination array does not have enough room, create a temporary
buffer.
+                strategy = IterationStrategy.BUFFER_TARGET;
+            } else if (srcPts != dstPts) {
+                strategy = IterationStrategy.ASCENDING;
             } else {
-                subTransform.transform((float[]) points, srcOff, (float[]) points, dstOff,
1);
+                strategy = IterationStrategy.suggest(srcOff, srcInc, dstOff, dstInc, numPts);
             }
-            srcOff += subDimSource;
-            dstOff += subDimTarget;
-            if (!copyTrailingFirst) {
-                System.arraycopy(points, srcOff, points, dstOff, numTrailingOrdinates);
+            switch (strategy) {
+                case ASCENDING: {
+                    break;
+                }
+                case DESCENDING: {
+                    srcCpk += (numPts-1) * srcInc; srcInc = -srcInc;
+                    subOff += (numPts-1) * dstInc; dstInc = -dstInc;
+                    break;
+                }
+                default: {
+                    subPts = newArray(subPts, Math.max(subDimSource, subDimTarget) * numPts);
+                    subOff = 0;
+                    break;
+                }
             }
-            srcOff += srcStep;
-            dstOff += dstStep;
+            int n = numPts;
+            do {
+                System.arraycopy(srcPts, srcCpk, subPts, subOff, subDimSource);
+                subOff += dstInc;
+                srcCpk += srcInc;
+            } while (--n != 0);
+        }
+        /*
+         * All sub-transform coordinates have been compacted as consecutive tuples.
+         * Convert them in-place, overwriting the previous values.
+         */
+        int subOff = (subPts == dstPts) ? dstOff : 0;
+        if (subPts instanceof double[]) {
+            subTransform.transform((double[]) subPts, subOff, (double[]) subPts, subOff,
numPts);
+        } else {
+            subTransform.transform( (float[]) subPts, subOff,  (float[]) subPts, subOff,
numPts);
+        }
+        /*
+         * Copies the transformed coordinates to their final location, inserting pass-through
+         * coordinates between them in the process. Note that we avoided to modify 'dstOff'
+         * and 'numPts' before this point, but now we are free to do so since this is the
last
+         * step.
+         */
+        int pasOff = numPts * numPassThrough;
+        subOff    += numPts * subDimTarget;
+        dstOff    += numPts * dimTarget;
+        if (--numPts >= 0) {
+            System.arraycopy(pasPts, pasOff -= numTrailingOrdinates,
+                             dstPts, dstOff -= numTrailingOrdinates, numTrailingOrdinates);
+            System.arraycopy(subPts, subOff -= subDimTarget,
+                             dstPts, dstOff -= subDimTarget, subDimTarget);
+            while (--numPts >= 0) {
+                System.arraycopy(pasPts, pasOff -= numPassThrough,
+                                 dstPts, dstOff -= numPassThrough, numPassThrough);
+                System.arraycopy(subPts, subOff -= subDimTarget,
+                                 dstPts, dstOff -= subDimTarget, subDimTarget);
+            }
+            System.arraycopy(pasPts, pasOff - firstAffectedOrdinate,
+                             dstPts, dstOff - firstAffectedOrdinate, firstAffectedOrdinate);
         }
-        return null;
     }
 
     /**
@@ -344,24 +409,8 @@ public class PassThroughTransform extends AbstractMathTransform implements
Seria
      * @throws TransformException if the {@linkplain #subTransform sub-transform} failed.
      */
     @Override
-    public void transform(double[] srcPts, int srcOff, final double[] dstPts, int dstOff,
int numPts) throws TransformException {
-        if (srcPts == dstPts) {
-            srcPts = (double[]) transformOverlapping(srcPts, srcOff, dstOff, numPts);
-            if (srcPts == null) return;
-            srcOff = 0;
-        }
-        final int subDimSource = subTransform.getSourceDimensions();
-        final int subDimTarget = subTransform.getTargetDimensions();
-        while (--numPts >= 0) {
-            System.arraycopy(      srcPts, srcOff,
-                                   dstPts, dstOff,   firstAffectedOrdinate);
-            subTransform.transform(srcPts, srcOff += firstAffectedOrdinate,
-                                   dstPts, dstOff += firstAffectedOrdinate, 1);
-            System.arraycopy(      srcPts, srcOff += subDimSource,
-                                   dstPts, dstOff += subDimTarget, numTrailingOrdinates);
-            srcOff += numTrailingOrdinates;
-            dstOff += numTrailingOrdinates;
-        }
+    public void transform(double[] srcPts, int srcOff, double[] dstPts, int dstOff, int numPts)
throws TransformException {
+        apply(srcPts, srcOff, dstPts, dstOff, numPts);
     }
 
     /**
@@ -370,24 +419,8 @@ public class PassThroughTransform extends AbstractMathTransform implements
Seria
      * @throws TransformException if the {@linkplain #subTransform sub-transform} failed.
      */
     @Override
-    public void transform(float[] srcPts, int srcOff, final float[] dstPts, int dstOff, int
numPts) throws TransformException {
-        if (srcPts == dstPts) {
-            srcPts = (float[]) transformOverlapping(srcPts, srcOff, dstOff, numPts);
-            if (srcPts == null) return;
-            srcOff = 0;
-        }
-        final int subDimSource = subTransform.getSourceDimensions();
-        final int subDimTarget = subTransform.getTargetDimensions();
-        while (--numPts >= 0) {
-            System.arraycopy(      srcPts, srcOff,
-                                   dstPts, dstOff,   firstAffectedOrdinate);
-            subTransform.transform(srcPts, srcOff += firstAffectedOrdinate,
-                                   dstPts, dstOff += firstAffectedOrdinate, 1);
-            System.arraycopy(      srcPts, srcOff += subDimSource,
-                                   dstPts, dstOff += subDimTarget, numTrailingOrdinates);
-            srcOff += numTrailingOrdinates;
-            dstOff += numTrailingOrdinates;
-        }
+    public void transform(float[] srcPts, int srcOff, float[] dstPts, int dstOff, int numPts)
throws TransformException {
+        apply(srcPts, srcOff, dstPts, dstOff, numPts);
     }
 
     /**
diff --git a/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/PassThroughTransformTest.java
b/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/PassThroughTransformTest.java
index 1f8acee..c24d007 100644
--- a/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/PassThroughTransformTest.java
+++ b/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/PassThroughTransformTest.java
@@ -40,7 +40,7 @@ import org.opengis.test.ToleranceModifier;
  * Tests {@link PassThroughTransform}.
  *
  * @author  Martin Desruisseaux (IRD, Geomatys)
- * @version 0.5
+ * @version 1.0
  * @since   0.5
  * @module
  */
@@ -75,7 +75,6 @@ public final strictfp class PassThroughTransformTest extends MathTransformTestCa
             final String message = e.getMessage();
             assertTrue(message, message.contains("numTrailingOrdinates"));
         }
-
     }
 
     /**
@@ -117,6 +116,28 @@ public final strictfp class PassThroughTransformTest extends MathTransformTestCa
     }
 
     /**
+     * Tests the general pass-through transform with a sub-transform going from 3D to 2D
points.
+     *
+     * @throws TransformException should never happen.
+     */
+    @Test
+    public void testDimensionDecrease() throws TransformException {
+        isInverseTransformSupported = false;
+        runTest(new PseudoTransform(3, 2), PassThroughTransform.class);
+    }
+
+    /**
+     * Tests the general pass-through transform with a sub-transform going from 2D to 3D
points.
+     *
+     * @throws TransformException should never happen.
+     */
+    @Test
+    public void testDimensionIncrease() throws TransformException {
+        isInverseTransformSupported = false;
+        runTest(new PseudoTransform(2, 3), PassThroughTransform.class);
+    }
+
+    /**
      * Tests a pass-through transform built using the given sub-transform.
      *
      * @param  subTransform   the sub-transform to use for building pass-through transform.
@@ -155,58 +176,64 @@ public final strictfp class PassThroughTransformTest extends MathTransformTestCa
      * and compares the result against the expected ones. This method computes itself the
expected results.
      *
      * @param  subTransform           the sub transform used by the current pass-through
transform.
-     * @param  firstAffectedOrdinate  first input/output dimension used by {@code subTransform}.
+     * @param  firstAffectedCoordinate  first input/output dimension used by {@code subTransform}.
      * @throws TransformException if a transform failed.
      */
-    private void verifyTransform(final MathTransform subTransform, final int firstAffectedOrdinate)
-            throws TransformException
-    {
+    private void verifyTransform(final MathTransform subTransform, final int firstAffectedCoordinate)
throws TransformException {
         validate();
         /*
          * Prepare two arrays:
          *   - passthrough data, to be given to the transform to be tested.
          *   - sub-transform data, which we will use internally for verifying the pass-through
work.
          */
-        final int      passthroughDim   = transform.getSourceDimensions();
-        final int      subTransformDim  = subTransform.getSourceDimensions();
-        final int      numPts           = ORDINATE_COUNT / passthroughDim;
-        final double[] passthroughData  = CoordinateDomain.RANGE_10.generateRandomInput(random,
passthroughDim, numPts);
-        final double[] subTransformData = new double[numPts * subTransformDim];
+        final int      sourceDim        = transform.getSourceDimensions();
+        final int      targetDim        = transform.getTargetDimensions();
+        final int      subSrcDim        = subTransform.getSourceDimensions();
+        final int      subTgtDim        = subTransform.getTargetDimensions();
+        final int      numPts           = ORDINATE_COUNT / sourceDim;
+        final double[] passthroughData  = CoordinateDomain.RANGE_10.generateRandomInput(random,
sourceDim, numPts);
+        final double[] subTransformData = new double[numPts * Math.max(subSrcDim, subTgtDim)];
         Arrays.fill(subTransformData, Double.NaN);
         for (int i=0; i<numPts; i++) {
-            System.arraycopy(passthroughData, firstAffectedOrdinate + i*passthroughDim,
-                             subTransformData, i*subTransformDim, subTransformDim);
+            System.arraycopy(passthroughData, firstAffectedCoordinate + i*sourceDim,
+                             subTransformData, i*subSrcDim, subSrcDim);
         }
-        assertFalse("Error building test arrays.", ArraysExt.hasNaN(subTransformData));
+        subTransform.transform(subTransformData, 0, subTransformData, 0, numPts);
+        assertFalse(ArraysExt.hasNaN(subTransformData));
         /*
-         * Build the array of expected data by applying ourself the sub-transform.
+         * Build the array of expected data by copying ourself the sub-transform results.
          */
-        subTransform.transform(subTransformData, 0, subTransformData, 0, numPts);
-        final double[] expectedData = passthroughData.clone();
+        final int numTrailingCoordinates = targetDim - subTgtDim - firstAffectedCoordinate;
+        final double[] expectedData = new double[targetDim * numPts];
         for (int i=0; i<numPts; i++) {
-            System.arraycopy(subTransformData, i*subTransformDim, expectedData,
-                             firstAffectedOrdinate + i*passthroughDim, subTransformDim);
+            int srcOffset = i * sourceDim;
+            int dstOffset = i * targetDim;
+            final int s = firstAffectedCoordinate + subSrcDim;
+            System.arraycopy(passthroughData,  srcOffset,   expectedData, dstOffset,   firstAffectedCoordinate);
+            System.arraycopy(subTransformData, i*subTgtDim, expectedData, dstOffset += firstAffectedCoordinate,
subTgtDim);
+            System.arraycopy(passthroughData,  srcOffset+s, expectedData, dstOffset + subTgtDim,
numTrailingCoordinates);
         }
-        assertEquals("Error building test arrays.", subTransform.isIdentity(),
-                Arrays.equals(passthroughData, expectedData));
+        assertEquals(subTransform.isIdentity(), Arrays.equals(passthroughData, expectedData));
         /*
          * Now process to the transform and compares the results with the expected ones.
          */
         tolerance         = 0;          // Results should be strictly identical because we
used the same inputs.
         toleranceModifier = null;
-        final double[] transformedData = new double[expectedData.length];
+        final double[] transformedData = new double[Math.max(sourceDim, targetDim) * numPts];
         transform.transform(passthroughData, 0, transformedData, 0, numPts);
-        assertCoordinatesEqual("Direct transform.", passthroughDim,
-                expectedData, 0, transformedData, 0, numPts, CalculationType.DIRECT_TRANSFORM);
+        assertCoordinatesEqual("PassThroughTransform results do not match the results computed
by this test.",
+                targetDim, expectedData, 0, transformedData, 0, numPts, CalculationType.DIRECT_TRANSFORM);
         /*
          * Test inverse transform.
          */
-        tolerance         = 1E-8;
-        toleranceModifier = ToleranceModifier.RELATIVE;
-        Arrays.fill(transformedData, Double.NaN);
-        transform.inverse().transform(expectedData, 0, transformedData, 0, numPts);
-        assertCoordinatesEqual("Inverse transform.", passthroughDim,
-                passthroughData, 0, transformedData, 0, numPts, CalculationType.INVERSE_TRANSFORM);
+        if (isInverseTransformSupported) {
+            tolerance         = 1E-8;
+            toleranceModifier = ToleranceModifier.RELATIVE;
+            Arrays.fill(transformedData, Double.NaN);
+            transform.inverse().transform(expectedData, 0, transformedData, 0, numPts);
+            assertCoordinatesEqual("Inverse of PassThroughTransform do not give back the
original data.",
+                    sourceDim, passthroughData, 0, transformedData, 0, numPts, CalculationType.INVERSE_TRANSFORM);
+        }
         /*
          * Verify the consistency between different 'transform(…)' methods.
          */
@@ -220,8 +247,8 @@ public final strictfp class PassThroughTransformTest extends MathTransformTestCa
         if (transform instanceof LinearTransform) {
             tolerance         = 1E-4;
             toleranceModifier = ToleranceModifier.RELATIVE;
-            assertCoordinatesEqual("A transformed value is wrong.", passthroughDim,
-                    expectedData, 0, targetAsFloat, 0, numPts, CalculationType.DIRECT_TRANSFORM);
+            assertCoordinatesEqual("PassThroughTransform.transform(…) variants produce
inconsistent results.",
+                    sourceDim, expectedData, 0, targetAsFloat, 0, numPts, CalculationType.DIRECT_TRANSFORM);
         }
     }
 }
diff --git a/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/PseudoTransform.java
b/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/PseudoTransform.java
index f0743d3..2c4056d 100644
--- a/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/PseudoTransform.java
+++ b/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/transform/PseudoTransform.java
@@ -38,7 +38,7 @@ import static java.lang.StrictMath.*;
  *     3003.3
  * }
  *
- * This transform can not compute {@linkplain #derivative derivative}.
+ * This transform is not invertible and can not compute {@linkplain #derivative derivative}.
  *
  * @author  Martin Desruisseaux (Geomatys)
  * @version 1.0


Mime
View raw message