sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] 02/03: Remove the use of `MathTransformFactory` in WraparoundTransform (except in `tryConcatenate`). All classes using WraparoundTransform had no particular factory to specify. Remove corresponding `throws FactoryException` statements.
Date Tue, 15 Sep 2020 12:51:55 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

commit 256767701e9a6adb3fa06c0e78455e064a4cee4c
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Mon Sep 14 16:39:40 2020 +0200

    Remove the use of `MathTransformFactory` in WraparoundTransform (except in `tryConcatenate`).
    All classes using WraparoundTransform had no particular factory to specify.
    Remove corresponding `throws FactoryException` statements.
---
 .../coverage/grid/CoordinateOperationFinder.java   | 38 +++++--------
 .../internal/referencing/WraparoundTransform.java  | 62 ++++++++++------------
 .../referencing/WraparoundTransformTest.java       | 12 ++---
 3 files changed, 43 insertions(+), 69 deletions(-)

diff --git a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/CoordinateOperationFinder.java
b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/CoordinateOperationFinder.java
index e2b7b4a..fea754d 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/CoordinateOperationFinder.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/CoordinateOperationFinder.java
@@ -29,12 +29,12 @@ import org.opengis.referencing.cs.CoordinateSystem;
 import org.opengis.referencing.crs.CoordinateReferenceSystem;
 import org.opengis.referencing.operation.CoordinateOperation;
 import org.opengis.referencing.operation.MathTransform;
-import org.opengis.referencing.operation.MathTransformFactory;
 import org.opengis.referencing.operation.TransformException;
 import org.apache.sis.util.collection.BackingStoreException;
 import org.apache.sis.internal.referencing.CoordinateOperations;
 import org.apache.sis.internal.referencing.WraparoundTransform;
 import org.apache.sis.metadata.iso.extent.DefaultGeographicBoundingBox;
+import org.apache.sis.referencing.operation.transform.MathTransforms;
 import org.apache.sis.geometry.Envelopes;
 import org.apache.sis.geometry.GeneralDirectPosition;
 import org.apache.sis.image.ImageProcessor;
@@ -161,14 +161,6 @@ final class CoordinateOperationFinder implements Supplier<double[]>
{
     private boolean isWraparoundApplied;
 
     /**
-     * The factory to use for {@link MathTransform} creations. For now this is fixed to SIS
implementation.
-     * But it may become a configurable reference in a future version if useful.
-     *
-     * @see org.apache.sis.coverage.grid.ImageRenderer#mtFactory
-     */
-    private final MathTransformFactory mtFactory;
-
-    /**
      * Creates a new finder.
      *
      * @param  source  the grid geometry which is the source of the coordinate operation
to find.
@@ -177,7 +169,6 @@ final class CoordinateOperationFinder implements Supplier<double[]>
{
     CoordinateOperationFinder(final GridGeometry source, final GridGeometry target) {
         this.source = source;
         this.target = target;
-        mtFactory = CoordinateOperations.factoryMT().caching(false);
     }
 
     /**
@@ -213,7 +204,7 @@ final class CoordinateOperationFinder implements Supplier<double[]>
{
      * <p>The transform returned by this method applies wraparound checks systematically
on every axes having
      * wraparound range. This method does not verify whether those checks are needed (i.e.
whether wraparound
      * can possibly happen). This is okay because this transform is used only for transforming
envelopes;
-     * it is not used for transforming pixel coordinates.<p>
+     * it is not used for transforming pixel coordinates.</p>
      *
      * <h4>Implementation note</h4>
      * After invocation of this method, the following fields are valid:
@@ -306,10 +297,10 @@ wraparound: if (mayRequireWraparound(cs)) {
                     if (median == null) break wraparound;
                     median = forwardOp.transform(median, null);
                 }
-                forwardOp = WraparoundTransform.forDomainOfUse(mtFactory, forwardOp, cs,
median);
+                forwardOp = WraparoundTransform.forDomainOfUse(forwardOp, cs, median);
             }
         }
-        return mtFactory.createConcatenatedTransform(tr, forwardOp);
+        return MathTransforms.concatenate(tr, forwardOp);
     }
 
     /**
@@ -325,17 +316,16 @@ wraparound: if (mayRequireWraparound(cs)) {
      * @param  gridToCRS  result of previous call to {@link #gridToCRS(PixelInCell)}, or
{@code null}
      *                    for reusing the {@link #isWraparoundNeeded} result of previous
invocation.
      * @return operation from target geospatial coordinates to source grid indices.
-     * @throws FactoryException if some transform steps can not be concatenated.
      * @throws TransformException if some coordinates can not be transformed.
      */
-    final MathTransform inverse(final MathTransform gridToCRS) throws FactoryException, TransformException
{
+    final MathTransform inverse(final MathTransform gridToCRS) throws TransformException
{
         final MathTransform tr = source.getGridToCRS(anchor).inverse();
         if (operation == null) {
             return tr;
         }
         if (inverseOp != null) {
             // Here, `inverseOp` contains the wraparound step if needed.
-            return mtFactory.createConcatenatedTransform(inverseOp, tr);
+            return MathTransforms.concatenate(inverseOp, tr);
         }
         /*
          * Need to compute transform with wraparound checks, but contrarily to `gridToCRS(…)`
we do not want
@@ -345,11 +335,11 @@ wraparound: if (mayRequireWraparound(cs)) {
          * appears to be necessary or not.
          */
         final MathTransform inverseNoWrap   = operation.getMathTransform().inverse();
-        final MathTransform crsToGridNoWrap = mtFactory.createConcatenatedTransform(inverseNoWrap,
tr);
+        final MathTransform crsToGridNoWrap = MathTransforms.concatenate(inverseNoWrap, tr);
         inverseOp = inverseNoWrap;
         crsToGrid = crsToGridNoWrap;
         isWraparoundApplied = false;
-check:  if (gridToCRS != null) try {
+check:  if (gridToCRS != null) {
             /*
              * We will do a more extensive check by converting all corners of source grid
to the target CRS,
              * then convert back to the source grid and see if coordinates match. Only if
coordinates do not
@@ -360,7 +350,7 @@ check:  if (gridToCRS != null) try {
             final Supplier<MathTransform> withWraparound = () -> {
                 try {
                     return applyWraparound(tr);
-                } catch (FactoryException | TransformException e) {
+                } catch (TransformException e) {
                     throw new BackingStoreException(e);
                 }
             };
@@ -376,9 +366,6 @@ check:  if (gridToCRS != null) try {
                 }
                 return cc;
             });
-        } catch (BackingStoreException e) {
-            throw e.unwrapOrRethrow(FactoryException.class);
-            // TransformException is handled in `WraparoundTransform.isNeeded(…)` instead
of here.
         }
         /*
          * At this point we determined whether wraparound is needed. The `inverseOp` and
`crsToGrid` fields
@@ -401,18 +388,17 @@ check:  if (gridToCRS != null) try {
      *
      * @param  tr  value of {@code source.getGridToCRS(anchor).inverse()}.
      * @return transform from geospatial target coordinates to source grid indices.
-     * @throws FactoryException if some transform steps can not be concatenated.
      * @throws TransformException if some coordinates can not be transformed.
      */
-    private MathTransform applyWraparound(final MathTransform tr) throws FactoryException,
TransformException {
+    private MathTransform applyWraparound(final MathTransform tr) throws TransformException
{
         if (!isWraparoundApplied) {
             isWraparoundApplied = true;
             final CoordinateSystem cs = operation.getSourceCRS().getCoordinateSystem();
             if (mayRequireWraparound(cs)) {
                 final DirectPosition median = median(source);
                 if (median != null) {
-                    inverseOp = WraparoundTransform.forDomainOfUse(mtFactory, inverseOp,
cs, median);
-                    crsToGrid = mtFactory.createConcatenatedTransform(inverseOp, tr);
+                    inverseOp = WraparoundTransform.forDomainOfUse(inverseOp, cs, median);
+                    crsToGrid = MathTransforms.concatenate(inverseOp, tr);
                 }
             }
         }
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/WraparoundTransform.java
b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/WraparoundTransform.java
index 9917d92..95b6e6d 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/WraparoundTransform.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/WraparoundTransform.java
@@ -30,7 +30,6 @@ import org.opengis.referencing.operation.MathTransformFactory;
 import org.opengis.referencing.operation.Matrix;
 import org.opengis.referencing.operation.NoninvertibleTransformException;
 import org.opengis.referencing.operation.TransformException;
-import org.apache.sis.referencing.factory.InvalidGeodeticParameterException;
 import org.apache.sis.referencing.operation.matrix.Matrices;
 import org.apache.sis.referencing.operation.matrix.MatrixSIS;
 import org.apache.sis.referencing.operation.transform.AbstractMathTransform;
@@ -143,18 +142,14 @@ public final class WraparoundTransform extends AbstractMathTransform
{
      * on an inspection of source and target CRS axes. For a method making decision based
on a domain of use,
      * see {@link #forDomainOfUse forDomainOfUse(…)} instead.</p>
      *
-     * @param  factory  the factory to use for creating math transforms.
      * @param  op       the coordinate operation for which to get the math transform.
      * @return the math transform for the given coordinate operation.
-     * @throws FactoryException if an error occurred while creating the math transform.
      */
-    public static MathTransform forTargetCRS(final MathTransformFactory factory, final CoordinateOperation
op)
-            throws FactoryException
-    {
+    public static MathTransform forTargetCRS(final CoordinateOperation op) {
         MathTransform tr = op.getMathTransform();
         final CoordinateSystem targetCS = op.getTargetCRS().getCoordinateSystem();
         for (final int wraparoundDimension : CoordinateOperations.wrapAroundChanges(op))
{
-            tr = factory.createConcatenatedTransform(tr, create(factory, targetCS.getDimension(),
+            tr = MathTransforms.concatenate(tr, create(targetCS.getDimension(),
                     wraparoundDimension, targetCS.getAxis(wraparoundDimension), Double.NaN));
         }
         return tr;
@@ -171,22 +166,18 @@ public final class WraparoundTransform extends AbstractMathTransform
{
      * (e.g. when transforming a raster) and we want that output coordinates to be continuous
      * in that domain, independently of axis ranges.</p>
      *
-     * @param  factory   the factory to use for creating math transforms.
      * @param  tr        the transform to augment with "wrap around" behavior.
      * @param  targetCS  the target coordinate system.
      * @param  median    the coordinates to put at the center of new ranges.
      * @return the math transform with wraparound if needed.
-     * @throws FactoryException if an error occurred while creating the math transform.
      */
-    public static MathTransform forDomainOfUse(final MathTransformFactory factory, MathTransform
tr,
-            final CoordinateSystem targetCS, final DirectPosition median) throws FactoryException
-    {
+    public static MathTransform forDomainOfUse(MathTransform tr, final CoordinateSystem targetCS,
final DirectPosition median) {
         final int dimension = targetCS.getDimension();
         for (int i=0; i<dimension; i++) {
             final CoordinateSystemAxis axis = targetCS.getAxis(i);
             if (RangeMeaning.WRAPAROUND.equals(axis.getRangeMeaning())) {
-                tr = factory.createConcatenatedTransform(tr,
-                        create(factory, dimension, i, targetCS.getAxis(i), median.getOrdinate(i)));
+                tr = MathTransforms.concatenate(tr,
+                        create(dimension, i, targetCS.getAxis(i), median.getOrdinate(i)));
             }
         }
         return tr;
@@ -197,18 +188,15 @@ public final class WraparoundTransform extends AbstractMathTransform
{
      * The wraparound is implemented by a concatenation of affine transform before
      * and after the {@link WraparoundTransform} instance.
      *
-     * @param  factory              the factory to use for creating math transforms.
      * @param  dimension            the number of source and target dimensions.
      * @param  wraparoundDimension  the dimension where "wrap around" behavior apply.
      * @param  axis                 the coordinate system axis in the "wrap around" dimension.
      * @param  median               the coordinate to put at the center of new range,
      *                              or {@link Double#NaN} for standard center of given axis.
      * @return the math transform with "wrap around" behavior in the specified dimension.
-     * @throws FactoryException in an error occurred while creating the math transform.
      */
-    private static MathTransform create(final MathTransformFactory factory, final int dimension,
-            final int wraparoundDimension, final CoordinateSystemAxis axis, final double
median)
-            throws FactoryException
+    private static MathTransform create(final int dimension, final int wraparoundDimension,
+            final CoordinateSystemAxis axis, final double median)
     {
         ArgumentChecks.ensureStrictlyPositive("dimension", dimension);
         ArgumentChecks.ensureBetween("wraparoundDimension", 0, dimension - 1, wraparoundDimension);
@@ -220,17 +208,21 @@ public final class WraparoundTransform extends AbstractMathTransform
{
             final MatrixSIS m = Matrices.createIdentity(dimension + 1);
             m.setElement(wraparoundDimension, wraparoundDimension, span);
             m.setElement(wraparoundDimension, dimension, Double.isNaN(median) ? minimum :
median - span/2);
-            final MathTransform denormalize = factory.createAffineTransform(m);
+            final MathTransform denormalize = MathTransforms.linear(m);
             final WraparoundTransform wraparound = create(dimension, wraparoundDimension);
             try {
-                return factory.createConcatenatedTransform(denormalize.inverse(),
-                       factory.createConcatenatedTransform(wraparound, denormalize));
+                /*
+                 * Do not use the 3-arguments method because we need to
+                 * control the order in which concatenation is applied.
+                 */
+                return MathTransforms.concatenate(denormalize.inverse(),
+                       MathTransforms.concatenate(wraparound, denormalize));
             } catch (NoninvertibleTransformException e) {
                 // Matrix is non-invertible only if the range given in argument is illegal.
                 cause = e;
             }
         }
-        throw new InvalidGeodeticParameterException(Errors.format(Errors.Keys.IllegalRange_2,
minimum, maximum), cause);
+        throw new IllegalArgumentException(Errors.format(Errors.Keys.IllegalRange_2, minimum,
maximum), cause);
     }
 
     /**
@@ -445,9 +437,10 @@ public final class WraparoundTransform extends AbstractMathTransform
{
                 final MathTransform middle = steps.get(1);
                 Matrix matrix = MathTransforms.getMatrix(middle);
                 if (matrix != null) try {
+                    final MathTransformsOrFactory mf = MathTransformsOrFactory.wrap(factory);
                     boolean modified = false;
                     MathTransform step2 = this;
-                    final MathTransform after = movable(matrix, factory);
+                    final MathTransform after = movable(matrix, mf);
                     if (after != null) {
                         /*
                          * Update the middle matrix with everything that we could not put
in `after`.
@@ -458,7 +451,7 @@ public final class WraparoundTransform extends AbstractMathTransform {
                         final Matrix remaining = Matrices.multiply(MathTransforms.getMatrix(after.inverse()),
matrix);
                         final WraparoundTransform redim = redim(remaining.getNumRow() - 1);
                         if (redim != null) {
-                            step2 = factory.createConcatenatedTransform(redim, after);
+                            step2 = mf.concatenate(redim, after);
                             matrix = remaining;
                             modified = true;
                         }
@@ -470,25 +463,24 @@ public final class WraparoundTransform extends AbstractMathTransform
{
                     MathTransform step1 = steps.get(0);
                     if (step1 instanceof WraparoundTransform) {
                         WraparoundTransform wb = (WraparoundTransform) step1;
-                        final MathTransform before = wb.movable(matrix, factory);
+                        final MathTransform before = wb.movable(matrix, mf);
                         if (before != null) {
                             final Matrix remaining = Matrices.multiply(matrix, MathTransforms.getMatrix(before.inverse()));
                             wb = wb.redim(remaining.getNumCol() - 1);
                             if (wb != null) {
-                                step1 = factory.createConcatenatedTransform(before, wb);
+                                step1 = mf.concatenate(before, wb);
                                 matrix = remaining;
                                 modified = true;
                             }
                         }
                     }
                     /*
-                     * Done moving the linear operations that we can move.
-                     * Put everything together.
+                     * Done moving the linear operations that we can move. Put everything
together.
+                     * Do not use the 3-arguments method because we need to control the order
in which
+                     * concatenation is applied.
                      */
                     if (modified) {
-                        return factory.createConcatenatedTransform(
-                               factory.createConcatenatedTransform(step1,
-                               factory.createAffineTransform(matrix)), step2);
+                        return mf.concatenate(mf.concatenate(step1, mf.linear(matrix)), step2);
                     }
                 } catch (NoninvertibleTransformException e) {
                     // Should not happen. But if it is the case, just abandon the optimization
effort.
@@ -504,10 +496,10 @@ public final class WraparoundTransform extends AbstractMathTransform
{
      * that can be processed indifferently before or after this {@code WraparoundTransform}.
      *
      * @param  matrix   the matrix to analyze.
-     * @param  factory  the factory given to {@link #tryConcatenate tryConcatenate(…)}.
+     * @param  factory  wrapper of the factory given to {@link #tryConcatenate tryConcatenate(…)}.
      * @return a transform processing only the movable parts, or {@code null} if identity.
      */
-    private MathTransform movable(Matrix matrix, final MathTransformFactory factory) throws
FactoryException {
+    private MathTransform movable(Matrix matrix, final MathTransformsOrFactory factory) throws
FactoryException {
         long canMoveAfter = Numerics.bitmask(dimension) - 1;
         canMoveAfter &= ~Numerics.bitmask(wraparoundDimension);
         /*
@@ -538,7 +530,7 @@ public final class WraparoundTransform extends AbstractMathTransform {
             }
         }
         if (!matrix.isIdentity()) {
-            return factory.createAffineTransform(matrix);
+            return factory.linear(matrix);
         }
         return null;
     }
diff --git a/core/sis-referencing/src/test/java/org/apache/sis/internal/referencing/WraparoundTransformTest.java
b/core/sis-referencing/src/test/java/org/apache/sis/internal/referencing/WraparoundTransformTest.java
index ec8b366..f860c10 100644
--- a/core/sis-referencing/src/test/java/org/apache/sis/internal/referencing/WraparoundTransformTest.java
+++ b/core/sis-referencing/src/test/java/org/apache/sis/internal/referencing/WraparoundTransformTest.java
@@ -18,10 +18,8 @@ package org.apache.sis.internal.referencing;
 
 import java.util.List;
 import java.util.Collections;
-import org.opengis.util.FactoryException;
 import org.opengis.referencing.operation.MathTransform;
 import org.opengis.referencing.operation.MathTransformFactory;
-import org.apache.sis.internal.system.DefaultFactories;
 import org.apache.sis.referencing.crs.HardCodedCRS;
 import org.apache.sis.referencing.cs.AxesConvention;
 import org.apache.sis.referencing.operation.AbstractCoordinateOperation;
@@ -67,11 +65,10 @@ public final strictfp class WraparoundTransformTest extends TestCase {
     /**
      * Tests wraparound on one axis.
      *
-     * @throws FactoryException if the transform can not be created.
      * @throws NoninvertibleMatrixException if the expected matrix can not be inverted.
      */
     @Test
-    public void testOneAxis() throws FactoryException, NoninvertibleMatrixException {
+    public void testOneAxis() throws NoninvertibleMatrixException {
         final AbstractCoordinateOperation op = new AbstractCoordinateOperation(
                 Collections.singletonMap(AbstractCoordinateOperation.NAME_KEY, "Wrapper"),
                 HardCodedCRS.WGS84_φλ.forConvention(AxesConvention.POSITIVE_RANGE),
@@ -82,7 +79,7 @@ public final strictfp class WraparoundTransformTest extends TestCase {
          * Wrararound is often (but not always) unnecessary on source coordinates if the
operation
          * uses trigonometric functions.
          */
-        final MathTransform wt = WraparoundTransform.forTargetCRS(DefaultFactories.forClass(MathTransformFactory.class),
op);
+        final MathTransform wt = WraparoundTransform.forTargetCRS(op);
         final List<MathTransform> steps = MathTransforms.getSteps(wt);
         assertEquals(3, steps.size());
         assertEquals(1, ((WraparoundTransform) steps.get(1)).wraparoundDimension);
@@ -111,11 +108,10 @@ public final strictfp class WraparoundTransformTest extends TestCase
{
      * transform between them. The absence of separation between the two {@link WraparoundTransform}s
is an
      * indirect test of {@link WraparoundTransform#tryConcatenate(boolean, MathTransform,
MathTransformFactory)}.
      *
-     * @throws FactoryException if the transform can not be created.
      * @throws NoninvertibleMatrixException if the expected matrix can not be inverted.
      */
     @Test
-    public void testTwoAxes() throws FactoryException, NoninvertibleMatrixException {
+    public void testTwoAxes() throws NoninvertibleMatrixException {
         final AbstractCoordinateOperation op = new AbstractCoordinateOperation(
                 Collections.singletonMap(AbstractCoordinateOperation.NAME_KEY, "Wrapper"),
                 HardCodedCRS.WGS84_3D_TIME.forConvention(AxesConvention.POSITIVE_RANGE),
@@ -126,7 +122,7 @@ public final strictfp class WraparoundTransformTest extends TestCase {
          * should have been moved by `WraparoundTransform.tryConcatenate(…)` in order to
combine them with initial
          * [normalization} and final {denormalization].
          */
-        final MathTransform wt = WraparoundTransform.forTargetCRS(DefaultFactories.forClass(MathTransformFactory.class),
op);
+        final MathTransform wt = WraparoundTransform.forTargetCRS(op);
         final List<MathTransform> steps = MathTransforms.getSteps(wt);
         assertEquals(4, steps.size());
         assertEquals(0, ((WraparoundTransform) steps.get(1)).wraparoundDimension);


Mime
View raw message