Merge pull request #11304 from rouault/fix_11299

DXF writer: do not set 0 as the value for DXF code 5 (HANDLE)
OSGeo · Nov 20, 2024 · bf808a0 · bf808a0
2 parents 52850a4 + 5ecc163
commit bf808a0
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 14 deletions.
diff --git 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
@@ -975,8 +975,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
@@ -16,6 +16,7 @@
 
 #include <cmath>
 #include <cstdlib>
+#include <limits>
 
 #include "ogr_dxf.h"
 #include "cpl_conv.h"
@@ -737,7 +738,7 @@ bool OGRDXFWriterDS::WriteNewLayerDefinitions(VSILFILE *fpOut)
             }
             else if (anDefaultLayerCode[i] == 5)
             {
-                long nIgnored;
+                unsigned int nIgnored;
                 if (!WriteEntityID(fpOut, nIgnored))
                     return false;
             }
@@ -780,7 +781,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");
@@ -821,7 +822,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");
@@ -896,7 +897,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");
@@ -945,7 +946,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)
@@ -1084,22 +1085,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<GIntBig>(std::numeric_limits<unsigned int>::max()))
     {
 
-        osEntityID.Printf("%X", (unsigned int)nPreferredFID);
+        osEntityID.Printf("%X", static_cast<unsigned int>(nPreferredFID));
         if (!CheckEntityID(osEntityID))
         {
             aosUsedEntities.insert(osEntityID);
             if (!WriteValue(fpIn, 5, osEntityID))
                 return false;
-            nAssignedFID = nPreferredFID;
+            nAssignedFID = static_cast<unsigned int>(nPreferredFID);
             return true;
         }
     }

diff --git 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");