From fdf5111a9a790926aacec75a07d30508a8ed9c91 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Wed, 16 Mar 2022 00:15:16 +0100 Subject: Fix comparison of GeodeticRefrenceFrame vs DynamicGeodeticReferenceFrame If comparing a DynamicGeodeticReferenceFrame object and its export to WKT1, which is a simple DATUM object, currently in non-strict comparison mode, we'd consider the datum to be equivalent to the dynamic datum, but not the reverse, which breaks the symmetric property of the isEquivalentTo() operation. So fix this, to consider both equivalent whatever the operand order. (in strict mode, the objects will be considered different of course) Spotted in the GDAL GeoTIFF CRS reader code: https://github.com/OSGeo/gdal/blob/f9d48bdcc8c90df20e53b5af5785f1e5d78910db/frmts/gtiff/gt_wkt_srs.cpp#L832 Do same change for vertical datum vs dynamic vertical datum. --- include/proj/datum.hpp | 8 +++++++ src/iso19111/datum.cpp | 61 +++++++++++++++++++++++++++++++++++++++++------- test/unit/test_datum.cpp | 53 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 8 deletions(-) diff --git a/include/proj/datum.hpp b/include/proj/datum.hpp index bf3dbcb7..ad388908 100644 --- a/include/proj/datum.hpp +++ b/include/proj/datum.hpp @@ -432,6 +432,10 @@ class PROJ_GCC_DLL GeodeticReferenceFrame : public Datum { util::IComparable::Criterion criterion = util::IComparable::Criterion::STRICT, const io::DatabaseContextPtr &dbContext = nullptr) const override; + + PROJ_INTERNAL bool isEquivalentToNoExactTypeCheck( + const util::IComparable *other, util::IComparable::Criterion criterion, + const io::DatabaseContextPtr &dbContext) const; //! @endcond protected: @@ -587,6 +591,10 @@ class PROJ_GCC_DLL VerticalReferenceFrame : public Datum { util::IComparable::Criterion::STRICT, const io::DatabaseContextPtr &dbContext = nullptr) const override; + PROJ_INTERNAL bool isEquivalentToNoExactTypeCheck( + const util::IComparable *other, util::IComparable::Criterion criterion, + const io::DatabaseContextPtr &dbContext) const; + PROJ_INTERNAL void _exportToWKT(io::WKTFormatter *formatter) const override; // throw(io::FormattingException) diff --git a/src/iso19111/datum.cpp b/src/iso19111/datum.cpp index 7c76061e..758eaec3 100644 --- a/src/iso19111/datum.cpp +++ b/src/iso19111/datum.cpp @@ -1414,7 +1414,8 @@ void GeodeticReferenceFrame::_exportToJSON( // --------------------------------------------------------------------------- //! @cond Doxygen_Suppress -bool GeodeticReferenceFrame::_isEquivalentTo( + +bool GeodeticReferenceFrame::isEquivalentToNoExactTypeCheck( const util::IComparable *other, util::IComparable::Criterion criterion, const io::DatabaseContextPtr &dbContext) const { auto otherGRF = dynamic_cast(other); @@ -1427,6 +1428,19 @@ bool GeodeticReferenceFrame::_isEquivalentTo( ellipsoid()->_isEquivalentTo(otherGRF->ellipsoid().get(), criterion, dbContext); } + +// --------------------------------------------------------------------------- + +bool GeodeticReferenceFrame::_isEquivalentTo( + const util::IComparable *other, util::IComparable::Criterion criterion, + const io::DatabaseContextPtr &dbContext) const { + if (criterion == Criterion::STRICT && + !util::isOfExactType(*other)) { + return false; + } + return isEquivalentToNoExactTypeCheck(other, criterion, dbContext); +} + //! @endcond // --------------------------------------------------------------------------- @@ -1558,11 +1572,20 @@ DynamicGeodeticReferenceFrame::deformationModelName() const { bool DynamicGeodeticReferenceFrame::_isEquivalentTo( const util::IComparable *other, util::IComparable::Criterion criterion, const io::DatabaseContextPtr &dbContext) const { - auto otherDGRF = dynamic_cast(other); - if (otherDGRF == nullptr || - !GeodeticReferenceFrame::_isEquivalentTo(other, criterion, dbContext)) { + if (criterion == Criterion::STRICT && + !util::isOfExactType(*other)) { return false; } + if (!GeodeticReferenceFrame::isEquivalentToNoExactTypeCheck( + other, criterion, dbContext)) { + return false; + } + auto otherDGRF = dynamic_cast(other); + if (otherDGRF == nullptr) { + // we can go here only if criterion != Criterion::STRICT, and thus + // given the above check we can consider the objects equivalent. + return true; + } return frameReferenceEpoch()._isEquivalentTo( otherDGRF->frameReferenceEpoch(), criterion) && metadata::Identifier::isEquivalentName( @@ -2101,7 +2124,7 @@ void VerticalReferenceFrame::_exportToJSON( // --------------------------------------------------------------------------- //! @cond Doxygen_Suppress -bool VerticalReferenceFrame::_isEquivalentTo( +bool VerticalReferenceFrame::isEquivalentToNoExactTypeCheck( const util::IComparable *other, util::IComparable::Criterion criterion, const io::DatabaseContextPtr &dbContext) const { auto otherVRF = dynamic_cast(other); @@ -2121,6 +2144,19 @@ bool VerticalReferenceFrame::_isEquivalentTo( } return true; } + +// --------------------------------------------------------------------------- + +bool VerticalReferenceFrame::_isEquivalentTo( + const util::IComparable *other, util::IComparable::Criterion criterion, + const io::DatabaseContextPtr &dbContext) const { + if (criterion == Criterion::STRICT && + !util::isOfExactType(*other)) { + return false; + } + return isEquivalentToNoExactTypeCheck(other, criterion, dbContext); +} + //! @endcond // --------------------------------------------------------------------------- @@ -2195,11 +2231,20 @@ DynamicVerticalReferenceFrame::deformationModelName() const { bool DynamicVerticalReferenceFrame::_isEquivalentTo( const util::IComparable *other, util::IComparable::Criterion criterion, const io::DatabaseContextPtr &dbContext) const { - auto otherDGRF = dynamic_cast(other); - if (otherDGRF == nullptr || - !VerticalReferenceFrame::_isEquivalentTo(other, criterion, dbContext)) { + if (criterion == Criterion::STRICT && + !util::isOfExactType(*other)) { return false; } + if (!VerticalReferenceFrame::isEquivalentToNoExactTypeCheck( + other, criterion, dbContext)) { + return false; + } + auto otherDGRF = dynamic_cast(other); + if (otherDGRF == nullptr) { + // we can go here only if criterion != Criterion::STRICT, and thus + // given the above check we can consider the objects equivalent. + return true; + } return frameReferenceEpoch()._isEquivalentTo( otherDGRF->frameReferenceEpoch(), criterion) && metadata::Identifier::isEquivalentName( diff --git a/test/unit/test_datum.cpp b/test/unit/test_datum.cpp index 26098d5c..457bde4d 100644 --- a/test/unit/test_datum.cpp +++ b/test/unit/test_datum.cpp @@ -287,6 +287,33 @@ TEST(datum, dynamic_geodetic_reference_frame) { drf->exportToWKT( WKTFormatter::create(WKTFormatter::Convention::WKT2_2019).get()), expected_wtk2_2019); + + EXPECT_TRUE(drf->isEquivalentTo(drf.get())); + EXPECT_TRUE( + drf->isEquivalentTo(drf.get(), IComparable::Criterion::EQUIVALENT)); + EXPECT_FALSE(drf->isEquivalentTo(createUnrelatedObject().get())); + + // "Same" datum, except that it is a non-dynamic one + auto datum = GeodeticReferenceFrame::create( + PropertyMap().set(IdentifiedObject::NAME_KEY, "test"), Ellipsoid::WGS84, + optional("My anchor"), PrimeMeridian::GREENWICH); + EXPECT_FALSE(datum->isEquivalentTo(drf.get())); + EXPECT_FALSE(drf->isEquivalentTo(datum.get())); + EXPECT_TRUE( + datum->isEquivalentTo(drf.get(), IComparable::Criterion::EQUIVALENT)); + EXPECT_TRUE( + drf->isEquivalentTo(datum.get(), IComparable::Criterion::EQUIVALENT)); + + auto unrelated_datum = GeodeticReferenceFrame::create( + PropertyMap().set(IdentifiedObject::NAME_KEY, "test2"), + Ellipsoid::WGS84, optional("My anchor"), + PrimeMeridian::GREENWICH); + EXPECT_FALSE(unrelated_datum->isEquivalentTo(drf.get())); + EXPECT_FALSE(drf->isEquivalentTo(unrelated_datum.get())); + EXPECT_FALSE(unrelated_datum->isEquivalentTo( + drf.get(), IComparable::Criterion::EQUIVALENT)); + EXPECT_FALSE(drf->isEquivalentTo(unrelated_datum.get(), + IComparable::Criterion::EQUIVALENT)); } // --------------------------------------------------------------------------- @@ -395,6 +422,32 @@ TEST(datum, dynamic_vertical_reference_frame) { drf->exportToWKT( WKTFormatter::create(WKTFormatter::Convention::WKT2_2019).get()), expected_wtk2_2019); + + EXPECT_TRUE(drf->isEquivalentTo(drf.get())); + EXPECT_TRUE( + drf->isEquivalentTo(drf.get(), IComparable::Criterion::EQUIVALENT)); + EXPECT_FALSE(drf->isEquivalentTo(createUnrelatedObject().get())); + + // "Same" datum, except that it is a non-dynamic one + auto datum = VerticalReferenceFrame::create( + PropertyMap().set(IdentifiedObject::NAME_KEY, "test"), + optional("My anchor"), optional()); + EXPECT_FALSE(datum->isEquivalentTo(drf.get())); + EXPECT_FALSE(drf->isEquivalentTo(datum.get())); + EXPECT_TRUE( + datum->isEquivalentTo(drf.get(), IComparable::Criterion::EQUIVALENT)); + EXPECT_TRUE( + drf->isEquivalentTo(datum.get(), IComparable::Criterion::EQUIVALENT)); + + auto unrelated_datum = VerticalReferenceFrame::create( + PropertyMap().set(IdentifiedObject::NAME_KEY, "test2"), + optional("My anchor"), optional()); + EXPECT_FALSE(unrelated_datum->isEquivalentTo(drf.get())); + EXPECT_FALSE(drf->isEquivalentTo(unrelated_datum.get())); + EXPECT_FALSE(unrelated_datum->isEquivalentTo( + drf.get(), IComparable::Criterion::EQUIVALENT)); + EXPECT_FALSE(drf->isEquivalentTo(unrelated_datum.get(), + IComparable::Criterion::EQUIVALENT)); } // --------------------------------------------------------------------------- -- cgit v1.2.3