buildingSMART / NextGen-IFC

61 stars 4 forks source link

Inline faces in IfcPolygonalFaceSet instead of an external IfcIndexedPolygonalFace #71

Open Moult opened 3 years ago

Moult commented 3 years ago

Description of the proposal:

Consider the following snippet - why can't the IfcIndexedPolygonalFace be in-lined (similar to IfcCartesianPointList3D into IfcPolygonalFaceSet? Wouldn't that be much, much more efficient?

#15=IFCINDEXEDPOLYGONALFACE((2,1,4,5));
#16=IFCINDEXEDPOLYGONALFACE((1,2,8,3));
#17=IFCINDEXEDPOLYGONALFACE((1,3,6,4));
#18=IFCINDEXEDPOLYGONALFACE((4,6,7,5));
#19=IFCINDEXEDPOLYGONALFACE((3,8,7,6));
#20=IFCINDEXEDPOLYGONALFACE((8,2,5,7));
#21=IFCCARTESIANPOINTLIST3D(((265.,4.,0.),(265.,6.,0.),(277.,4.,0.),(265.,4.,2.),(265.,6.,2.),(277.,4.,2.),(277.,6.,2.),(277.,6.,0.)));
#22=IFCPOLYGONALFACESET(#21,$,(#15,#16,#17,#18,#19,#20),$);
#23=IFCSHAPEREPRESENTATION(#10,'Body','Tessellation',(#22));

The proposal:

#21=IFCCARTESIANPOINTLIST3D(((265.,4.,0.),(265.,6.,0.),(277.,4.,0.),(265.,4.,2.),(265.,6.,2.),(277.,4.,2.),(277.,6.,2.),(277.,6.,0.)));
#22=IFCPOLYGONALFACESET(#21,$,((2,1,4,5),(1,2,8,3),(1,3,6,4),(4,6,7,5),(3,8,7,6),(8,2,5,7)),$);
#23=IFCSHAPEREPRESENTATION(#10,'Body','Tessellation',(#22));

The proposal is 58% of the original size in bytes.

Describe how it contributes to the objectives (https://github.com/buildingSMART/NextGen-IFC/wiki/Towards-a-technology-independent-IFC): Simpler structure, same outcome.

Is this a proposal to 'add', 'remove' of 'change' entities in the schema (pick one): CHANGE+REMOVE

What do we win: Files shrink to half their filesize otherwise.

What do we lose: nothing?

Schema impact: Change the data type of one of the attributes

Instance model impact: NA

Backwards compatible: No

Automatic migration possible: Yes

Additional implications: NA

- Note that not all points need to be satisfied! Backwards compatibility and file size are not concerns.

jmirtsch commented 3 years ago

The faces are inline for Triangulate Face Set, but I would suggest the logic for polygonal is that a face can have inner loops via the subclass https://standards.buildingsmart.org/IFC/DEV/IFC4_3/RC1/HTML/link/ifcindexedpolygonalfacewithvoids.htm

It is possible that a list of lists of integer could be used, I do agree there would be a significant efficiency in doing this.

Moult commented 3 years ago

Considering the relative rarity of mesh programs supporting inner loops (perhaps sketchup is one), I propose: IfcPolygonalFaceSet and IfcPolygonlFaceSetWithVoids. The former is as I have proposed, the latter could be (an off-the-top-of-my-head suggestion):

#22=IFCPOLYGONALFACESETWITHVOIDS(#21,$,((2,1,4,5),(1,2,8,3),(1,3,6,4),(4,6,7,5),(3,8,7,6),(8,2,5,7)),$,(1,3),((A,B,C,D),(E,F,G,H)));

2 added attributes are:

The filesize savings are too great to be ignored :)

Moult commented 3 years ago

My little snippet yesterday doesn't take into account multiple inner loops, but you get the idea :)

TLiebich commented 3 years ago

Thanks @Moult - its a good and valid proposal. Actually, when developing support for tessellation in IFC4, there was one proposal to only use a single instance to exchange a polygonal face set (even with voids), using a multidimensional list of integers. Leading to:

`

22=IFCPOLYGONALFACESET(#21,$,(((2,1,4,5)),((1,2,8,3),(5,4,3,2)),((1,3,6,4)),((4,6,7,5)),((3,8,7,6)),((8,2,5,7))),$);

`

It was then not used fearing that a three-dimensional list would be too complex. The inner loop was mandatory in order to use it to superseed IFCFACETEDBREP which offers this possibility. But splitting it into a simple polygonal face set without inner loops and one with inner loops makes sense.

hlg commented 3 years ago

... be in-lined similar to IfcCartesianPointList3D into IfcPolygonalFaceSet

Where is IfcCartesianPointList3D "inlined"? I probably missed or misunderstood something.

It was then not used fearing that a three-dimensional list would be too complex.

I think it is confirmed that addition of two-dimensional arrays in IFC4 definitely caused substantial implementation and refactoring efforts in existing software. Just want to mention this, even though forward compatibility is not a concern here.

Moult commented 3 years ago

@hlg sorry for the phrasing confusion, I meant that IfcCartesianPointList3D has in-lined vertex coordinates.

aothms commented 3 years ago

I agree with @TLiebich that the nested lists is a effective way to keep the pairing between facets and inner loops. Like so:

TYPE IfcIndexedPolygonalFace = LIST[1:?] OF LIST [3:?] OF IfcPositiveInteger; END_TYPE;

Essentially this in "indexed WKT". https://en.wikipedia.org/wiki/Well-known_text_representation_of_geometry WKT also has a polygon definition as a sequence outer followed by inner bounds.

As said elsewhere we loose the two inherited inverse attributes:

LayerAssignment : SET [0:1] OF IfcPresentationLayerAssignment FOR AssignedItems; StyledByItem : SET [0:1] OF IfcStyledItem FOR Item;

So it's a question of whether styling needs to be supported on individual facets. We have the IfcIndexedColourMap obviously so it's partly redundant, but style > colour. And now it's unclear how IfcIndexedColourMap will interplay with StyledByItem so removing StyledByItem is a quick way to iron that out.