From 063ebb46667bb0e6098fac1e2bb58750951fdc00 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Wed, 20 Nov 2024 02:31:57 +0100 Subject: [PATCH] DXF writer: do not set 0 as the value for DXF code 5 (HANDLE) as it is apparently a reserved value. Fixes #11299 --- autotest/ogr/ogr_dxf.py | 8 ++++++++ ogr/ogrsf_frmts/dxf/ogr_dxf.h | 4 ++-- ogr/ogrsf_frmts/dxf/ogrdxfwriterds.cpp | 25 ++++++++++++++--------- ogr/ogrsf_frmts/dxf/ogrdxfwriterlayer.cpp | 4 ++-- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/autotest/ogr/ogr_dxf.py b/autotest/ogr/ogr_dxf.py index 1cd665593482..3075dfdb510c 100644 --- a/autotest/ogr/ogr_dxf.py +++ b/autotest/ogr/ogr_dxf.py @@ -436,20 +436,28 @@ def test_ogr_dxf_12(tmp_path): dst_feat = ogr.Feature(feature_def=lyr.GetLayerDefn()) dst_feat.SetGeometryDirectly(ogr.CreateGeometryFromWkt("LINESTRING(10 12, 60 65)")) + dst_feat.SetFID(0) lyr.CreateFeature(dst_feat) + # 80 is the minimum handle value we set in case of inapproriate or unsed + # initial FID value + assert dst_feat.GetFID() >= 80 dst_feat = None dst_feat = ogr.Feature(feature_def=lyr.GetLayerDefn()) dst_feat.SetGeometryDirectly( ogr.CreateGeometryFromWkt("POLYGON((0 0,100 0,100 100,0 0))") ) + dst_feat.SetFID(79) lyr.CreateFeature(dst_feat) + assert dst_feat.GetFID() == 79 dst_feat = None # Test 25D linestring with constant Z (#5210) dst_feat = ogr.Feature(feature_def=lyr.GetLayerDefn()) dst_feat.SetGeometryDirectly(ogr.CreateGeometryFromWkt("LINESTRING(1 2 10,3 4 10)")) + dst_feat.SetFID(79) lyr.CreateFeature(dst_feat) + assert dst_feat.GetFID() > 79 dst_feat = None # Test 25D linestring with different Z (#5210) diff --git a/ogr/ogrsf_frmts/dxf/ogr_dxf.h b/ogr/ogrsf_frmts/dxf/ogr_dxf.h index b8793c9cb88b..e26ed1395580 100644 --- a/ogr/ogrsf_frmts/dxf/ogr_dxf.h +++ b/ogr/ogrsf_frmts/dxf/ogr_dxf.h @@ -972,8 +972,8 @@ class OGRDXFWriterDS final : public GDALDataset CSLConstList papszOptions) override; bool CheckEntityID(const char *pszEntityID); - bool WriteEntityID(VSILFILE *fp, long &nAssignedFID, - long nPreferredFID = OGRNullFID); + bool WriteEntityID(VSILFILE *fp, unsigned int &nAssignedFID, + GIntBig nPreferredFID = OGRNullFID); void UpdateExtent(OGREnvelope *psEnvelope); }; diff --git a/ogr/ogrsf_frmts/dxf/ogrdxfwriterds.cpp b/ogr/ogrsf_frmts/dxf/ogrdxfwriterds.cpp index 7db9da389420..a5775957e042 100644 --- a/ogr/ogrsf_frmts/dxf/ogrdxfwriterds.cpp +++ b/ogr/ogrsf_frmts/dxf/ogrdxfwriterds.cpp @@ -16,6 +16,7 @@ #include #include +#include #include "ogr_dxf.h" #include "cpl_conv.h" @@ -680,7 +681,7 @@ bool OGRDXFWriterDS::WriteNewLayerDefinitions(VSILFILE *fpOut) } else if (anDefaultLayerCode[i] == 5) { - long nIgnored; + unsigned int nIgnored; if (!WriteEntityID(fpOut, nIgnored)) return false; } @@ -723,7 +724,7 @@ bool OGRDXFWriterDS::WriteNewLineTypeRecords(VSILFILE *fpIn) for (const auto &oPair : oNewLineTypes) { bRet &= WriteValue(fpIn, 0, "LTYPE"); - long nIgnored; + unsigned int nIgnored; bRet &= WriteEntityID(fpIn, nIgnored); bRet &= WriteValue(fpIn, 100, "AcDbSymbolTableRecord"); bRet &= WriteValue(fpIn, 100, "AcDbLinetypeTableRecord"); @@ -764,7 +765,7 @@ bool OGRDXFWriterDS::WriteNewTextStyleRecords(VSILFILE *fpIn) for (auto &oPair : oNewTextStyles) { bRet &= WriteValue(fpIn, 0, "STYLE"); - long nIgnored; + unsigned int nIgnored; bRet &= WriteEntityID(fpIn, nIgnored); bRet &= WriteValue(fpIn, 100, "AcDbSymbolTableRecord"); bRet &= WriteValue(fpIn, 100, "AcDbTextStyleTableRecord"); @@ -839,7 +840,7 @@ bool OGRDXFWriterDS::WriteNewBlockRecords(VSILFILE *fpIn) /* -------------------------------------------------------------------- */ bRet &= WriteValue(fpIn, 0, "BLOCK_RECORD"); - long nIgnored; + unsigned int nIgnored; bRet &= WriteEntityID(fpIn, nIgnored); bRet &= WriteValue(fpIn, 100, "AcDbSymbolTableRecord"); bRet &= WriteValue(fpIn, 100, "AcDbBlockTableRecord"); @@ -888,7 +889,7 @@ bool OGRDXFWriterDS::WriteNewBlockDefinitions(VSILFILE *fpIn) poThisBlockFeat->GetFieldAsString("Block")); bRet &= WriteValue(fpIn, 0, "BLOCK"); - long nIgnored; + unsigned int nIgnored; bRet &= WriteEntityID(fpIn, nIgnored); bRet &= WriteValue(fpIn, 100, "AcDbEntity"); if (strlen(poThisBlockFeat->GetFieldAsString("Layer")) > 0) @@ -1027,22 +1028,26 @@ bool OGRDXFWriterDS::CheckEntityID(const char *pszEntityID) /* WriteEntityID() */ /************************************************************************/ -bool OGRDXFWriterDS::WriteEntityID(VSILFILE *fpIn, long &nAssignedFID, - long nPreferredFID) +bool OGRDXFWriterDS::WriteEntityID(VSILFILE *fpIn, unsigned int &nAssignedFID, + GIntBig nPreferredFID) { CPLString osEntityID; - if (nPreferredFID != OGRNullFID) + // From https://github.com/OSGeo/gdal/issues/11299 it seems that 0 is an + // invalid handle value. + if (nPreferredFID > 0 && + nPreferredFID <= + static_cast(std::numeric_limits::max())) { - osEntityID.Printf("%X", (unsigned int)nPreferredFID); + osEntityID.Printf("%X", static_cast(nPreferredFID)); if (!CheckEntityID(osEntityID)) { aosUsedEntities.insert(osEntityID); if (!WriteValue(fpIn, 5, osEntityID)) return false; - nAssignedFID = nPreferredFID; + nAssignedFID = static_cast(nPreferredFID); return true; } } diff --git a/ogr/ogrsf_frmts/dxf/ogrdxfwriterlayer.cpp b/ogr/ogrsf_frmts/dxf/ogrdxfwriterlayer.cpp index 499da0bbe640..923ac2e1907e 100644 --- a/ogr/ogrsf_frmts/dxf/ogrdxfwriterlayer.cpp +++ b/ogr/ogrsf_frmts/dxf/ogrdxfwriterlayer.cpp @@ -169,8 +169,8 @@ OGRErr OGRDXFWriterLayer::WriteCore(OGRFeature *poFeature) /* Also, for reasons I don't understand these ids seem to have */ /* to start somewhere around 0x50 hex (80 decimal). */ /* -------------------------------------------------------------------- */ - long nGotFID = -1; - poDS->WriteEntityID(fp, nGotFID, (int)poFeature->GetFID()); + unsigned int nGotFID = 0; + poDS->WriteEntityID(fp, nGotFID, poFeature->GetFID()); poFeature->SetFID(nGotFID); WriteValue(100, "AcDbEntity");