xBimTeam / XbimGeometry

XbimGeometry contains the CLR interop libraries and the c++ engine used to compute the 3D geometry of models.
https://xbimteam.github.io/
Other
254 stars 128 forks source link

calls to IXbimSolid.Intersection are affected by previous calls of that method. #382

Open ido-cu opened 2 years ago

ido-cu commented 2 years ago

calls to IXbimSolid.Intersection to some elements may change the results of independent calls to IXbimSolid.Intersection.

Assemblies and versions affected:

Xbim.Essentials Version=5.1.323 Xbim.Geometry Version=5.1.403

Steps (or code) to reproduce the issue:

IntersectionTest.zip

I added a testing app in C# (.net 6). When running it as it is - the "corrupted" intersection result will return zero intersections. When commenting the first intersection (for problemElement) - the "corrupted" intersection result will return 1 intersections.

Minimal file to reproduce the issue:

BREP files for reproduction are attached within the testing app.

Expected behavior:

Independent calls to IXbimSolid.Intersection will not affect each other.

Actual behavior or exception details:

Independent calls to IXbimSolid.Intersection change the results of other calls.

CBenghi commented 2 years ago

Hello @ido-cu, I can reproduce and confirm the issue. looking at our code it looks like we pass the parameters directly to OCCT to perform the intersection, it is opencascade that modifies the parameters passed. Indeed boolean operations have a SetNonDestructive() method that prevents parameters from being modified. I'm not sure if changing this might impact the performance of the entire geometry engine, as a lot of memory copies might be happening for boolean operations.

CBenghi commented 2 years ago

Hi again,

that's confirmed, Boolean operations are, by default, potentially altering the input shapes in occt. By adding SetNonDestructive(Standard_True); to the call we can prevent this, but this feels like it might impact the behaviour the the engine a lot, I need input from @SteveLockley before changing this.

Best, Claudio

CBenghi commented 2 years ago

One last thing, if you need to test the fix for yourself, here's a minimal patch:

diff --git a/Xbim.Geometry.Engine/OCC/src/BRepAlgoAPI/BRepAlgoAPI_Common.cxx b/Xbim.Geometry.Engine/OCC/src/BRepAlgoAPI/BRepAlgoAPI_Common.cxx
index 5060b995..b0ddb65a 100644
--- a/Xbim.Geometry.Engine/OCC/src/BRepAlgoAPI/BRepAlgoAPI_Common.cxx
+++ b/Xbim.Geometry.Engine/OCC/src/BRepAlgoAPI/BRepAlgoAPI_Common.cxx
@@ -56,6 +56,7 @@ BRepAlgoAPI_Common::BRepAlgoAPI_Common(const TopoDS_Shape& S1,
                                        const Message_ProgressRange& theRange)
 : BRepAlgoAPI_BooleanOperation(S1, S2, BOPAlgo_COMMON)
 {
+  SetNonDestructive(Standard_True);
   Build(theRange);
 }
 //=======================================================================
CBenghi commented 2 years ago

See https://github.com/xBimTeam/XbimGeometry/commit/5431db58f26b682b40ffaca56533171647ce7c9c for added test case in a branch.

ido-cu commented 2 years ago

Ok, thanks! Do you know when you'll make the decision whether you're going to fix it or not? and if you're going to fix it, when will you release a new version?

CBenghi commented 2 years ago

Not easy to plan on that. I suggest that you reload the original solid if you have to operate multiple independent operations.

Il Ven 6 Mag 2022, 16:53 ido-cu @.***> ha scritto:

Ok, thanks! Do you know when you'll make the decision whether you're going to fix it or not? and if you're going to fix it, when will you release a new version?

— Reply to this email directly, view it on GitHub https://github.com/xBimTeam/XbimGeometry/issues/382#issuecomment-1119708715, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJY7MOAAOJCMP6IAWDX7GDVIUW6TANCNFSM5U6THEUQ . You are receiving this because you commented.Message ID: @.***>

ido-cu commented 2 years ago

ok, I'll try that.

should I leave this issue opened or close it?

CBenghi commented 2 years ago

Leave it open, thanks

Il Sab 7 Mag 2022, 19:01 ido-cu @.***> ha scritto:

ok, I'll try that. should I leave this issue opened or close it?

— Reply to this email directly, view it on GitHub https://github.com/xBimTeam/XbimGeometry/issues/382#issuecomment-1120241466, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJY7MMNV3XZCOMN3WDUSJTVI2OXPANCNFSM5U6THEUQ . You are receiving this because you commented.Message ID: @.***>