PROJStringFormatting: change order of emission of push/pop w.r.t axis swap/unitconvert to avoid useless simplification rules

2 files changed, 33 insertions, 136 deletions

diff --git a/src/iso19111/coordinateoperation.cpp b/src/iso19111/coordinateoperation.cpp
index d66afd0d..fdcb6af8 100644
--- a/src/iso19111/coordinateoperation.cpp
+++ b/src/iso19111/coordinateoperation.cpp
@@ -8101,7 +8101,7 @@ static void ThrowExceptionNotGeodeticGeographic(const char *trfrm_name) {
 // ---------------------------------------------------------------------------
 
 static void setupPROJGeodeticSourceCRS(io::PROJStringFormatter *formatter,
-                                       const crs::CRSNNPtr &crs,
+                                       const crs::CRSNNPtr &crs, bool addPushV3,
                                        const char *trfrm_name) {
     auto sourceCRSGeog = dynamic_cast<const crs::GeographicCRS *>(crs.get());
     if (sourceCRSGeog) {
@@ -8109,6 +8109,11 @@ static void setupPROJGeodeticSourceCRS(io::PROJStringFormatter *formatter,
         sourceCRSGeog->_exportToPROJString(formatter);
         formatter->stopInversion();
 
+        if (addPushV3) {
+            formatter->addStep("push");
+            formatter->addParam("v_3");
+        }
+
         formatter->addStep("cart");
         sourceCRSGeog->ellipsoid()->_exportToPROJString(formatter);
     } else {
@@ -8124,7 +8129,7 @@ static void setupPROJGeodeticSourceCRS(io::PROJStringFormatter *formatter,
 // ---------------------------------------------------------------------------
 
 static void setupPROJGeodeticTargetCRS(io::PROJStringFormatter *formatter,
-                                       const crs::CRSNNPtr &crs,
+                                       const crs::CRSNNPtr &crs, bool addPopV3,
                                        const char *trfrm_name) {
     auto targetCRSGeog = dynamic_cast<const crs::GeographicCRS *>(crs.get());
     if (targetCRSGeog) {
@@ -8132,6 +8137,11 @@ static void setupPROJGeodeticTargetCRS(io::PROJStringFormatter *formatter,
         formatter->setCurrentStepInverted(true);
         targetCRSGeog->ellipsoid()->_exportToPROJString(formatter);
 
+        if (addPopV3) {
+            formatter->addStep("pop");
+            formatter->addParam("v_3");
+        }
+
         targetCRSGeog->_exportToPROJString(formatter);
     } else {
         auto targetCRSGeod = dynamic_cast<const crs::GeodeticCRS *>(crs.get());
@@ -8223,19 +8233,19 @@ void Transformation::_exportToPROJString(
         double z =
             parameterValueNumericAsSI(EPSG_CODE_PARAMETER_Z_AXIS_TRANSLATION);
 
-        if (methodEPSGCode == EPSG_CODE_METHOD_COORDINATE_FRAME_GEOGRAPHIC_2D ||
-            methodEPSGCode ==
-                EPSG_CODE_METHOD_TIME_DEPENDENT_COORDINATE_FRAME_GEOGRAPHIC_2D ||
-            methodEPSGCode == EPSG_CODE_METHOD_POSITION_VECTOR_GEOGRAPHIC_2D ||
-            methodEPSGCode ==
-                EPSG_CODE_METHOD_TIME_DEPENDENT_POSITION_VECTOR_GEOGRAPHIC_2D ||
-            methodEPSGCode ==
-                EPSG_CODE_METHOD_GEOCENTRIC_TRANSLATION_GEOGRAPHIC_2D) {
-            formatter->addStep("push");
-            formatter->addParam("v_3");
-        }
+        bool addPushPopV3 =
+            (methodEPSGCode ==
+                 EPSG_CODE_METHOD_COORDINATE_FRAME_GEOGRAPHIC_2D ||
+             methodEPSGCode ==
+                 EPSG_CODE_METHOD_TIME_DEPENDENT_COORDINATE_FRAME_GEOGRAPHIC_2D ||
+             methodEPSGCode == EPSG_CODE_METHOD_POSITION_VECTOR_GEOGRAPHIC_2D ||
+             methodEPSGCode ==
+                 EPSG_CODE_METHOD_TIME_DEPENDENT_POSITION_VECTOR_GEOGRAPHIC_2D ||
+             methodEPSGCode ==
+                 EPSG_CODE_METHOD_GEOCENTRIC_TRANSLATION_GEOGRAPHIC_2D);
 
-        setupPROJGeodeticSourceCRS(formatter, sourceCRS(), "Helmert");
+        setupPROJGeodeticSourceCRS(formatter, sourceCRS(), addPushPopV3,
+                                   "Helmert");
 
         formatter->addStep("helmert");
         formatter->addParam("x", x);
@@ -8299,19 +8309,8 @@ void Transformation::_exportToPROJString(
             }
         }
 
-        setupPROJGeodeticTargetCRS(formatter, targetCRS(), "Helmert");
-
-        if (methodEPSGCode == EPSG_CODE_METHOD_COORDINATE_FRAME_GEOGRAPHIC_2D ||
-            methodEPSGCode ==
-                EPSG_CODE_METHOD_TIME_DEPENDENT_COORDINATE_FRAME_GEOGRAPHIC_2D ||
-            methodEPSGCode == EPSG_CODE_METHOD_POSITION_VECTOR_GEOGRAPHIC_2D ||
-            methodEPSGCode ==
-                EPSG_CODE_METHOD_TIME_DEPENDENT_POSITION_VECTOR_GEOGRAPHIC_2D ||
-            methodEPSGCode ==
-                EPSG_CODE_METHOD_GEOCENTRIC_TRANSLATION_GEOGRAPHIC_2D) {
-            formatter->addStep("pop");
-            formatter->addParam("v_3");
-        }
+        setupPROJGeodeticTargetCRS(formatter, targetCRS(), addPushPopV3,
+                                   "Helmert");
 
         return;
     }
@@ -8359,15 +8358,13 @@ void Transformation::_exportToPROJString(
         double pz = parameterValueNumericAsSI(
             EPSG_CODE_PARAMETER_ORDINATE_3_EVAL_POINT);
 
-        if (methodEPSGCode ==
-                EPSG_CODE_METHOD_MOLODENSKY_BADEKAS_PV_GEOGRAPHIC_2D ||
-            methodEPSGCode ==
-                EPSG_CODE_METHOD_MOLODENSKY_BADEKAS_CF_GEOGRAPHIC_2D) {
-            formatter->addStep("push");
-            formatter->addParam("v_3");
-        }
+        bool addPushPopV3 =
+            (methodEPSGCode ==
+                 EPSG_CODE_METHOD_MOLODENSKY_BADEKAS_PV_GEOGRAPHIC_2D ||
+             methodEPSGCode ==
+                 EPSG_CODE_METHOD_MOLODENSKY_BADEKAS_CF_GEOGRAPHIC_2D);
 
-        setupPROJGeodeticSourceCRS(formatter, sourceCRS(),
+        setupPROJGeodeticSourceCRS(formatter, sourceCRS(), addPushPopV3,
                                    "Molodensky-Badekas");
 
         formatter->addStep("molobadekas");
@@ -8387,17 +8384,9 @@ void Transformation::_exportToPROJString(
             formatter->addParam("convention", "coordinate_frame");
         }
 
-        setupPROJGeodeticTargetCRS(formatter, targetCRS(),
+        setupPROJGeodeticTargetCRS(formatter, targetCRS(), addPushPopV3,
                                    "Molodensky-Badekas");
 
-        if (methodEPSGCode ==
-                EPSG_CODE_METHOD_MOLODENSKY_BADEKAS_PV_GEOGRAPHIC_2D ||
-            methodEPSGCode ==
-                EPSG_CODE_METHOD_MOLODENSKY_BADEKAS_CF_GEOGRAPHIC_2D) {
-            formatter->addStep("pop");
-            formatter->addParam("v_3");
-        }
-
         return;
     }
 
diff --git a/src/iso19111/io.cpp b/src/iso19111/io.cpp
index f602c7c0..a1608464 100644
--- a/src/iso19111/io.cpp
+++ b/src/iso19111/io.cpp
@@ -5102,98 +5102,6 @@ const std::string &PROJStringFormatter::toString() const {
                 }
             }
 
-            // axisswap order=2,1, pop/push v_3, axisswap order=2,1 -> can
-            // suppress axisswap
-            if (i + 1 < d->steps_.size() && prevStep.name == "axisswap" &&
-                (curStep.name == "push" || curStep.name == "pop") &&
-                prevStepParamCount == 1 &&
-                prevStep.paramValues[0].equals("order", "2,1") &&
-                curStepParamCount == 1 && curStep.paramValues[0].key == "v_3") {
-                auto iterNext = iterCur;
-                ++iterNext;
-                auto &nextStep = *iterNext;
-                if (nextStep.name == "axisswap" &&
-                    nextStep.paramValues.size() == 1 &&
-                    nextStep.paramValues[0].equals("order", "2,1")) {
-                    d->steps_.erase(iterPrev);
-                    d->steps_.erase(iterNext);
-                    changeDone = true;
-                    break;
-                }
-            }
-
-            // push v_3, axisswap order=2,1, pop v_3 -> can suppress push/pop
-            if (i + 1 < d->steps_.size() && prevStep.name == "push" &&
-                prevStepParamCount == 1 &&
-                prevStep.paramValues[0].key == "v_3" &&
-                curStep.name == "axisswap" && curStepParamCount == 1 &&
-                curStep.paramValues[0].equals("order", "2,1")) {
-                auto iterNext = iterCur;
-                ++iterNext;
-                auto &nextStep = *iterNext;
-                if (nextStep.name == "pop" &&
-                    nextStep.paramValues.size() == 1 &&
-                    nextStep.paramValues[0].key == "v_3") {
-                    d->steps_.erase(iterPrev);
-                    d->steps_.erase(iterNext);
-                    changeDone = true;
-                    break;
-                }
-            }
-
-            // push v_3, unitconvert xy_in xy_out, axisswap order=2,1, pop v_3
-            // -> can suppress push/pop
-            if (i + 2 < d->steps_.size() && prevStep.name == "push" &&
-                prevStepParamCount == 1 &&
-                prevStep.paramValues[0].key == "v_3" &&
-                curStep.name == "unitconvert" && curStepParamCount == 2 &&
-                curStep.paramValues[0].key == "xy_in" &&
-                curStep.paramValues[1].key == "xy_out") {
-                auto iterNext = iterCur;
-                ++iterNext;
-                auto &nextStep = *iterNext;
-                auto iterNextNext = iterNext;
-                ++iterNextNext;
-                auto &nextNextStep = *iterNextNext;
-                if (nextStep.name == "axisswap" &&
-                    nextStep.paramValues.size() == 1 &&
-                    nextStep.paramValues[0].equals("order", "2,1") &&
-                    nextNextStep.name == "pop" &&
-                    nextNextStep.paramValues.size() == 1 &&
-                    nextNextStep.paramValues[0].key == "v_3") {
-                    d->steps_.erase(iterPrev);
-                    d->steps_.erase(iterNextNext);
-                    changeDone = true;
-                    break;
-                }
-            }
-
-            // unitconvert xy_in=A xy_out=B, pop/push v_3, unitconvert xy_in=B
-            // xy_out=A -> can suppress unitconvert
-            if (i + 1 < d->steps_.size() && prevStep.name == "unitconvert" &&
-                (curStep.name == "push" || curStep.name == "pop") &&
-                prevStepParamCount == 2 &&
-                prevStep.paramValues[0].key == "xy_in" &&
-                prevStep.paramValues[1].key == "xy_out" &&
-                curStepParamCount == 1 && curStep.paramValues[0].key == "v_3") {
-                auto iterNext = iterCur;
-                ++iterNext;
-                auto &nextStep = *iterNext;
-                if (nextStep.name == "unitconvert" &&
-                    nextStep.paramValues.size() == 2 &&
-                    nextStep.paramValues[0].key == "xy_in" &&
-                    nextStep.paramValues[1].key == "xy_out" &&
-                    nextStep.paramValues[0].value ==
-                        prevStep.paramValues[1].value &&
-                    nextStep.paramValues[1].value ==
-                        prevStep.paramValues[0].value) {
-                    d->steps_.erase(iterPrev);
-                    d->steps_.erase(iterNext);
-                    changeDone = true;
-                    break;
-                }
-            }
-
             // for practical purposes WGS84 and GRS80 ellipsoids are
             // equivalents (cartesian transform between both lead to differences
             // of the order of 1e-14 deg..).