From fadf31f5ddfca6bcac2569ee867bcf720ffa6bbf Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Wed, 8 Jan 2025 02:24:37 +0100 Subject: [PATCH] Add a OGRGeometryFactory::GetDefaultArcStepSize() method to get value of OGR_ARC_STEPSIZE config option --- autotest/cpp/test_ogr.cpp | 27 +++++++++++++ ogr/gml2ogrgeometry.cpp | 7 +--- ogr/ogr_geometry.h | 2 + ogr/ogrgeometryfactory.cpp | 46 ++++++++++++++++++++--- ogr/ogrpgeogeometry.cpp | 4 +- ogr/ogrsf_frmts/dwg/ogrdgnv8layer.cpp | 2 +- ogr/ogrsf_frmts/ili/ogrili1datasource.cpp | 11 +----- ogr/ogrsf_frmts/ili/ogrili1layer.cpp | 12 ++---- port/cpl_known_config_options.h | 2 +- 9 files changed, 79 insertions(+), 34 deletions(-) diff --git a/autotest/cpp/test_ogr.cpp b/autotest/cpp/test_ogr.cpp index e748568ad8c0..e6605d9f03b8 100644 --- a/autotest/cpp/test_ogr.cpp +++ b/autotest/cpp/test_ogr.cpp @@ -4571,4 +4571,31 @@ TEST_F(test_ogr, OGRFieldDefnGetFieldTypeByName) } } +// Test OGRGeometryFactory::GetDefaultArcStepSize() +TEST_F(test_ogr, GetDefaultArcStepSize) +{ + if (CPLGetConfigOption("OGR_ARC_STEPSIZE", nullptr) == nullptr) + { + EXPECT_EQ(OGRGeometryFactory::GetDefaultArcStepSize(), 4.0); + } + { + CPLConfigOptionSetter oSetter("OGR_ARC_STEPSIZE", "0.00001", + /* bSetOnlyIfUndefined = */ false); + CPLErrorHandlerPusher oErrorHandler(CPLQuietErrorHandler); + EXPECT_EQ(OGRGeometryFactory::GetDefaultArcStepSize(), 1e-2); + EXPECT_TRUE( + strstr(CPLGetLastErrorMsg(), + "Too small value for OGR_ARC_STEPSIZE. Clamping it to")); + } + { + CPLConfigOptionSetter oSetter("OGR_ARC_STEPSIZE", "190", + /* bSetOnlyIfUndefined = */ false); + CPLErrorHandlerPusher oErrorHandler(CPLQuietErrorHandler); + EXPECT_EQ(OGRGeometryFactory::GetDefaultArcStepSize(), 180); + EXPECT_TRUE( + strstr(CPLGetLastErrorMsg(), + "Too large value for OGR_ARC_STEPSIZE. Clamping it to")); + } +} + } // namespace diff --git a/ogr/gml2ogrgeometry.cpp b/ogr/gml2ogrgeometry.cpp index bd4a59adf386..1ad0ba5c44b1 100644 --- a/ogr/gml2ogrgeometry.cpp +++ b/ogr/gml2ogrgeometry.cpp @@ -1876,9 +1876,7 @@ GML2OGRGeometry_XMLNode_Internal(const CPLXMLNode *psNode, const char *pszId, if (bSRSUnitIsDegree && dfUOMConv > 0) { auto poLS = std::make_unique(); - // coverity[tainted_data] - const double dfStep = - CPLAtof(CPLGetConfigOption("OGR_ARC_STEPSIZE", "4")); + const double dfStep = OGRGeometryFactory::GetDefaultArcStepSize(); const double dfSign = dfStartAngle < dfEndAngle ? 1 : -1; for (double dfAngle = dfStartAngle; (dfAngle - dfEndAngle) * dfSign < 0; @@ -2010,8 +2008,7 @@ GML2OGRGeometry_XMLNode_Internal(const CPLXMLNode *psNode, const char *pszId, if (bSRSUnitIsDegree && dfUOMConv > 0) { auto poLS = std::make_unique(); - const double dfStep = - CPLAtof(CPLGetConfigOption("OGR_ARC_STEPSIZE", "4")); + const double dfStep = OGRGeometryFactory::GetDefaultArcStepSize(); for (double dfAngle = 0; dfAngle < 360; dfAngle += dfStep) { double dfLong = 0.0; diff --git a/ogr/ogr_geometry.h b/ogr/ogr_geometry.h index 414d5d2829b4..42dd9a69d0e1 100644 --- a/ogr/ogr_geometry.h +++ b/ogr/ogr_geometry.h @@ -4381,6 +4381,8 @@ class CPL_DLL OGRGeometryFactory char **papszOptions, const TransformWithOptionsCache &cache = TransformWithOptionsCache()); + static double GetDefaultArcStepSize(); + static OGRGeometry * approximateArcAngles(double dfX, double dfY, double dfZ, double dfPrimaryRadius, double dfSecondaryAxis, diff --git a/ogr/ogrgeometryfactory.cpp b/ogr/ogrgeometryfactory.cpp index 86d6c6b4b3db..7d09f1b7b180 100644 --- a/ogr/ogrgeometryfactory.cpp +++ b/ogr/ogrgeometryfactory.cpp @@ -4197,13 +4197,47 @@ void OGR_GeomTransformer_Destroy(OGRGeomTransformerH hTransformer) } /************************************************************************/ -/* OGRGF_GetDefaultStepSize() */ +/* OGRGeometryFactory::GetDefaultArcStepSize() */ /************************************************************************/ -static double OGRGF_GetDefaultStepSize() +/** Return the default value of the angular step used when stroking curves + * as lines. Defaults to 4 degrees. + * Can be modified by setting the OGR_ARC_STEPSIZE configuration option. + * Valid values are in [1e-2, 180] degree range. + * @since 3.11 + */ + +/* static */ +double OGRGeometryFactory::GetDefaultArcStepSize() { - // coverity[tainted_data] - return CPLAtofM(CPLGetConfigOption("OGR_ARC_STEPSIZE", "4")); + const double dfVal = CPLAtofM(CPLGetConfigOption("OGR_ARC_STEPSIZE", "4")); + constexpr double MIN_VAL = 1e-2; + if (dfVal < MIN_VAL) + { + static bool bWarned = false; + if (!bWarned) + { + bWarned = true; + CPLError(CE_Warning, CPLE_AppDefined, + "Too small value for OGR_ARC_STEPSIZE. Clamping it to %f", + MIN_VAL); + } + return MIN_VAL; + } + constexpr double MAX_VAL = 180; + if (dfVal > MAX_VAL) + { + static bool bWarned = false; + if (!bWarned) + { + bWarned = true; + CPLError(CE_Warning, CPLE_AppDefined, + "Too large value for OGR_ARC_STEPSIZE. Clamping it to %f", + MAX_VAL); + } + return MAX_VAL; + } + return dfVal; } /************************************************************************/ @@ -4266,7 +4300,7 @@ OGRGeometry *OGRGeometryFactory::approximateArcAngles( // Support default arc step setting. if (dfMaxAngleStepSizeDegrees < 1e-6) { - dfMaxAngleStepSizeDegrees = OGRGF_GetDefaultStepSize(); + dfMaxAngleStepSizeDegrees = OGRGeometryFactory::GetDefaultArcStepSize(); } // Determine maximum interpolation gap. This is the largest straight-line @@ -5459,7 +5493,7 @@ OGRLineString *OGRGeometryFactory::curveToLineString( // support default arc step setting. if (dfMaxAngleStepSizeDegrees < 1e-6) { - dfMaxAngleStepSizeDegrees = OGRGF_GetDefaultStepSize(); + dfMaxAngleStepSizeDegrees = OGRGeometryFactory::GetDefaultArcStepSize(); } double dfStep = dfMaxAngleStepSizeDegrees / 180 * M_PI; diff --git a/ogr/ogrpgeogeometry.cpp b/ogr/ogrpgeogeometry.cpp index af0a1eff784c..86005a315b3c 100644 --- a/ogr/ogrpgeogeometry.cpp +++ b/ogr/ogrpgeogeometry.cpp @@ -1835,10 +1835,8 @@ static OGRCurve *OGRShapeCreateCompoundCurve(int nPartStartIdx, int nPartPoints, dfStartAngle += 2 * M_PI; else if (dfEndAngle + M_PI < dfStartAngle) dfEndAngle += 2 * M_PI; - // coverity[tainted_data] const double dfStepSizeRad = - CPLAtofM(CPLGetConfigOption("OGR_ARC_STEPSIZE", "4")) / 180.0 * - M_PI; + OGRGeometryFactory::GetDefaultArcStepSize() / 180.0 * M_PI; const double dfLengthTangentStart = (dfX1 - dfX0) * (dfX1 - dfX0) + (dfY1 - dfY0) * (dfY1 - dfY0); const double dfLengthTangentEnd = diff --git a/ogr/ogrsf_frmts/dwg/ogrdgnv8layer.cpp b/ogr/ogrsf_frmts/dwg/ogrdgnv8layer.cpp index 00b8dd9835f4..48d23edd9a39 100644 --- a/ogr/ogrsf_frmts/dwg/ogrdgnv8layer.cpp +++ b/ogr/ogrsf_frmts/dwg/ogrdgnv8layer.cpp @@ -574,7 +574,7 @@ static void ProcessCurve(OGRFeature *poFeature, const CPLString &osPen, else poSC = new OGRLineString(); const double dfArcStepSize = - CPLAtofM(CPLGetConfigOption("OGR_ARC_STEPSIZE", "4")); + OGRGeometryFactory::GetDefaultArcStepSize(); if (!ellipse.isNull()) { nPoints = std::max(2, static_cast(360 / dfArcStepSize)); diff --git a/ogr/ogrsf_frmts/ili/ogrili1datasource.cpp b/ogr/ogrsf_frmts/ili/ogrili1datasource.cpp index bc9f8ba3dd47..6dd848af7bdd 100644 --- a/ogr/ogrsf_frmts/ili/ogrili1datasource.cpp +++ b/ogr/ogrsf_frmts/ili/ogrili1datasource.cpp @@ -148,19 +148,12 @@ int OGRILI1DataSource::Open(const char *pszNewName, char **papszOpenOptionsIn, if (osModelFilename.length() > 0) poReader->ReadModel(poImdReader, osModelFilename.c_str(), this); - int bResetConfigOption = FALSE; - if (EQUAL(CPLGetConfigOption("OGR_ARC_STEPSIZE", ""), "")) - { - bResetConfigOption = TRUE; - CPLSetThreadLocalConfigOption("OGR_ARC_STEPSIZE", "0.96"); - } + CPLConfigOptionSetter oSetter("OGR_ARC_STEPSIZE", "0.96", + /* bSetOnlyIfUndefined = */ true); // Parse model and read data - without surface join and area polygonizing. poReader->ReadFeatures(); - if (bResetConfigOption) - CPLSetThreadLocalConfigOption("OGR_ARC_STEPSIZE", nullptr); - return TRUE; } diff --git a/ogr/ogrsf_frmts/ili/ogrili1layer.cpp b/ogr/ogrsf_frmts/ili/ogrili1layer.cpp index 2d4b615e035f..b188c17dceaa 100644 --- a/ogr/ogrsf_frmts/ili/ogrili1layer.cpp +++ b/ogr/ogrsf_frmts/ili/ogrili1layer.cpp @@ -444,12 +444,9 @@ OGRErr OGRILI1Layer::CreateField(const OGRFieldDefn *poField, void OGRILI1Layer::JoinGeomLayers() { bGeomsJoined = true; - bool bResetConfigOption = false; - if (EQUAL(CPLGetConfigOption("OGR_ARC_STEPSIZE", ""), "")) - { - bResetConfigOption = true; - CPLSetThreadLocalConfigOption("OGR_ARC_STEPSIZE", "0.96"); - } + + CPLConfigOptionSetter oSetter("OGR_ARC_STEPSIZE", "0.96", + /* bSetOnlyIfUndefined = */ true); for (GeomFieldInfos::const_iterator it = oGeomFieldInfos.begin(); it != oGeomFieldInfos.end(); ++it) @@ -477,9 +474,6 @@ void OGRILI1Layer::JoinGeomLayers() } } } - - if (bResetConfigOption) - CPLSetThreadLocalConfigOption("OGR_ARC_STEPSIZE", nullptr); } void OGRILI1Layer::JoinSurfaceLayer(OGRILI1Layer *poSurfaceLineLayer, diff --git a/port/cpl_known_config_options.h b/port/cpl_known_config_options.h index 942f4612ce1b..cbac9be2bbde 100644 --- a/port/cpl_known_config_options.h +++ b/port/cpl_known_config_options.h @@ -663,7 +663,7 @@ constexpr static const char* const apszKnownConfigOptions[] = "OGR_API_SPY_SNAPSHOT_PATH", // from ograpispy.cpp "OGR_APPLY_GEOM_SET_PRECISION", // from ogr2ogr_lib.cpp, ogrlayer.cpp "OGR_ARC_MAX_GAP", // from ogrgeometryfactory.cpp - "OGR_ARC_STEPSIZE", // from gml2ogrgeometry.cpp, ogrdgnv8layer.cpp, ogrgeometryfactory.cpp, ogrili1datasource.cpp, ogrili1layer.cpp, ogrpgeogeometry.cpp + "OGR_ARC_STEPSIZE", // from ogrgeometryfactory.cpp "OGR_ARROW_COMPUTE_GEOMETRY_TYPE", // from ogrfeatherlayer.cpp "OGR_ARROW_LOAD_FILE_SYSTEM_FACTORIES", // from ogrfeatherdriver.cpp "OGR_ARROW_MEM_LIMIT", // from ograrrowarrayhelper.cpp