From 7fe057ba8fb08b5c4517692c15c9ef06fa8a4aba Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Mon, 2 Jan 2023 13:02:19 +0100 Subject: [PATCH] ProjectedCRS::identify(): relax trust in id/authority in definition to identify Fixes use case of https://lists.osgeo.org/pipermail/qgis-user/2023-January/052299.html with likely buggy definition: ``` PROJCS["ETRS_1989_UTM_Zone_32N_6Stellen",GEOGCS["GCS_ETRS_1989",DATUM["D_ETRS_1989",SPHEROID["GRS_1980",6378137.0,298.257222101]],PRIMEM["Greenwich",0.0],UNIT["Degree",0.0174532925199433]],PROJECTION["Transverse_Mercator"],PARAMETER["False_Easting",500000.0],PARAMETER["False_Northing",0.0],PARAMETER["Central_Meridian",9.0],PARAMETER["Scale_Factor",0.9996],PARAMETER["Latitude_Of_Origin",0.0],UNIT["Meter",1.0],AUTHORITY["ESRI",102328]] ``` that should rather be identified to EPSG:25832 ('ETRS_1989_UTM_Zone_32N') rather than ESRI:102328 ('PE_PCS_ETRS_1989_UTM_32N_7STELLEN') --- src/iso19111/crs.cpp | 18 +++++++++++------- test/unit/test_crs.cpp | 34 +++++++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/src/iso19111/crs.cpp b/src/iso19111/crs.cpp index 6dbb801179..a63d76ead5 100644 --- a/src/iso19111/crs.cpp +++ b/src/iso19111/crs.cpp @@ -4829,7 +4829,8 @@ ProjectedCRS::identify(const io::AuthorityFactoryPtr &authorityFactory) const { } const bool l_implicitCS = hasImplicitCS(); - const auto addCRS = [&](const ProjectedCRSNNPtr &crs, const bool eqName) { + const auto addCRS = [&](const ProjectedCRSNNPtr &crs, const bool eqName, + bool hasNonMatchingId) { const auto &l_unit = cs->axisList()[0]->unit(); if (_isEquivalentTo(crs.get(), util::IComparable::Criterion:: @@ -4849,7 +4850,7 @@ ProjectedCRS::identify(const io::AuthorityFactoryPtr &authorityFactory) const { util::IComparable::Criterion::EQUIVALENT, dbContext))) { if (crs->nameStr() == thisName) { res.clear(); - res.emplace_back(crs, 100); + res.emplace_back(crs, hasNonMatchingId ? 70 : 100); } else { res.emplace_back(crs, eqName ? 90 : 70); } @@ -4890,6 +4891,7 @@ ProjectedCRS::identify(const io::AuthorityFactoryPtr &authorityFactory) const { ci_equal(thisName, "unnamed"); bool foundEquivalentName = false; + bool hasNonMatchingId = false; if (hasCodeCompatibleOfAuthorityFactory(this, authorityFactory)) { // If the CRS has already an id, check in the database for the // official object, and verify that they are equivalent. @@ -4906,11 +4908,14 @@ ProjectedCRS::identify(const io::AuthorityFactoryPtr &authorityFactory) const { EQUIVALENT_EXCEPT_AXIS_ORDER_GEOGCRS, dbContext); res.emplace_back(crs, match ? 100 : 25); - return res; + if (match) { + return res; + } } catch (const std::exception &) { } } } + hasNonMatchingId = true; } else if (!insignificantName) { for (int ipass = 0; ipass < 2; ipass++) { const bool approximateMatch = ipass == 1; @@ -4926,7 +4931,7 @@ ProjectedCRS::identify(const io::AuthorityFactoryPtr &authorityFactory) const { thisName.c_str(), pairObjName.second.c_str()); foundEquivalentName |= eqName; - if (addCRS(crsNN, eqName).second == 100) { + if (addCRS(crsNN, eqName, false).second == 100) { return res; } } @@ -4962,8 +4967,7 @@ ProjectedCRS::identify(const io::AuthorityFactoryPtr &authorityFactory) const { // Sort results res.sort(lambdaSort); - if (!hasCodeCompatibleOfAuthorityFactory(this, authorityFactory) && - !foundEquivalentName && (res.empty() || res.front().second < 50)) { + if (!foundEquivalentName && (res.empty() || res.front().second < 50)) { std::set> alreadyKnown; for (const auto &pair : res) { const auto &ids = pair.first->identifiers(); @@ -4985,7 +4989,7 @@ ProjectedCRS::identify(const io::AuthorityFactoryPtr &authorityFactory) const { continue; } - addCRS(crs, insignificantName); + addCRS(crs, insignificantName, hasNonMatchingId); } res.sort(lambdaSort); diff --git a/test/unit/test_crs.cpp b/test/unit/test_crs.cpp index 053700a66e..9257902273 100644 --- a/test/unit/test_crs.cpp +++ b/test/unit/test_crs.cpp @@ -2617,7 +2617,9 @@ TEST(crs, projectedCRS_identify_db) { sourceCRS->baseCRS(), sourceCRS->derivingConversion(), sourceCRS->coordinateSystem()); auto res = crs->identify(factoryEPSG); - EXPECT_EQ(res.size(), 0U); + EXPECT_EQ(res.size(), 1U); + EXPECT_EQ(res.front().first->getEPSGCode(), 2172); + EXPECT_EQ(res.front().second, 70); } { // Existing code, but not matching content @@ -2632,8 +2634,8 @@ TEST(crs, projectedCRS_identify_db) { sourceCRS->coordinateSystem()); auto res = crs->identify(factoryEPSG); ASSERT_EQ(res.size(), 1U); - EXPECT_EQ(res.front().first->getEPSGCode(), 32631); - EXPECT_EQ(res.front().second, 25); + EXPECT_EQ(res.front().first->getEPSGCode(), 2172); + EXPECT_EQ(res.front().second, 70); } { // Identify by exact name @@ -3252,6 +3254,32 @@ TEST(crs, projectedCRS_identify_db) { EXPECT_EQ(res.front().first->getEPSGCode(), 8353); EXPECT_EQ(res.front().second, 100); } + { + // Identify from a pseudo WKT ESRI with has an AUTHORITY node that + // points to another object. + // Cf + // https://lists.osgeo.org/pipermail/qgis-user/2023-January/052299.html + auto obj = WKTParser().attachDatabaseContext(dbContext).createFromWKT( + "PROJCS[\"ETRS_1989_UTM_Zone_32N_6Stellen\"," + "GEOGCS[\"GCS_ETRS_1989\",DATUM[\"D_ETRS_1989\"," + "SPHEROID[\"GRS_1980\",6378137.0,298.257222101]]," + "PRIMEM[\"Greenwich\",0.0],UNIT[\"Degree\",0.0174532925199433]]," + "PROJECTION[\"Transverse_Mercator\"]," + "PARAMETER[\"False_Easting\",500000.0]," + "PARAMETER[\"False_Northing\",0.0]," + "PARAMETER[\"Central_Meridian\",9.0]," + "PARAMETER[\"Scale_Factor\",0.9996]," + "PARAMETER[\"Latitude_Of_Origin\",0.0]," + "UNIT[\"Meter\",1.0]," + "AUTHORITY[\"ESRI\",\"102328\"]]"); + auto crs = nn_dynamic_pointer_cast(obj); + ASSERT_TRUE(crs != nullptr); + auto allFactory = AuthorityFactory::create(dbContext, std::string()); + auto res = crs->identify(allFactory); + ASSERT_GE(res.size(), 1U); + EXPECT_EQ(res.front().first->getEPSGCode(), 25832); + EXPECT_EQ(res.front().second, 70); + } } // ---------------------------------------------------------------------------