Return-Path: X-Original-To: apmail-sis-commits-archive@www.apache.org Delivered-To: apmail-sis-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 26C5B18215 for ; Wed, 20 Apr 2016 22:05:03 +0000 (UTC) Received: (qmail 30332 invoked by uid 500); 20 Apr 2016 22:05:03 -0000 Delivered-To: apmail-sis-commits-archive@sis.apache.org Received: (qmail 30300 invoked by uid 500); 20 Apr 2016 22:05:03 -0000 Mailing-List: contact commits-help@sis.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: sis-dev@sis.apache.org Delivered-To: mailing list commits@sis.apache.org Received: (qmail 30291 invoked by uid 99); 20 Apr 2016 22:05:03 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd2-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 20 Apr 2016 22:05:03 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd2-us-west.apache.org (ASF Mail Server at spamd2-us-west.apache.org) with ESMTP id A58AF1A0961 for ; Wed, 20 Apr 2016 22:05:02 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd2-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 1.799 X-Spam-Level: * X-Spam-Status: No, score=1.799 tagged_above=-999 required=6.31 tests=[KAM_ASCII_DIVIDERS=0.8, KAM_LAZY_DOMAIN_SECURITY=1, RP_MATCHES_RCVD=-0.001] autolearn=disabled Received: from mx2-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd2-us-west.apache.org [10.40.0.9]) (amavisd-new, port 10024) with ESMTP id PSnq3MoKDSfq for ; Wed, 20 Apr 2016 22:04:59 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx2-lw-eu.apache.org (ASF Mail Server at mx2-lw-eu.apache.org) with ESMTP id F064A5F1F0 for ; Wed, 20 Apr 2016 22:04:58 +0000 (UTC) Received: from svn01-us-west.apache.org (svn.apache.org [10.41.0.6]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id EC546E0095 for ; Wed, 20 Apr 2016 22:04:57 +0000 (UTC) Received: from svn01-us-west.apache.org (localhost [127.0.0.1]) by svn01-us-west.apache.org (ASF Mail Server at svn01-us-west.apache.org) with ESMTP id 973213A0140 for ; Wed, 20 Apr 2016 22:04:57 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: svn commit: r1740204 - in /sis/branches/JDK8/core/sis-referencing/src: main/java/org/apache/sis/referencing/cs/ main/java/org/apache/sis/referencing/operation/ test/java/org/apache/sis/referencing/operation/ Date: Wed, 20 Apr 2016 22:04:57 -0000 To: commits@sis.apache.org From: desruisseaux@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20160420220457.973213A0140@svn01-us-west.apache.org> Author: desruisseaux Date: Wed Apr 20 22:04:56 2016 New Revision: 1740204 URL: http://svn.apache.org/viewvc?rev=1740204&view=rev Log: Fix a mismatched dimension when transforming coordinates using position vector transformation from geographic 2D to geographic 3D domains. Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/cs/AbstractCS.java sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CRSPair.java sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/CoordinateOperationRegistryTest.java Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/cs/AbstractCS.java URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/cs/AbstractCS.java?rev=1740204&r1=1740203&r2=1740204&view=diff ============================================================================== --- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/cs/AbstractCS.java [UTF-8] (original) +++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/cs/AbstractCS.java [UTF-8] Wed Apr 20 22:04:56 2016 @@ -391,9 +391,10 @@ public class AbstractCS extends Abstract * @return {@code true} if both objects are equal. */ @Override + @SuppressWarnings("fallthrough") public boolean equals(final Object object, final ComparisonMode mode) { if (object == this) { - return true; // Slight optimization. + return true; // Slight optimization. } if (!super.equals(object, mode)) { return false; @@ -403,6 +404,14 @@ public class AbstractCS extends Abstract // No need to check the class - this check has been done by super.equals(…). return Arrays.equals(axes, ((AbstractCS) object).axes); } + case DEBUG: { + final int d1 = axes.length; + final int d2 = ((CoordinateSystem) object).getDimension(); + if (d1 != d2) { + throw new AssertionError(Errors.format(Errors.Keys.MismatchedDimension_2, d1, d2)); + } + // Fall through + } default: { final CoordinateSystem that = (CoordinateSystem) object; final int dimension = getDimension(); Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CRSPair.java URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CRSPair.java?rev=1740204&r1=1740203&r2=1740204&view=diff ============================================================================== --- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CRSPair.java [UTF-8] (original) +++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CRSPair.java [UTF-8] Wed Apr 20 22:04:56 2016 @@ -16,6 +16,8 @@ */ package org.apache.sis.referencing.operation; +import org.opengis.referencing.cs.EllipsoidalCS; +import org.opengis.referencing.cs.CoordinateSystem; import org.opengis.referencing.crs.CoordinateReferenceSystem; import org.opengis.referencing.IdentifiedObject; import org.apache.sis.referencing.AbstractIdentifiedObject; @@ -79,8 +81,9 @@ final class CRSPair { } /** - * Returns the name of the GeoAPI interface implemented by the specified object, - * followed by the name between brackets. + * Returns the name of the GeoAPI interface implemented by the specified object. In the GeographicCRS + * or EllipsoidalCS cases, the trailing CRS or CS suffix is replaced by the number of dimensions + * (e.g. "Geographic3D"). */ static String label(final IdentifiedObject object) { if (object == null) { @@ -92,7 +95,18 @@ final class CRSPair { } else { type = Classes.getLeafInterfaces(object.getClass(), IdentifiedObject.class)[0]; } - String label = Classes.getShortName(type); + String suffix, label = Classes.getShortName(type); + if (label.endsWith((suffix = "CRS")) || label.endsWith(suffix = "CS")) { + Object cs = object; + if (object instanceof CoordinateReferenceSystem) { + cs = ((CoordinateReferenceSystem) object).getCoordinateSystem(); + } + if (cs instanceof EllipsoidalCS) { + final StringBuilder sb = new StringBuilder(label); + sb.setLength(label.length() - suffix.length()); + label = sb.append(((CoordinateSystem) cs).getDimension()).append('D').toString(); + } + } String name = IdentifiedObjects.getName(object, null); if (name != null) { int i = 30; // Arbitrary length threshold. Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java?rev=1740204&r1=1740203&r2=1740204&view=diff ============================================================================== --- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java [UTF-8] (original) +++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java [UTF-8] Wed Apr 20 22:04:56 2016 @@ -38,6 +38,7 @@ import org.opengis.referencing.Identifie import org.opengis.referencing.NoSuchAuthorityCodeException; import org.opengis.referencing.crs.CoordinateReferenceSystem; import org.opengis.referencing.crs.GeodeticCRS; +import org.opengis.referencing.crs.SingleCRS; import org.opengis.referencing.cs.EllipsoidalCS; import org.opengis.referencing.operation.*; @@ -294,7 +295,8 @@ class CoordinateOperationRegistry { * need to modify the coordinate operation in order to match the new number of dimensions. */ if (combine != 0) { - operation = propagateVertical(operation, source != sourceCRS, target != targetCRS); + operation = propagateVertical(sourceCRS, source != sourceCRS, + targetCRS, target != targetCRS, operation); if (operation == null) { continue; } @@ -303,8 +305,12 @@ class CoordinateOperationRegistry { return operation; } } catch (IllegalArgumentException | ConversionException e) { - throw new FactoryException(Errors.format( - Errors.Keys.CanNotInstantiate_1, new CRSPair(sourceCRS, targetCRS)), e); + String message = Errors.format(Errors.Keys.CanNotInstantiate_1, new CRSPair(sourceCRS, targetCRS)); + String details = e.getLocalizedMessage(); + if (details != null) { + message = message + ' ' + details; + } + throw new FactoryException(message, e); } } } @@ -538,7 +544,8 @@ class CoordinateOperationRegistry { final MathTransformFactory mtFactory) throws IllegalArgumentException, ConversionException, FactoryException { - assert Utilities.deepEquals(sourceCRS, targetCRS, ComparisonMode.ALLOW_VARIANT); + assert ReferencingUtilities.getDimension(sourceCRS) != ReferencingUtilities.getDimension(targetCRS) + || Utilities.deepEquals(sourceCRS, targetCRS, ComparisonMode.ALLOW_VARIANT); final Matrix m = CoordinateSystems.swapAndScaleAxes(sourceCRS.getCoordinateSystem(), targetCRS.getCoordinateSystem()); return (m.isIdentity()) ? null : mtFactory.createAffineTransform(m); } @@ -692,16 +699,20 @@ class CoordinateOperationRegistry { * best effort basis. In any cases, the {@link #complete} method should be invoked * after this one in order to ensure that the source and target CRS are the expected ones.

* - * @param operation the original (typically two-dimensional) coordinate operation. + * @param sourceCRS the potentially three-dimensional source CRS * @param source3D {@code true} for adding ellipsoidal height in source coordinates. + * @param targetCRS the potentially three-dimensional target CRS * @param target3D {@code true} for adding ellipsoidal height in target coordinates. + * @param operation the original (typically two-dimensional) coordinate operation. * @return a coordinate operation with the source and/or target coordinates made 3D, * or {@code null} if this method does not know how to create the operation. * @throws IllegalArgumentException if the operation method can not have the desired number of dimensions. * @throws FactoryException if an error occurred while creating the coordinate operation. */ - private CoordinateOperation propagateVertical(final CoordinateOperation operation, - final boolean source3D, final boolean target3D) throws IllegalArgumentException, FactoryException + private CoordinateOperation propagateVertical(final CoordinateReferenceSystem sourceCRS, final boolean source3D, + final CoordinateReferenceSystem targetCRS, final boolean target3D, + final CoordinateOperation operation) + throws IllegalArgumentException, FactoryException { final List operations = new ArrayList<>(); if (operation instanceof ConcatenatedOperation) { @@ -709,8 +720,8 @@ class CoordinateOperationRegistry { } else { operations.add(operation); } - if ((source3D && !propagateVertical(operations.listIterator(), true)) || - (target3D && !propagateVertical(operations.listIterator(operations.size()), false))) + if ((source3D && !propagateVertical(sourceCRS, targetCRS, operations.listIterator(), true)) || + (target3D && !propagateVertical(sourceCRS, targetCRS, operations.listIterator(operations.size()), false))) { return null; } @@ -727,13 +738,18 @@ class CoordinateOperationRegistry { * Appends a vertical axis in the source CRS of the first step {@code forward = true} or in * the target CRS of the last step {@code forward = false} of the given operations chain. * + * @param source3D the potentially three-dimensional source CRS + * @param target3D the potentially three-dimensional target CRS * @param operations the chain of operations in which to add a vertical axis. * @param forward {@code true} for adding the vertical axis at the beginning, or * {@code false} for adding the vertical axis at the end. * @return {@code true} on success. * @throws IllegalArgumentException if the operation method can not have the desired number of dimensions. */ - private boolean propagateVertical(final ListIterator operations, final boolean forward) + private boolean propagateVertical(final CoordinateReferenceSystem source3D, + final CoordinateReferenceSystem target3D, + final ListIterator operations, + final boolean forward) throws IllegalArgumentException, FactoryException { while (forward ? operations.hasNext() : operations.hasPrevious()) { @@ -771,18 +787,18 @@ class CoordinateOperationRegistry { if (op instanceof SingleOperation) { final MathTransformFactory mtFactory = factorySIS.getMathTransformFactory(); if (mtFactory instanceof DefaultMathTransformFactory) { - final CoordinateReferenceSystem source3D = toGeodetic3D(sourceCRS); - final CoordinateReferenceSystem target3D = toGeodetic3D(targetCRS); + if (forward) sourceCRS = toGeodetic3D(sourceCRS, source3D); + else targetCRS = toGeodetic3D(targetCRS, target3D); final MathTransform mt; try { mt = ((DefaultMathTransformFactory) mtFactory).createParameterizedTransform( ((SingleOperation) op).getParameterValues(), - ReferencingUtilities.createTransformContext(source3D, target3D, null)); + ReferencingUtilities.createTransformContext(sourceCRS, targetCRS, null)); } catch (InvalidGeodeticParameterException e) { log(e); break; } - operations.set(recreate(op, source3D, target3D, mt, mtFactory.getLastMethodUsed())); + operations.set(recreate(op, sourceCRS, targetCRS, mt, mtFactory.getLastMethodUsed())); return true; } } @@ -813,7 +829,7 @@ class CoordinateOperationRegistry { * conform to the definition provided by the authority. */ final MathTransform mt = factorySIS.getMathTransformFactory().createAffineTransform(matrix); - operations.set(recreate(op, toGeodetic3D(sourceCRS), toGeodetic3D(targetCRS), mt, null)); + operations.set(recreate(op, toGeodetic3D(sourceCRS, source3D), toGeodetic3D(targetCRS, target3D), mt, null)); } /* * If we processed the operation that change the number of dimensions, we are done. @@ -829,13 +845,27 @@ class CoordinateOperationRegistry { * If the given CRS is two-dimensional, append an ellipsoidal height to it. * It is caller's responsibility to ensure that the given CRS is geographic. */ - private CoordinateReferenceSystem toGeodetic3D(CoordinateReferenceSystem crs) throws FactoryException { + private CoordinateReferenceSystem toGeodetic3D(CoordinateReferenceSystem crs, + final CoordinateReferenceSystem candidate) throws FactoryException + { assert (crs instanceof GeodeticCRS) && (crs.getCoordinateSystem() instanceof EllipsoidalCS) : crs; - if (crs.getCoordinateSystem().getDimension() == 2) { - crs = ReferencingServices.getInstance().createCompoundCRS(factorySIS.getCRSFactory(), factorySIS.getCSFactory(), - derivedFrom(crs), crs, CommonCRS.Vertical.ELLIPSOIDAL.crs()); + if (crs.getCoordinateSystem().getDimension() != 2) { + return crs; + } + /* + * The check for same class is a cheap way to ensure that the two CRS implement the same GeoAPI interface. + * This test is stricter than necessary, but the result should still not wrong if we miss an opportunity + * to return the existing instance. + */ + if (crs.getClass() == candidate.getClass() && candidate.getCoordinateSystem().getDimension() == 3) { + if (Utilities.equalsIgnoreMetadata(((SingleCRS) crs).getDatum(), ((SingleCRS) candidate).getDatum())) { + return candidate; // Keep the existing instance since it may contain useful metadata. + } } - return crs; + return ReferencingServices.getInstance().createCompoundCRS( + factorySIS.getCRSFactory(), + factorySIS.getCSFactory(), + derivedFrom(crs), crs, CommonCRS.Vertical.ELLIPSOIDAL.crs()); } /** Modified: sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/CoordinateOperationRegistryTest.java URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/CoordinateOperationRegistryTest.java?rev=1740204&r1=1740203&r2=1740204&view=diff ============================================================================== --- sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/CoordinateOperationRegistryTest.java [UTF-8] (original) +++ sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/CoordinateOperationRegistryTest.java [UTF-8] Wed Apr 20 22:04:56 2016 @@ -328,7 +328,7 @@ public final strictfp class CoordinateOp } else { assertEpsgNameWithoutIdentifierEqual("NTF (Paris) to WGS 84 (1)", operation); assertEpsgNameWithoutIdentifierEqual("NTF (Paris)", operation.getSourceCRS()); - assertEpsgNameWithoutIdentifierEqual("WGS 84", operation.getTargetCRS()); + assertEquals("name", "WGS 84", operation.getTargetCRS().getName().getCode()); assertEpsgNameWithoutIdentifierEqual("NTF (Paris) to NTF (1)", step1); assertEpsgNameWithoutIdentifierEqual("NTF to WGS 84 (1)", step2); }