createOperations(): fix pipeline generation when a CRS has terms like +datum=NAD27 and +over, where the datum was just replaced by its ellipsoid

5 files changed, 122 insertions, 43 deletions

diff --git a/include/proj/io.hpp b/include/proj/io.hpp
index 9c99fc0e..5b1a8588 100644
--- a/include/proj/io.hpp
+++ b/include/proj/io.hpp
@@ -432,8 +432,8 @@ class PROJ_GCC_DLL PROJStringFormatter {
     PROJ_INTERNAL void popOmitZUnitConversion();
     PROJ_INTERNAL bool omitZUnitConversion() const;
 
-    PROJ_INTERNAL void setDropEarlyBindingsTerms(bool drop);
-    PROJ_INTERNAL bool getDropEarlyBindingsTerms() const;
+    PROJ_INTERNAL void setLegacyCRSToCRSContext(bool legacyContext);
+    PROJ_INTERNAL bool getLegacyCRSToCRSContext() const;
 
     PROJ_INTERNAL const DatabaseContextPtr &databaseContext() const;
 
diff --git a/src/iso19111/coordinateoperation.cpp b/src/iso19111/coordinateoperation.cpp
index dc700445..e9524163 100644
--- a/src/iso19111/coordinateoperation.cpp
+++ b/src/iso19111/coordinateoperation.cpp
@@ -12064,7 +12064,7 @@ CoordinateOperationFactory::Private::createOperations(
         }
         auto projFormatter = io::PROJStringFormatter::create();
         projFormatter->setCRSExport(true);
-        projFormatter->setDropEarlyBindingsTerms(true);
+        projFormatter->setLegacyCRSToCRSContext(true);
         projFormatter->startInversion();
         sourceProjExportable->_exportToPROJString(projFormatter.get());
         auto geogSrc =
diff --git a/src/iso19111/crs.cpp b/src/iso19111/crs.cpp
index b9694ba2..ad4acaff 100644
--- a/src/iso19111/crs.cpp
+++ b/src/iso19111/crs.cpp
@@ -1308,7 +1308,7 @@ void GeodeticCRS::addDatumInfoToPROJString(
     const auto &nadgrids = formatter->getHDatumExtension();
     const auto &l_datum = datum();
     if (formatter->getCRSExport() && l_datum && TOWGS84Params.empty() &&
-        nadgrids.empty() && !formatter->getDropEarlyBindingsTerms()) {
+        nadgrids.empty()) {
         if (l_datum->_isEquivalentTo(
                 datum::GeodeticReferenceFrame::EPSG_6326.get(),
                 util::IComparable::Criterion::EQUIVALENT)) {
@@ -2083,8 +2083,31 @@ void GeographicCRS::_exportToPROJString(
         primeMeridian()->longitude().getSIValue() != 0.0 ||
         !formatter->getTOWGS84Parameters().empty() ||
         !formatter->getHDatumExtension().empty()) {
-        formatter->addStep("longlat");
-        addDatumInfoToPROJString(formatter);
+
+        bool done = false;
+        if (formatter->getLegacyCRSToCRSContext() &&
+            formatter->getHDatumExtension().empty() &&
+            formatter->getTOWGS84Parameters().empty()) {
+            done = true;
+            const auto &l_datum = datum();
+            if (l_datum &&
+                l_datum->_isEquivalentTo(
+                    datum::GeodeticReferenceFrame::EPSG_6326.get(),
+                    util::IComparable::Criterion::EQUIVALENT)) {
+                formatter->addStep("longlat");
+                formatter->addParam("ellps", "WGS84");
+            } else if (l_datum &&
+                       l_datum->_isEquivalentTo(
+                           datum::GeodeticReferenceFrame::EPSG_6267.get(),
+                           util::IComparable::Criterion::EQUIVALENT)) {
+                formatter->addStep("longlat");
+                formatter->addParam("datum", "NAD27");
+            }
+        }
+        if (!done) {
+            formatter->addStep("longlat");
+            addDatumInfoToPROJString(formatter);
+        }
     }
     if (!formatter->getCRSExport()) {
         addAngularUnitConvertAndAxisSwap(formatter);
@@ -3132,7 +3155,8 @@ void ProjectedCRS::addUnitConvertAndAxisSwap(io::PROJStringFormatter *formatter,
                 formatter->addParam("units", projUnit);
             }
         }
-    } else if (formatter->getCRSExport()) {
+    } else if (formatter->getCRSExport() &&
+               !formatter->getLegacyCRSToCRSContext()) {
         formatter->addParam("units", "m");
     }
 
@@ -4143,11 +4167,6 @@ void BoundCRS::_exportToPROJString(
             "baseCRS of BoundCRS cannot be exported as a PROJ string");
     }
 
-    if (formatter->getDropEarlyBindingsTerms()) {
-        crs_exportable->_exportToPROJString(formatter);
-        return;
-    }
-
     auto vdatumProj4GridName = getVDatumPROJ4GRIDS();
     if (!vdatumProj4GridName.empty()) {
         formatter->setVDatumExtension(vdatumProj4GridName);
diff --git a/src/iso19111/io.cpp b/src/iso19111/io.cpp
index f4d93ef4..3b630a10 100644
--- a/src/iso19111/io.cpp
+++ b/src/iso19111/io.cpp
@@ -6145,7 +6145,7 @@ struct PROJStringFormatter::Private {
     bool addNoDefs_ = true;
     bool coordOperationOptimizations_ = false;
     bool crsExport_ = false;
-    bool dropEarlyBindingsTerms_ = false;
+    bool legacyCRSToCRSContext_ = false;
 
     std::string result_{};
 
@@ -6723,7 +6723,7 @@ void PROJStringFormatter::Private::appendToResult(const char *str) {
 static void
 PROJStringSyntaxParser(const std::string &projString, std::vector<Step> &steps,
                        std::vector<Step::KeyValue> &globalParamValues,
-                       std::string &title, bool dropEarlyBindingsTerms) {
+                       std::string &title) {
     const char *c_str = projString.c_str();
     std::vector<std::string> tokens;
 
@@ -6811,27 +6811,14 @@ PROJStringSyntaxParser(const std::string &projString, std::vector<Step> &steps,
             } else if (word != "step") {
                 const auto pos = word.find('=');
                 auto key = word.substr(0, pos);
-                if (!(dropEarlyBindingsTerms &&
-                      (key == "towgs84" || key == "nadgrids" ||
-                       key == "geoidgrids"))) {
-                    auto pair = (pos != std::string::npos)
-                                    ? Step::KeyValue(key, word.substr(pos + 1))
-                                    : Step::KeyValue(key);
-                    if (dropEarlyBindingsTerms && key == "datum") {
-                        const auto datums = pj_get_datums_ref();
-                        for (int i = 0; datums[i].id != nullptr; i++) {
-                            if (pair.value == datums[i].id) {
-                                pair.key = "ellps";
-                                pair.value = datums[i].ellipse_id;
-                                break;
-                            }
-                        }
-                    }
-                    if (steps.empty()) {
-                        globalParamValues.push_back(pair);
-                    } else {
-                        steps.back().paramValues.push_back(pair);
-                    }
+
+                auto pair = (pos != std::string::npos)
+                                ? Step::KeyValue(key, word.substr(pos + 1))
+                                : Step::KeyValue(key);
+                if (steps.empty()) {
+                    globalParamValues.push_back(pair);
+                } else {
+                    steps.back().paramValues.push_back(pair);
                 }
             }
         }
@@ -6906,8 +6893,7 @@ void PROJStringFormatter::ingestPROJString(
 {
     std::vector<Step> steps;
     std::string title;
-    PROJStringSyntaxParser(str, steps, d->globalParamValues_, title,
-                           d->dropEarlyBindingsTerms_);
+    PROJStringSyntaxParser(str, steps, d->globalParamValues_, title);
     d->steps_.insert(d->steps_.end(), steps.begin(), steps.end());
 }
 
@@ -7175,14 +7161,14 @@ bool PROJStringFormatter::omitZUnitConversion() const {
 
 // ---------------------------------------------------------------------------
 
-void PROJStringFormatter::setDropEarlyBindingsTerms(bool drop) {
-    d->dropEarlyBindingsTerms_ = drop;
+void PROJStringFormatter::setLegacyCRSToCRSContext(bool legacyContext) {
+    d->legacyCRSToCRSContext_ = legacyContext;
 }
 
 // ---------------------------------------------------------------------------
 
-bool PROJStringFormatter::getDropEarlyBindingsTerms() const {
-    return d->dropEarlyBindingsTerms_;
+bool PROJStringFormatter::getLegacyCRSToCRSContext() const {
+    return d->legacyCRSToCRSContext_;
 }
 
 // ---------------------------------------------------------------------------
@@ -8671,7 +8657,7 @@ PROJStringParser::createFromPROJString(const std::string &projString) {
     d->globalParamValues_.clear();
     d->projString_ = projString;
     PROJStringSyntaxParser(projString, d->steps_, d->globalParamValues_,
-                           d->title_, false);
+                           d->title_);
 
     if (d->steps_.empty()) {
         const auto &vunits = d->getGlobalParamValue("vunits");
diff --git a/test/unit/test_operation.cpp b/test/unit/test_operation.cpp
index 1a9b69d5..f95f94bd 100644
--- a/test/unit/test_operation.cpp
+++ b/test/unit/test_operation.cpp
@@ -7059,7 +7059,81 @@ TEST(operation, createOperation_fallback_to_proj4_strings) {
     EXPECT_EQ(op->exportToPROJString(PROJStringFormatter::create().get()),
               "+proj=pipeline +step +proj=axisswap +order=2,1 "
               "+step +proj=unitconvert +xy_in=deg +xy_out=rad "
-              "+step +proj=longlat +geoc +ellps=WGS84 "
+              "+step +proj=longlat +geoc +datum=WGS84 "
+              "+step +proj=unitconvert +xy_in=rad +xy_out=deg");
+}
+
+// ---------------------------------------------------------------------------
+
+TEST(
+    operation,
+    createOperation_fallback_to_proj4_strings_regular_with_datum_to_projliteral) {
+    auto objSrc = PROJStringParser().createFromPROJString(
+        "+proj=utm +zone=11 +datum=NAD27 +type=crs");
+    auto src = nn_dynamic_pointer_cast<CRS>(objSrc);
+    ASSERT_TRUE(src != nullptr);
+
+    auto objDst = PROJStringParser().createFromPROJString(
+        "+proj=longlat +datum=WGS84 +over +type=crs");
+    auto dst = nn_dynamic_pointer_cast<CRS>(objDst);
+    ASSERT_TRUE(dst != nullptr);
+
+    auto op = CoordinateOperationFactory::create()->createOperation(
+        NN_CHECK_ASSERT(src), NN_CHECK_ASSERT(dst));
+    ASSERT_TRUE(op != nullptr);
+    EXPECT_EQ(op->exportToPROJString(PROJStringFormatter::create().get()),
+              "+proj=pipeline "
+              "+step +inv +proj=utm +zone=11 +datum=NAD27 "
+              "+step +proj=longlat +datum=WGS84 +over "
+              "+step +proj=unitconvert +xy_in=rad +xy_out=deg");
+}
+
+// ---------------------------------------------------------------------------
+
+TEST(
+    operation,
+    createOperation_fallback_to_proj4_strings_regular_with_nadgrids_to_projliteral) {
+    auto objSrc = PROJStringParser().createFromPROJString(
+        "+proj=utm +zone=11 +ellps=clrk66 +nadgrids=@conus +type=crs");
+    auto src = nn_dynamic_pointer_cast<CRS>(objSrc);
+    ASSERT_TRUE(src != nullptr);
+
+    auto objDst = PROJStringParser().createFromPROJString(
+        "+proj=longlat +datum=WGS84 +over +type=crs");
+    auto dst = nn_dynamic_pointer_cast<CRS>(objDst);
+    ASSERT_TRUE(dst != nullptr);
+
+    auto op = CoordinateOperationFactory::create()->createOperation(
+        NN_CHECK_ASSERT(src), NN_CHECK_ASSERT(dst));
+    ASSERT_TRUE(op != nullptr);
+    EXPECT_EQ(op->exportToPROJString(PROJStringFormatter::create().get()),
+              "+proj=pipeline "
+              "+step +inv +proj=utm +zone=11 +ellps=clrk66 +nadgrids=@conus "
+              "+step +proj=longlat +datum=WGS84 +over "
+              "+step +proj=unitconvert +xy_in=rad +xy_out=deg");
+}
+
+// ---------------------------------------------------------------------------
+
+TEST(operation,
+     createOperation_fallback_to_proj4_strings_projliteral_to_projliteral) {
+    auto objSrc = PROJStringParser().createFromPROJString(
+        "+proj=utm +zone=11 +datum=NAD27 +over +type=crs");
+    auto src = nn_dynamic_pointer_cast<CRS>(objSrc);
+    ASSERT_TRUE(src != nullptr);
+
+    auto objDst = PROJStringParser().createFromPROJString(
+        "+proj=longlat +datum=WGS84 +over +type=crs");
+    auto dst = nn_dynamic_pointer_cast<CRS>(objDst);
+    ASSERT_TRUE(dst != nullptr);
+
+    auto op = CoordinateOperationFactory::create()->createOperation(
+        NN_CHECK_ASSERT(src), NN_CHECK_ASSERT(dst));
+    ASSERT_TRUE(op != nullptr);
+    EXPECT_EQ(op->exportToPROJString(PROJStringFormatter::create().get()),
+              "+proj=pipeline "
+              "+step +inv +proj=utm +zone=11 +datum=NAD27 +over "
+              "+step +proj=longlat +datum=WGS84 +over "
               "+step +proj=unitconvert +xy_in=rad +xy_out=deg");
 }