diff --git a/src/iso19111/datum.cpp b/src/iso19111/datum.cpp index 80dde967cd..8c89c2d70c 100644 --- a/src/iso19111/datum.cpp +++ b/src/iso19111/datum.cpp @@ -1557,9 +1557,14 @@ bool GeodeticReferenceFrame::_isEquivalentTo( bool GeodeticReferenceFrame::hasEquivalentNameToUsingAlias( const IdentifiedObject *other, const io::DatabaseContextPtr &dbContext) const { - if (nameStr() == "unknown" || other->nameStr() == "unknown") { + if (nameStr() == other->nameStr() || nameStr() == "unknown" || + other->nameStr() == "unknown") { return true; } + if (ci_starts_with(nameStr(), UNKNOWN_BASED_ON) || + ci_starts_with(other->nameStr(), UNKNOWN_BASED_ON)) { + return false; + } if (dbContext) { if (!identifiers().empty()) { const auto &id = identifiers().front(); diff --git a/src/iso19111/operation/coordinateoperationfactory.cpp b/src/iso19111/operation/coordinateoperationfactory.cpp index 0e1f35c997..5ea611371b 100644 --- a/src/iso19111/operation/coordinateoperationfactory.cpp +++ b/src/iso19111/operation/coordinateoperationfactory.cpp @@ -1720,7 +1720,7 @@ void CoordinateOperationFactory::Private::buildCRSIds( } } } - if (ids.empty()) { + if (ids.empty() && !ci_equal(crs->nameStr(), "unknown")) { std::vector allowedObjects; auto geogCRS = dynamic_cast(crs.get()); if (geogCRS) { @@ -1810,6 +1810,13 @@ CoordinateOperationFactory::Private::findOpsInRegistryDirect( const auto &authFactory = context.context->getAuthorityFactory(); assert(authFactory); + if ((sourceCRS->identifiers().empty() && + ci_equal(sourceCRS->nameStr(), "unknown")) || + (targetCRS->identifiers().empty() && + ci_equal(targetCRS->nameStr(), "unknown"))) { + return {}; + } + #ifdef TRACE_CREATE_OPERATIONS ENTER_BLOCK("findOpsInRegistryDirect(" + objectAsStr(sourceCRS.get()) + " --> " + objectAsStr(targetCRS.get()) + ")"); @@ -1991,21 +1998,39 @@ CoordinateOperationFactory::Private::findsOpsInRegistryWithIntermediate( objectAsStr(sourceCRS.get()) + " --> " + objectAsStr(targetCRS.get()) + ")"); #endif - const auto &authFactory = context.context->getAuthorityFactory(); assert(authFactory); + const auto dbContext = authFactory->databaseContext().as_nullable(); + const auto geodSrc = dynamic_cast(sourceCRS.get()); + const auto datumSrc = + geodSrc ? geodSrc->datumNonNull(dbContext).as_nullable() : nullptr; + // Optimization: check if the source/target CRS have no chance to match + // a database entry + if (datumSrc && datumSrc->identifiers().empty() && + (ci_equal(datumSrc->nameStr(), "unknown") || + ci_starts_with(datumSrc->nameStr(), UNKNOWN_BASED_ON))) + return {}; + if (!geodSrc && dynamic_cast(sourceCRS.get())) + return {}; + const auto geodDst = dynamic_cast(targetCRS.get()); + const auto datumDst = + geodDst ? geodDst->datumNonNull(dbContext).as_nullable() : nullptr; + if (datumDst && datumDst->identifiers().empty() && + (ci_equal(datumDst->nameStr(), "unknown") || + ci_starts_with(datumDst->nameStr(), UNKNOWN_BASED_ON))) + return {}; + if (!geodDst && dynamic_cast(targetCRS.get())) + return {}; + std::list> sourceIds; buildCRSIds(sourceCRS, context, sourceIds); if (sourceIds.empty()) { - auto geodSrc = dynamic_cast(sourceCRS.get()); - auto geodDst = dynamic_cast(targetCRS.get()); if (geodSrc) { const std::string originatingAuthSrc = geodSrc->getOriginatingAuthName(); - const auto dbContext = authFactory->databaseContext().as_nullable(); const auto candidatesSrcGeod(findCandidateGeodCRSForDatum( - authFactory, geodSrc, geodSrc->datumNonNull(dbContext))); + authFactory, geodSrc, NN_NO_CHECK(datumSrc))); std::vector res; for (const auto &candidateSrcGeod : candidatesSrcGeod) { // Restrict to using only objects that have the same authority @@ -2022,11 +2047,11 @@ CoordinateOperationFactory::Private::findsOpsInRegistryWithIntermediate( (dynamic_cast( candidateSrcGeod.get()) != nullptr))) { if (geodDst) { - const auto srcDatum = + const auto candidateSrcDatum = candidateSrcGeod->datumNonNull(dbContext); - const auto dstDatum = geodDst->datumNonNull(dbContext); - const bool sameGeodeticDatum = - isSameGeodeticDatum(srcDatum, dstDatum, dbContext); + const bool sameGeodeticDatum = isSameGeodeticDatum( + candidateSrcDatum, NN_NO_CHECK(datumDst), + dbContext); if (sameGeodeticDatum) { continue; } @@ -3078,10 +3103,21 @@ void CoordinateOperationFactory::Private::createOperationsWithDatumPivot( const auto &authFactory = context.context->getAuthorityFactory(); const auto &dbContext = authFactory->databaseContext(); - const auto candidatesSrcGeod(findCandidateGeodCRSForDatum( - authFactory, geodSrc, geodSrc->datumNonNull(dbContext.as_nullable()))); - const auto candidatesDstGeod(findCandidateGeodCRSForDatum( - authFactory, geodDst, geodDst->datumNonNull(dbContext.as_nullable()))); + const auto srcDatum = geodSrc->datumNonNull(dbContext.as_nullable()); + if (srcDatum->identifiers().empty() && + (ci_equal(srcDatum->nameStr(), "unknown") || + ci_starts_with(srcDatum->nameStr(), UNKNOWN_BASED_ON))) + return; + const auto dstDatum = geodDst->datumNonNull(dbContext.as_nullable()); + if (dstDatum->identifiers().empty() && + (ci_equal(dstDatum->nameStr(), "unknown") || + ci_starts_with(dstDatum->nameStr(), UNKNOWN_BASED_ON))) + return; + + const auto candidatesSrcGeod( + findCandidateGeodCRSForDatum(authFactory, geodSrc, srcDatum)); + const auto candidatesDstGeod( + findCandidateGeodCRSForDatum(authFactory, geodDst, dstDatum)); const bool sourceAndTargetAre3D = geodSrc->coordinateSystem()->axisList().size() == 3 && diff --git a/test/unit/test_operationfactory.cpp b/test/unit/test_operationfactory.cpp index 2cf5765ad3..e406834a81 100644 --- a/test/unit/test_operationfactory.cpp +++ b/test/unit/test_operationfactory.cpp @@ -684,10 +684,12 @@ TEST(operation, geogCRS_to_geogCRS_compatible_unknown_celestial_body) { auto srcCRS = nn_dynamic_pointer_cast(objSrc); auto objDst = createFromUserInput("+proj=longlat +R=10001 +type=crs", dbContext); + const auto queryCounterBefore = dbContext->getQueryCounter(); auto dstCRS = nn_dynamic_pointer_cast(objDst); auto ctxt = CoordinateOperationContext::create(authFactory, nullptr, 0.0); auto list = CoordinateOperationFactory::create()->createOperations( NN_NO_CHECK(srcCRS), NN_NO_CHECK(dstCRS), ctxt); + EXPECT_EQ(dbContext->getQueryCounter(), queryCounterBefore); ASSERT_EQ(list.size(), 1U); EXPECT_EQ(list[0]->exportToPROJString(PROJStringFormatter::create().get()), "+proj=noop"); @@ -3063,11 +3065,13 @@ TEST(operation, createOperation_boundCRS_identified_by_datum) { "+ellps=clrk80ign +step +proj=pop +v_3 +step +proj=utm +zone=32 " "+ellps=clrk80ign"); - auto authFactory = - AuthorityFactory::create(DatabaseContext::create(), std::string()); + auto dbContext = DatabaseContext::create(); + auto authFactory = AuthorityFactory::create(dbContext, std::string()); auto ctxt = CoordinateOperationContext::create(authFactory, nullptr, 0.0); + const auto queryCounterBefore = dbContext->getQueryCounter(); auto list = CoordinateOperationFactory::create()->createOperations( NN_CHECK_ASSERT(src), NN_CHECK_ASSERT(dest), ctxt); + EXPECT_EQ(dbContext->getQueryCounter(), queryCounterBefore); ASSERT_EQ(list.size(), 1U); EXPECT_TRUE(list[0]->isEquivalentTo(op.get())); } @@ -3092,6 +3096,17 @@ TEST(operation, boundCRS_of_clrk_66_geogCRS_to_nad83_geogCRS) { "+proj=pipeline +step +proj=unitconvert +xy_in=deg +xy_out=rad " "+step +proj=hgridshift +grids=ntv1_can.dat,conus " "+step +proj=unitconvert +xy_in=rad +xy_out=deg"); + + auto dbContext = DatabaseContext::create(); + auto authFactory = AuthorityFactory::create(dbContext, std::string()); + auto ctxt = CoordinateOperationContext::create(authFactory, nullptr, 0.0); + const auto queryCounterBefore = dbContext->getQueryCounter(); + auto list = CoordinateOperationFactory::create()->createOperations( + NN_CHECK_ASSERT(src), NN_CHECK_ASSERT(dest), ctxt); + // Two extra queries related to the conus grid + EXPECT_EQ(dbContext->getQueryCounter(), queryCounterBefore + 2); + ASSERT_EQ(list.size(), 1U); + EXPECT_TRUE(list[0]->isEquivalentTo(op.get())); } // ---------------------------------------------------------------------------