Skip to content

Commit 119ed32

Browse files
rouaultrouault
authored
Merge pull request #4775 from rouault/optimize_inv_helmert_and_opposite_inv_helmert
Optimize +step +proj=helmert +inv +x=X +y=Y +z=Z +step +proj=helmert +inv +x=-X +y=-Y +z=-Z as +proj=noop
2 parents 297e693 + c364aef commit 119ed32

2 files changed

Lines changed: 167 additions & 25 deletions

File tree

‎src/iso19111/io.cpp‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9495,7 +9495,6 @@ const std::string &PROJStringFormatter::toString() const {
94959495
}
94969496

94979497
if (curStep.name == "helmert" && prevStep.name == "helmert" &&
9498-
!curStep.inverted && !prevStep.inverted &&
94999498
curStepParamCount == 3 &&
95009499
curStepParamCount == prevStepParamCount) {
95019500
std::map<std::string, double> leftParamsMap;
@@ -9520,12 +9519,18 @@ const std::string &PROJStringFormatter::toString() const {
95209519
rightParamsMap.find(y) != rightParamsMap.end() &&
95219520
rightParamsMap.find(z) != rightParamsMap.end()) {
95229521

9523-
const double xSum = leftParamsMap[x] + rightParamsMap[x];
9524-
const double ySum = leftParamsMap[y] + rightParamsMap[y];
9525-
const double zSum = leftParamsMap[z] + rightParamsMap[z];
9522+
const double signLeft = prevStep.inverted ? -1 : 1;
9523+
const double signRight = curStep.inverted ? -1 : 1;
9524+
const double xSum = signLeft * leftParamsMap[x] +
9525+
signRight * rightParamsMap[x];
9526+
const double ySum = signLeft * leftParamsMap[y] +
9527+
signRight * rightParamsMap[y];
9528+
const double zSum = signLeft * leftParamsMap[z] +
9529+
signRight * rightParamsMap[z];
95269530
if (xSum == 0.0 && ySum == 0.0 && zSum == 0.0) {
95279531
deletePrevAndCurIter();
95289532
} else {
9533+
prevStep.inverted = false;
95299534
prevStep.paramValues[0] =
95309535
Step::KeyValue("x", internal::toString(xSum));
95319536
prevStep.paramValues[1] =
@@ -9542,7 +9547,7 @@ const std::string &PROJStringFormatter::toString() const {
95429547

95439548
// Helmert followed by its inverse is a no-op
95449549
if (curStep.name == "helmert" && prevStep.name == "helmert" &&
9545-
!curStep.inverted && !prevStep.inverted &&
9550+
(curStep.inverted == prevStep.inverted) &&
95469551
curStepParamCount == prevStepParamCount) {
95479552
std::set<std::string> leftParamsSet;
95489553
std::set<std::string> rightParamsSet;

‎test/unit/test_io.cpp‎

Lines changed: 157 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10201,31 +10201,137 @@ TEST(io, projstringformatter_helmert_7_param_noop) {
1020110201
// ---------------------------------------------------------------------------
1020210202

1020310203
TEST(io, projstringformatter_merge_consecutive_helmert_3_param) {
10204-
auto fmt = PROJStringFormatter::create();
10205-
fmt->addStep("helmert");
10206-
fmt->addParam("x", 10);
10207-
fmt->addParam("y", 20);
10208-
fmt->addParam("z", 30);
10209-
fmt->addStep("helmert");
10210-
fmt->addParam("x", -1);
10211-
fmt->addParam("y", -2);
10212-
fmt->addParam("z", -3);
10213-
EXPECT_EQ(fmt->toString(), "+proj=helmert +x=9 +y=18 +z=27");
10204+
{
10205+
auto fmt = PROJStringFormatter::create();
10206+
fmt->addStep("helmert");
10207+
fmt->addParam("x", 10);
10208+
fmt->addParam("y", 20);
10209+
fmt->addParam("z", 30);
10210+
10211+
fmt->addStep("helmert");
10212+
fmt->addParam("x", -1);
10213+
fmt->addParam("y", -2);
10214+
fmt->addParam("z", -3);
10215+
EXPECT_EQ(fmt->toString(), "+proj=helmert +x=9 +y=18 +z=27");
10216+
}
10217+
10218+
{
10219+
auto fmt = PROJStringFormatter::create();
10220+
fmt->addStep("helmert");
10221+
fmt->addParam("x", 10);
10222+
fmt->addParam("y", 20);
10223+
fmt->addParam("z", 30);
10224+
10225+
fmt->addStep("helmert");
10226+
fmt->setCurrentStepInverted(true);
10227+
fmt->addParam("x", -10);
10228+
fmt->addParam("y", -20);
10229+
fmt->addParam("z", -30);
10230+
EXPECT_EQ(fmt->toString(), "+proj=helmert +x=20 +y=40 +z=60");
10231+
}
10232+
10233+
{
10234+
auto fmt = PROJStringFormatter::create();
10235+
fmt->addStep("helmert");
10236+
fmt->setCurrentStepInverted(true);
10237+
fmt->addParam("x", 10);
10238+
fmt->addParam("y", 20);
10239+
fmt->addParam("z", 30);
10240+
10241+
fmt->addStep("helmert");
10242+
fmt->addParam("x", -10);
10243+
fmt->addParam("y", -20);
10244+
fmt->addParam("z", -30);
10245+
EXPECT_EQ(fmt->toString(), "+proj=helmert +x=-20 +y=-40 +z=-60");
10246+
}
1021410247
}
1021510248

1021610249
// ---------------------------------------------------------------------------
1021710250

1021810251
TEST(io, projstringformatter_merge_consecutive_helmert_3_param_noop) {
10219-
auto fmt = PROJStringFormatter::create();
10220-
fmt->addStep("helmert");
10221-
fmt->addParam("x", 10);
10222-
fmt->addParam("y", 20);
10223-
fmt->addParam("z", 30);
10224-
fmt->addStep("helmert");
10225-
fmt->addParam("x", -10);
10226-
fmt->addParam("y", -20);
10227-
fmt->addParam("z", -30);
10228-
EXPECT_EQ(fmt->toString(), "+proj=noop");
10252+
{
10253+
auto fmt = PROJStringFormatter::create();
10254+
fmt->addStep("helmert");
10255+
fmt->addParam("x", 10);
10256+
fmt->addParam("y", 20);
10257+
fmt->addParam("z", 30);
10258+
10259+
fmt->addStep("helmert");
10260+
fmt->addParam("x", -10);
10261+
fmt->addParam("y", -20);
10262+
fmt->addParam("z", -30);
10263+
EXPECT_EQ(fmt->toString(), "+proj=noop");
10264+
}
10265+
10266+
{
10267+
auto fmt = PROJStringFormatter::create();
10268+
fmt->addStep("helmert");
10269+
fmt->setCurrentStepInverted(true);
10270+
fmt->addParam("x", 10);
10271+
fmt->addParam("y", 20);
10272+
fmt->addParam("z", 30);
10273+
10274+
fmt->addStep("helmert");
10275+
fmt->setCurrentStepInverted(true);
10276+
fmt->addParam("x", -10);
10277+
fmt->addParam("y", -20);
10278+
fmt->addParam("z", -30);
10279+
EXPECT_EQ(fmt->toString(), "+proj=noop");
10280+
}
10281+
}
10282+
10283+
// ---------------------------------------------------------------------------
10284+
10285+
TEST(io, projstringformatter_merge_consecutive_helmert_7_param_noop) {
10286+
{
10287+
auto fmt = PROJStringFormatter::create();
10288+
fmt->addStep("helmert");
10289+
fmt->addParam("x", 10);
10290+
fmt->addParam("y", 20);
10291+
fmt->addParam("z", 30);
10292+
fmt->addParam("rx", 1);
10293+
fmt->addParam("ry", 2);
10294+
fmt->addParam("rz", 3);
10295+
fmt->addParam("s", 4);
10296+
fmt->addParam("convention", "position_vector");
10297+
10298+
fmt->addStep("helmert");
10299+
fmt->addParam("x", -10);
10300+
fmt->addParam("y", -20);
10301+
fmt->addParam("z", -30);
10302+
fmt->addParam("rx", -1);
10303+
fmt->addParam("ry", -2);
10304+
fmt->addParam("rz", -3);
10305+
fmt->addParam("s", -4);
10306+
fmt->addParam("convention", "position_vector");
10307+
EXPECT_EQ(fmt->toString(), "+proj=noop");
10308+
}
10309+
10310+
{
10311+
auto fmt = PROJStringFormatter::create();
10312+
fmt->addStep("helmert");
10313+
fmt->setCurrentStepInverted(true);
10314+
fmt->addParam("x", 10);
10315+
fmt->addParam("y", 20);
10316+
fmt->addParam("z", 30);
10317+
fmt->addParam("rx", 1);
10318+
fmt->addParam("ry", 2);
10319+
fmt->addParam("rz", 3);
10320+
fmt->addParam("s", 4);
10321+
fmt->addParam("convention", "position_vector");
10322+
10323+
fmt->addStep("helmert");
10324+
fmt->setCurrentStepInverted(true);
10325+
fmt->addParam("x", -10);
10326+
fmt->addParam("y", -20);
10327+
fmt->addParam("z", -30);
10328+
fmt->addParam("rx", -1);
10329+
fmt->addParam("ry", -2);
10330+
fmt->addParam("rz", -3);
10331+
fmt->addParam("s", -4);
10332+
fmt->addParam("convention", "position_vector");
10333+
EXPECT_EQ(fmt->toString(), "+proj=noop");
10334+
}
1022910335
}
1023010336

1023110337
// ---------------------------------------------------------------------------
@@ -10242,11 +10348,37 @@ TEST(io, projstringformatter_merge_inverted_helmert_with_opposite_conventions) {
1024210348
fmt->addParam("rz", 3);
1024310349
fmt->addParam("s", 4);
1024410350
fmt->addParam("convention", "position_vector");
10351+
10352+
fmt->addStep("helmert");
10353+
fmt->setCurrentStepInverted(true);
10354+
fmt->addParam("x", 10);
10355+
fmt->addParam("y", 20);
10356+
fmt->addParam("z", 30);
10357+
fmt->addParam("rx", -1);
10358+
fmt->addParam("ry", -2);
10359+
fmt->addParam("rz", -3);
10360+
fmt->addParam("s", 4);
10361+
fmt->addParam("convention", "coordinate_frame");
10362+
EXPECT_EQ(fmt->toString(), "+proj=noop");
10363+
}
10364+
10365+
{
10366+
auto fmt = PROJStringFormatter::create();
1024510367
fmt->addStep("helmert");
1024610368
fmt->setCurrentStepInverted(true);
1024710369
fmt->addParam("x", 10);
1024810370
fmt->addParam("y", 20);
1024910371
fmt->addParam("z", 30);
10372+
fmt->addParam("rx", 1);
10373+
fmt->addParam("ry", 2);
10374+
fmt->addParam("rz", 3);
10375+
fmt->addParam("s", 4);
10376+
fmt->addParam("convention", "position_vector");
10377+
10378+
fmt->addStep("helmert");
10379+
fmt->addParam("x", 10);
10380+
fmt->addParam("y", 20);
10381+
fmt->addParam("z", 30);
1025010382
fmt->addParam("rx", -1);
1025110383
fmt->addParam("ry", -2);
1025210384
fmt->addParam("rz", -3);
@@ -10267,6 +10399,7 @@ TEST(io, projstringformatter_merge_inverted_helmert_with_opposite_conventions) {
1026710399
fmt->addParam("rz", 3);
1026810400
fmt->addParam("s", 4);
1026910401
fmt->addParam("convention", "coordinate_frame");
10402+
1027010403
fmt->addStep("helmert");
1027110404
fmt->addParam("x", 10);
1027210405
fmt->addParam("y", 20);
@@ -10291,6 +10424,7 @@ TEST(io, projstringformatter_merge_inverted_helmert_with_opposite_conventions) {
1029110424
fmt->addParam("rz", 3);
1029210425
fmt->addParam("s", 4);
1029310426
fmt->addParam("convention", "position_vector");
10427+
1029410428
fmt->addStep("helmert");
1029510429
// fmt->setCurrentStepInverted(true); <== CAUSE
1029610430
fmt->addParam("x", 10);
@@ -10316,6 +10450,7 @@ TEST(io, projstringformatter_merge_inverted_helmert_with_opposite_conventions) {
1031610450
fmt->addParam("rz", 3);
1031710451
fmt->addParam("s", 4);
1031810452
fmt->addParam("convention", "position_vector");
10453+
1031910454
fmt->addStep("helmert");
1032010455
fmt->setCurrentStepInverted(true);
1032110456
fmt->addParam("x", 10);
@@ -10341,6 +10476,7 @@ TEST(io, projstringformatter_merge_inverted_helmert_with_opposite_conventions) {
1034110476
fmt->addParam("rz", 3);
1034210477
fmt->addParam("s", 4);
1034310478
fmt->addParam("convention", "position_vector");
10479+
1034410480
fmt->addStep("helmert");
1034510481
fmt->setCurrentStepInverted(true);
1034610482
fmt->addParam("x", -10); // <== CAUSE
@@ -10366,6 +10502,7 @@ TEST(io, projstringformatter_merge_inverted_helmert_with_opposite_conventions) {
1036610502
fmt->addParam("rz", 2);
1036710503
fmt->addParam("s", 4);
1036810504
fmt->addParam("convention", "position_vector");
10505+
1036910506
fmt->addStep("helmert");
1037010507
fmt->setCurrentStepInverted(true);
1037110508
fmt->addParam("x", 10);

0 commit comments

Comments
 (0)