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
255 stars 128 forks source link

ShapeUpgrade_UnifySameDomain causes a freeze #483

Open santiagoIT opened 4 months ago

santiagoIT commented 4 months ago

Opening the attached IFC is not possible using the self-compiled XBim-Explorer (developer) branch.

During opening the program hangs and after a long wait the process dies.

What is causing the issue is a particular IfcBeam (#60042): image

The problem happens when the ShapeUpgrade_UnifySameDomain is executed: image1

if that line is commented out, the file opens fine.

This is the precise line where the program seems to hang forever:

image

In this case the solid is the result of calculated CSG solids. Is it really necessary to unify faces, edges after a boolean operation? I have zero experience with Open-Cascade, but from experience with other modelers it seems odd to me.

I would expect to do something like that if a BREP is read in from an IFC file. In that case it makes sense to unify faces/edges.

Would be great to have some feedback. Ideally it is not always necessary to perform the face/edge cleanup.

A side remark:

The last oficial release of the XbimViewer (v4 I believe) opens the file just fine.

This is how the beam looks like (using a different IFC Viewer):

image

Let me know if I should try to look deeper into the problem. Maybe I am able figure something out, or if I hope it is not always necessary to do the face/edge cleanup. That is also something I could implement if you help me understand when not to execute it.

testFile.zip

andyward commented 4 months ago

Perhaps @Ibrahim5aad can shed some light?

Ibrahim5aad commented 3 months ago

Hi @santiagoIT, sorry I wasn't able to pick this earlier. Sadly, this is a long-standing issue for the ShapeUpgrade_UnifySameDomain algorithm, for some particular topologies and under some fuzzy tolerances in the Boolean operation, the output shape can cause the the unifier to hang and get stuck in some infinite loop. We provided some problematic shape before to a tracking ticket on OCC tracker, but it is still an open issue: https://tracker.dev.opencascade.org/view.php?id=32949

One thing you can do is to tweak the fuzzy tolerance of the Boolean operation a little bit . You don't need to dig into the codebase for that. The V5 engine have some application setting called FuzzyFactor that you can use. It is a factor that the model tolerance is multiplied with and used as the fuzzy tolerance of the Boolean operation.

image

The default value is 10.

image

I tried to set it to 1 and I got the IfcBeam (#60042) processed fine. I hope this can be of help to you.

santiagoIT commented 3 months ago

@Ibrahim5aad @andyward Thank you for the valuable info. Indeed, by setting the FuzzyFactor to 1 the file opens fine.

Would it make sense to expose the public static values in XbimGeometryCreator to managed code, so that they can be set/modified at runtime? My suggestion would be to expand: IXbimGeometryCreator with the following properties:

static int BooleanTimeOut;
static double FuzzyFactor;
static double LinearDeflectionInMM;
static double AngularDeflectionInRadians;
static bool IgnoreIfcSweptDiskSolidParams;

Then managed code could set those values. That would be beneficial for us since we provide a class library.

Let me know your thoughts/suggestions, I could then create a pull request.

santiagoIT commented 3 months ago

Now that I think about it further, the better approach would be add a static class: XbimGeometryCreatorSettings to Xbim.Geometry.Engine and expose the properties there.

andyward commented 2 months ago

Hi @santiagoIT

Agreed I'd avoid extending IXbimGeometryCreator - partly since the implementation may not be singleton and the lifetime of the GE is not always in the user's control. So you'd potentially find your setting being missed out

A static settings class would work - albeit there may be edge-case concurrency issues (but note the GE is not fully re-entrant anyway).

In the net6 branch I would probably adopt the Options pattern and have an IOptionsMonitor<GeometrySettings> passed in the GE constructor. That way you could change the settings dynamically at runtime.