Ifcopenshell: IfcGeom::Kernel::wire_intersections fails for occt 7.4.0

[tpaviot]

latest ifcopenshell python from v0.6.0 branch fails at loading/displaying any ifc file. No debug information, only a segfault.

After an investigation using valgrind on Unbuntu 18.04, the method
```c++
IfcGeom::Kernel::wire_intersections

seems to be the cause. valgrind reports the failure at the line 

```c++
GeomAPI_ExtremaCurveCurve ecc(
                BRep_Tool::Curve(wd->Edge(i + 1), u11, u12),
                BRep_Tool::Curve(wd->Edge(j + 1), u21, u22)
            );

I modified this function to return false in any case, and could load any 3d representation as it used to work with previous versions.

[sanderboer]

I think I fixed it by changing the class instantiation to:

static Handle(Geom_Curve) c0 = BRep_Tool::Curve(wd->Edge(i + 1), u11, u12); static Handle(Geom_Curve) c1 = BRep_Tool::Curve(wd->Edge(j + 1), u21, u22); GeomAPI_ExtremaCurveCurve ecc(c0, c1);`

I had this issue crash blender and freecad when importing a door I made in freecad.

segfault_door.zip

[aothms]

Interesting. I haven't had a chance yet to look into 7.4.

According to the Docs:

Curve()
Returns the 3D curve of the edge. May be a Null handle. In and the parameter range. It can be a copy if there is a Location.

The copy in case of location could be a clue, but since the Handle should behave like a smart pointer it shouldn't make a difference.

@sanderboer does the static make a difference or is a normal local variable sufficient?

@tpaviot would be great if you can verify the fix from @sanderboer on your files

[sanderboer]

occt refs:

GeomAPI_ExtremaCurveCurve (const Handle< Geom_Curve > &C1, const Handle< Geom_Curve > &C2)

static Handle< Geom_Curve > Curve (const TopoDS_Edge &E, Standard_Real &First, Standard_Real &Last)

I tried omitting the 'static' storage class specifiers alltogether, segfault, replaced it with 'const', segfault. Only the 'thread_local' and 'static' specifiers don't segfault.

I am inclined to think this is not occt related, but threads related. as static and thread_local both declare duration according to the cppref cppref

[sanderboer]

Freecad:

The exported ifc used to crash blender when importing into blender:

no more !
=)

[tpaviot]

@aothms yep @sanderboer's fix works perfectly (tested on Ubuntu 18.04)

[sanderboer]

FYI, I did some digging and I was told "const does not bind to rvalues".

see here for the answer to my stackexchange question

I checked 6.9.1 docs and it appears the GeomExtremaCurveCurve object interface has not changed, but maybe they implemented some move semantics under the hood in 7.4.0 where this becomes critical ?

[tpaviot]

@sanderboer could you create a PR including this fix for merge into v0.6.0 branch ?

[sanderboer]

I did so a couple of days ago, please review Thomas' concerns.

https://github.com/IfcOpenShell/IfcOpenShell/pull/784

[aothms]

@sanderboer @tpaviot I stepped through the entire occt algorithm and I think I found the culprit. In case of parallel edges the ExtCC class no longer stores the points (and therefore parameters). It only stores the squared distance (mySqDist, which in a way makes sense). NbSolutions is based on the size of mySqDist but since the Points and Parameters are not stored, the subsequent call to Parameters() fails. Somehow this is a change in behaviour with previous versions of occt. I added a condition based on IsParallel https://github.com/IfcOpenShell/IfcOpenShell/commit/df81490f86798153f253d55cfec9231cf5a5fabe

If you have time would be great if you can verify.

[sanderboer]

Surely a hero's journey. It is said that many who step into the occt code are never heard from again...

[sanderboer]

The testfile convert completes succesfully.

[aothms]

Thanks for testing.