IfcAdvancedBrep for example file cube-advanced-brep

[christophKaiser]

In this pull request, IfcAdvancedBrep will be implemented to run the example file cube-advanced-brep.ifc.
This includes the implementation and adjustment of a few other functions, too. A rough overview of the involved functions was given in a comment in the corresponding issue #60.

Finally, this PR will close #60.
Probably, issue #58 could be closed, as well. However, this will be investigated at the right time in future. [currently, the non-linked issue is intended]

UPDATE 21.02.2022: This PR will close #58, as well.

[christophKaiser]

While I have analysed the function convertIfcFaceList in commit 346e611 and 032606c, I came over a 'TODO Stefan':

Open-Infra-Platform/Core/src/IfcGeometryConverter/FaceConverter.h

Lines 750 to 766 in 1f1fc35

	// Loop through all faces
	for (auto it = faces.cbegin(); it != faces.cend(); ++it)
	{ //TODO Stefan
	EXPRESSReference<typename IfcEntityTypesT::IfcFace> face = (*it);
	if (face.expired()) {
	throw oip::ReferenceExpiredException(face);
	}
	if (!convertIfcFace(face, pos, polygon, polygonIndices))
	{
	throw oip::InconsistentGeometryException(face, "IFC Face conversion failed with these faces");
	}
	}
	// IfcFaceList can be a closed or open shell, so let the calling function decide where to put it
	itemData->open_or_closed_polyhedrons.push_back(polygon);

(Note: The cited code is without the new changes.)

It was added in commit 2aec12f last summer. However, I wasn't able to find more information about that.
Currently, the function does its purposed task.

@pjanck Should I leave that comment there for the moment, or can you give me more information about it?

My next task in this PR will be an analyse of the function convertIfcFace. This will lead in a (probably) small refactoring of this function and the implementation of its subtype IfcFaceSurface, which is the supertype of IfcAdvancedFace (which I will need at the end).

[pjanck]

Good analysis of the task!

1. About the TODO Stefan: no idea what I meant there. If the function does its task, you can safely delete it.
2. You can change the for loop to for( const auto& face : faces ) - perhaps that was the only purpose of the TODO?
3. convertIfcFace - it seems that the function returns bool - this can be changed to be more in parallel with other functions. I trust you here to judge by yourself what to do. Perhaps it is needed, that a bool is returned. In case of doubt, ask.

[pjanck]

Good start!

[christophKaiser]

[Refactoring of convertIfcFace is still in process.]

Like promised in commit 3ae4b05 “Fixed semi-bug of double triangulation of triangular ifcFaceBound”, this comment gives more details about the fixed issue – especially screenshots of an example.

Before this commit, faces with an exactly triangular bound loop (ifcFaceBound) were added twice to the variable polygon (of type PolyhedronData). First, the triangle face was added in the if-block if (loopVertices3D.size() == 3) (in convertIfcFace). The second adding occurred inside of the function triangulateFace which is called in convertIfcFace. This became implicitly visible by the variable polygon.faceCount, respectively polygon.faceIndices.
Moreover, the first adding has not gave respect to holes inside the outer triangle bound, respectively they were covered by the first added triangle face as shown in the following screenshot:

After this fix, the code decides between the case of only one triangular bound loop without an inner bound (= hole), and the case of an arbitrary number of vertices and / or holes inside the face. Now, the hole in my example is visible:

The relevant ifc-data of my example:

#38= IFCFACE((#202,#203));

#202= IFCFACEOUTERBOUND(#366,.T.);
#203= IFCFACEBOUND(#367,.T.);

#366= IFCPOLYLOOP((#527,#526,#606));
#367= IFCPOLYLOOP((#528,#608,#529));

#526= IFCCARTESIANPOINT((0.,0.,0.));
#527= IFCCARTESIANPOINT((1000.,0.,0.));
#606= IFCCARTESIANPOINT((0.,1000.,0.));

#528= IFCCARTESIANPOINT((250.,250.,0.));
#529= IFCCARTESIANPOINT((500.,250.,0.));
#608= IFCCARTESIANPOINT((250.,500.,0.));

[jschlenger]

Good start. Excellent description of what needs to be supported in order to be able to display the example file.

[pjanck]

Good work so far!

[jschlenger]

Looks very good to me.
We should merge this pull request soon.

[pjanck]

Please mark it as ready for review, then I'll go through the code once more.

[christophKaiser]

Commit ac64eb1 updates the Doxygen comments, which are related to the parameters polyhedron and polyhedronIndices.

A detailed explanation about these parameters is in the description of the function convertIfcFace. As shown in the screenshot below, this section is separated with a bold header:

Furthermore, all functions which have polyhedron and polyhedronIndices as parameter refer to this description by the Doxygen command \see. This generates the section 'See also', as shown in the screenshot:

[pjanck]

Great work!

Small suggestions, but this can definitely be merged.

[jschlenger]

I believe we can merge after the last few comments of pjanck are resolved

[pjanck]

Awesome work!