From dd95ac79a58b31d8b6cc22f60a37c96cc7636302 Mon Sep 17 00:00:00 2001
From: Christophe Geuzaine <cgeuzaine@uliege.be>
Date: Thu, 12 May 2022 17:38:54 +0200
Subject: [PATCH] new Geometry.OCCSafeUnbind to disable fast unbind
 optimization (e.g. for tag backward compatibility): cf. #1928

---
 CHANGELOG.txt               |  6 +++--
 examples/api/crack.py       |  2 +-
 examples/api/crack3d.py     |  2 +-
 src/common/Context.h        |  2 +-
 src/common/DefaultOptions.h |  3 +++
 src/common/Options.cpp      |  6 +++++
 src/common/Options.h        |  1 +
 src/geo/GModelIO_OCC.cpp    | 54 ++++++++++++++++++-------------------
 8 files changed, 44 insertions(+), 32 deletions(-)

diff --git a/CHANGELOG.txt b/CHANGELOG.txt
index 93000cfb24..7b59f21d18 100644
--- a/CHANGELOG.txt
+++ b/CHANGELOG.txt
@@ -1,6 +1,8 @@
 4.10.2 (Work-in-progress): fixed regression introduced in 4.9 for recombined
-meshes with boundary layers; new HealShapes command in .geo files; simplified
-calculation of OCC STL bounding boxes; generalized Crack plugin; small bug fixes.
+meshes with boundary layers; new Geometry.OCCSafeUnbind option to disable
+boolean optimization introduced in 4.10.0 (for backward compatibility); new
+HealShapes command in .geo files; simplified calculation of OCC STL bounding
+boxes; generalized Crack plugin; small bug fixes.
 
 4.10.1 (May 1, 2022): small bug fixes.
 
diff --git a/examples/api/crack.py b/examples/api/crack.py
index 9082ea28b3..a391a11a6a 100644
--- a/examples/api/crack.py
+++ b/examples/api/crack.py
@@ -31,7 +31,7 @@ gmsh.model.addPhysicalGroup(1, new_lines, 101)
 gmsh.model.mesh.generate(2)
 
 gmsh.plugin.setNumber("Crack", "PhysicalGroup", 101)
-# gmsh.plugin.setNumber("Crack", "DebugView", 1)
+gmsh.plugin.setNumber("Crack", "DebugView", 1)
 gmsh.plugin.run("Crack")
 
 # save all the elements in the mesh (even those that do not belong to any
diff --git a/examples/api/crack3d.py b/examples/api/crack3d.py
index f98166287c..620319ca34 100644
--- a/examples/api/crack3d.py
+++ b/examples/api/crack3d.py
@@ -28,7 +28,7 @@ gmsh.model.mesh.generate(3)
 # "crack" the mesh by duplicating the elements and nodes on the small surface
 gmsh.plugin.setNumber("Crack", "Dimension", 2)
 gmsh.plugin.setNumber("Crack", "PhysicalGroup", phys)
-#gmsh.plugin.setNumber("Crack", "DebugView", 1)
+gmsh.plugin.setNumber("Crack", "DebugView", 1)
 gmsh.plugin.run("Crack")
 
 # save all the elements in the mesh (even those that do not belong to any
diff --git a/src/common/Context.h b/src/common/Context.h
index 3cf6f789d2..50e73b066d 100644
--- a/src/common/Context.h
+++ b/src/common/Context.h
@@ -97,7 +97,7 @@ struct contextGeometryOptions {
   int autoCoherence;
   int autoExtrude; // FIXME: temporary for auto-extrude testing
   double tolerance, toleranceBoolean, snap[3], transform[3][3], offset[3];
-  int occAutoFix, occAutoEmbed;
+  int occAutoFix, occAutoEmbed, occSafeUnbind;
   int occFixDegenerated, occFixSmallEdges, occFixSmallFaces;
   int occSewFaces, occMakeSolids, occParallel, occBooleanPreserveNumbering;
   int occBoundsUseSTL, occDisableSTL, occImportLabels, occUnionUnify;
diff --git a/src/common/DefaultOptions.h b/src/common/DefaultOptions.h
index 539a6e7adb..14bf8b01e9 100644
--- a/src/common/DefaultOptions.h
+++ b/src/common/DefaultOptions.h
@@ -954,6 +954,9 @@ StringXNumber GeometryOptions_Number[] = {
     "OpenCASCADE kernel" },
   { F|O, "OCCParallel" , opt_geometry_occ_parallel , 0. ,
     "Use multi-threaded OpenCASCADE boolean operators" },
+  { F|O, "OCCSafeUnbind" , opt_geometry_occ_safe_unbind , 0. ,
+    "Revert to safe (i.e. with recursive checks on boundaries) unbinding of entities "
+    "in boolean operations and geometrical transformations" },
   { F|O, "OCCScaling" , opt_geometry_occ_scaling , 1. ,
     "Scale STEP, IGES and BRep models by the given factor when importing them with the "
     "OpenCASCADE kernel" },
diff --git a/src/common/Options.cpp b/src/common/Options.cpp
index 674e31f2d4..b592925f01 100644
--- a/src/common/Options.cpp
+++ b/src/common/Options.cpp
@@ -4571,6 +4571,12 @@ double opt_geometry_occ_auto_embed(OPT_ARGS_NUM)
   return CTX::instance()->geom.occAutoEmbed;
 }
 
+double opt_geometry_occ_safe_unbind(OPT_ARGS_NUM)
+{
+  if(action & GMSH_SET) CTX::instance()->geom.occSafeUnbind = val ? 1 : 0;
+  return CTX::instance()->geom.occSafeUnbind;
+}
+
 double opt_geometry_occ_auto_fix(OPT_ARGS_NUM)
 {
   if(action & GMSH_SET) CTX::instance()->geom.occAutoFix = val ? 1 : 0;
diff --git a/src/common/Options.h b/src/common/Options.h
index 96cc661f6a..de3fb5e0b8 100644
--- a/src/common/Options.h
+++ b/src/common/Options.h
@@ -399,6 +399,7 @@ double opt_geometry_surface_type(OPT_ARGS_NUM);
 double opt_geometry_light(OPT_ARGS_NUM);
 double opt_geometry_light_two_side(OPT_ARGS_NUM);
 double opt_geometry_occ_auto_embed(OPT_ARGS_NUM);
+double opt_geometry_occ_safe_unbind(OPT_ARGS_NUM);
 double opt_geometry_occ_auto_fix(OPT_ARGS_NUM);
 double opt_geometry_occ_bounds_use_stl(OPT_ARGS_NUM);
 double opt_geometry_occ_disable_stl(OPT_ARGS_NUM);
diff --git a/src/geo/GModelIO_OCC.cpp b/src/geo/GModelIO_OCC.cpp
index d99b082d20..ddff7e90f7 100644
--- a/src/geo/GModelIO_OCC.cpp
+++ b/src/geo/GModelIO_OCC.cpp
@@ -142,9 +142,6 @@
 #include <XCAFDoc_ShapeTool.hxx>
 #endif
 
-// define this to deactive the optimizations introduced in #1240:
-// #define SAFE_UNBIND
-
 // for debugging:
 template <class T>
 void writeBrep(const T &shapes, const std::string &fileName = "debug.brep")
@@ -3672,11 +3669,12 @@ bool OCC_Internals::booleanOperator(
       if(remove) {
         int d = inDimTags[i].first;
         int t = inDimTags[i].second;
-#ifdef SAFE_UNBIND
-        if(_isBound(d, t)) _unbind(_find(d, t), d, t, true);
-#else
-        if(_isBound(d, t)) _unbindWithoutChecks(_find(d, t));
-#endif
+        if(_isBound(d, t)) {
+          if(CTX::instance()->geom.occSafeUnbind)
+            _unbind(_find(d, t), d, t, true);
+          else
+            _unbindWithoutChecks(_find(d, t));
+        }
       }
     }
     _multiBind(result, tag, outDimTags, (tag >= 0) ? true : false, true,
@@ -3694,11 +3692,12 @@ bool OCC_Internals::booleanOperator(
       int tag = inDimTags[i].second;
       bool remove = (i < numObjects) ? removeObject : removeTool;
       if(mapDeleted[i]) { // deleted
-#ifdef SAFE_UNBIND
-        if(remove) _unbind(mapOriginal[i], dim, tag, true);
-#else
-        if(remove) _unbindWithoutChecks(mapOriginal[i]);
-#endif
+        if(remove) {
+          if(CTX::instance()->geom.occSafeUnbind)
+            _unbind(mapOriginal[i], dim, tag, true);
+          else
+            _unbindWithoutChecks(mapOriginal[i]);
+        }
         Msg::Debug("BOOL (%d,%d) deleted", dim, tag);
       }
       else if(mapModified[i].Extent() == 0) { // not modified
@@ -3960,20 +3959,21 @@ bool OCC_Internals::_transform(
   for(std::size_t i = 0; i < inDimTags.size(); i++) {
     int dim = inDimTags[i].first;
     int tag = inDimTags[i].second;
-#ifdef SAFE_UNBIND
-    // safe, but slow: _unbind() has linear complexity with respect to the number
-    // of entities in the model (due to the dependency checking of upward
-    // adjencencies and the maximum tag update). Using this in a for loop to
-    // translate copies of entities leads to quadratic complexity.
-    _unbind(inShapes[i], dim, tag, true);
-#else
-    // bypass it by unbinding the shape and all its subshapes without checking
-    // dependencies: this is a bit dangerous, as one could translate e.g. the
-    // face of a cube (this is not allowed!) - which will unbind the face of the
-    // cube. But the original face will actually be re-bound (with a warning) at
-    // the next syncronization point, so it's not too bad...
-    _unbindWithoutChecks(inShapes[i]);
-#endif
+    if(CTX::instance()->geom.occSafeUnbind) {
+      // safe, but slow: _unbind() has linear complexity with respect to the number
+      // of entities in the model (due to the dependency checking of upward
+      // adjencencies and the maximum tag update). Using this in a for loop to
+      // translate copies of entities leads to quadratic complexity.
+      _unbind(inShapes[i], dim, tag, true);
+    }
+    else {
+      // bypass it by unbinding the shape and all its subshapes without checking
+      // dependencies: this is a bit dangerous, as one could translate e.g. the
+      // face of a cube (this is not allowed!) - which will unbind the face of
+      // the cube. But the original face will actually be re-bound (with a
+      // warning) at the next syncronization point, so it's not too bad...
+      _unbindWithoutChecks(inShapes[i]);
+    }
     // TODO: it would be even better to code a rebind() function to reuse the
     // tags not only of the shape, but of all the sub-shapes as well
     _bind(outShapes[i], dim, tag, true);
-- 
GitLab