sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] 01/02: Refactor a bit DatumShiftGridGroup for computing sub-grid coordinates in a more direct way than using AffineTransform. This change allows us to fix a bug, where the translation that we got was not adjusted for the different size of the cell.
Date Fri, 14 Feb 2020 23:55:52 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 327b2df53b3e6074e93f85426024d0b82fb6d575
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Sat Feb 15 00:43:28 2020 +0100

    Refactor a bit DatumShiftGridGroup for computing sub-grid coordinates in a more direct
way than using AffineTransform.
    This change allows us to fix a bug, where the translation that we got was not adjusted
for the different size of the cell.
---
 .../internal/referencing/j2d/MosaicCalculator.java |   4 +-
 .../apache/sis/internal/referencing/j2d/Tile.java  |  99 ++----------
 .../referencing/provider/DatumShiftGridFile.java   |   8 +-
 .../referencing/provider/DatumShiftGridGroup.java  | 179 ++++++++++++++-------
 4 files changed, 134 insertions(+), 156 deletions(-)

diff --git a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/MosaicCalculator.java
b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/MosaicCalculator.java
index b0654cd..e1f4445 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/MosaicCalculator.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/MosaicCalculator.java
@@ -511,8 +511,6 @@ public class MosaicCalculator {
      */
     @Override
     public String toString() {
-        final List<Tile> tiles = new ArrayList<>(this.tiles.values());
-        Collections.sort(tiles);
-        return Tile.toString(tiles, 400);
+        return Tile.toString(tiles.values(), 400);
     }
 }
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/Tile.java
b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/Tile.java
index ec755eb..847fb42 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/Tile.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/j2d/Tile.java
@@ -104,7 +104,7 @@ import org.apache.sis.io.TableAppender;
  * @since   1.1
  * @module
  */
-public class Tile implements Comparable<Tile>, Serializable {
+public class Tile implements Serializable {
     /**
      * For cross-version compatibility during serialization.
      */
@@ -506,88 +506,13 @@ public class Tile implements Comparable<Tile>, Serializable {
         return 0;
     }
 
-    /**
-     * Compares two tiles for optimal order in sequential reads. Default implementation sorts
by
-     * increasing {@linkplain #getImageIndex image index}. This ordering allows efficient
access
-     * for tiles that are stored sequentially in a file.
-     *
-     * <p>For tiles having the same image index, additional criterion are used like
increasing
-     * subsampling, increasing <var>y</var> then increasing <var>x</var>
coordinates.
-     * But the actual set of additional criterion may change in any future version.
-     *
-     * @param  other  the tile to compare with.
-     * @return -1 if this tile should be read before {@code other},
-     *         +1 if it should be read after or 0 if equal.
-     */
-    @Override
-    public final int compareTo(final Tile other) {
-        int c = getImageIndex() - other.getImageIndex();
-        if (c == 0) {
-            /*
-             * From this point it does not matter much for disk access. But we continue to
-             * define criterions for consistency with `equals(Object)` method. We compare
-             * subsampling first because it may be undefined while it is needed for (x,y)
-             * ordering. Undefined subsampling will be ordered first (this is arbitrary).
-             */
-            final int sy =  this.ySubsampling;
-            final int oy = other.ySubsampling;
-            c = sy - oy;
-            if (c == 0) {
-                final int sx =  this.xSubsampling;
-                final int ox = other.xSubsampling;
-                c = sx - ox;
-                if (c == 0) {
-                    c = (y * sy) - (other.y * oy);
-                    if (c == 0) {
-                        c = (x * sx) - (other.x * ox);
-                    }
-                }
-            }
-        }
-        return c;
-    }
-
-    /**
-     * Compares this tile with the specified one for equality. Two tiles are considered equal
-     * if they are of the same class and have the same {@linkplain #getRegion() region} and
-     * same {@linkplain #getSubsampling() subsampling}. Subclasses should override if there
-     * is more properties to compare such as image format and index.
-     *
-     * @param  object  the object to compare with.
-     * @return {@code true} if both objects are equal.
+    /*
+     * Intentionally no implementation for `equals()` and `hashCode()`. Tile is an "almost
immutable" class
+     * which can still be modified (only once) by MocaicCalculator, or by read operations
during `getSize()`
+     * or `getRegion()` execution. This causes confusing behavior when used in an HashMap.
We are better to
+     * rely on system identity. For example `DatumShiftGridGroup` rely on the capability
to locate Tiles in
+     * HashMap before and after they have been processed by `MosaicCalculator`.
      */
-    @Override
-    public boolean equals(final Object object) {
-        if (object == this) {
-            return true;
-        }
-        if (object != null && object.getClass() == getClass()) {
-            final Tile that = (Tile) object;
-            if (this.x == that.x  &&  this.y == that.y &&
-                this.xSubsampling == that.xSubsampling &&
-                this.ySubsampling == that.ySubsampling)
-            {
-                /*
-                 * Compares width and height only if they are defined in both tiles.  We
do not
-                 * invoke `getRegion()` because it may be expensive and useless anyway: If
both
-                 * tiles have the same image reader, image index and input, then logically
they
-                 * must have the same size - invoking `getRegion()` would read exactly the
same
-                 * image twice.
-                 */
-                return (width  == 0 || that.width  == 0 || width  == that.width) &&
-                       (height == 0 || that.height == 0 || height == that.height);
-            }
-        }
-        return false;
-    }
-
-    /**
-     * Returns a hash code value for this tile.
-     */
-    @Override
-    public int hashCode() {
-        return x + 37*y;
-    }
 
     /**
      * Returns a string representation of this tile for debugging purposes.
@@ -632,9 +557,8 @@ public class Tile implements Comparable<Tile>, Serializable {
     }
 
     /**
-     * Returns a string representation of a collection of tiles. The tiles are formatted
in a
-     * table in iteration order. Tip: consider sorting the tiles before to invoke this method;
-     * tiles are {@link Comparable} for this purpose.
+     * Returns a string representation of a collection of tiles.
+     * The tiles are formatted in a table in iteration order.
      *
      * <p>This method is not public because it can consume a large amount of memory
(the underlying
      * {@link StringBuffer} can be quite large). Users are encouraged to use the method expecting
a
@@ -658,9 +582,8 @@ public class Tile implements Comparable<Tile>, Serializable {
     }
 
     /**
-     * Formats a collection of tiles in a table. The tiles are appended in iteration order.
-     * Tip: consider sorting the tiles before to invoke this method;
-     * tiles are {@link Comparable} for this purpose.
+     * Formats a collection of tiles in a table.
+     * The tiles are appended in iteration order.
      *
      * @param tiles    the tiles to format in a table.
      * @param out      where to write the table.
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/provider/DatumShiftGridFile.java
b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/provider/DatumShiftGridFile.java
index 1826544..c9c3794 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/provider/DatumShiftGridFile.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/provider/DatumShiftGridFile.java
@@ -71,11 +71,11 @@ abstract class DatumShiftGridFile<C extends Quantity<C>, T extends
Quantity<T>>
     static final Cache<Object, DatumShiftGridFile<?,?>> CACHE = new Cache<Object,
DatumShiftGridFile<?,?>>(4, 32*1024, true) {
         @Override protected int cost(final DatumShiftGridFile<?,?> grid) {
             int p = 1;
-            for (final Object array : grid.getData()) {
-                if (array instanceof DatumShiftGridFile<?,?>) {
-                    p += cost((DatumShiftGridFile<?,?>) array);
+            for (final Object data : grid.getData()) {
+                if (data instanceof DatumShiftGridFile<?,?>) {
+                    p += cost((DatumShiftGridFile<?,?>) data);          // When `grid`
is a DatumShiftGridGroup.
                 } else {
-                    p *= Array.getLength(array);
+                    p *= Array.getLength(data);                         // short[], float[]
or double[].
                 }
             }
             return p;
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/provider/DatumShiftGridGroup.java
b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/provider/DatumShiftGridGroup.java
index d9b5d32..e634375 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/provider/DatumShiftGridGroup.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/provider/DatumShiftGridGroup.java
@@ -20,12 +20,14 @@ import java.util.Map;
 import java.util.List;
 import java.nio.file.Path;
 import java.io.IOException;
+import java.awt.Dimension;
 import java.awt.Rectangle;
-import java.awt.geom.Point2D;
 import java.awt.geom.AffineTransform;
+import java.util.LinkedHashMap;
 import javax.measure.Quantity;
 import org.opengis.util.FactoryException;
 import org.opengis.referencing.operation.NoninvertibleTransformException;
+import org.apache.sis.referencing.operation.transform.InterpolatedTransform;
 import org.apache.sis.internal.referencing.j2d.AffineTransform2D;
 import org.apache.sis.internal.referencing.j2d.MosaicCalculator;
 import org.apache.sis.internal.referencing.j2d.Tile;
@@ -35,10 +37,8 @@ import org.apache.sis.internal.util.CollectionsExt;
 
 /**
  * A group of datum shift grids. This is used when a NTv2 file contains more than one grid
with no common parent.
- * This class creates a synthetic parent with an affine transform approximating all grids.
The affine transform is
- * close to identity transform. Its main purpose is to locate a grid during inverse transforms,
before refinements
- * using the real grids.  So a "best match" transform (for example estimated using least
squares method) would not
- * be useful because the differences would be small compared to grid cell sizes.
+ * This class creates a synthetic parent which always delegate its work to a child (as opposed
to more classical
+ * trees where the parent can do some work if no child can).
  *
  * @author  Martin Desruisseaux (Geomatys)
  * @version 1.1
@@ -47,66 +47,79 @@ import org.apache.sis.internal.util.CollectionsExt;
  */
 final class DatumShiftGridGroup<C extends Quantity<C>, T extends Quantity<T>>
extends DatumShiftGridFile<C,T> {
     /**
-     * For each {@code subgrids[i]}, {@code regions[i]} is the range of indices valid of
that grid.
-     *
-     * @todo We should replace by an R-Tree. For now we assume that the array is small enough.
+     * The bounds of a sub-grid, together with the subsampling level compared to the grid
having the finest resolution.
+     * All values in this class are integers, but nevertheless stored as {@code double} for
avoiding to cast them every
+     * time {@link #interpolateInCell(double, double, double[])} is executed.
      */
-    private final Rectangle[] regions;
+    private static final class Region {
+        /** Grid bounds in units of the grid having finest resolution. */
+        private final double xmin, xmax, ymin, ymax;
+
+        /** Subsampling compared to the grid having finest resolution. */
+        private final double sx, sy;
+
+        /** Creates a new instance from the given {@link MosaicCalculator} result. */
+        Region(final Tile tile) throws IOException {
+            final Rectangle r = tile.getAbsoluteRegion();
+            final Dimension s = tile.getSubsampling();
+            xmin = r.getMinX();
+            xmax = r.getMaxX();
+            ymin = r.getMinY();
+            ymax = r.getMaxY();
+            sx   = s.width;
+            sy   = s.height;
+        }
+
+        /** Tests whether the given coordinates are included in this region. */
+        final boolean contains(final double x, final double y) {
+            return x >= xmin && x <= xmax && y >= ymin &&
y <= ymax;
+        }
+
+        /** Converts a coordinate from the parent grid to this grid. */
+        final double x(final double p) {return (p - xmin) / sx;}
+        final double y(final double p) {return (p - ymin) / sy;}
+
+        /** Returns the subsampling (compared to the grid having finest resolution) in the
specified dimension. */
+        final double relativeCellSize(final int dim) {
+            switch (dim) {
+                case 0:  return sx;
+                case 1:  return sy;
+                default: throw new IndexOutOfBoundsException();
+            }
+        }
+    }
 
     /**
-     * Converts indices from this grid to indices in sub-grids.
+     * For each {@code subgrids[i]}, {@code regions[i]} is the range of indices valid of
that grid.
+     * This array will be used only as a fallback if the {@code MathTransform} has not been
able to
+     * find the sub-grid itself. Since it should be rarely used, we do not bother using a
R-Tree.
      */
-    private final AffineTransform[] toSubGrids;
+    private final Region[] regions;
 
     /**
      * Creates a new group for the given list of sub-grids. That list shall contain at least
2 elements.
      * The first sub-grid is taken as a template for setting parameter values such as filename
(all list
      * elements should declare the same filename parameters, so the selected element should
not matter).
      *
-     * @param grids      sub-grids with their indices range. The array is declared as {@code
Tile[]}
-     *                   because this is the type returned by {@link MosaicCalculator#tiles()},
but
-     *                   each element shall be an instance of {@link Region}.
-     * @param gridToCRS  conversion from grid indices to "real world" coordinates.
-     * @param nx         number of cells along the <var>x</var> axis in the grid.
-     * @param ny         number of cells along the <var>y</var> axis in the grid.
+     * @param  tiles      the tiles computed by {@link MosaicCalculator}.
+     * @param  grids      sub-grids associated to tiles computed by {@link MosaicCalculator}.
+     * @param  gridToCRS  conversion from grid indices to "real world" coordinates.
+     * @param  nx         number of cells along the <var>x</var> axis in the
grid.
+     * @param  ny         number of cells along the <var>y</var> axis in the
grid.
      * @throws IOException declared because {@link Tile#getRegion()} declares it, but should
not happen.
      */
     @SuppressWarnings({"rawtypes", "unchecked"})
-    private DatumShiftGridGroup(final Tile[] grids, final AffineTransform2D gridToCRS, final
int nx, final int ny)
+    private DatumShiftGridGroup(final Tile[] tiles, final Map<Tile,DatumShiftGridFile<C,T>>
grids,
+            final AffineTransform2D gridToCRS, final int nx, final int ny)
             throws IOException, NoninvertibleTransformException
     {
-        super((DatumShiftGridFile<C,T>) ((Region) grids[0]).grid, gridToCRS.inverse(),
nx, ny);
-        subgrids   = new DatumShiftGridFile[grids.length];
-        regions    = new Rectangle[grids.length];
-        toSubGrids = new AffineTransform[grids.length];
-        for (int i=0; i<grids.length; i++) {
-            final Region r = (Region) grids[i];
-            final AffineTransform tr = new AffineTransform(gridToCRS);
-            tr.preConcatenate((AffineTransform) r.grid.getCoordinateToGrid());
-            subgrids  [i] = (DatumShiftGridFile<C,T>) r.grid;
-            regions   [i] = r.getAbsoluteRegion();
-            toSubGrids[i] = tr;
-        }
-    }
-
-    /**
-     * A sub-grid wrapped with information about the region where it applies.
-     * The region is expressed as indices in a larger grid. That larger grid
-     * is what {@link MosaicCalculator} will try to infer.
-     */
-    @SuppressWarnings("serial")
-    private static final class Region extends Tile {
-        /** The wrapped sub-grid. */
-        final DatumShiftGridFile<?,?> grid;
-
-        /**
-         * Creates a new wrapper for the given sub-grid.
-         *
-         * @param size  value of {@link DatumShiftGridFile#getGridSize()}.
-         */
-        Region(final DatumShiftGridFile<?,?> grid, final int[] size) throws NoninvertibleTransformException
{
-            super(new Rectangle(size[0], size[1]), (AffineTransform) grid.getCoordinateToGrid().inverse());
-            this.grid = grid;
+        super(grids.get(tiles[0]), gridToCRS.inverse(), nx, ny);
+        final int n = grids.size();
+        regions  = new Region[n];
+        subgrids = new DatumShiftGridFile[n];
+        for (int i=0; i<n; i++) {
+            regions [i] = new Region(tiles[i]);
+            subgrids[i] = grids.get(tiles[i]);
         }
     }
 
@@ -124,8 +137,16 @@ final class DatumShiftGridGroup<C extends Quantity<C>, T extends
Quantity<T>> ex
             throws IOException, FactoryException, NoninvertibleTransformException
     {
         final MosaicCalculator mosaic = new MosaicCalculator(null);
+        final Map<Tile,DatumShiftGridFile<C,T>> grids = new LinkedHashMap<>();
         for (final DatumShiftGridFile<C,T> grid : subgrids) {
-            mosaic.add(new Region(grid, grid.getGridSize()));
+            final int[] size = grid.getGridSize();
+            final Tile tile = new Tile(new Rectangle(size[0], size[1]),
+                    (AffineTransform) grid.getCoordinateToGrid().inverse());
+            if (mosaic.add(tile)) {                                     // Should never be
false, but check anyway.
+                if (grids.put(tile, grid) != null) {
+                    throw new AssertionError(tile);                     // Should never happen
(paranoiac check).
+                }
+            }
         }
         final Map.Entry<Tile,Tile[]> result = CollectionsExt.singletonOrNull(mosaic.tiles().entrySet());
         if (result == null) {
@@ -133,7 +154,7 @@ final class DatumShiftGridGroup<C extends Quantity<C>, T extends
Quantity<T>> ex
         }
         final Tile global = result.getKey();
         final Rectangle r = global.getRegion();
-        return new DatumShiftGridGroup<>(result.getValue(), global.getGridToCRS(),
r.width, r.height);
+        return new DatumShiftGridGroup<>(result.getValue(), grids, global.getGridToCRS(),
r.width, r.height);
     }
 
     /**
@@ -142,9 +163,8 @@ final class DatumShiftGridGroup<C extends Quantity<C>, T extends
Quantity<T>> ex
      */
     private DatumShiftGridGroup(final DatumShiftGridGroup<C,T> other, final DatumShiftGridFile<C,T>[]
data) {
         super(other);
-        subgrids   = data;
-        regions    = other.regions;
-        toSubGrids = other.toSubGrids;
+        subgrids = data;
+        regions  = other.regions;
     }
 
     /**
@@ -182,6 +202,9 @@ final class DatumShiftGridGroup<C extends Quantity<C>, T extends
Quantity<T>> ex
 
     /**
      * Returns the translation stored at the given two-dimensional grid indices for the given
dimension.
+     * This method is defined for consistency with {@link #interpolateInCell(double, double,
double[])}
+     * but should never be invoked. The {@link InterpolatedTransform} class will rather invoke
the
+     * {@code interpolateInCell} method for efficiency.
      *
      * @param  dim    the dimension of the translation vector component to get.
      * @param  gridX  the grid index on the <var>x</var> axis, from 0 inclusive
to {@code gridSize[0]} exclusive.
@@ -191,15 +214,49 @@ final class DatumShiftGridGroup<C extends Quantity<C>, T extends
Quantity<T>> ex
     @Override
     public double getCellValue(final int dim, final int gridX, final int gridY) {
         for (int i=0; i<regions.length; i++) {
-            final Rectangle r = regions[i];
+            final Region r = regions[i];
             if (r.contains(gridX, gridY)) {
-                Point2D pt = new Point2D.Double(gridX, gridY);
-                pt = toSubGrids[i].transform(pt, pt);
-                return subgrids[i].getCellValue(dim,
-                        Math.toIntExact(Math.round(pt.getX())),
-                        Math.toIntExact(Math.round(pt.getY())));
+                double shift = subgrids[i].getCellValue(dim,
+                        Math.toIntExact(Math.round(r.x(gridX))),
+                        Math.toIntExact(Math.round(r.y(gridY))));
+                /*
+                 * If the translations have been divided by the cell size, we may need to
compensate.
+                 * The size of the cells of the grid used below may be bigger than the cells
of this
+                 * pseudo-grid.
+                 */
+                if (isCellValueRatio()) {
+                    shift *= r.relativeCellSize(dim);
+                }
+                return shift;
             }
         }
         throw new IndexOutOfBoundsException();
     }
+
+    /**
+     * Interpolates the translation to apply for the given two-dimensional grid indices.
The result is stored
+     * in the given {@code vector} array. This method is invoked only as a fallback if the
transform has not
+     * been able to use directly one of the child transforms. Consequently this implementation
does not need
+     * to be very fast.
+     *
+     * @param  gridX   first grid coordinate of the point for which to get the translation.
+     * @param  gridY   second grid coordinate of the point for which to get the translation.
+     * @param  vector  a pre-allocated array where to write the translation vector.
+     */
+    @Override
+    public void interpolateInCell(final double gridX, final double gridY, final double[]
vector) {
+        for (int i=0; i<regions.length; i++) {
+            final Region r = regions[i];
+            if (r.contains(gridX, gridY)) {
+                subgrids[i].interpolateInCell(r.x(gridX), r.y(gridY), vector);
+                if (isCellValueRatio()) {
+                    for (int dim=0; dim < INTERPOLATED_DIMENSIONS; dim++) {
+                        vector[dim] *= r.relativeCellSize(dim);
+                    }
+                }
+                return;
+            }
+        }
+        super.interpolateInCell(gridX, gridY, vector);
+    }
 }


Mime
View raw message