Skip to content

Commit

Permalink
Merge pull request #4166 from rouault/fix_wrong_utm_zone_south
Browse files Browse the repository at this point in the history 
Fix wrong EPSG conversion code for UTM south
  • Loading branch information
@rouault
rouault authored Jun 10, 2024
2 parents e0e1482 + 41be409 commit d467ecb
Show file tree
Hide file tree
Showing 4 changed files with 167 additions and 13 deletions.
61 changes: 50 additions & 11 deletions src/iso19111/io.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1289,7 +1289,8 @@ struct WKTParser::Private {

BaseObjectNNPtr build(const WKTNodeNNPtr &node);

IdentifierPtr buildId(const WKTNodeNNPtr &node, bool tolerant,
IdentifierPtr buildId(const WKTNodeNNPtr &parentNode,
const WKTNodeNNPtr &node, bool tolerant,
bool removeInverseOf);

PropertyMap &buildProperties(const WKTNodeNNPtr &node,
Expand Down Expand Up @@ -1605,7 +1606,8 @@ double WKTParser::Private::asDouble(const WKTNodeNNPtr &node) {

// ---------------------------------------------------------------------------

IdentifierPtr WKTParser::Private::buildId(const WKTNodeNNPtr &node,
IdentifierPtr WKTParser::Private::buildId(const WKTNodeNNPtr &parentNode,
const WKTNodeNNPtr &node,
bool tolerant, bool removeInverseOf) {
const auto *nodeP = node->GP();
const auto &nodeChildren = nodeP->children();
Expand Down Expand Up @@ -1638,6 +1640,26 @@ IdentifierPtr WKTParser::Private::buildId(const WKTNodeNNPtr &node,
}

auto code = stripQuotes(nodeChildren[1]);

// Prior to PROJ 9.5, when synthetizing an ID for a CONVERSION UTM Zone
// south, we generated a wrong value. Auto-fix that
const auto &parentNodeKeyword(parentNode->GP()->value());
if (parentNodeKeyword == WKTConstants::CONVERSION &&
codeSpace == Identifier::EPSG) {
const auto &parentNodeChildren = parentNode->GP()->children();
if (!parentNodeChildren.empty()) {
const auto parentNodeName(stripQuotes(parentNodeChildren[0]));
if (ci_starts_with(parentNodeName, "UTM Zone ") &&
parentNodeName.find('S') != std::string::npos) {
const int nZone =
atoi(parentNodeName.c_str() + strlen("UTM Zone "));
if (nZone >= 1 && nZone <= 60) {
code = internal::toString(16100 + nZone);
}
}
}
}

auto &citationNode = nodeP->lookForChild(WKTConstants::CITATION);
auto &uriNode = nodeP->lookForChild(WKTConstants::URI);

Expand Down Expand Up @@ -1695,7 +1717,7 @@ PropertyMap &WKTParser::Private::buildProperties(const WKTNodeNNPtr &node,
const auto &subNodeName(subNode->GP()->value());
if (ci_equal(subNodeName, WKTConstants::ID) ||
ci_equal(subNodeName, WKTConstants::AUTHORITY)) {
auto id = buildId(subNode, true, removeInverseOf);
auto id = buildId(node, subNode, true, removeInverseOf);
if (id) {
identifiers->add(NN_NO_CHECK(id));
}
Expand Down Expand Up @@ -2328,7 +2350,7 @@ GeodeticReferenceFrameNNPtr WKTParser::Private::buildGeodeticReferenceFrame(
auto &idNode = nodeP->lookForChild(WKTConstants::AUTHORITY);
if (!isNull(idNode)) {
try {
auto id = buildId(idNode, false, false);
auto id = buildId(node, idNode, false, false);
auto authFactory2 = AuthorityFactory::create(
NN_NO_CHECK(dbContext_), *id->codeSpace());
auto dbDatum =
Expand Down Expand Up @@ -3208,7 +3230,7 @@ WKTParser::Private::buildGeodeticCRS(const WKTNodeNNPtr &node) {
const auto &subNodeName(subNode->GP()->value());
if (ci_equal(subNodeName, WKTConstants::ID) ||
ci_equal(subNodeName, WKTConstants::AUTHORITY)) {
auto id = buildId(subNode, true, false);
auto id = buildId(node, subNode, true, false);
if (id) {
try {
auto authFactory = AuthorityFactory::create(
Expand Down Expand Up @@ -4502,7 +4524,7 @@ WKTParser::Private::buildProjectedCRS(const WKTNodeNNPtr &node) {
auto &idNode = nodeP->lookForChild(WKTConstants::ID);
if (!isNull(idNode)) {
try {
auto id = buildId(idNode, false, false);
auto id = buildId(node, idNode, false, false);
auto authFactory = AuthorityFactory::create(
NN_NO_CHECK(dbContext_), *id->codeSpace());
auto projCRS =
Expand Down Expand Up @@ -5762,7 +5784,7 @@ BaseObjectNNPtr WKTParser::Private::build(const WKTNodeNNPtr &node) {
if (ci_equal(name, WKTConstants::ID) ||
ci_equal(name, WKTConstants::AUTHORITY)) {
return util::nn_static_pointer_cast<BaseObject>(
NN_NO_CHECK(buildId(node, false, false)));
NN_NO_CHECK(buildId(node, node, false, false)));
}

if (ci_equal(name, WKTConstants::COORDINATEMETADATA)) {
Expand Down Expand Up @@ -5792,7 +5814,8 @@ class JSONParser {
static Length getLength(const json &j, const char *key);
static Measure getMeasure(const json &j);

IdentifierNNPtr buildId(const json &j, bool removeInverseOf);
IdentifierNNPtr buildId(const json &parentJ, const json &j,
bool removeInverseOf);
static ObjectDomainPtr buildObjectDomain(const json &j);
PropertyMap buildProperties(const json &j, bool removeInverseOf = false,
bool nameRequired = true);
Expand Down Expand Up @@ -6124,7 +6147,8 @@ ObjectDomainPtr JSONParser::buildObjectDomain(const json &j) {

// ---------------------------------------------------------------------------

IdentifierNNPtr JSONParser::buildId(const json &j, bool removeInverseOf) {
IdentifierNNPtr JSONParser::buildId(const json &parentJ, const json &j,
bool removeInverseOf) {

PropertyMap propertiesId;
auto codeSpace(getString(j, "authority"));
Expand Down Expand Up @@ -6178,6 +6202,21 @@ IdentifierNNPtr JSONParser::buildId(const json &j, bool removeInverseOf) {
throw ParsingException("Unexpected type for value of \"code\"");
}

// Prior to PROJ 9.5, when synthetizing an ID for a CONVERSION UTM Zone
// south, we generated a wrong value. Auto-fix that
if (parentJ.contains("type") && getType(parentJ) == "Conversion" &&
codeSpace == Identifier::EPSG && parentJ.contains("name")) {
const auto parentNodeName(getName(parentJ));
if (ci_starts_with(parentNodeName, "UTM Zone ") &&
parentNodeName.find('S') != std::string::npos) {
const int nZone =
atoi(parentNodeName.c_str() + strlen("UTM Zone "));
if (nZone >= 1 && nZone <= 60) {
code = internal::toString(16100 + nZone);
}
}
}

if (!version.empty()) {
propertiesId.set(Identifier::VERSION_KEY, version);
}
Expand Down Expand Up @@ -6216,13 +6255,13 @@ PropertyMap JSONParser::buildProperties(const json &j, bool removeInverseOf,
throw ParsingException(
"Unexpected type for value of \"ids\" child");
}
identifiers->add(buildId(idJ, removeInverseOf));
identifiers->add(buildId(j, idJ, removeInverseOf));
}
map.set(IdentifiedObject::IDENTIFIERS_KEY, identifiers);
} else if (j.contains("id")) {
auto idJ = getObject(j, "id");
auto identifiers = ArrayOfBaseObject::create();
identifiers->add(buildId(idJ, removeInverseOf));
identifiers->add(buildId(j, idJ, removeInverseOf));
map.set(IdentifiedObject::IDENTIFIERS_KEY, identifiers);
}

Expand Down
6 changes: 5 additions & 1 deletion src/iso19111/operation/conversion.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ getUTMConversionProperty(const util::PropertyMap &properties, int zone,
conversionName += (north ? 'N' : 'S');

return createMapNameEPSGCode(conversionName,
(north ? 16000 : 17000) + zone);
(north ? 16000 : 16100) + zone);
} else {
return properties;
}
Expand Down Expand Up @@ -1752,6 +1752,8 @@ ConversionNNPtr Conversion::createPopularVisualisationPseudoMercator(

// ---------------------------------------------------------------------------

// clang-format off

/** \brief Instantiate a conversion based on the
* <a href="../../../operations/projections/merc.html">
* Mercator</a> projection method, using its spherical formulation
Expand Down Expand Up @@ -1781,6 +1783,8 @@ ConversionNNPtr Conversion::createMercatorSpherical(
createParams(centerLat, centerLong, falseEasting, falseNorthing));
}

// clang-format on

// ---------------------------------------------------------------------------

/** \brief Instantiate a conversion based on the
Expand Down
111 changes: 111 additions & 0 deletions test/unit/test_io.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4244,6 +4244,36 @@ TEST(wkt_parse, conversion_proj_based) {

// ---------------------------------------------------------------------------

TEST(wkt_parse, conversion_utm_zone_south_wrong_id) {

auto wkt = "CONVERSION[\"UTM zone 55S\","
" METHOD[\"Transverse Mercator\","
" ID[\"EPSG\",9807]],"
" PARAMETER[\"Latitude of natural origin\",0,"
" ANGLEUNIT[\"Degree\",0.0174532925199433],"
" ID[\"EPSG\",8801]],"
" PARAMETER[\"Longitude of natural origin\",147,"
" ANGLEUNIT[\"Degree\",0.0174532925199433],"
" ID[\"EPSG\",8802]],"
" PARAMETER[\"Scale factor at natural origin\",0.9996,"
" SCALEUNIT[\"unity\",1],"
" ID[\"EPSG\",8805]],"
" PARAMETER[\"False easting\",500000,"
" LENGTHUNIT[\"metre\",1],"
" ID[\"EPSG\",8806]],"
" PARAMETER[\"False northing\",10000000,"
" LENGTHUNIT[\"metre\",1],"
" ID[\"EPSG\",8807]],"
" ID[\"EPSG\",17055]]"; // wrong code

auto obj = WKTParser().createFromWKT(wkt);
auto conv = nn_dynamic_pointer_cast<Conversion>(obj);
ASSERT_TRUE(conv != nullptr);
EXPECT_EQ(conv->getEPSGCode(), 16155); // code fixed on import
}

// ---------------------------------------------------------------------------

TEST(wkt_parse, CONCATENATEDOPERATION) {

auto transf_1 = Transformation::create(
Expand Down Expand Up @@ -14904,6 +14934,87 @@ TEST(json_import, projected_crs) {

// ---------------------------------------------------------------------------

TEST(json_import, conversion_utm_zone_south_wrong_id) {

auto json = "{\n"
" \"type\": \"Conversion\",\n"
" \"name\": \"UTM zone 55S\",\n"
" \"method\": {\n"
" \"name\": \"Transverse Mercator\",\n"
" \"id\": {\n"
" \"authority\": \"EPSG\",\n"
" \"code\": 9807\n"
" }\n"
" },\n"
" \"parameters\": [\n"
" {\n"
" \"name\": \"Latitude of natural origin\",\n"
" \"value\": 0,\n"
" \"unit\": {\n"
" \"type\": \"AngularUnit\",\n"
" \"name\": \"Degree\",\n"
" \"conversion_factor\": 0.0174532925199433\n"
" },\n"
" \"id\": {\n"
" \"authority\": \"EPSG\",\n"
" \"code\": 8801\n"
" }\n"
" },\n"
" {\n"
" \"name\": \"Longitude of natural origin\",\n"
" \"value\": 147,\n"
" \"unit\": {\n"
" \"type\": \"AngularUnit\",\n"
" \"name\": \"Degree\",\n"
" \"conversion_factor\": 0.0174532925199433\n"
" },\n"
" \"id\": {\n"
" \"authority\": \"EPSG\",\n"
" \"code\": 8802\n"
" }\n"
" },\n"
" {\n"
" \"name\": \"Scale factor at natural origin\",\n"
" \"value\": 0.9996,\n"
" \"unit\": \"unity\",\n"
" \"id\": {\n"
" \"authority\": \"EPSG\",\n"
" \"code\": 8805\n"
" }\n"
" },\n"
" {\n"
" \"name\": \"False easting\",\n"
" \"value\": 500000,\n"
" \"unit\": \"metre\",\n"
" \"id\": {\n"
" \"authority\": \"EPSG\",\n"
" \"code\": 8806\n"
" }\n"
" },\n"
" {\n"
" \"name\": \"False northing\",\n"
" \"value\": 10000000,\n"
" \"unit\": \"metre\",\n"
" \"id\": {\n"
" \"authority\": \"EPSG\",\n"
" \"code\": 8807\n"
" }\n"
" }\n"
" ],\n"
" \"id\": {\n"
" \"authority\": \"EPSG\",\n"
" \"code\": 17055\n" // wrong code
" }\n"
"}";

auto obj = createFromUserInput(json, nullptr);
auto conv = nn_dynamic_pointer_cast<Conversion>(obj);
ASSERT_TRUE(conv != nullptr);
EXPECT_EQ(conv->getEPSGCode(), 16155); // code fixed on import
}

// ---------------------------------------------------------------------------

TEST(json_import, projected_crs_with_geocentric_base) {
auto json = "{\n"
" \"$schema\": \"foo\",\n"
Expand Down
2 changes: 1 addition & 1 deletion test/unit/test_operation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,7 @@ TEST(operation, utm_export) {
" PARAMETER[\"False northing\",10000000,\n"
" LENGTHUNIT[\"metre\",1],\n"
" ID[\"EPSG\",8807]],\n"
" ID[\"EPSG\",17001]]");
" ID[\"EPSG\",16101]]");

EXPECT_EQ(
conv->exportToWKT(
Expand Down

0 comments on commit d467ecb

Please sign in to comment.