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: Bug fixes in optimizations (translation case not always detected, existing transforms not reused).
Date Wed, 16 Sep 2020 19:19:54 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 a6615f9  Bug fixes in optimizations (translation case not always detected, existing
transforms not reused).
a6615f9 is described below

commit a6615f92077c0d911f7ebc520a66917d7c463545
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Wed Sep 16 21:18:40 2020 +0200

    Bug fixes in optimizations (translation case not always detected, existing transforms
not reused).
---
 .../internal/referencing/WraparoundTransform.java  | 50 +++++++++++++---------
 .../operation/transform/ProjectiveTransform.java   |  4 +-
 2 files changed, 32 insertions(+), 22 deletions(-)

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 0131f6a..46a9287 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
@@ -397,7 +397,8 @@ public final class WraparoundTransform extends AbstractMathTransform implements
         final List<MathTransform> steps = MathTransforms.getSteps(other);
         final int count = steps.size();
         if (count >= 2) {
-            Matrix middle = MathTransforms.getMatrix(steps.get(applyOtherFirst ? count -
1 : 0));
+            final MathTransform middleTr = steps.get(applyOtherFirst ? count - 1 : 0);
+            Matrix middle = MathTransforms.getMatrix(middleTr);
             if (middle != null) try {
                 /*
                  * We have a matrix between this `WraparoundTransform` and something else,
@@ -409,7 +410,7 @@ public final class WraparoundTransform extends AbstractMathTransform implements
                 final MathTransformsOrFactory mf = MathTransformsOrFactory.wrap(factory);
                 boolean modified = false;
                 MathTransform step1 = this;
-                MathTransform move = movable(middle, mf);
+                MathTransform move = movable(middleTr, middle, mf);
                 if (move != null) {
                     /*
                      * Update the middle matrix with everything that we could not put in
`move`.
@@ -441,7 +442,7 @@ public final class WraparoundTransform extends AbstractMathTransform implements
                 MathTransform step2 = steps.get(applyOtherFirst ? count - 2 : 1);
                 if (step2 instanceof WraparoundTransform) {
                     WraparoundTransform redim = (WraparoundTransform) step2;
-                    move = redim.movable(middle, mf);
+                    move = redim.movable(null, middle, mf);
                     if (move != null) {
                         final Matrix remaining = remaining(!applyOtherFirst, move, middle);
                         redim = redim.redim(!applyOtherFirst, remaining);
@@ -482,13 +483,16 @@ public final class WraparoundTransform extends AbstractMathTransform
implements
      * Returns a transform based on the given matrix but converting only coordinates in dimensions
      * that can be processed indifferently before or after this {@code WraparoundTransform}.
      *
+     * @param  tr       the transform of the matrix to analyze, or {@code null} if none.
      * @param  matrix   the matrix to analyze.
      * @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 MathTransformsOrFactory factory) throws
FactoryException {
-        long canMoveAfter = Numerics.bitmask(dimension) - 1;
-        canMoveAfter &= ~Numerics.bitmask(wraparoundDimension);
+    private MathTransform movable(MathTransform tr, Matrix matrix, final MathTransformsOrFactory
factory)
+            throws FactoryException
+    {
+        final long moveAll = Numerics.bitmask(dimension) - 1;
+        long canMoveAfter = moveAll;
         /*
          * If any matrix row (output coordinate) uses the wraparound dimension as input,
          * then we can not move that row because the coordinate value may not be the same
@@ -496,28 +500,34 @@ public final class WraparoundTransform extends AbstractMathTransform
implements
          */
         if (matrix.getNumCol() - 1 > wraparoundDimension) {
             for (int j = matrix.getNumRow(); --j >= 0;) {
-                if (matrix.getElement(j, wraparoundDimension) != 0) {
+                final double v = matrix.getElement(j, wraparoundDimension);
+                if (v != (j == wraparoundDimension ? 1 : 0)) {
                     canMoveAfter &= ~Numerics.bitmask(j);
                 }
             }
         }
         if (canMoveAfter != 0) {
-            /*
-             * Create a matrix which will convert coordinates in all dimensions
-             * that we can process before or after this `WraparoundTransform`.
-             * We start with a copy and set to identity the rows that we can not move.
-             */
-            matrix = Matrices.copy(matrix);
-            for (int j = matrix.getNumRow() - 1; --j >=0;) {
-                if ((canMoveAfter & Numerics.bitmask(j)) == 0) {
-                    for (int i=matrix.getNumCol(); --i >= 0;) {
-                        matrix.setElement(j, i, (i == j) ? 1 : 0);
+            if (canMoveAfter != moveAll) {
+                /*
+                 * Create a matrix which will convert coordinates in all dimensions
+                 * that we can process before or after this `WraparoundTransform`.
+                 * We start with a copy and set to identity the rows that we can not move.
+                 */
+                matrix = Matrices.copy(matrix);
+                for (int j = matrix.getNumRow() - 1; --j >=0;) {
+                    if ((canMoveAfter & Numerics.bitmask(j)) == 0) {
+                        for (int i=matrix.getNumCol(); --i >= 0;) {
+                            matrix.setElement(j, i, (i == j) ? 1 : 0);
+                        }
                     }
                 }
+            } else if (tr != null) {
+                return tr;
+                // OTherwise `matrix` can be used as-is.
+            }
+            if (!matrix.isIdentity()) {
+                return factory.linear(matrix);
             }
-        }
-        if (!matrix.isIdentity()) {
-            return factory.linear(matrix);
         }
         return null;
     }
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/ProjectiveTransform.java
b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/ProjectiveTransform.java
index 5dc40ff..f8d0175 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/ProjectiveTransform.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/transform/ProjectiveTransform.java
@@ -119,8 +119,8 @@ class ProjectiveTransform extends AbstractLinearTransform implements ExtendedPre
             if (v != 0) {
                 final int row  = (i / numCol);
                 final int col  = (i % numCol);
-                isScale       &= row == col;
-                isTranslation &= (col == lastColumn) || (isScale && v == 1);
+                isScale       &= (col == row);
+                isTranslation &= (col == lastColumn) || (col == row && v == 1);
                 if (!(isScale | isTranslation)) {
                     return this;
                 }


Mime
View raw message