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 de19b988f90d84c05efb4ea2f34a1729df5bc0e5 Author: Martin Desruisseaux <[email protected]> AuthorDate: Sun Feb 22 11:45:45 2026 +0100 Finer-grain detection of when the `CoordinateOperation` can be cached for a pair of CRS. In particular, we now detect the case when the Area Of Interest (AOI) has not been used. It covers a lot of cases, in particular map projections from grid geometry to target CRS. --- .../operation/CoordinateOperationContext.java | 31 ++++++++++- .../operation/CoordinateOperationFinder.java | 30 +++-------- .../operation/CoordinateOperationRegistry.java | 63 +++++++++++++++++----- .../DefaultCoordinateOperationFactory.java | 58 ++++++++++---------- .../referencing/operation/SubOperationInfo.java | 8 +-- .../sis/referencing/operation/package-info.java | 2 +- 6 files changed, 124 insertions(+), 68 deletions(-) diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationContext.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationContext.java index 1df1f3f0d0..93c24183af 100644 --- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationContext.java +++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationContext.java @@ -62,7 +62,7 @@ import org.apache.sis.measure.Longitude; * to Texas, but not suitable to the rest of North-America. * * @author Martin Desruisseaux (Geomatys) - * @version 1.6 + * @version 1.7 * @since 0.7 * * @todo Should also take the country of a {@link java.util.Locale}. The EPSG database contains ISO2 and ISO3 @@ -98,6 +98,15 @@ public class CoordinateOperationContext implements Localized { */ private transient Filter logFilter; + /** + * Whether the coordinate operation created with this context depends on the context values. + * This flag is set to {@code true} when {@link CoordinateOperationFinder} has used a contextual information such + * as the area of interest, desired accuracy, constant coordinate values or operation filter for choosing a result. + * + * @see #resultWasContextSensitive() + */ + boolean resultWasContextSensitive; + /** * Creates a new context with no area of interest and the best accuracy available. */ @@ -414,4 +423,24 @@ public class CoordinateOperationContext implements Localized { } logger.log(record); } + + /** + * Returns whether the coordinate operation created with this context depends on the context values. + * This is {@code true} if the last {@link CoordinateOperationFinder} that used this context needed + * information such as + * the {@linkplain #getAreaOfInterest() area of interest}, + * the {@linkplain #getDesiredAccuracy() desired accuracy}, + * some {@linkplain #getConstantCoordinates() constant coordinate values} or + * the {@linkplain #getOperationFilter() operation filter} for choosing a result. + * If this flag is {@code false}, then multiple calls to {@code createOperation(…)} + * with the same pair of <abbr>CRS</abbr> are likely to provide the same result no matter the context. + * This information information is useful for deciding whether to cache a result. + * + * @return whether the coordinate operation created with this context depends on the context values. + * + * @since 1.7 + */ + public boolean resultWasContextSensitive() { + return resultWasContextSensitive; + } } diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationFinder.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationFinder.java index ca2d90fdd4..be099c5216 100644 --- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationFinder.java +++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationFinder.java @@ -113,7 +113,7 @@ import org.apache.sis.util.resources.Vocabulary; * </ul> * * @author Martin Desruisseaux (Geomatys) - * @version 1.5 + * @version 1.7 * * @see DefaultCoordinateOperationFactory#createOperation(CoordinateReferenceSystem, CoordinateReferenceSystem, CoordinateOperationContext) * @@ -146,14 +146,6 @@ public class CoordinateOperationFinder extends CoordinateOperationRegistry { */ private final boolean canReadFromCache; - /** - * Whether the operation can be cached. This flag shall be {@code false} if - * the operation depends on parameters that may vary between two executions. - * - * @see #canStoreInCache() - */ - private boolean canStoreInCache; - /** * Creates a new instance for the given factory and context. * @@ -172,7 +164,6 @@ public class CoordinateOperationFinder extends CoordinateOperationRegistry { identifierOfStepCRS = new HashMap<>(8); previousSearches = new HashMap<>(8); canReadFromCache = (context == null || context.canReadFromCache()) && (factory == factorySIS); - canStoreInCache = (context == null); } /** @@ -253,13 +244,13 @@ public class CoordinateOperationFinder extends CoordinateOperationRegistry { /* * If this method is invoked recursively, verify if the requested operation is already in the cache. * We do not perform this verification on the first invocation because it was already verified by - * DefaultCoordinateOperationFactory.createOperation(…). We do not block if the operation is in + * `DefaultCoordinateOperationFactory.createOperation(…)`. We do not block if the operation is in * process of being computed in another thread because of the risk of deadlock. If the operation * is not in the cache, store the key in our internal map for preventing infinite recursion. */ final CRSPair key = new CRSPair(sourceCRS, targetCRS); if (canReadFromCache && stopAtFirst && !previousSearches.isEmpty()) { - final CoordinateOperation op = factorySIS.cache.peek(key); + final CoordinateOperation op = factorySIS.getCachedOperation(key); if (op != null) return asList(op); // Must be a modifiable list as per this method contract. } if (previousSearches.put(key, Boolean.TRUE) != null) { @@ -559,6 +550,7 @@ public class CoordinateOperationFinder extends CoordinateOperationRegistry { final var impl = (DefaultGeodeticDatum) sourceDatum; datumShift = impl.getPositionVectorTransformation(targetDatum, areaOfInterest); if (datumShift != null) typeOfChange = DATUM_SHIFT; + resultWasContextSensitive(specifiedAOI); } else if (targetDatum instanceof DefaultGeodeticDatum) { final var impl = (DefaultGeodeticDatum) targetDatum; Matrix matrix = impl.getPositionVectorTransformation(sourceDatum, areaOfInterest); @@ -570,6 +562,7 @@ public class CoordinateOperationFinder extends CoordinateOperationRegistry { matrix = null; } datumShift = matrix; + resultWasContextSensitive(specifiedAOI); } else { datumShift = null; } @@ -586,6 +579,7 @@ public class CoordinateOperationFinder extends CoordinateOperationRegistry { } method = builder.getMethod(); parameters = builder.parametersForMetadata(); + resultWasContextSensitive(specifiedAccuracy); } /* * Adjust the accuracy information if the datum shift has been computed by an indirect path. @@ -967,7 +961,7 @@ public class CoordinateOperationFinder extends CoordinateOperationRegistry { final SubOperationInfo info = infos[i]; final CoordinateReferenceSystem source = stepComponents[i]; final CoordinateReferenceSystem target = info.targetComponent; - canStoreInCache &= info.canStoreInCache(); + resultWasContextSensitive(info.resultWasContextSensitive()); /* * In order to compute `stepTargetCRS`, replace in-place a single element in `stepComponents`. * For each step except the last one, `stepTargetCRS` is a mix of target CRS and source CRS. @@ -1021,7 +1015,7 @@ public class CoordinateOperationFinder extends CoordinateOperationRegistry { targetCRS.getCoordinateSystem().getDimension()); operation = concatenate(operation, createFromAffineTransform(CONSTANTS, stepSourceCRS, targetCRS, null, m)); for (int i = stepComponents.length; i < infos.length; i++) { - canStoreInCache &= infos[i].canStoreInCache(); + resultWasContextSensitive(infos[i].resultWasContextSensitive()); } } return asList(operation); @@ -1304,12 +1298,4 @@ public class CoordinateOperationFinder extends CoordinateOperationRegistry { Resources.Keys.NonInvertibleOperation_1, CRSPair.label(crs.getConversionFromBase(), locale)); } - - /** - * Returns whether the operation can be cached. This is {@code false} if - * the operation depends on parameters that may vary between two executions. - */ - final boolean canStoreInCache() { - return canStoreInCache; - } } diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java index bb4f667fdc..c3f05e75e7 100644 --- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java +++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java @@ -230,13 +230,30 @@ class CoordinateOperationRegistry { */ protected Extent areaOfInterest; + /** + * Whether a non-default {@link #areaOfInterest} value was specified explicitly by the context. + * If this flag is {@code true}, then {@link context#operationIsContextSensitive} should be set + * to {@code true} when {@link #areaOfInterest} is used. + */ + final boolean specifiedAOI; + /** * The desired accuracy in metres, or 0 for the best accuracy available. + * When a new {@code CoordinateOperationFinder} instance is created with a non-null + * {@link CoordinateOperationContext}, the context is used for initializing this value. + * After initialization, this field may be updated at implementation discretion. * * @see CoordinateOperationContext#getDesiredAccuracy() */ protected double desiredAccuracy; + /** + * Whether a non-default {@link #desiredAccuracy} value was specified explicitly by the context. + * If this flag is {@code true}, then {@link context#operationIsContextSensitive} should be set + * to {@code true} when {@link #desiredAccuracy} is used. + */ + final boolean specifiedAccuracy; + /** * {@code true} if {@code search(…)} should stop after the first coordinate operation found. * This field is set to {@code true} when the caller is interested only in the "best" operation @@ -299,6 +316,8 @@ class CoordinateOperationRegistry { areaOfInterest = context.getAreaOfInterest(); desiredAccuracy = context.getDesiredAccuracy(); } + specifiedAOI = (areaOfInterest != null); + specifiedAccuracy = (desiredAccuracy > 0); } /** @@ -656,15 +675,18 @@ class CoordinateOperationRegistry { * then we need to get one from the CRS. This is necessary for preventing the transformation from * NAD27 to NAD83 in Idaho to select the transform for Alaska (since the latter has a larger area). */ - CoordinateOperationSorter.sort(operations, Extents.getGeographicBoundingBox(areaOfInterest)); + if (operations.size() > 1) { + resultWasContextSensitive(specifiedAOI); + CoordinateOperationSorter.sort(operations, Extents.getGeographicBoundingBox(areaOfInterest)); + } final ListIterator<CoordinateOperation> it = operations.listIterator(); while (it.hasNext()) { /* * At this point we filtered a CoordinateOperation by looking only at its metadata. * Code following this point will need the full coordinate operation, including its - * MathTransform. So if we got a deferred operation, we need to resolve it now. + * `MathTransform`. So if we got a deferred operation, we need to resolve it now. * Conversely, we should not use metadata below this point because the call to - * inverse(CoordinateOperation) is not guaranteed to preserve all metadata. + * `inverse(CoordinateOperation)` is not guaranteed to preserve all metadata. */ CoordinateOperation operation = it.next(); try { @@ -705,21 +727,38 @@ class CoordinateOperationRegistry { * FactoryException propagate. */ operation = complete(operation, sourceCRS, targetCRS); - final Predicate<CoordinateOperation> filter = (context != null) ? context.getOperationFilter() : null; - if (filter == null || filter.test(operation)) { - if (stopAtFirst) { - operations.clear(); - operations.add(operation); - break; + if (context != null) { + final Predicate<CoordinateOperation> filter = context.getOperationFilter(); + if (filter != null) { + resultWasContextSensitive(true); + if (!filter.test(operation)) { + it.remove(); + continue; + } } - it.set(operation); - } else { - it.remove(); } + if (stopAtFirst) { + operations.clear(); + operations.add(operation); + break; + } + it.set(operation); } return operations; } + /** + * If the given flag is {@code true}, declares that the search for coordinate operations + * depends on the values of the {@code context} argument specified at construction time. + * + * @param flag whether the coordinate operation depends on the context according the caller. + */ + final void resultWasContextSensitive(final boolean flag) { + if (flag && context != null) { + context.resultWasContextSensitive = true; + } + } + /** * Creates the inverse of the given single operation. * If this operation succeed, then the returned coordinate operation has the following properties: diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/DefaultCoordinateOperationFactory.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/DefaultCoordinateOperationFactory.java index 6efdc5f69c..bd6f8f728f 100644 --- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/DefaultCoordinateOperationFactory.java +++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/DefaultCoordinateOperationFactory.java @@ -80,7 +80,7 @@ import org.apache.sis.util.resources.Errors; * This class is safe for multi-thread usage if all referenced factories are thread-safe. * * @author Martin Desruisseaux (IRD, Geomatys) - * @version 1.5 + * @version 1.7 * @since 0.6 */ public class DefaultCoordinateOperationFactory extends AbstractFactory implements CoordinateOperationFactory { @@ -124,12 +124,12 @@ public class DefaultCoordinateOperationFactory extends AbstractFactory implement /** * The cache of coordinate operations found for a given pair of source and target CRS. - * If current implementation, we cache only operations found without context (otherwise - * we would need to take in account the area of interest and desired accuracy in the key). + * We cache only operations found when {@link CoordinateOperationContext} values were + * not used (otherwise we would need to store some context values in the key). * * @see #createOperation(CoordinateReferenceSystem, CoordinateReferenceSystem, CoordinateOperationContext) */ - final Cache<CRSPair,CoordinateOperation> cache; + private final Cache<CRSPair, CoordinateOperation> cache; /** * The default factory instance. @@ -665,35 +665,37 @@ next: for (SingleCRS component : CRS.getSingleComponents(targetCRS)) { final CoordinateOperationContext context) throws OperationNotFoundException, FactoryException { - final Cache.Handler<CoordinateOperation> handler; - CoordinateOperation op; - if (context == null) { - final CRSPair key = new CRSPair(sourceCRS, targetCRS); - op = cache.peek(key); - if (op != null) { - return op; - } - handler = cache.lock(key); - } else { - // We currently do not cache the operation when the result may depend on the context (see `this.cache` javadoc). - handler = null; - op = null; - } - boolean canStoreInCache = true; - try { - if (handler == null || (op = handler.peek()) == null) { - final CoordinateOperationFinder finder = createOperationFinder(getFactorySIS(), context); - op = finder.createOperation(sourceCRS, targetCRS); - canStoreInCache = finder.canStoreInCache(); - } - } finally { - if (handler != null) { - handler.putAndUnlock(canStoreInCache ? op : null); + final var key = new CRSPair(sourceCRS, targetCRS); + CoordinateOperation op = getCachedOperation(key); + if (op == null) { + boolean resultWasContextSensitive = false; + final Cache.Handler<CoordinateOperation> handler = cache.lock(key); + try { + op = handler.peek(); + if (op == null) { + final CoordinateOperationFinder finder = createOperationFinder(getFactorySIS(), context); + op = finder.createOperation(sourceCRS, targetCRS); + if (context != null) { + resultWasContextSensitive = context.resultWasContextSensitive(); + } + } + } finally { + handler.putAndUnlock(resultWasContextSensitive ? null : op); } } return op; } + /** + * Returns an operation for the given pair of <abbr>CRS</abbr> if that operation is already in the cache. + * + * @param key pair of <abbr>CRS</abbr> for which to get an operation. + * @return the cached operation for the given pair of <abbr>CRS</abbr>, or {@code null} if none. + */ + final CoordinateOperation getCachedOperation(final CRSPair key) { + return cache.peek(key); + } + /** * Finds or creates operations for conversions or transformations between two coordinate reference systems. * If at least one operation exists, they are returned in preference order: the operation having the widest diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/SubOperationInfo.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/SubOperationInfo.java index 676674b804..1e85a2f4df 100644 --- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/SubOperationInfo.java +++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/SubOperationInfo.java @@ -479,10 +479,10 @@ otherRow: for (int j = last.getNumRow() - 1; --j >= 0;) { // Ignor } /** - * Whether this operation can be cached. This flag shall be {@code false} if - * the operation depends on parameters that may vary between two executions. + * Whether the operation result depends on the values of the user-supplied {@link CoordinateOperationContext}. + * This flag shall be {@code true} if the operation depends on parameters that may vary between two executions. */ - final boolean canStoreInCache() { - return constantCoordinates == null; + final boolean resultWasContextSensitive() { + return constantCoordinates != null; } } diff --git a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/package-info.java b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/package-info.java index d9b422b818..ef22720164 100644 --- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/package-info.java +++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/package-info.java @@ -72,7 +72,7 @@ * for example by specifying the area of interest. * * @author Martin Desruisseaux (IRD, Geomatys) - * @version 1.6 + * @version 1.7 * @since 0.6 */ @XmlSchema(location = "http://schemas.opengis.net/gml/3.2.1/coordinateOperations.xsd";,
