From e11df09bbc49f72019e4162a697a09f7bb067045 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Thu, 5 Sep 2024 23:14:11 +0200 Subject: [PATCH 1/8] OGRGeometryCollection::operator=(): slightly more efficient approach --- ogr/ogrgeometrycollection.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ogr/ogrgeometrycollection.cpp b/ogr/ogrgeometrycollection.cpp index 5cc636667cfb..26a8d4a1bd6f 100644 --- a/ogr/ogrgeometrycollection.cpp +++ b/ogr/ogrgeometrycollection.cpp @@ -99,9 +99,15 @@ OGRGeometryCollection::operator=(const OGRGeometryCollection &other) OGRGeometry::operator=(other); - for (const auto &poSubGeom : other) + papoGeoms = static_cast( + VSI_CALLOC_VERBOSE(sizeof(OGRGeometry *), other.nGeomCount)); + if (papoGeoms) { - addGeometry(poSubGeom); + nGeomCount = other.nGeomCount; + for (int i = 0; i < other.nGeomCount; i++) + { + papoGeoms[i] = other.papoGeoms[i]->clone(); + } } } return *this; From fc1977bac8fda3be4aff75bbf3abbf76efdcd469 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Thu, 5 Sep 2024 23:37:47 +0200 Subject: [PATCH 2/8] OGRGeometryCollection::operator=(): prevent illegal use --- autotest/cpp/test_ogr.cpp | 21 +++++++++++++++++++++ ogr/ogrgeometrycollection.cpp | 11 +++++++++++ 2 files changed, 32 insertions(+) diff --git a/autotest/cpp/test_ogr.cpp b/autotest/cpp/test_ogr.cpp index 9d1ce22be918..0dc910fdc557 100644 --- a/autotest/cpp/test_ogr.cpp +++ b/autotest/cpp/test_ogr.cpp @@ -313,6 +313,27 @@ TEST_F(test_ogr, SpatialReference_leak_copy_constructor) testCopyEquals(); } +// Test crazy usage of OGRGeometryCollection copy constructor +TEST_F(test_ogr, OGRGeometryCollection_copy_constructor_illegal_use) +{ + OGRGeometryCollection gc; + gc.addGeometryDirectly(new OGRPoint(1, 2)); + + OGRMultiPolygon mp; + mp.addGeometryDirectly(new OGRPolygon()); + + OGRGeometryCollection *mp_as_gc = ∓ + CPLErrorReset(); + { + CPLErrorHandlerPusher oPusher(CPLQuietErrorHandler); + *mp_as_gc = gc; + } + EXPECT_STREQ(CPLGetLastErrorMsg(), + "Illegal use of OGRGeometryCollection::operator=(): trying to " + "assign an incomptible sub-geometry"); + EXPECT_TRUE(mp.IsEmpty()); +} + TEST_F(test_ogr, geometry_get_point) { { diff --git a/ogr/ogrgeometrycollection.cpp b/ogr/ogrgeometrycollection.cpp index 26a8d4a1bd6f..e30ee062d321 100644 --- a/ogr/ogrgeometrycollection.cpp +++ b/ogr/ogrgeometrycollection.cpp @@ -99,6 +99,17 @@ OGRGeometryCollection::operator=(const OGRGeometryCollection &other) OGRGeometry::operator=(other); + for (const auto *poOtherSubGeom : other) + { + if (!isCompatibleSubType(poOtherSubGeom->getGeometryType())) + { + CPLError(CE_Failure, CPLE_AppDefined, + "Illegal use of OGRGeometryCollection::operator=(): " + "trying to assign an incomptible sub-geometry"); + return *this; + } + } + papoGeoms = static_cast( VSI_CALLOC_VERBOSE(sizeof(OGRGeometry *), other.nGeomCount)); if (papoGeoms) From 9ac12687960652d2d2ae49f0d583d5209c2ad0f8 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Thu, 5 Sep 2024 23:51:43 +0200 Subject: [PATCH 3/8] OGRCurvePolygon::operator=(): prevent illegal use --- autotest/cpp/test_ogr.cpp | 33 +++++++++++++++++++++++++++++++++ ogr/ogr_geometry.h | 6 ++++-- ogr/ogrcurvepolygon.cpp | 25 ++++++++++++++++++------- ogr/ogrpolygon.cpp | 10 +++++----- 4 files changed, 60 insertions(+), 14 deletions(-) diff --git a/autotest/cpp/test_ogr.cpp b/autotest/cpp/test_ogr.cpp index 0dc910fdc557..350a25c77256 100644 --- a/autotest/cpp/test_ogr.cpp +++ b/autotest/cpp/test_ogr.cpp @@ -334,6 +334,39 @@ TEST_F(test_ogr, OGRGeometryCollection_copy_constructor_illegal_use) EXPECT_TRUE(mp.IsEmpty()); } +// Test crazy usage of OGRCurvePolygon copy constructor +TEST_F(test_ogr, OGRCurvePolygon_copy_constructor_illegal_use) +{ + OGRCurvePolygon cp; + auto poCC = new OGRCircularString(); + poCC->addPoint(0, 0); + poCC->addPoint(1, 1); + poCC->addPoint(2, 0); + poCC->addPoint(1, -1); + poCC->addPoint(0, 0); + cp.addRingDirectly(poCC); + + OGRPolygon poly; + auto poLR = new OGRLinearRing(); + poLR->addPoint(0, 0); + poLR->addPoint(1, 1); + poLR->addPoint(2, 0); + poLR->addPoint(1, -1); + poLR->addPoint(0, 0); + poly.addRingDirectly(poLR); + + OGRCurvePolygon *poly_as_cp = &poly; + CPLErrorReset(); + { + CPLErrorHandlerPusher oPusher(CPLQuietErrorHandler); + *poly_as_cp = cp; + } + EXPECT_STREQ(CPLGetLastErrorMsg(), + "Illegal use of OGRCurvePolygon::operator=(): trying to " + "assign an incomptible sub-geometry"); + EXPECT_TRUE(poly.IsEmpty()); +} + TEST_F(test_ogr, geometry_get_point) { { diff --git a/ogr/ogr_geometry.h b/ogr/ogr_geometry.h index 7a1e0b8d57d5..4fe4b8cea976 100644 --- a/ogr/ogr_geometry.h +++ b/ogr/ogr_geometry.h @@ -2446,7 +2446,8 @@ class CPL_DLL OGRCurvePolygon : public OGRSurface private: OGRBoolean IntersectsPoint(const OGRPoint *p) const; OGRBoolean ContainsPoint(const OGRPoint *p) const; - virtual int checkRing(OGRCurve *poNewRing) const; + virtual bool checkRing(const OGRCurve *poNewRing, + bool bOnlyType = false) const; OGRErr addRingDirectlyInternal(OGRCurve *poCurve, int bNeedRealloc); static OGRErr addCurveDirectlyFromWkt(OGRGeometry *poSelf, OGRCurve *poCurve); @@ -2661,7 +2662,8 @@ class CPL_DLL OGRPolygon : public OGRCurvePolygon friend class OGRPolyhedralSurface; friend class OGRTriangulatedSurface; - virtual int checkRing(OGRCurve *poNewRing) const override; + virtual bool checkRing(const OGRCurve *poNewRing, + bool bOnlyType = false) const override; virtual OGRErr importFromWKTListOnly(const char **ppszInput, int bHasZ, int bHasM, OGRRawPoint *&paoPoints, int &nMaxPoints, double *&padfZ); diff --git a/ogr/ogrcurvepolygon.cpp b/ogr/ogrcurvepolygon.cpp index 39e182b6cdab..3e9ebc6d8425 100644 --- a/ogr/ogrcurvepolygon.cpp +++ b/ogr/ogrcurvepolygon.cpp @@ -73,6 +73,17 @@ OGRCurvePolygon &OGRCurvePolygon::operator=(const OGRCurvePolygon &other) { OGRSurface::operator=(other); + for (const auto *poRing : other.oCC) + { + if (!checkRing(poRing, /* bOnlyType = */ true)) + { + CPLError(CE_Failure, CPLE_AppDefined, + "Illegal use of OGRCurvePolygon::operator=(): " + "trying to assign an incomptible sub-geometry"); + return *this; + } + } + oCC = other.oCC; } return *this; @@ -340,9 +351,9 @@ OGRErr OGRCurvePolygon::addRing(const OGRCurve *poNewRing) /* checkRing() */ /************************************************************************/ -int OGRCurvePolygon::checkRing(OGRCurve *poNewRing) const +bool OGRCurvePolygon::checkRing(const OGRCurve *poNewRing, bool bOnlyType) const { - if (!poNewRing->IsEmpty() && !poNewRing->get_IsClosed()) + if (!bOnlyType && !poNewRing->IsEmpty() && !poNewRing->get_IsClosed()) { // This configuration option name must be the same as in // OGRPolygon::checkRing() @@ -351,7 +362,7 @@ int OGRCurvePolygon::checkRing(OGRCurve *poNewRing) const if (pszEnvVar != nullptr && !CPLTestBool(pszEnvVar)) { CPLError(CE_Failure, CPLE_AppDefined, "Non closed ring detected."); - return FALSE; + return false; } else { @@ -366,19 +377,19 @@ int OGRCurvePolygon::checkRing(OGRCurve *poNewRing) const if (wkbFlatten(poNewRing->getGeometryType()) == wkbLineString) { - if (poNewRing->getNumPoints() < 4) + if (!bOnlyType && poNewRing->getNumPoints() < 4) { - return FALSE; + return false; } if (EQUAL(poNewRing->getGeometryName(), "LINEARRING")) { CPLError(CE_Failure, CPLE_AppDefined, "Linearring not allowed."); - return FALSE; + return false; } } - return TRUE; + return true; } /************************************************************************/ diff --git a/ogr/ogrpolygon.cpp b/ogr/ogrpolygon.cpp index b0ac26ee7b13..c1663696c7cc 100644 --- a/ogr/ogrpolygon.cpp +++ b/ogr/ogrpolygon.cpp @@ -267,17 +267,17 @@ OGRLinearRing *OGRPolygon::stealInteriorRing(int iRing) /* checkRing() */ /************************************************************************/ -int OGRPolygon::checkRing(OGRCurve *poNewRing) const +bool OGRPolygon::checkRing(const OGRCurve *poNewRing, bool bOnlyType) const { if (poNewRing == nullptr || !(EQUAL(poNewRing->getGeometryName(), "LINEARRING"))) { CPLError(CE_Failure, CPLE_AppDefined, "Wrong curve type. Expected LINEARRING."); - return FALSE; + return false; } - if (!poNewRing->IsEmpty() && !poNewRing->get_IsClosed()) + if (!bOnlyType && !poNewRing->IsEmpty() && !poNewRing->get_IsClosed()) { // This configuration option name must be the same as in // OGRCurvePolygon::checkRing() @@ -286,7 +286,7 @@ int OGRPolygon::checkRing(OGRCurve *poNewRing) const if (pszEnvVar != nullptr && !CPLTestBool(pszEnvVar)) { CPLError(CE_Failure, CPLE_AppDefined, "Non closed ring detected."); - return FALSE; + return false; } else { @@ -299,7 +299,7 @@ int OGRPolygon::checkRing(OGRCurve *poNewRing) const } } - return TRUE; + return true; } /*! @endcond */ From 4767cca82cd9a774172b74a08b7f3c90e0d72de2 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Thu, 5 Sep 2024 20:13:52 +0200 Subject: [PATCH 4/8] OGRTriangulatedSurface::operator=(): simplify by making base operator to call empty() --- autotest/cpp/test_ogr.cpp | 5 +++-- ogr/ogrgeometry.cpp | 1 + ogr/ogrgeometrycollection.cpp | 2 -- ogr/ogrpoint.cpp | 5 ++++- ogr/ogrtriangulatedsurface.cpp | 8 ++------ 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/autotest/cpp/test_ogr.cpp b/autotest/cpp/test_ogr.cpp index 350a25c77256..bf9b0ca9eb67 100644 --- a/autotest/cpp/test_ogr.cpp +++ b/autotest/cpp/test_ogr.cpp @@ -262,7 +262,7 @@ template <> OGRPolyhedralSurface *make() template void testCopyEquals() { - T *poOrigin = make(); + auto poOrigin = std::unique_ptr(make()); ASSERT_TRUE(nullptr != poOrigin); T value2(*poOrigin); @@ -288,7 +288,8 @@ template void testCopyEquals() << poOrigin->getGeometryName() << ": assignment operator changed a value"; - OGRGeometryFactory::destroyGeometry(poOrigin); + value3 = T(); + ASSERT_TRUE(value3.IsEmpty()); } // Test if copy constructor and assignment operators succeeds on copying the diff --git a/ogr/ogrgeometry.cpp b/ogr/ogrgeometry.cpp index 988ae800ef9a..e18e1b467526 100644 --- a/ogr/ogrgeometry.cpp +++ b/ogr/ogrgeometry.cpp @@ -137,6 +137,7 @@ OGRGeometry &OGRGeometry::operator=(const OGRGeometry &other) { if (this != &other) { + empty(); assignSpatialReference(other.getSpatialReference()); flags = other.flags; } diff --git a/ogr/ogrgeometrycollection.cpp b/ogr/ogrgeometrycollection.cpp index e30ee062d321..04ad4ad5b247 100644 --- a/ogr/ogrgeometrycollection.cpp +++ b/ogr/ogrgeometrycollection.cpp @@ -95,8 +95,6 @@ OGRGeometryCollection::operator=(const OGRGeometryCollection &other) { if (this != &other) { - empty(); - OGRGeometry::operator=(other); for (const auto *poOtherSubGeom : other) diff --git a/ogr/ogrpoint.cpp b/ogr/ogrpoint.cpp index 7a6a58f3d83a..59ce7fa39949 100644 --- a/ogr/ogrpoint.cpp +++ b/ogr/ogrpoint.cpp @@ -158,7 +158,10 @@ OGRPoint &OGRPoint::operator=(const OGRPoint &other) { if (this != &other) { - OGRGeometry::operator=(other); + // Slightly more efficient to avoid OGRGeometry::operator=(other); + // but do what it does to avoid a call to empty() + assignSpatialReference(other.getSpatialReference()); + flags = other.flags; x = other.x; y = other.y; diff --git a/ogr/ogrtriangulatedsurface.cpp b/ogr/ogrtriangulatedsurface.cpp index 76c17585bab8..349d2265e229 100644 --- a/ogr/ogrtriangulatedsurface.cpp +++ b/ogr/ogrtriangulatedsurface.cpp @@ -72,13 +72,9 @@ OGRTriangulatedSurface::operator=(const OGRTriangulatedSurface &other) // of OGRPolyhedralSurface since it will be confused by a multipolygon // of triangles. OGRSurface::operator=(other); - empty(); - set3D(other.Is3D()); - setMeasured(other.IsMeasured()); - assignSpatialReference(other.getSpatialReference()); - for (int i = 0; i < other.oMP.nGeomCount; i++) + for (const auto *poPoly : other.oMP) { - OGRTriangulatedSurface::addGeometry(other.oMP.getGeometryRef(i)); + OGRTriangulatedSurface::addGeometry(poPoly); } } return *this; From 3f6ac094c8643b40e722e9630e8793d148e3b364 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Thu, 5 Sep 2024 22:40:09 +0200 Subject: [PATCH 5/8] Doxyfile: deal with CPL_NON_FINAL --- Doxyfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Doxyfile b/Doxyfile index 1fd627a3489d..b22e38ec20c7 100644 --- a/Doxyfile +++ b/Doxyfile @@ -943,7 +943,8 @@ PREDEFINED = HAVE_DLFCN_H \ CPLSTRING_METHOD_DLL= \ CPL_NO_RETURN= \ EXPERIMENTAL_CPL_WARN_UNUSED_RESULT= \ - CPL_MULTIPROC_PTHREAD + CPL_MULTIPROC_PTHREAD \ + CPL_NON_FINAL= # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then # this tag can be used to specify a list of macro names that should be expanded. From 0d737dd79967de41a5c070694a916aa7546c8cbc Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Thu, 5 Sep 2024 22:30:13 +0200 Subject: [PATCH 6/8] OGR geometry classes: mark default constructors directly in .h and removed useless overridden destructor for better code generation --- ogr/ogr_geometry.h | 96 +++++++++++++++++++--------------- ogr/ogrcircularstring.cpp | 16 ------ ogr/ogrcompoundcurve.cpp | 16 ------ ogr/ogrcurve.cpp | 18 ------- ogr/ogrcurvecollection.cpp | 6 --- ogr/ogrcurvepolygon.cpp | 16 ------ ogr/ogrgeometrycollection.cpp | 10 ---- ogr/ogrlinearring.cpp | 15 +----- ogr/ogrlinestring.cpp | 26 --------- ogr/ogrmulticurve.cpp | 16 ------ ogr/ogrmultilinestring.cpp | 16 ------ ogr/ogrmultipoint.cpp | 16 ------ ogr/ogrmultipolygon.cpp | 16 ------ ogr/ogrmultisurface.cpp | 16 ------ ogr/ogrpoint.cpp | 6 --- ogr/ogrpolygon.cpp | 16 ------ ogr/ogrpolyhedralsurface.cpp | 21 -------- ogr/ogrtriangle.cpp | 22 -------- ogr/ogrtriangulatedsurface.cpp | 22 -------- 19 files changed, 56 insertions(+), 330 deletions(-) diff --git a/ogr/ogr_geometry.h b/ogr/ogr_geometry.h index 4fe4b8cea976..dd6709ef7bd9 100644 --- a/ogr/ogr_geometry.h +++ b/ogr/ogr_geometry.h @@ -1145,7 +1145,6 @@ class CPL_DLL OGRPoint : public OGRGeometry OGRPoint(double x, double y, double z, double m); OGRPoint(const OGRPoint &other); static OGRPoint *createXYM(double x, double y, double m); - ~OGRPoint() override; OGRPoint &operator=(const OGRPoint &other); @@ -1316,8 +1315,8 @@ class CPL_DLL OGRCurve : public OGRGeometry { protected: //! @cond Doxygen_Suppress - OGRCurve(); - OGRCurve(const OGRCurve &other); + OGRCurve() = default; + OGRCurve(const OGRCurve &other) = default; virtual OGRCurveCasterToLineString GetCasterToLineString() const = 0; virtual OGRCurveCasterToLinearRing GetCasterToLinearRing() const = 0; @@ -1349,8 +1348,6 @@ class CPL_DLL OGRCurve : public OGRGeometry friend inline ConstIterator end(const OGRCurve *); public: - ~OGRCurve() override; - //! @cond Doxygen_Suppress OGRCurve &operator=(const OGRCurve &other); //! @endcond @@ -1512,11 +1509,11 @@ class CPL_DLL OGRSimpleCurve : public OGRCurve //! @cond Doxygen_Suppress friend class OGRGeometry; - int nPointCount; + int nPointCount = 0; int m_nPointCapacity = 0; - OGRRawPoint *paoPoints; - double *padfZ; - double *padfM; + OGRRawPoint *paoPoints = nullptr; + double *padfZ = nullptr; + double *padfM = nullptr; bool Make3D(); void Make2D(); @@ -1530,7 +1527,9 @@ class CPL_DLL OGRSimpleCurve : public OGRCurve virtual double get_LinearArea() const; - OGRSimpleCurve(); + /** Constructor */ + OGRSimpleCurve() = default; + OGRSimpleCurve(const OGRSimpleCurve &other); private: @@ -1774,9 +1773,9 @@ class CPL_DLL OGRLineString : public OGRSimpleCurve static OGRLinearRing *CastToLinearRing(OGRLineString *poLS); public: - OGRLineString(); + /** Create an empty line string. */ + OGRLineString() = default; OGRLineString(const OGRLineString &other); - ~OGRLineString() override; OGRLineString &operator=(const OGRLineString &other); @@ -1880,10 +1879,10 @@ class CPL_DLL OGRLinearRing : public OGRLineString static OGRLineString *CastToLineString(OGRLinearRing *poLR); public: - OGRLinearRing(); + /** Constructor */ + OGRLinearRing() = default; OGRLinearRing(const OGRLinearRing &other); - explicit OGRLinearRing(OGRLinearRing *); - ~OGRLinearRing() override; + explicit OGRLinearRing(const OGRLinearRing *); OGRLinearRing &operator=(const OGRLinearRing &other); @@ -1962,9 +1961,10 @@ class CPL_DLL OGRCircularString : public OGRSimpleCurve //! @endcond public: - OGRCircularString(); + /** Create an empty circular string. */ + OGRCircularString() = default; + OGRCircularString(const OGRCircularString &other); - ~OGRCircularString() override; OGRCircularString &operator=(const OGRCircularString &other); @@ -2072,7 +2072,7 @@ class CPL_DLL OGRCurveCollection OGRCurve **papoCurves = nullptr; public: - OGRCurveCollection(); + OGRCurveCollection() = default; OGRCurveCollection(const OGRCurveCollection &other); ~OGRCurveCollection(); @@ -2203,9 +2203,10 @@ class CPL_DLL OGRCompoundCurve : public OGRCurve //! @endcond public: - OGRCompoundCurve(); + /** Create an empty compound curve. */ + OGRCompoundCurve() = default; + OGRCompoundCurve(const OGRCompoundCurve &other); - ~OGRCompoundCurve() override; OGRCompoundCurve &operator=(const OGRCompoundCurve &other); @@ -2469,9 +2470,10 @@ class CPL_DLL OGRCurvePolygon : public OGRSurface static OGRPolygon *CastToPolygon(OGRCurvePolygon *poCP); public: - OGRCurvePolygon(); + /** Create an empty curve polygon. */ + OGRCurvePolygon() = default; + OGRCurvePolygon(const OGRCurvePolygon &); - ~OGRCurvePolygon() override; OGRCurvePolygon &operator=(const OGRCurvePolygon &other); @@ -2676,9 +2678,10 @@ class CPL_DLL OGRPolygon : public OGRCurvePolygon //! @endcond public: - OGRPolygon(); + /** Create an empty polygon. */ + OGRPolygon() = default; + OGRPolygon(const OGRPolygon &other); - ~OGRPolygon() override; OGRPolygon &operator=(const OGRPolygon &other); @@ -2846,12 +2849,13 @@ class CPL_DLL OGRTriangle : public OGRPolygon //! @endcond public: - OGRTriangle(); + /** Constructor. */ + OGRTriangle() = default; OGRTriangle(const OGRPoint &p, const OGRPoint &q, const OGRPoint &r); OGRTriangle(const OGRTriangle &other); OGRTriangle(const OGRPolygon &other, OGRErr &eErr); OGRTriangle &operator=(const OGRTriangle &other); - ~OGRTriangle() override; + virtual const char *getGeometryName() const override; virtual OGRwkbGeometryType getGeometryType() const override; virtual OGRTriangle *clone() const override; @@ -2927,7 +2931,9 @@ class CPL_DLL OGRGeometryCollection : public OGRGeometry virtual OGRBoolean isCompatibleSubType(OGRwkbGeometryType) const; public: - OGRGeometryCollection(); + /** Create an empty geometry collection. */ + OGRGeometryCollection() = default; + OGRGeometryCollection(const OGRGeometryCollection &other); ~OGRGeometryCollection() override; @@ -3109,9 +3115,10 @@ class CPL_DLL OGRMultiSurface : public OGRGeometryCollection virtual OGRBoolean isCompatibleSubType(OGRwkbGeometryType) const override; public: - OGRMultiSurface(); + /** Create an empty multi surface collection. */ + OGRMultiSurface() = default; + OGRMultiSurface(const OGRMultiSurface &other); - ~OGRMultiSurface() override; OGRMultiSurface &operator=(const OGRMultiSurface &other); @@ -3276,9 +3283,10 @@ class CPL_DLL OGRMultiPolygon : public OGRMultiSurface //! @endcond public: - OGRMultiPolygon(); + /** Create an empty multi polygon collection. */ + OGRMultiPolygon() = default; + OGRMultiPolygon(const OGRMultiPolygon &other); - ~OGRMultiPolygon() override; OGRMultiPolygon &operator=(const OGRMultiPolygon &other); @@ -3438,9 +3446,11 @@ class CPL_DLL OGRPolyhedralSurface : public OGRSurface //! @endcond public: - OGRPolyhedralSurface(); + /** Create an empty PolyhedralSurface */ + OGRPolyhedralSurface() = default; + OGRPolyhedralSurface(const OGRPolyhedralSurface &poGeom); - ~OGRPolyhedralSurface() override; + OGRPolyhedralSurface &operator=(const OGRPolyhedralSurface &other); /** Type of child elements. */ @@ -3612,9 +3622,10 @@ class CPL_DLL OGRTriangulatedSurface : public OGRPolyhedralSurface //! @endcond public: - OGRTriangulatedSurface(); + /** Constructor */ + OGRTriangulatedSurface() = default; + OGRTriangulatedSurface(const OGRTriangulatedSurface &other); - ~OGRTriangulatedSurface(); /** Type of child elements. */ typedef OGRTriangle ChildType; @@ -3746,9 +3757,10 @@ class CPL_DLL OGRMultiPoint : public OGRGeometryCollection virtual OGRBoolean isCompatibleSubType(OGRwkbGeometryType) const override; public: - OGRMultiPoint(); + /** Create an empty multi point collection. */ + OGRMultiPoint() = default; + OGRMultiPoint(const OGRMultiPoint &other); - ~OGRMultiPoint() override; OGRMultiPoint &operator=(const OGRMultiPoint &other); @@ -3903,9 +3915,10 @@ class CPL_DLL OGRMultiCurve : public OGRGeometryCollection virtual OGRBoolean isCompatibleSubType(OGRwkbGeometryType) const override; public: - OGRMultiCurve(); + /** Create an empty multi curve collection. */ + OGRMultiCurve() = default; + OGRMultiCurve(const OGRMultiCurve &other); - ~OGRMultiCurve() override; OGRMultiCurve &operator=(const OGRMultiCurve &other); @@ -4055,9 +4068,10 @@ class CPL_DLL OGRMultiLineString : public OGRMultiCurve virtual OGRBoolean isCompatibleSubType(OGRwkbGeometryType) const override; public: - OGRMultiLineString(); + /** Create an empty multi line string collection. */ + OGRMultiLineString() = default; + OGRMultiLineString(const OGRMultiLineString &other); - ~OGRMultiLineString() override; OGRMultiLineString &operator=(const OGRMultiLineString &other); diff --git a/ogr/ogrcircularstring.cpp b/ogr/ogrcircularstring.cpp index 49eff5b31039..2df237fd4d66 100644 --- a/ogr/ogrcircularstring.cpp +++ b/ogr/ogrcircularstring.cpp @@ -31,16 +31,6 @@ static inline double dist(double x0, double y0, double x1, double y1) return std::sqrt((x1 - x0) * (x1 - x0) + (y1 - y0) * (y1 - y0)); } -/************************************************************************/ -/* OGRCircularString() */ -/************************************************************************/ - -/** - * \brief Create an empty circular string. - */ - -OGRCircularString::OGRCircularString() = default; - /************************************************************************/ /* OGRCircularString( const OGRCircularString& ) */ /************************************************************************/ @@ -56,12 +46,6 @@ OGRCircularString::OGRCircularString() = default; OGRCircularString::OGRCircularString(const OGRCircularString &) = default; -/************************************************************************/ -/* ~OGRCircularString() */ -/************************************************************************/ - -OGRCircularString::~OGRCircularString() = default; - /************************************************************************/ /* operator=( const OGRCircularString& ) */ /************************************************************************/ diff --git a/ogr/ogrcompoundcurve.cpp b/ogr/ogrcompoundcurve.cpp index 16cdc06447e7..54f0fc0c74db 100644 --- a/ogr/ogrcompoundcurve.cpp +++ b/ogr/ogrcompoundcurve.cpp @@ -22,16 +22,6 @@ #include "ogr_p.h" #include "ogr_spatialref.h" -/************************************************************************/ -/* OGRCompoundCurve() */ -/************************************************************************/ - -/** - * \brief Create an empty compound curve. - */ - -OGRCompoundCurve::OGRCompoundCurve() = default; - /************************************************************************/ /* OGRCompoundCurve( const OGRCompoundCurve& ) */ /************************************************************************/ @@ -47,12 +37,6 @@ OGRCompoundCurve::OGRCompoundCurve() = default; OGRCompoundCurve::OGRCompoundCurve(const OGRCompoundCurve &) = default; -/************************************************************************/ -/* ~OGRCompoundCurve() */ -/************************************************************************/ - -OGRCompoundCurve::~OGRCompoundCurve() = default; - /************************************************************************/ /* operator=( const OGRCompoundCurve&) */ /************************************************************************/ diff --git a/ogr/ogrcurve.cpp b/ogr/ogrcurve.cpp index 2bb1d871b6c3..4e8b7d77f255 100644 --- a/ogr/ogrcurve.cpp +++ b/ogr/ogrcurve.cpp @@ -15,24 +15,6 @@ //! @cond Doxygen_Suppress -/************************************************************************/ -/* OGRCurve() */ -/************************************************************************/ - -OGRCurve::OGRCurve() = default; - -/************************************************************************/ -/* ~OGRCurve() */ -/************************************************************************/ - -OGRCurve::~OGRCurve() = default; - -/************************************************************************/ -/* OGRCurve( const OGRCurve& ) */ -/************************************************************************/ - -OGRCurve::OGRCurve(const OGRCurve &) = default; - /************************************************************************/ /* operator=( const OGRCurve& ) */ /************************************************************************/ diff --git a/ogr/ogrcurvecollection.cpp b/ogr/ogrcurvecollection.cpp index 0a5105561ba0..83f3ae2e063a 100644 --- a/ogr/ogrcurvecollection.cpp +++ b/ogr/ogrcurvecollection.cpp @@ -28,12 +28,6 @@ //! @cond Doxygen_Suppress -/************************************************************************/ -/* OGRCurveCollection() */ -/************************************************************************/ - -OGRCurveCollection::OGRCurveCollection() = default; - /************************************************************************/ /* OGRCurveCollection( const OGRCurveCollection& ) */ /************************************************************************/ diff --git a/ogr/ogrcurvepolygon.cpp b/ogr/ogrcurvepolygon.cpp index 3e9ebc6d8425..49795945ac43 100644 --- a/ogr/ogrcurvepolygon.cpp +++ b/ogr/ogrcurvepolygon.cpp @@ -23,16 +23,6 @@ #include "ogr_p.h" #include "ogr_spatialref.h" -/************************************************************************/ -/* OGRCurvePolygon() */ -/************************************************************************/ - -/** - * \brief Create an empty curve polygon. - */ - -OGRCurvePolygon::OGRCurvePolygon() = default; - /************************************************************************/ /* OGRCurvePolygon( const OGRCurvePolygon& ) */ /************************************************************************/ @@ -48,12 +38,6 @@ OGRCurvePolygon::OGRCurvePolygon() = default; OGRCurvePolygon::OGRCurvePolygon(const OGRCurvePolygon &) = default; -/************************************************************************/ -/* ~OGRCurvePolygon() */ -/************************************************************************/ - -OGRCurvePolygon::~OGRCurvePolygon() = default; - /************************************************************************/ /* operator=( const OGRCurvePolygon&) */ /************************************************************************/ diff --git a/ogr/ogrgeometrycollection.cpp b/ogr/ogrgeometrycollection.cpp index 04ad4ad5b247..d64f68766ef2 100644 --- a/ogr/ogrgeometrycollection.cpp +++ b/ogr/ogrgeometrycollection.cpp @@ -28,16 +28,6 @@ #include "ogr_p.h" #include "ogr_spatialref.h" -/************************************************************************/ -/* OGRGeometryCollection() */ -/************************************************************************/ - -/** - * \brief Create an empty geometry collection. - */ - -OGRGeometryCollection::OGRGeometryCollection() = default; - /************************************************************************/ /* OGRGeometryCollection( const OGRGeometryCollection& ) */ /************************************************************************/ diff --git a/ogr/ogrlinearring.cpp b/ogr/ogrlinearring.cpp index 968e9ae5bb36..0a0dffcd6e8e 100644 --- a/ogr/ogrlinearring.cpp +++ b/ogr/ogrlinearring.cpp @@ -24,13 +24,6 @@ #include "ogr_geometry.h" #include "ogr_p.h" -/************************************************************************/ -/* OGRLinearRing() */ -/************************************************************************/ - -/** Constructor */ -OGRLinearRing::OGRLinearRing() = default; - /************************************************************************/ /* OGRLinearRing( const OGRLinearRing& ) */ /************************************************************************/ @@ -46,12 +39,6 @@ OGRLinearRing::OGRLinearRing() = default; OGRLinearRing::OGRLinearRing(const OGRLinearRing &) = default; -/************************************************************************/ -/* ~OGRLinearRing() */ -/************************************************************************/ - -OGRLinearRing::~OGRLinearRing() = default; - /************************************************************************/ /* OGRLinearRing() */ /************************************************************************/ @@ -59,7 +46,7 @@ OGRLinearRing::~OGRLinearRing() = default; /** Constructor * @param poSrcRing source ring. */ -OGRLinearRing::OGRLinearRing(OGRLinearRing *poSrcRing) +OGRLinearRing::OGRLinearRing(const OGRLinearRing *poSrcRing) { if (poSrcRing == nullptr) diff --git a/ogr/ogrlinestring.cpp b/ogr/ogrlinestring.cpp index 708b0e3d1fee..b48431a395cf 100644 --- a/ogr/ogrlinestring.cpp +++ b/ogr/ogrlinestring.cpp @@ -39,16 +39,6 @@ int DoubleToIntClamp(double dfValue) } // namespace -/************************************************************************/ -/* OGRSimpleCurve() */ -/************************************************************************/ - -/** Constructor */ -OGRSimpleCurve::OGRSimpleCurve() - : nPointCount(0), paoPoints(nullptr), padfZ(nullptr), padfM(nullptr) -{ -} - /************************************************************************/ /* OGRSimpleCurve( const OGRSimpleCurve& ) */ /************************************************************************/ @@ -2796,16 +2786,6 @@ OGRPointIterator *OGRSimpleCurve::getPointIterator() const return new OGRSimpleCurvePointIterator(this); } -/************************************************************************/ -/* OGRLineString() */ -/************************************************************************/ - -/** - * \brief Create an empty line string. - */ - -OGRLineString::OGRLineString() = default; - /************************************************************************/ /* OGRLineString( const OGRLineString& ) */ /************************************************************************/ @@ -2821,12 +2801,6 @@ OGRLineString::OGRLineString() = default; OGRLineString::OGRLineString(const OGRLineString &) = default; -/************************************************************************/ -/* ~OGRLineString() */ -/************************************************************************/ - -OGRLineString::~OGRLineString() = default; - /************************************************************************/ /* operator=( const OGRLineString& ) */ /************************************************************************/ diff --git a/ogr/ogrmulticurve.cpp b/ogr/ogrmulticurve.cpp index 4f298dd37a1b..383a8d4f6686 100644 --- a/ogr/ogrmulticurve.cpp +++ b/ogr/ogrmulticurve.cpp @@ -20,16 +20,6 @@ #include "ogr_core.h" #include "ogr_p.h" -/************************************************************************/ -/* OGRMultiCurve() */ -/************************************************************************/ - -/** - * \brief Create an empty multi curve collection. - */ - -OGRMultiCurve::OGRMultiCurve() = default; - /************************************************************************/ /* OGRMultiCurve( const OGRMultiCurve& ) */ /************************************************************************/ @@ -45,12 +35,6 @@ OGRMultiCurve::OGRMultiCurve() = default; OGRMultiCurve::OGRMultiCurve(const OGRMultiCurve &) = default; -/************************************************************************/ -/* ~OGRMultiCurve() */ -/************************************************************************/ - -OGRMultiCurve::~OGRMultiCurve() = default; - /************************************************************************/ /* operator=( const OGRMultiCurve&) */ /************************************************************************/ diff --git a/ogr/ogrmultilinestring.cpp b/ogr/ogrmultilinestring.cpp index 1e5837648bbe..2961e4dc67c1 100644 --- a/ogr/ogrmultilinestring.cpp +++ b/ogr/ogrmultilinestring.cpp @@ -20,16 +20,6 @@ #include "ogr_core.h" #include "ogr_p.h" -/************************************************************************/ -/* OGRMultiLineString() */ -/************************************************************************/ - -/** - * \brief Create an empty multi line string collection. - */ - -OGRMultiLineString::OGRMultiLineString() = default; - /************************************************************************/ /* OGRMultiLineString( const OGRMultiLineString& ) */ /************************************************************************/ @@ -45,12 +35,6 @@ OGRMultiLineString::OGRMultiLineString() = default; OGRMultiLineString::OGRMultiLineString(const OGRMultiLineString &) = default; -/************************************************************************/ -/* ~OGRMultiLineString() */ -/************************************************************************/ - -OGRMultiLineString::~OGRMultiLineString() = default; - /************************************************************************/ /* operator=( const OGRMultiCurve&) */ /************************************************************************/ diff --git a/ogr/ogrmultipoint.cpp b/ogr/ogrmultipoint.cpp index 1763d1293d80..d1f9d6cb6639 100644 --- a/ogr/ogrmultipoint.cpp +++ b/ogr/ogrmultipoint.cpp @@ -25,16 +25,6 @@ #include "ogr_core.h" #include "ogr_p.h" -/************************************************************************/ -/* OGRMultiPoint() */ -/************************************************************************/ - -/** - * \brief Create an empty multi point collection. - */ - -OGRMultiPoint::OGRMultiPoint() = default; - /************************************************************************/ /* OGRMultiPoint( const OGRMultiPoint& ) */ /************************************************************************/ @@ -50,12 +40,6 @@ OGRMultiPoint::OGRMultiPoint() = default; OGRMultiPoint::OGRMultiPoint(const OGRMultiPoint &) = default; -/************************************************************************/ -/* ~OGRMultiPoint() */ -/************************************************************************/ - -OGRMultiPoint::~OGRMultiPoint() = default; - /************************************************************************/ /* operator=( const OGRMultiPoint&) */ /************************************************************************/ diff --git a/ogr/ogrmultipolygon.cpp b/ogr/ogrmultipolygon.cpp index d31aafdc9daa..d22d27e277a0 100644 --- a/ogr/ogrmultipolygon.cpp +++ b/ogr/ogrmultipolygon.cpp @@ -18,16 +18,6 @@ #include "ogr_core.h" #include "ogr_p.h" -/************************************************************************/ -/* OGRMultiPolygon() */ -/************************************************************************/ - -/** - * \brief Create an empty multi polygon collection. - */ - -OGRMultiPolygon::OGRMultiPolygon() = default; - /************************************************************************/ /* OGRMultiPolygon( const OGRMultiPolygon& ) */ /************************************************************************/ @@ -43,12 +33,6 @@ OGRMultiPolygon::OGRMultiPolygon() = default; OGRMultiPolygon::OGRMultiPolygon(const OGRMultiPolygon &) = default; -/************************************************************************/ -/* ~OGRMultiPolygon() */ -/************************************************************************/ - -OGRMultiPolygon::~OGRMultiPolygon() = default; - /************************************************************************/ /* operator=( const OGRMultiPolygon&) */ /************************************************************************/ diff --git a/ogr/ogrmultisurface.cpp b/ogr/ogrmultisurface.cpp index 4803acdf135c..6502d41157b4 100644 --- a/ogr/ogrmultisurface.cpp +++ b/ogr/ogrmultisurface.cpp @@ -21,22 +21,6 @@ #include "ogr_core.h" #include "ogr_p.h" -/************************************************************************/ -/* OGRMultiSurface() */ -/************************************************************************/ - -/** - * \brief Create an empty multi surface collection. - */ - -OGRMultiSurface::OGRMultiSurface() = default; - -/************************************************************************/ -/* ~OGRMultiSurface() */ -/************************************************************************/ - -OGRMultiSurface::~OGRMultiSurface() = default; - /************************************************************************/ /* OGRMultiSurface( const OGRMultiSurface& ) */ /************************************************************************/ diff --git a/ogr/ogrpoint.cpp b/ogr/ogrpoint.cpp index 59ce7fa39949..c3c503eb3246 100644 --- a/ogr/ogrpoint.cpp +++ b/ogr/ogrpoint.cpp @@ -135,12 +135,6 @@ OGRPoint *OGRPoint::createXYM(double x, double y, double m) OGRPoint::OGRPoint(const OGRPoint &) = default; -/************************************************************************/ -/* ~OGRPoint() */ -/************************************************************************/ - -OGRPoint::~OGRPoint() = default; - /************************************************************************/ /* operator=( const OGRPoint& ) */ /************************************************************************/ diff --git a/ogr/ogrpolygon.cpp b/ogr/ogrpolygon.cpp index c1663696c7cc..1a8794665c90 100644 --- a/ogr/ogrpolygon.cpp +++ b/ogr/ogrpolygon.cpp @@ -27,16 +27,6 @@ #include "ogr_sfcgal.h" #include "ogr_p.h" -/************************************************************************/ -/* OGRPolygon() */ -/************************************************************************/ - -/** - * \brief Create an empty polygon. - */ - -OGRPolygon::OGRPolygon() = default; - /************************************************************************/ /* OGRPolygon( const OGRPolygon& ) */ /************************************************************************/ @@ -52,12 +42,6 @@ OGRPolygon::OGRPolygon() = default; OGRPolygon::OGRPolygon(const OGRPolygon &) = default; -/************************************************************************/ -/* ~OGRPolygon() */ -/************************************************************************/ - -OGRPolygon::~OGRPolygon() = default; - /************************************************************************/ /* operator=( const OGRPolygon&) */ /************************************************************************/ diff --git a/ogr/ogrpolyhedralsurface.cpp b/ogr/ogrpolyhedralsurface.cpp index a22d6f76e7e7..23db6e1718ab 100644 --- a/ogr/ogrpolyhedralsurface.cpp +++ b/ogr/ogrpolyhedralsurface.cpp @@ -20,16 +20,6 @@ #include -/************************************************************************/ -/* OGRPolyhedralSurface() */ -/************************************************************************/ - -/** - * \brief Create an empty PolyhedralSurface - */ - -OGRPolyhedralSurface::OGRPolyhedralSurface() = default; - /************************************************************************/ /* OGRPolyhedralSurface( const OGRPolyhedralSurface& ) */ /************************************************************************/ @@ -42,17 +32,6 @@ OGRPolyhedralSurface::OGRPolyhedralSurface() = default; OGRPolyhedralSurface::OGRPolyhedralSurface(const OGRPolyhedralSurface &) = default; -/************************************************************************/ -/* ~OGRPolyhedralSurface() */ -/************************************************************************/ - -/** - * \brief Destructor - * - */ - -OGRPolyhedralSurface::~OGRPolyhedralSurface() = default; - /************************************************************************/ /* operator=( const OGRPolyhedralSurface&) */ /************************************************************************/ diff --git a/ogr/ogrtriangle.cpp b/ogr/ogrtriangle.cpp index 4f5e39e4e082..4e54600d99e5 100644 --- a/ogr/ogrtriangle.cpp +++ b/ogr/ogrtriangle.cpp @@ -20,17 +20,6 @@ /* OGRTriangle() */ /************************************************************************/ -/** - * \brief Constructor. - * - */ - -OGRTriangle::OGRTriangle() = default; - -/************************************************************************/ -/* OGRTriangle() */ -/************************************************************************/ - /** * \brief Copy constructor. * @@ -92,17 +81,6 @@ OGRTriangle::OGRTriangle(const OGRPoint &p, const OGRPoint &q, oCC.addCurveDirectly(this, poCurve, TRUE); } -/************************************************************************/ -/* ~OGRTriangle() */ -/************************************************************************/ - -/** - * \brief Destructor - * - */ - -OGRTriangle::~OGRTriangle() = default; - /************************************************************************/ /* operator=( const OGRGeometry&) */ /************************************************************************/ diff --git a/ogr/ogrtriangulatedsurface.cpp b/ogr/ogrtriangulatedsurface.cpp index 349d2265e229..9470370e628e 100644 --- a/ogr/ogrtriangulatedsurface.cpp +++ b/ogr/ogrtriangulatedsurface.cpp @@ -16,17 +16,6 @@ #include "ogr_p.h" #include "ogr_api.h" -/************************************************************************/ -/* OGRTriangulatedSurface() */ -/************************************************************************/ - -/** - * \brief Constructor. - * - */ - -OGRTriangulatedSurface::OGRTriangulatedSurface() = default; - /************************************************************************/ /* OGRTriangulatedSurface( const OGRTriangulatedSurface& ) */ /************************************************************************/ @@ -43,17 +32,6 @@ OGRTriangulatedSurface::OGRTriangulatedSurface( *this = other; } -/************************************************************************/ -/* ~OGRTriangulatedSurface() */ -/************************************************************************/ - -/** - * \brief Destructor - * - */ - -OGRTriangulatedSurface::~OGRTriangulatedSurface() = default; - /************************************************************************/ /* operator=( const OGRTriangulatedSurface&) */ /************************************************************************/ From 868c6048071876468ecd9af284d05cfde10fadca Mon Sep 17 00:00:00 2001 From: Daniel Baston Date: Thu, 10 Oct 2024 14:35:04 -0400 Subject: [PATCH 7/8] autotest: add tests for OGR AddCurveDirectly methods --- autotest/cpp/test_ogr.cpp | 190 ++++++++++++++++++++++++++++---------- 1 file changed, 139 insertions(+), 51 deletions(-) diff --git a/autotest/cpp/test_ogr.cpp b/autotest/cpp/test_ogr.cpp index bf9b0ca9eb67..08dee2511369 100644 --- a/autotest/cpp/test_ogr.cpp +++ b/autotest/cpp/test_ogr.cpp @@ -4241,69 +4241,157 @@ TEST_F(test_ogr, OGRCurve_reversePoints) TEST_F(test_ogr, transformWithOptions) { // Projected CRS to national geographic CRS (not including poles or antimeridian) - { - OGRGeometry *poGeom = nullptr; - OGRGeometryFactory::createFromWkt( - "LINESTRING(700000 6600000, 700001 6600001)", nullptr, &poGeom); - ASSERT_NE(poGeom, nullptr); + OGRGeometry *poGeom = nullptr; + OGRGeometryFactory::createFromWkt( + "LINESTRING(700000 6600000, 700001 6600001)", nullptr, &poGeom); + ASSERT_NE(poGeom, nullptr); - OGRSpatialReference oEPSG_2154; - oEPSG_2154.importFromEPSG(2154); // "RGF93 v1 / Lambert-93" - OGRSpatialReference oEPSG_4171; - oEPSG_4171.importFromEPSG(4171); // "RGF93 v1" - oEPSG_4171.SetAxisMappingStrategy(OAMS_TRADITIONAL_GIS_ORDER); - auto poCT = std::unique_ptr( - OGRCreateCoordinateTransformation(&oEPSG_2154, &oEPSG_4171)); - OGRGeometryFactory::TransformWithOptionsCache oCache; - poGeom = OGRGeometryFactory::transformWithOptions(poGeom, poCT.get(), - nullptr, oCache); - EXPECT_NEAR(poGeom->toLineString()->getX(0), 3, 1e-8); - EXPECT_NEAR(poGeom->toLineString()->getY(0), 46.5, 1e-8); + OGRSpatialReference oEPSG_2154; + oEPSG_2154.importFromEPSG(2154); // "RGF93 v1 / Lambert-93" + OGRSpatialReference oEPSG_4171; + oEPSG_4171.importFromEPSG(4171); // "RGF93 v1" + oEPSG_4171.SetAxisMappingStrategy(OAMS_TRADITIONAL_GIS_ORDER); + auto poCT = std::unique_ptr( + OGRCreateCoordinateTransformation(&oEPSG_2154, &oEPSG_4171)); + OGRGeometryFactory::TransformWithOptionsCache oCache; + poGeom = OGRGeometryFactory::transformWithOptions(poGeom, poCT.get(), + nullptr, oCache); + EXPECT_NEAR(poGeom->toLineString()->getX(0), 3, 1e-8); + EXPECT_NEAR(poGeom->toLineString()->getY(0), 46.5, 1e-8); - delete poGeom; - } + delete poGeom; +} #ifdef HAVE_GEOS + +// Test OGRGeometryFactory::transformWithOptions() +TEST_F(test_ogr, transformWithOptions_GEOS) +{ // Projected CRS to national geographic CRS including antimeridian - { - OGRGeometry *poGeom = nullptr; - OGRGeometryFactory::createFromWkt( - "LINESTRING(657630.64 4984896.17,815261.43 4990738.26)", nullptr, - &poGeom); - ASSERT_NE(poGeom, nullptr); + OGRGeometry *poGeom = nullptr; + OGRGeometryFactory::createFromWkt( + "LINESTRING(657630.64 4984896.17,815261.43 4990738.26)", nullptr, + &poGeom); + ASSERT_NE(poGeom, nullptr); - OGRSpatialReference oEPSG_6329; - oEPSG_6329.importFromEPSG(6329); // "NAD83(2011) / UTM zone 60N" - OGRSpatialReference oEPSG_6318; - oEPSG_6318.importFromEPSG(6318); // "NAD83(2011)" - oEPSG_6318.SetAxisMappingStrategy(OAMS_TRADITIONAL_GIS_ORDER); - auto poCT = std::unique_ptr( - OGRCreateCoordinateTransformation(&oEPSG_6329, &oEPSG_6318)); - OGRGeometryFactory::TransformWithOptionsCache oCache; - poGeom = OGRGeometryFactory::transformWithOptions(poGeom, poCT.get(), - nullptr, oCache); - EXPECT_EQ(poGeom->getGeometryType(), wkbMultiLineString); - if (poGeom->getGeometryType() == wkbMultiLineString) + OGRSpatialReference oEPSG_6329; + oEPSG_6329.importFromEPSG(6329); // "NAD83(2011) / UTM zone 60N" + OGRSpatialReference oEPSG_6318; + oEPSG_6318.importFromEPSG(6318); // "NAD83(2011)" + oEPSG_6318.SetAxisMappingStrategy(OAMS_TRADITIONAL_GIS_ORDER); + auto poCT = std::unique_ptr( + OGRCreateCoordinateTransformation(&oEPSG_6329, &oEPSG_6318)); + OGRGeometryFactory::TransformWithOptionsCache oCache; + poGeom = OGRGeometryFactory::transformWithOptions(poGeom, poCT.get(), + nullptr, oCache); + EXPECT_EQ(poGeom->getGeometryType(), wkbMultiLineString); + if (poGeom->getGeometryType() == wkbMultiLineString) + { + const auto poMLS = poGeom->toMultiLineString(); + EXPECT_EQ(poMLS->getNumGeometries(), 2); + if (poMLS->getNumGeometries() == 2) { - const auto poMLS = poGeom->toMultiLineString(); - EXPECT_EQ(poMLS->getNumGeometries(), 2); - if (poMLS->getNumGeometries() == 2) + const auto poLS = poMLS->getGeometryRef(0); + EXPECT_EQ(poLS->getNumPoints(), 2); + if (poLS->getNumPoints() == 2) { - const auto poLS = poMLS->getGeometryRef(0); - EXPECT_EQ(poLS->getNumPoints(), 2); - if (poLS->getNumPoints() == 2) - { - EXPECT_NEAR(poLS->getX(0), 179, 1e-6); - EXPECT_NEAR(poLS->getY(0), 45, 1e-6); - EXPECT_NEAR(poLS->getX(1), 180, 1e-6); - EXPECT_NEAR(poLS->getY(1), 45.004384301691303, 1e-6); - } + EXPECT_NEAR(poLS->getX(0), 179, 1e-6); + EXPECT_NEAR(poLS->getY(0), 45, 1e-6); + EXPECT_NEAR(poLS->getX(1), 180, 1e-6); + EXPECT_NEAR(poLS->getY(1), 45.004384301691303, 1e-6); } } - - delete poGeom; } + + delete poGeom; +} #endif + +// Test OGRCurvePolygon::addRingDirectly +TEST_F(test_ogr, OGRCurvePolygon_addRingDirectly) +{ + OGRCurvePolygon cp; + OGRGeometry *ring; + + // closed CircularString + OGRGeometryFactory::createFromWkt( + "CIRCULARSTRING (0 0, 1 1, 2 0, 1 -1, 0 0)", nullptr, &ring); + ASSERT_TRUE(ring); + EXPECT_EQ(cp.addRingDirectly(ring->toCurve()), OGRERR_NONE); + + // open CircularString + OGRGeometryFactory::createFromWkt("CIRCULARSTRING (0 0, 1 1, 2 0)", nullptr, + &ring); + ASSERT_TRUE(ring); + { + CPLConfigOptionSetter oSetter("OGR_GEOMETRY_ACCEPT_UNCLOSED_RING", "NO", + false); + ASSERT_EQ(cp.addRingDirectly(ring->toCurve()), + OGRERR_UNSUPPORTED_GEOMETRY_TYPE); + } + EXPECT_EQ(cp.addRingDirectly(ring->toCurve()), OGRERR_NONE); + + // closed CompoundCurve + OGRGeometryFactory::createFromWkt( + "COMPOUNDCURVE( CIRCULARSTRING (0 0, 1 1, 2 0), (2 0, 0 0))", nullptr, + &ring); + ASSERT_TRUE(ring); + EXPECT_EQ(cp.addRingDirectly(ring->toCurve()), OGRERR_NONE); + + // closed LineString + OGRGeometryFactory::createFromWkt("LINESTRING (0 0, 1 0, 1 1, 0 1, 0 0)", + nullptr, &ring); + ASSERT_TRUE(ring); + EXPECT_EQ(cp.addRingDirectly(ring->toCurve()), OGRERR_NONE); + + // LinearRing + auto lr = std::make_unique(); + lr->addPoint(0, 0); + lr->addPoint(1, 0); + lr->addPoint(1, 1); + lr->addPoint(0, 1); + lr->addPoint(0, 0); + ASSERT_TRUE(ring); + ASSERT_EQ(cp.addRingDirectly(lr.get()), OGRERR_UNSUPPORTED_GEOMETRY_TYPE); +} + +// Test OGRPolygon::addRingDirectly +TEST_F(test_ogr, OGRPolygon_addRingDirectly) +{ + OGRPolygon p; + OGRGeometry *ring; + + // closed CircularString + OGRGeometryFactory::createFromWkt( + "CIRCULARSTRING (0 0, 1 1, 2 0, 1 -1, 0 0)", nullptr, &ring); + ASSERT_TRUE(ring); + EXPECT_EQ(p.addRingDirectly(ring->toCurve()), + OGRERR_UNSUPPORTED_GEOMETRY_TYPE); + delete ring; + + // closed LineString + OGRGeometryFactory::createFromWkt("LINESTRING (0 0, 1 0, 1 1, 0 1, 0 0)", + nullptr, &ring); + ASSERT_TRUE(ring); + EXPECT_EQ(p.addRingDirectly(ring->toCurve()), + OGRERR_UNSUPPORTED_GEOMETRY_TYPE); + delete ring; + + // open LineString + OGRGeometryFactory::createFromWkt("LINESTRING (0 0, 1 0)", nullptr, &ring); + ASSERT_TRUE(ring); + EXPECT_EQ(p.addRingDirectly(ring->toCurve()), + OGRERR_UNSUPPORTED_GEOMETRY_TYPE); + delete ring; + + // LinearRing + auto lr = std::make_unique(); + lr->addPoint(0, 0); + lr->addPoint(1, 0); + lr->addPoint(1, 1); + lr->addPoint(0, 1); + lr->addPoint(0, 0); + ASSERT_EQ(p.addRingDirectly(lr.release()), OGRERR_NONE); } } // namespace From fad6dd5e22a221bd27bcff7eaa42169c37a8a136 Mon Sep 17 00:00:00 2001 From: Daniel Baston Date: Thu, 10 Oct 2024 15:04:53 -0400 Subject: [PATCH 8/8] OGRGeometry: Factor isRingCorrectType out of checkRing --- ogr/ogr_geometry.h | 11 +++++++---- ogr/ogrcurvepolygon.cpp | 28 ++++++++++++++++++---------- ogr/ogrpolygon.cpp | 16 ++++++++++++---- 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/ogr/ogr_geometry.h b/ogr/ogr_geometry.h index dd6709ef7bd9..abd66ace0e8f 100644 --- a/ogr/ogr_geometry.h +++ b/ogr/ogr_geometry.h @@ -2447,8 +2447,10 @@ class CPL_DLL OGRCurvePolygon : public OGRSurface private: OGRBoolean IntersectsPoint(const OGRPoint *p) const; OGRBoolean ContainsPoint(const OGRPoint *p) const; - virtual bool checkRing(const OGRCurve *poNewRing, - bool bOnlyType = false) const; + + virtual bool isRingCorrectType(const OGRCurve *poRing) const; + + virtual bool checkRing(const OGRCurve *poNewRing) const; OGRErr addRingDirectlyInternal(OGRCurve *poCurve, int bNeedRealloc); static OGRErr addCurveDirectlyFromWkt(OGRGeometry *poSelf, OGRCurve *poCurve); @@ -2664,8 +2666,9 @@ class CPL_DLL OGRPolygon : public OGRCurvePolygon friend class OGRPolyhedralSurface; friend class OGRTriangulatedSurface; - virtual bool checkRing(const OGRCurve *poNewRing, - bool bOnlyType = false) const override; + virtual bool isRingCorrectType(const OGRCurve *poRing) const override; + + virtual bool checkRing(const OGRCurve *poNewRing) const override; virtual OGRErr importFromWKTListOnly(const char **ppszInput, int bHasZ, int bHasM, OGRRawPoint *&paoPoints, int &nMaxPoints, double *&padfZ); diff --git a/ogr/ogrcurvepolygon.cpp b/ogr/ogrcurvepolygon.cpp index 49795945ac43..559eb89c420a 100644 --- a/ogr/ogrcurvepolygon.cpp +++ b/ogr/ogrcurvepolygon.cpp @@ -59,7 +59,7 @@ OGRCurvePolygon &OGRCurvePolygon::operator=(const OGRCurvePolygon &other) for (const auto *poRing : other.oCC) { - if (!checkRing(poRing, /* bOnlyType = */ true)) + if (!isRingCorrectType(poRing)) { CPLError(CE_Failure, CPLE_AppDefined, "Illegal use of OGRCurvePolygon::operator=(): " @@ -331,13 +331,27 @@ OGRErr OGRCurvePolygon::addRing(const OGRCurve *poNewRing) return eErr; } +/************************************************************************/ +/* isRingCorrectType() */ +/************************************************************************/ +bool OGRCurvePolygon::isRingCorrectType(const OGRCurve *poRing) const +{ + return poRing && !EQUAL(poRing->getGeometryName(), "LINEARRING"); +} + /************************************************************************/ /* checkRing() */ /************************************************************************/ -bool OGRCurvePolygon::checkRing(const OGRCurve *poNewRing, bool bOnlyType) const +bool OGRCurvePolygon::checkRing(const OGRCurve *poNewRing) const { - if (!bOnlyType && !poNewRing->IsEmpty() && !poNewRing->get_IsClosed()) + if (!isRingCorrectType(poNewRing)) + { + CPLError(CE_Failure, CPLE_AppDefined, "Linearring not allowed."); + return false; + } + + if (!poNewRing->IsEmpty() && !poNewRing->get_IsClosed()) { // This configuration option name must be the same as in // OGRPolygon::checkRing() @@ -361,14 +375,8 @@ bool OGRCurvePolygon::checkRing(const OGRCurve *poNewRing, bool bOnlyType) const if (wkbFlatten(poNewRing->getGeometryType()) == wkbLineString) { - if (!bOnlyType && poNewRing->getNumPoints() < 4) - { - return false; - } - - if (EQUAL(poNewRing->getGeometryName(), "LINEARRING")) + if (poNewRing->getNumPoints() < 4) { - CPLError(CE_Failure, CPLE_AppDefined, "Linearring not allowed."); return false; } } diff --git a/ogr/ogrpolygon.cpp b/ogr/ogrpolygon.cpp index 1a8794665c90..b654a9584443 100644 --- a/ogr/ogrpolygon.cpp +++ b/ogr/ogrpolygon.cpp @@ -247,21 +247,29 @@ OGRLinearRing *OGRPolygon::stealInteriorRing(int iRing) } /*! @cond Doxygen_Suppress */ + +/************************************************************************/ +/* isRingCorrectType() */ +/************************************************************************/ +bool OGRPolygon::isRingCorrectType(const OGRCurve *poRing) const +{ + return poRing != nullptr && EQUAL(poRing->getGeometryName(), "LINEARRING"); +} + /************************************************************************/ /* checkRing() */ /************************************************************************/ -bool OGRPolygon::checkRing(const OGRCurve *poNewRing, bool bOnlyType) const +bool OGRPolygon::checkRing(const OGRCurve *poNewRing) const { - if (poNewRing == nullptr || - !(EQUAL(poNewRing->getGeometryName(), "LINEARRING"))) + if (!isRingCorrectType(poNewRing)) { CPLError(CE_Failure, CPLE_AppDefined, "Wrong curve type. Expected LINEARRING."); return false; } - if (!bOnlyType && !poNewRing->IsEmpty() && !poNewRing->get_IsClosed()) + if (!poNewRing->IsEmpty() && !poNewRing->get_IsClosed()) { // This configuration option name must be the same as in // OGRCurvePolygon::checkRing()