Skip to content

Commit

Permalink
createOperations(): tune name and remarks of some vertical transforma…
Browse files Browse the repository at this point in the history 
…tions
  • Loading branch information
@rouault
rouault committed Sep 21, 2022
1 parent 966ea2b commit ecc13ab
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 26 deletions.
62 changes: 51 additions & 11 deletions src/iso19111/operation/coordinateoperationfactory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2118,7 +2118,6 @@ struct MyPROJStringExportableHorizVerticalHorizPROJBased final
EPSG_CODE_METHOD_GEOCENTRIC_TRANSLATION_GEOGRAPHIC_2D ||
methodEPSGCode ==
EPSG_CODE_METHOD_GEOCENTRIC_TRANSLATION_GEOGRAPHIC_3D;

if ((bGeocentricTranslation &&
!(transf->parameterValueNumericAsSI(
EPSG_CODE_PARAMETER_X_AXIS_TRANSLATION) == 0 &&
Expand Down Expand Up @@ -2413,6 +2412,34 @@ static CoordinateOperationNNPtr createHorizVerticalHorizPROJBased(
ops.emplace_back(opGeogCRStoDstCRS);
}

std::vector<CoordinateOperationNNPtr> opsForRemarks;
std::vector<CoordinateOperationNNPtr> opsForAccuracy;
std::string opName;
if (ops.size() == 3 && opGeogCRStoDstCRS->inverse()->_isEquivalentTo(
opSrcCRSToGeogCRS.get(),
util::IComparable::Criterion::EQUIVALENT)) {
opsForRemarks.emplace_back(opSrcCRSToGeogCRS);
opsForRemarks.emplace_back(verticalTransform);

// Only taking into account the accuracy of the vertical transform when
// opSrcCRSToGeogCRS and opGeogCRStoDstCRS are reversed and cancel
// themselves would make sense. Unfortunately it causes
// EPSG:4313+5710 (BD72 + Ostend height) to EPSG:9707
// (WGS 84 + EGM96 height) to use a non-ideal pipeline.
// opsForAccuracy.emplace_back(verticalTransform);
opsForAccuracy = ops;

opName = verticalTransform->nameStr() + " using ";
if (!starts_with(opSrcCRSToGeogCRS->nameStr(), "Inverse of"))
opName += opSrcCRSToGeogCRS->nameStr();
else
opName += opGeogCRStoDstCRS->nameStr();
} else {
opsForRemarks = ops;
opsForAccuracy = ops;
opName = computeConcatenatedName(ops);
}

bool hasBallparkTransformation = false;
for (const auto &op : ops) {
hasBallparkTransformation |= op->hasBallparkTransformation();
Expand All @@ -2426,21 +2453,20 @@ static CoordinateOperationNNPtr createHorizVerticalHorizPROJBased(
throw InvalidOperationEmptyIntersection(msg);
}
auto properties = util::PropertyMap();
properties.set(common::IdentifiedObject::NAME_KEY,
computeConcatenatedName(ops));
properties.set(common::IdentifiedObject::NAME_KEY, opName);

if (extent) {
properties.set(common::ObjectUsage::DOMAIN_OF_VALIDITY_KEY,
NN_NO_CHECK(extent));
}

const auto remarks = getRemarks(ops);
const auto remarks = getRemarks(opsForRemarks);
if (!remarks.empty()) {
properties.set(common::IdentifiedObject::REMARKS_KEY, remarks);
}

std::vector<metadata::PositionalAccuracyNNPtr> accuracies;
const double accuracy = getAccuracy(ops);
const double accuracy = getAccuracy(opsForAccuracy);
if (accuracy >= 0.0) {
accuracies.emplace_back(
metadata::PositionalAccuracy::create(toString(accuracy)));
Expand Down Expand Up @@ -5674,6 +5700,7 @@ void CoordinateOperationFactory::Private::createOperationsCompoundToCompound(
const bool hasNonTrivialTargetTransf =
hasNonTrivialTransf(opsGeogToTarget);
double bestAccuracy = -1;
size_t bestStepCount = 0;
if (hasNonTrivialSrcTransf && hasNonTrivialTargetTransf) {
const auto opsGeogSrcToGeogDst =
createOperations(intermGeogSrc, intermGeogDst, context);
Expand Down Expand Up @@ -5715,10 +5742,15 @@ void CoordinateOperationFactory::Private::createOperationsCompoundToCompound(
op3},
disallowEmptyIntersection));
const double accuracy = getAccuracy(res.back());
const size_t stepCount =
getStepCount(res.back());
if (accuracy >= 0 &&
(bestAccuracy < 0 ||
accuracy < bestAccuracy)) {
(accuracy < bestAccuracy ||
(accuracy == bestAccuracy &&
stepCount < bestStepCount)))) {
bestAccuracy = accuracy;
bestStepCount = stepCount;
}
} catch (const std::exception &) {
}
Expand All @@ -5729,11 +5761,12 @@ void CoordinateOperationFactory::Private::createOperationsCompoundToCompound(
}

const auto createOpsInTwoSteps =
[&res,
bestAccuracy](const std::vector<CoordinateOperationNNPtr> &ops1,
const std::vector<CoordinateOperationNNPtr> &ops2) {
[&res, bestAccuracy,
bestStepCount](const std::vector<CoordinateOperationNNPtr> &ops1,
const std::vector<CoordinateOperationNNPtr> &ops2) {
std::vector<CoordinateOperationNNPtr> res2;
double bestAccuracy2 = -1;
size_t bestStepCount2 = 0;

// In first pass, exclude (horizontal) ballpark operations, but
// accept them in second pass.
Expand Down Expand Up @@ -5768,10 +5801,15 @@ void CoordinateOperationFactory::Private::createOperationsCompoundToCompound(
disallowEmptyIntersection));
const double accuracy =
getAccuracy(res2.back());
const size_t stepCount =
getStepCount(res2.back());
if (accuracy >= 0 &&
(bestAccuracy2 < 0 ||
accuracy < bestAccuracy2)) {
(accuracy < bestAccuracy2 ||
(accuracy == bestAccuracy2 &&
stepCount < bestStepCount2)))) {
bestAccuracy2 = accuracy;
bestStepCount2 = stepCount;
}
} catch (const std::exception &) {
}
Expand All @@ -5782,7 +5820,9 @@ void CoordinateOperationFactory::Private::createOperationsCompoundToCompound(
// Keep the results of this new attempt, if there are better
// than the previous ones
if (bestAccuracy2 >= 0 &&
(bestAccuracy < 0 || bestAccuracy2 < bestAccuracy)) {
(bestAccuracy < 0 || (bestAccuracy2 < bestAccuracy ||
(bestAccuracy2 == bestAccuracy &&
bestStepCount2 < bestStepCount)))) {
res = std::move(res2);
}
};
Expand Down
5 changes: 2 additions & 3 deletions test/unit/test_c_api.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5160,10 +5160,9 @@ TEST_F(CApi, proj_create_vertical_crs_ex_with_geog_crs) {
ASSERT_TRUE(name != nullptr);
EXPECT_EQ(name,
std::string("Inverse of UTM zone 11N + "
"NAD83(2011) to WGS 84 (1) + "
"Conversion from myVertCRS to myVertCRS (metre) + "
"Transformation from myVertCRS (metre) to WGS 84 + "
"Inverse of NAD83(2011) to WGS 84 (1)"));
"Transformation from myVertCRS (metre) to WGS 84 "
"using NAD83(2011) to WGS 84 (1)"));

auto proj_5 = proj_as_proj_string(m_ctxt, P, PJ_PROJ_5, nullptr);
ASSERT_NE(proj_5, nullptr);
Expand Down
20 changes: 8 additions & 12 deletions test/unit/test_operationfactory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4741,10 +4741,9 @@ TEST(operation, compoundCRS_to_compoundCRS_WGS84_EGM96_to_WGS84_Belfast) {
auto list = CoordinateOperationFactory::create()->createOperations(
NN_NO_CHECK(srcCrs), NN_NO_CHECK(destCrs), ctxt);
ASSERT_GE(list.size(), 1U);
EXPECT_EQ(list[0]->nameStr(), "Inverse of WGS 84 to EGM96 height (1) + "
"Inverse of ETRS89 to WGS 84 (1) + "
"ETRS89 to Belfast height (2) + "
"ETRS89 to WGS 84 (1)");
EXPECT_EQ(list[0]->nameStr(),
"Inverse of WGS 84 to EGM96 height (1) + "
"ETRS89 to Belfast height (2) using ETRS89 to WGS 84 (1)");
EXPECT_EQ(list[0]->exportToPROJString(PROJStringFormatter::create().get()),
"+proj=pipeline +step +proj=axisswap +order=2,1 "
"+step +proj=unitconvert +xy_in=deg +xy_out=rad "
Expand Down Expand Up @@ -5016,9 +5015,8 @@ TEST(operation, compoundCRS_to_compoundCRS_issue_3328) {
NN_NO_CHECK(src), NN_NO_CHECK(dst), ctxt);
ASSERT_GE(list.size(), 1U);
EXPECT_EQ(list[0]->nameStr(), "Inverse of WGS 84 to EGM96 height (1) + "
"Inverse of NAD83(CSRS) to WGS 84 (2) + "
"NAD83(CSRS) to CGVD28 height (1) + "
"NAD83(CSRS) to WGS 84 (2)");
"NAD83(CSRS) to CGVD28 height (1) "
"using NAD83(CSRS) to WGS 84 (2)");
EXPECT_EQ(list[0]->exportToPROJString(PROJStringFormatter::create().get()),
"+proj=pipeline "
"+step +proj=push +v_1 +v_2 "
Expand Down Expand Up @@ -5073,12 +5071,10 @@ TEST(
NN_NO_CHECK(src), NN_NO_CHECK(dst), ctxt);
ASSERT_GE(list.size(), 1U);
EXPECT_EQ(list[0]->nameStr(),
"Ballpark geographic offset from "
"NAD83(CSRS) to NAD83(CSRS)v6 + "
"Inverse of NAD83(CSRS)v6 to CGVD28 height (1) + "
"NAD83(CSRS)v6 to CGVD2013(CGG2013) height (1) + "
"Ballpark geographic offset from "
"NAD83(CSRS)v6 to NAD83(CSRS)");
"NAD83(CSRS)v6 to CGVD2013(CGG2013) height (1) "
"using Ballpark geographic offset "
"from NAD83(CSRS) to NAD83(CSRS)v6");
}
#if 0
// Note: below situation is no longer triggered since EPSG v10.066 update
Expand Down

0 comments on commit ecc13ab

Please sign in to comment.