sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] 03/03: Better control on when wraparound is applied.
Date Fri, 02 Oct 2020 19:09:28 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 ba4ad908f20822d09dc011a70fb00b8158e22783
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Fri Oct 2 21:07:01 2020 +0200

    Better control on when wraparound is applied.
---
 .../coverage/grid/CoordinateOperationFinder.java   | 166 ++++++++++++++-------
 .../apache/sis/coverage/grid/GridDerivation.java   |   3 +-
 .../org/apache/sis/coverage/grid/GridGeometry.java |   2 +-
 .../sis/coverage/grid/ResampledGridCoverage.java   |   4 +-
 .../internal/referencing/WraparoundInEnvelope.java |  20 ++-
 5 files changed, 134 insertions(+), 61 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 15fea1f..b71c7f5 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
@@ -135,14 +135,21 @@ final class CoordinateOperationFinder implements Supplier<double[]>
{
      *
      * @see #isWraparoundNeeded
      * @see #isWraparoundApplied
-     * @see #inverse(boolean)
+     * @see #inverse()
      */
     private MathTransform inverseChangeOfCRS;
 
     /**
      * Transform from “grid coordinates of the source” to “geospatial coordinates of
the target”.
      * This is the concatenation of {@link #source} "grid to CRS" with {@link #forwardChangeOfCRS},
-     * cached for possible reuse by {@link #inverse(boolean)}.
+     * possibly with wraparound handling and cached for reuse by {@link #inverse()}:
+     *
+     * {@preformat java
+     *     forwardChangeOfCRS = changeOfCRS.getMathTransform();
+     *     // + wraparound handling if applicable.
+     *     gridToCRS = source.getGridToCRS(anchor);
+     *     gridToCRS = MathTransforms.concatenate(gridToCRS, forwardChangeOfCRS);
+     * }
      *
      * @see #gridToCRS()
      */
@@ -150,14 +157,29 @@ final class CoordinateOperationFinder implements Supplier<double[]>
{
 
     /**
      * Transform from the target CRS to the source grid, with {@link WraparoundTransform}
applied if needed.
-     * This is the concatenation of {@link #inverseChangeOfCRS} with inverse of {@link #source}
"grid to CRS".
+     * This is the concatenation of {@link #inverseChangeOfCRS} with inverse of {@link #source}
"grid to CRS",
+     * possibly with wraparound handling:
+     *
+     * {@preformat java
+     *     inverseChangeOfCRS = forwardChangeOfCRS.inverse();
+     *     // + wraparound handling if applicable.
+     *     crsToGrid = gridToCRS.inverse();
+     *     crsToGrid = MathTransforms.concatenate(inverseChangeOfCRS, crsToGrid);
+     * }
      *
-     * @see #inverse(boolean)
+     * @see #inverse()
      * @see #applyWraparound(MathTransform)
      */
     private MathTransform crsToGrid;
 
     /**
+     * Whether the {@link #isWraparoundNeeded} value has been determined. This flag controls
whether to perform
+     * a more extensive check of wraparound occurrence. This flag should be {@code false}
the first time that
+     * {@link #inverse()} is invoked and {@code true} the next time.
+     */
+    private boolean isWraparoundNeedVerified;
+
+    /**
      * Whether {@link #inverseChangeOfCRS} needs to include a {@link WraparoundTransform}
step. We do this check
      * only for {@link #inverseChangeOfCRS} because it is the transform which will be executed
for every pixels.
      * By contrast, {@link #forwardChangeOfCRS} will systematically contain a {@link WraparoundTransform}
step
@@ -181,6 +203,25 @@ final class CoordinateOperationFinder implements Supplier<double[]>
{
     private boolean isWraparoundApplied;
 
     /**
+     * Whether to disable completely all wraparounds checks.
+     * If {@code true}, then calculation done in this class should be equivalent to following
code:
+     *
+     * {@preformat java
+     *     forwardChangeOfCRS = changeOfCRS.getMathTransform();
+     *     inverseChangeOfCRS = forwardChangeOfCRS.inverse();
+     *     gridToCRS          = source.getGridToCRS(anchor);
+     *     crsToGrid          = gridToCRS.inverse();
+     *     gridToCRS          = MathTransforms.concatenate(gridToCRS, forwardChangeOfCRS);
+     *     crsToGrid          = MathTransforms.concatenate(inverseChangeOfCRS, crsToGrid);
+     * }
+     *
+     * <b>Tip:</b> searching usage of this field should help to identify code
doing wraparound handling.
+     *
+     * @see #nowraparound()
+     */
+    private boolean isWraparoundDisabled;
+
+    /**
      * Creates a new finder initialized to {@link PixelInCell#CELL_CORNER} anchor.
      *
      * @param  source  the grid geometry which is the source of the coordinate operation
to find.
@@ -220,6 +261,22 @@ final class CoordinateOperationFinder implements Supplier<double[]>
{
     }
 
     /**
+     * Disables completely all wraparounds operation.
+     *
+     * @see #isWraparoundDisabled
+     */
+    final void nowraparound() {
+        gridToCRS                = null;        // For forcing recomputation.
+        crsToGrid                = null;
+        forwardChangeOfCRS       = null;
+        inverseChangeOfCRS       = null;
+        isWraparoundNeeded       = false;
+        isWraparoundApplied      = true;
+        isWraparoundNeedVerified = true;
+        isWraparoundDisabled     = true;
+    }
+
+    /**
      * Returns the CRS of the source grid geometry. If neither the source and target grid
geometry
      * define a CRS, then this method returns {@code null}.
      *
@@ -297,7 +354,7 @@ final class CoordinateOperationFinder implements Supplier<double[]>
{
      * Computes the transform from “grid coordinates of the source” to “grid coordinates
of the target”.
      * This is a concatenation of {@link #gridToCRS()} with target "CRS to grid" transform.
      *
-     * <p><b>WARNING:</b> this method may return a mutable transform.
+     * <p><b>WARNING:</b> this method may return a mutable transform (unless
{@link #nowraparound()} has been invoked).
      * That transform should be only short lived (e.g. just the time to transform an envelope).
      * See {@link org.apache.sis.internal.referencing.WraparoundInEnvelope#transform(MathTransform,
Envelope)}.</p>
      *
@@ -329,11 +386,11 @@ final class CoordinateOperationFinder implements Supplier<double[]>
{
      * <h4>Implementation note</h4>
      * After invocation of this method, the following fields are valid:
      * <ul>
-     *   <li>{@link #changeOfCRS} — cached for {@link #inverse(boolean)} usage.</li>
+     *   <li>{@link #changeOfCRS} — cached for {@link #inverse()} usage.</li>
      *   <li>{@link #forwardChangeOfCRS} — cached for next invocation of this {@code
gridToCRS()} method.</li>
      * </ul>
      *
-     * <p><b>WARNING:</b> this method may return a mutable transform.
+     * <p><b>WARNING:</b> this method may return a mutable transform (unless
{@link #nowraparound()} has been invoked).
      * That transform should be only short lived (e.g. just the time to transform an envelope).
      * See {@link org.apache.sis.internal.referencing.WraparoundInEnvelope#transform(MathTransform,
Envelope)}.</p>
      *
@@ -358,16 +415,20 @@ final class CoordinateOperationFinder implements Supplier<double[]>
{
                  */
 apply:          if (forwardChangeOfCRS == null) {
                     forwardChangeOfCRS = changeOfCRS.getMathTransform();
-                    DirectPosition sourceMedian = median(source, forwardChangeOfCRS);
-                    DirectPosition targetMedian = median(target, null);
-                    if (targetMedian == null) {
-                        if (sourceMedian == null) break apply;
-                        targetMedian = sourceMedian;
-                        sourceMedian = null;
+                    if (!isWraparoundDisabled) {
+                        DirectPosition sourceMedian = median(source, forwardChangeOfCRS);
+                        DirectPosition targetMedian = median(target, null);
+                        if (targetMedian == null) {
+                            if (sourceMedian == null) {
+                                break apply;
+                            }
+                            targetMedian = sourceMedian;
+                            sourceMedian = null;
+                        }
+                        final WraparoundApplicator ap = new WraparoundApplicator(sourceMedian,
+                                targetMedian, changeOfCRS.getTargetCRS().getCoordinateSystem());
+                        forwardChangeOfCRS = ap.forDomainOfUse(forwardChangeOfCRS);
                     }
-                    final WraparoundApplicator ap = new WraparoundApplicator(sourceMedian,
-                            targetMedian, changeOfCRS.getTargetCRS().getCoordinateSystem());
-                    forwardChangeOfCRS = ap.forDomainOfUse(forwardChangeOfCRS);
                 }
                 gridToCRS = MathTransforms.concatenate(gridToCRS, forwardChangeOfCRS);
             }
@@ -380,55 +441,56 @@ apply:          if (forwardChangeOfCRS == null) {
      * This is similar to invoking {@link MathTransform#inverse()} on {@link #gridToCRS()},
except in the way
      * wraparounds are handled.
      *
-     * <p>The {@code checkWraparound} argument controls whether to perform a more extensive
check of wraparound occurrence.
-     * The argument should be {@code true} the first time that {@code inverse(…)} is invoked
and {@code false} next time.</p>
-     *
      * @return operation from target geospatial coordinates to source grid indices.
      * @throws FactoryException if no operation can be found between the source and target
CRS.
      * @throws TransformException if some coordinates can not be transformed.
      */
-    final MathTransform inverse(final boolean checkWraparound) throws FactoryException, TransformException
{
+    final MathTransform inverse() throws FactoryException, TransformException {
         final MathTransform sourceCrsToGrid = source.getGridToCRS(anchor).inverse();
         final CoordinateOperation changeOfCRS = changeOfCRS();
         if (changeOfCRS == null) {
             return sourceCrsToGrid;
         }
         if (inverseChangeOfCRS == null) {
-            final MathTransform inverseNoWrap = inverseChangeOfCRS = changeOfCRS.getMathTransform().inverse();
-            isWraparoundApplied = false;
-            if (checkWraparound) {
-                /*
-                 * Need to compute transform with wraparound checks, but contrarily to `gridToCRS()`
we do not want
-                 * `WraparoundTransform` to be systematically inserted. This is for performance
reasons, because the
-                 * transform returned by this method will be applied on every pixels of destination
image. We create
-                 * both transforms with and without wraparound, and check if their results
differ.
-                 *
-                 * We give precedence to corners specified by target extent. The corners
specified by source extent
-                 * are used only as a fallback if the target extent has not been specified,
in which case we assume
-                 * that caller will fallback on source extent transformed to target coordinates.
 The target extent
-                 * is preferred because it may cover only a sub-region of the source, or
conversely it may be world.
-                 * If smaller, wraparound may become useless (i.e. sub-region may not cross
anti-meridian anymore).
-                 * If larger with [-180 … +180]° longitude range, the use of source extent
may fail to detect that
-                 * a part of the raster need to be rendered on each side of the [-180 …
+180]° range.
-                 */
-                final MathTransform crsToGridNoWrap = MathTransforms.concatenate(inverseNoWrap,
sourceCrsToGrid);
-                if (target.isDefined(GridGeometry.EXTENT | GridGeometry.CRS)) {
-                    if (applyWraparound(sourceCrsToGrid)) {
-                        isWraparoundNeeded = isWraparoundNeeded(target.getExtent(),
-                                target.getGridToCRS(anchor), crsToGridNoWrap, null);
+            inverseChangeOfCRS = changeOfCRS.getMathTransform().inverse();
+            if (!isWraparoundDisabled) {
+                isWraparoundApplied = false;
+                if (!isWraparoundNeedVerified) {
+                    isWraparoundNeedVerified = true;
+                    /*
+                     * Need to compute transform with wraparound checks, but contrarily to
`gridToCRS()` we do not want
+                     * `WraparoundTransform` to be systematically inserted. This is for performance
reasons, because the
+                     * transform returned by this method will be applied on every pixels
of destination image. We create
+                     * both transforms with and without wraparound, and check if their results
differ.
+                     *
+                     * We give precedence to corners specified by target extent. The corners
specified by source extent
+                     * are used only as a fallback if the target extent has not been specified,
in which case we assume
+                     * that caller will fallback on source extent transformed to target coordinates.
 The target extent
+                     * is preferred because it may cover only a sub-region of the source,
or conversely it may be world.
+                     * If smaller, wraparound may become useless (i.e. sub-region may not
cross anti-meridian anymore).
+                     * If larger with [-180 … +180]° longitude range, the use of source
extent may fail to detect that
+                     * a part of the raster need to be rendered on each side of the [-180
… +180]° range.
+                     */
+                    final MathTransform inverseNoWrap = inverseChangeOfCRS;
+                    final MathTransform crsToGridNoWrap = MathTransforms.concatenate(inverseNoWrap,
sourceCrsToGrid);
+                    if (target.isDefined(GridGeometry.EXTENT | GridGeometry.CRS)) {
+                        if (applyWraparound(sourceCrsToGrid)) {
+                            isWraparoundNeeded = isWraparoundNeeded(target.getExtent(),
+                                    target.getGridToCRS(anchor), crsToGridNoWrap, null);
+                        }
+                    } else if (source.isDefined(GridGeometry.EXTENT)) {
+                        isWraparoundNeeded = isWraparoundNeeded(source.getExtent(),
+                                gridToCRS(), crsToGridNoWrap, sourceCrsToGrid);
+                    }
+                    if (!isWraparoundNeeded) {
+                        inverseChangeOfCRS = inverseNoWrap;     // Discard the transform
that was applying wraparound.
+                        crsToGrid = crsToGridNoWrap;
                     }
-                } else if (source.isDefined(GridGeometry.EXTENT)) {
-                    isWraparoundNeeded = isWraparoundNeeded(source.getExtent(),
-                            gridToCRS(), crsToGridNoWrap, sourceCrsToGrid);
                 }
-                if (!isWraparoundNeeded) {
-                    inverseChangeOfCRS = inverseNoWrap;     // Discard the transform that
was applying wraparound.
-                    crsToGrid = crsToGridNoWrap;
+                if (isWraparoundNeeded) {
+                    applyWraparound(sourceCrsToGrid);           // Update `inverseChangeOfCRS`
if possible.
                 }
             }
-            if (isWraparoundNeeded) {
-                applyWraparound(sourceCrsToGrid);           // Update `inverseChangeOfCRS`
if possible.
-            }
         }
         /*
          * Here, `inverseChangeOfCRS` already contains the wraparound step if needed.
@@ -662,7 +724,7 @@ apply:          if (forwardChangeOfCRS == null) {
      * Configures the accuracy hints on the given processor.
      *
      * <h4>Pre-requite</h4>
-     * This method assumes that {@link #gridToCRS()} or {@link #inverse(boolean)}
+     * This method assumes that {@link #gridToCRS()} or {@link #inverse()}
      * has already been invoked before this method.
      */
     final void setAccuracyOf(final ImageProcessor processor) {
diff --git a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridDerivation.java
b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridDerivation.java
index 8e6a3c4..ad9e95b 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridDerivation.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridDerivation.java
@@ -428,7 +428,8 @@ public class GridDerivation {
                 final CoordinateOperationFinder finder = new CoordinateOperationFinder(gridOfInterest,
base);
                 final MathTransform mapCorners = finder.gridToGrid();
                 finder.setAnchor(PixelInCell.CELL_CENTER);
-                mapCenters = finder.gridToGrid();
+                finder.nowraparound();
+                mapCenters = finder.gridToGrid();                               // We will
use only the scale factors.
                 clipExtent(domain.toCRS(mapCorners, mapCenters, null));
             } catch (FactoryException | TransformException e) {
                 throw new IllegalGridGeometryException(e, "gridOfInterest");
diff --git a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridGeometry.java
b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridGeometry.java
index 586bde7..e02e8fa 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridGeometry.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridGeometry.java
@@ -1313,7 +1313,7 @@ public class GridGeometry implements LenientComparable, Serializable
{
         finder.setAnchor(anchor);
         final MathTransform tr;
         try {
-            tr = finder.inverse(true);
+            tr = finder.inverse();
         } catch (FactoryException e) {
             throw new TransformException(e);
         }
diff --git a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/ResampledGridCoverage.java
b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/ResampledGridCoverage.java
index 6fbe4c6..906194f 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/ResampledGridCoverage.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/ResampledGridCoverage.java
@@ -233,10 +233,10 @@ final class ResampledGridCoverage extends GridCoverage {
          */
         // Finder is initialized to PixelInCell.CELL_CORNER.
         final MathTransform sourceCornerToCRS = changeOfCRS.gridToCRS();
-        final MathTransform crsToSourceCorner = changeOfCRS.inverse(true);
+        final MathTransform crsToSourceCorner = changeOfCRS.inverse();
         changeOfCRS.setAnchor(PixelInCell.CELL_CENTER);
         final MathTransform sourceCenterToCRS = changeOfCRS.gridToCRS();
-        final MathTransform crsToSourceCenter = changeOfCRS.inverse(false);
+        final MathTransform crsToSourceCenter = changeOfCRS.inverse();
         /*
          * Compute the transform from target grid to target CRS. This transform may be unspecified,
          * in which case we need to compute a default transform trying to preserve resolution
at the
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/WraparoundInEnvelope.java
b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/WraparoundInEnvelope.java
index f45e038..f246339 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/WraparoundInEnvelope.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/WraparoundInEnvelope.java
@@ -45,6 +45,10 @@ import org.apache.sis.util.ComparisonMode;
  * Since this strategy breaks usual {@link org.apache.sis.referencing.operation.transform.AbstractMathTransform}
  * contract about immutability, this class should be used only for temporary transforms to
apply on an envelope.
  *
+ * <h2>Initial state</h2>
+ * On initialization, the {@linkplain #limit} is not applied and this class behaves like
{@link WraparoundTransform}
+ * parent class. The limit is enabled when {@link #transform(MathTransform, Envelope)} is
invoked.
+ *
  * @author  Martin Desruisseaux (Geomatys)
  * @version 1.1
  * @since   1.1
@@ -96,12 +100,12 @@ public final class WraparoundInEnvelope extends WraparoundTransform {
                          final double period, final double sourceMedian)
     {
         super(dimension, wraparoundDimension, period);
+        minCycles = maxCycles = limit = Double.NaN;
         this.sourceMedian = sourceMedian;
         if (ap.lock == null) {
             ap.lock = this;
         }
         lock = ap.lock;
-        reset();
     }
 
     /**
@@ -168,11 +172,14 @@ public final class WraparoundInEnvelope extends WraparoundTransform
{
     }
 
     /**
-     * Resets this transform to its initial state.
+     * Resets this transform to the {@link #limit} value for an initial transform,
+     * or disable the use of limit.
+     *
+     * @param  enabled  whether to enable the {@linkplain #limit}.
      */
-    private void reset() {
+    private void reset(final boolean enabled) {
         synchronized (lock) {
-            minCycles = maxCycles = limit = Math.rint(sourceMedian / period);
+            minCycles = maxCycles = limit = enabled ? Math.rint(sourceMedian / period) :
Double.NaN;
         }
     }
 
@@ -220,6 +227,9 @@ public final class WraparoundInEnvelope extends WraparoundTransform {
             return Envelopes.transform(transform, envelope);
         }
         synchronized (wraparounds[0].lock) {
+            for (final WraparoundInEnvelope tr : wraparounds) {
+                tr.reset(true);
+            }
             final GeneralEnvelope result = Envelopes.transform(transform, envelope);
             for (;;) {
                 boolean done = false;
@@ -230,7 +240,7 @@ public final class WraparoundInEnvelope extends WraparoundTransform {
                 result.add(Envelopes.transform(transform, envelope));
             }
             for (final WraparoundInEnvelope tr : wraparounds) {
-                tr.reset();
+                tr.reset(false);
             }
             return result;
         }


Mime
View raw message