Skip to content

Commit

Permalink
Merge pull request #11153 from dbaston/ogrfeature-set-unique-ptr
Browse files Browse the repository at this point in the history 
OGRFeature: Add SetGeometry overloads taking unique_ptr<OGRGeometry>
  • Loading branch information
@rouault
rouault authored Oct 29, 2024
2 parents 1c4caf3 + 1c459f5 commit 1fd36d7
Show file tree
Hide file tree
Showing 3 changed files with 155 additions and 27 deletions.
63 changes: 63 additions & 0 deletions autotest/cpp/test_ogr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4394,4 +4394,67 @@ TEST_F(test_ogr, OGRPolygon_addRingDirectly)
ASSERT_EQ(p.addRingDirectly(lr.release()), OGRERR_NONE);
}

TEST_F(test_ogr, OGRFeature_SetGeometry)
{
OGRFeatureDefn *poFeatureDefn = new OGRFeatureDefn();
poFeatureDefn->Reference();

OGRFeature oFeat(poFeatureDefn);
std::unique_ptr<OGRGeometry> poGeom;
OGRGeometry *poTmpGeom;
ASSERT_EQ(
OGRGeometryFactory::createFromWkt("POINT (3 7)", nullptr, &poTmpGeom),
OGRERR_NONE);
poGeom.reset(poTmpGeom);
ASSERT_EQ(oFeat.SetGeometry(std::move(poGeom)), OGRERR_NONE);
EXPECT_EQ(oFeat.GetGeometryRef()->toPoint()->getX(), 3);
EXPECT_EQ(oFeat.GetGeometryRef()->toPoint()->getY(), 7);

// set it again to make sure previous feature geometry is freed
ASSERT_EQ(
OGRGeometryFactory::createFromWkt("POINT (2 8)", nullptr, &poTmpGeom),
OGRERR_NONE);
poGeom.reset(poTmpGeom);
ASSERT_EQ(oFeat.SetGeometry(std::move(poGeom)), OGRERR_NONE);
EXPECT_EQ(oFeat.GetGeometryRef()->toPoint()->getX(), 2);
EXPECT_EQ(oFeat.GetGeometryRef()->toPoint()->getY(), 8);

poFeatureDefn->Release();
}

TEST_F(test_ogr, OGRFeature_SetGeomField)
{
OGRFeatureDefn *poFeatureDefn = new OGRFeatureDefn();
poFeatureDefn->Reference();

OGRGeomFieldDefn oGeomField("second", wkbPoint);
poFeatureDefn->AddGeomFieldDefn(&oGeomField);

OGRFeature oFeat(poFeatureDefn);

// failure
{
std::unique_ptr<OGRGeometry> poGeom;
OGRGeometry *poTmpGeom;
ASSERT_EQ(OGRGeometryFactory::createFromWkt("POINT (3 7)", nullptr,
&poTmpGeom),
OGRERR_NONE);
poGeom.reset(poTmpGeom);
EXPECT_EQ(oFeat.SetGeomField(13, std::move(poGeom)), OGRERR_FAILURE);
}

// success
{
std::unique_ptr<OGRGeometry> poGeom;
OGRGeometry *poTmpGeom;
ASSERT_EQ(OGRGeometryFactory::createFromWkt("POINT (3 7)", nullptr,
&poTmpGeom),
OGRERR_NONE);
poGeom.reset(poTmpGeom);
EXPECT_EQ(oFeat.SetGeomField(1, std::move(poGeom)), OGRERR_NONE);
}

poFeatureDefn->Release();
}

} // namespace
2 changes: 2 additions & 0 deletions ogr/ogr_feature.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1178,6 +1178,7 @@ class CPL_DLL OGRFeature

OGRErr SetGeometryDirectly(OGRGeometry *);
OGRErr SetGeometry(const OGRGeometry *);
OGRErr SetGeometry(std::unique_ptr<OGRGeometry>);
OGRGeometry *GetGeometryRef();
const OGRGeometry *GetGeometryRef() const;
OGRGeometry *StealGeometry() CPL_WARN_UNUSED_RESULT;
Expand Down Expand Up @@ -1209,6 +1210,7 @@ class CPL_DLL OGRFeature
const OGRGeometry *GetGeomFieldRef(const char *pszFName) const;
OGRErr SetGeomFieldDirectly(int iField, OGRGeometry *);
OGRErr SetGeomField(int iField, const OGRGeometry *);
OGRErr SetGeomField(int iField, std::unique_ptr<OGRGeometry>);

void Reset();

Expand Down
117 changes: 90 additions & 27 deletions ogr/ogrfeature.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ OGRFeatureDefnH OGR_F_GetDefnRef(OGRFeatureH hFeat)
/**
* \brief Set feature geometry.
*
* This method updates the features geometry, and operate exactly as
* This method updates the features geometry, and operates the same as
* SetGeometry(), except that this method assumes ownership of the
* passed geometry (even in case of failure of that function).
*
Expand All @@ -456,11 +456,12 @@ OGRFeatureDefnH OGR_F_GetDefnRef(OGRFeatureH hFeat)
OGRErr OGRFeature::SetGeometryDirectly(OGRGeometry *poGeomIn)

{
if (GetGeomFieldCount() > 0)
return SetGeomFieldDirectly(0, poGeomIn);
if (poGeomIn == GetGeometryRef())
{
return OGRERR_NONE;
}

delete poGeomIn;
return OGRERR_FAILURE;
return SetGeomField(0, std::unique_ptr<OGRGeometry>(poGeomIn));
}

/************************************************************************/
Expand All @@ -470,7 +471,7 @@ OGRErr OGRFeature::SetGeometryDirectly(OGRGeometry *poGeomIn)
/**
* \brief Set feature geometry.
*
* This function updates the features geometry, and operate exactly as
* This function updates the features geometry, and operates the same as
* SetGeometry(), except that this function assumes ownership of the
* passed geometry (even in case of failure of that function).
*
Expand Down Expand Up @@ -506,7 +507,7 @@ OGRErr OGR_F_SetGeometryDirectly(OGRFeatureH hFeat, OGRGeometryH hGeom)
/**
* \brief Set feature geometry.
*
* This method updates the features geometry, and operate exactly as
* This method updates the features geometry, and operates the same as
* SetGeometryDirectly(), except that this method does not assume ownership
* of the passed geometry, but instead makes a copy of it.
*
Expand Down Expand Up @@ -542,7 +543,7 @@ OGRErr OGRFeature::SetGeometry(const OGRGeometry *poGeomIn)
/**
* \brief Set feature geometry.
*
* This function updates the features geometry, and operate exactly as
* This function updates the features geometry, and operates the same as
* SetGeometryDirectly(), except that this function does not assume ownership
* of the passed geometry, but instead makes a copy of it.
*
Expand Down Expand Up @@ -570,6 +571,37 @@ OGRErr OGR_F_SetGeometry(OGRFeatureH hFeat, OGRGeometryH hGeom)
OGRGeometry::FromHandle(hGeom));
}

/************************************************************************/
/* SetGeometry() */
/************************************************************************/

/**
* \brief Set feature geometry.
*
* This method is the same as the C function OGR_F_SetGeometryDirectly().
*
* @note This method has only an effect on the in-memory feature object. If
* this object comes from a layer and the modifications must be serialized back
* to the datasource, OGR_L_SetFeature() must be used afterwards. Or if this is
* a new feature, OGR_L_CreateFeature() must be used afterwards.
*
* @param poGeomIn new geometry to apply to feature. Passing NULL value here
* is correct and it will result in deallocation of currently assigned geometry
* without assigning new one.
*
* @return OGRERR_NONE if successful, or OGR_UNSUPPORTED_GEOMETRY_TYPE if
* the geometry type is illegal for the OGRFeatureDefn (checking not yet
* implemented).
*
* @since GDAL 3.11
*/

OGRErr OGRFeature::SetGeometry(std::unique_ptr<OGRGeometry> poGeomIn)

{
return SetGeomField(0, std::move(poGeomIn));
}

/************************************************************************/
/* StealGeometry() */
/************************************************************************/
Expand Down Expand Up @@ -907,7 +939,7 @@ OGRGeometryH OGR_F_GetGeomFieldRef(OGRFeatureH hFeat, int iField)
/**
* \brief Set feature geometry of a specified geometry field.
*
* This method updates the features geometry, and operate exactly as
* This method updates the features geometry, and operates the same as
* SetGeomField(), except that this method assumes ownership of the
* passed geometry (even in case of failure of that function).
*
Expand All @@ -926,21 +958,13 @@ OGRGeometryH OGR_F_GetGeomFieldRef(OGRFeatureH hFeat, int iField)
*/

OGRErr OGRFeature::SetGeomFieldDirectly(int iField, OGRGeometry *poGeomIn)

{
if (iField < 0 || iField >= GetGeomFieldCount())
{
delete poGeomIn;
return OGRERR_FAILURE;
}

if (papoGeometries[iField] != poGeomIn)
if (poGeomIn && poGeomIn == GetGeomFieldRef(iField))
{
delete papoGeometries[iField];
papoGeometries[iField] = poGeomIn;
return OGRERR_NONE;
}

return OGRERR_NONE;
return SetGeomField(iField, std::unique_ptr<OGRGeometry>(poGeomIn));
}

/************************************************************************/
Expand All @@ -950,7 +974,7 @@ OGRErr OGRFeature::SetGeomFieldDirectly(int iField, OGRGeometry *poGeomIn)
/**
* \brief Set feature geometry of a specified geometry field.
*
* This function updates the features geometry, and operate exactly as
* This function updates the features geometry, and operates the same as
* SetGeomField(), except that this function assumes ownership of the
* passed geometry (even in case of failure of that function).
*
Expand Down Expand Up @@ -985,7 +1009,7 @@ OGRErr OGR_F_SetGeomFieldDirectly(OGRFeatureH hFeat, int iField,
/**
* \brief Set feature geometry of a specified geometry field.
*
* This method updates the features geometry, and operate exactly as
* This method updates the features geometry, and operates the same as
* SetGeomFieldDirectly(), except that this method does not assume ownership
* of the passed geometry, but instead makes a copy of it.
*
Expand Down Expand Up @@ -1031,7 +1055,7 @@ OGRErr OGRFeature::SetGeomField(int iField, const OGRGeometry *poGeomIn)
/**
* \brief Set feature geometry of a specified geometry field.
*
* This function updates the features geometry, and operate exactly as
* This function updates the features geometry, and operates the same as
* SetGeometryDirectly(), except that this function does not assume ownership
* of the passed geometry, but instead makes a copy of it.
*
Expand All @@ -1055,6 +1079,45 @@ OGRErr OGR_F_SetGeomField(OGRFeatureH hFeat, int iField, OGRGeometryH hGeom)
iField, OGRGeometry::FromHandle(hGeom));
}

/************************************************************************/
/* SetGeomField() */
/************************************************************************/

/**
* \brief Set feature geometry of a specified geometry field.
*
* This method is the same as the C function OGR_F_SetGeomFieldDirectly().
*
* @param iField geometry field to set.
* @param poGeomIn new geometry to apply to feature. Passing NULL value here
* is correct and it will result in deallocation of currently assigned geometry
* without assigning new one.
*
* @return OGRERR_NONE if successful, or OGRERR_FAILURE if the index is invalid,
* or OGRERR_UNSUPPORTED_GEOMETRY_TYPE if the geometry type is illegal for the
* OGRFeatureDefn (checking not yet implemented).
*
* @since GDAL 3.11
*/

OGRErr OGRFeature::SetGeomField(int iField,
std::unique_ptr<OGRGeometry> poGeomIn)

{
if (iField < 0 || iField >= GetGeomFieldCount())
{
return OGRERR_FAILURE;
}

if (papoGeometries[iField] != poGeomIn.get())
{
delete papoGeometries[iField];
papoGeometries[iField] = poGeomIn.release();
}

return OGRERR_NONE;
}

/************************************************************************/
/* Clone() */
/************************************************************************/
Expand Down Expand Up @@ -6688,7 +6751,7 @@ const char *OGR_F_GetStyleString(OGRFeatureH hFeat)
/**
* \brief Set feature style string.
*
* This method operate exactly as OGRFeature::SetStyleStringDirectly() except
* This method operates the same as OGRFeature::SetStyleStringDirectly() except
* that it does not assume ownership of the passed string, but instead makes a
* copy of it.
*
Expand Down Expand Up @@ -6716,7 +6779,7 @@ void OGRFeature::SetStyleString(const char *pszString)
/**
* \brief Set feature style string.
*
* This method operate exactly as OGR_F_SetStyleStringDirectly() except that it
* This method operates the same as OGR_F_SetStyleStringDirectly() except that it
* does not assume ownership of the passed string, but instead makes a copy of
* it.
*
Expand All @@ -6741,7 +6804,7 @@ void OGR_F_SetStyleString(OGRFeatureH hFeat, const char *pszStyle)
/**
* \brief Set feature style string.
*
* This method operate exactly as OGRFeature::SetStyleString() except that it
* This method operates the same as OGRFeature::SetStyleString() except that it
* assumes ownership of the passed string.
*
* This method is the same as the C function OGR_F_SetStyleStringDirectly().
Expand All @@ -6762,7 +6825,7 @@ void OGRFeature::SetStyleStringDirectly(char *pszString)
/**
* \brief Set feature style string.
*
* This method operate exactly as OGR_F_SetStyleString() except that it assumes
* This method operates the same as OGR_F_SetStyleString() except that it assumes
* ownership of the passed string.
*
* This function is the same as the C++ method
Expand Down

0 comments on commit 1fd36d7

Please sign in to comment.