diff --git a/docs/coverage.md b/docs/coverage.md index aff1465..84527eb 100644 --- a/docs/coverage.md +++ b/docs/coverage.md @@ -42,7 +42,7 @@ the page cannot quietly disagree with the gate. | Model | Elements | Extrude | Brep | OBB | Empty | OBB rate | Collapsed | AABB ratio p50 | p90 | max | |---|---|---|---|---|---|---|---|---|---|---| | `ifcopenhouse` | 40 | 34 | 0 | 0 | 6 | 0.0% | 0 | 1.0000 | 1.0000 | 1.0000 | -| `duplex_a` | 218 | 146 | 0 | 69 | 3 | 32.1% | 0 | 1.0000 | 1.0000 | 1.1156 | +| `duplex_a` | 218 | 146 | 65 | 4 | 3 | 1.9% | 0 | 1.0000 | 1.0000 | 1.1156 | | `fzk_haus` | 85 | 16 | 64 | 2 | 3 | 2.4% | 0 | 1.0000 | 1.0000 | 1.0334 | ## What falls back, and how often @@ -86,7 +86,6 @@ at least one unhandled item type below. | Item type | Occurrences | |---|---| -| `IFCFACEBASEDSURFACEMODEL` | 65 | | `IFCPOLYGONALBOUNDEDHALFSPACE` | 11 | ## Known Gate 1 violations diff --git a/geometry/brep.go b/geometry/brep.go index 78b83c4..d56675a 100644 --- a/geometry/brep.go +++ b/geometry/brep.go @@ -10,15 +10,23 @@ const ( attrBoundOrientation = 1 // IfcFaceBound.Orientation attrLoopPolygon = 0 // IfcPolyLoop.Polygon attrSbsmBoundary = 0 // IfcShellBasedSurfaceModel.SbsmBoundary + attrFbsmFaces = 0 // IfcFaceBasedSurfaceModel.FbsmFaces ) -// shellBasedSurfaceModelMesh unions the faces of every shell (IfcClosedShell -// or IfcOpenShell) in an IfcShellBasedSurfaceModel's SbsmBoundary set, raw -// units. Used by multi-shell family instances (e.g. a door's frame/leaf/ -// hardware, each its own shell) that mix this representation type with plain -// IfcFacetedBrep siblings. -func shellBasedSurfaceModelMesh(sbsm *step.Instance) (verts []float32, tris []uint32, ok bool) { - boundaryV, has := sbsm.Get(attrSbsmBoundary) +// surfaceModelMesh unions the faces of every shell or face set in a surface +// model's boundary attribute, raw units. +// +// Shared by IfcShellBasedSurfaceModel (SbsmBoundary, a SET of IfcShell) and +// IfcFaceBasedSurfaceModel (FbsmFaces, a SET of IfcConnectedFaceSet). Two +// different schema entities with the same shape: a set of things brepMesh +// already tessellates. attr is a parameter rather than assumed, because the +// two constants agreeing at 0 is a fact about the schema, not a rule. +// +// Used by multi-shell family instances (a door's frame/leaf/hardware, each its +// own shell) that mix these representation types with plain IfcFacetedBrep +// siblings in the same element. +func surfaceModelMesh(m *step.Instance, attr int) (verts []float32, tris []uint32, ok bool) { + boundaryV, has := m.Get(attr) if !has || boundaryV.Kind != step.KindList { return nil, nil, false } diff --git a/geometry/brep_test.go b/geometry/brep_test.go index f135f87..dd457f9 100644 --- a/geometry/brep_test.go +++ b/geometry/brep_test.go @@ -1,6 +1,7 @@ package geometry import ( + "strings" "testing" "github.com/blox-eng/goifc/model" @@ -33,3 +34,71 @@ func TestBrep_FacetedFaces(t *testing.T) { t.Errorf("brep world min = %v, want {10 20 5}", e.BBoxMin) } } + +// IfcFaceBasedSurfaceModel had no dispatch case at all, so every element built +// from one degraded to a bounding box. It is the top entry of the measured gap +// list (65 occurrences), and in duplex_a it hides 235 IfcConnectedFaceSets no +// other path can reach. +func TestFaceBasedSurfaceModel_Tessellates(t *testing.T) { + f, err := step.ParseFile("testdata/synthetic/face_based_surface_model.ifc") + if err != nil { + t.Fatal(err) + } + r, err := model.Extract(f) + if err != nil { + t.Fatal(err) + } + s, err := Build(f, r) + if err != nil { + t.Fatal(err) + } + e := s.Elements[0] + if e.Source != SourceBrep { + t.Fatalf("source = %q, want brep — the element fell back to a box", e.Source) + } + // Two quad faces -> 2 triangles each -> 12 indices, same as known_box.ifc. + if len(e.Tris) != 12 { + t.Errorf("tris = %d indices, want 12", len(e.Tris)) + } + if !closeVec(e.BBoxMin, [3]float64{10, 20, 5}, 1e-6) { + t.Errorf("world min = %v, want {10 20 5}", e.BBoxMin) + } +} + +// An empty or unusable FbsmFaces set must decline so the element falls back to +// its OBB box. Returning an empty mesh that still claims SourceBrep would +// produce a zero-volume element — the "Collapsed" case the coverage page +// exists to surface, reported as a success. +// +// Note what the second case does and does not pin. surfaceModelMesh and +// brepMesh do not type-check set members: brepMesh reads attribute 0 of +// whatever it is handed. An IfcCartesianPoint's attribute 0 is Coordinates, +// which IS a list, so it gets that far — and declines only because the list +// holds reals rather than entity references. So this pins "attribute 0 is not +// a list of face refs", not "the member is rejected for having the wrong +// type". A wrong-typed member whose attribute 0 happened to be a list of +// references would still be walked as a shell. +func TestFaceBasedSurfaceModel_EmptyDeclines(t *testing.T) { + for _, tc := range []struct{ name, faces string }{ + {"empty set", "()"}, + {"member whose attribute 0 is not a list of face refs", "(#22)"}, // an IfcCartesianPoint + } { + t.Run(tc.name, func(t *testing.T) { + f, err := step.Parse(strings.NewReader( + "ISO-10303-21;\nHEADER;\nFILE_SCHEMA(('IFC4'));\nENDSEC;\nDATA;\n" + + "#22=IFCCARTESIANPOINT((0.,0.,0.));\n" + + "#61=IFCFACEBASEDSURFACEMODEL(" + tc.faces + ");\n" + + "ENDSEC;\nEND-ISO-10303-21;\n")) + if err != nil { + t.Fatal(err) + } + item, ok := f.ByID(61) + if !ok { + t.Fatal("fixture instance #61 missing") + } + if _, _, gotOK := surfaceModelMesh(item, attrFbsmFaces); gotOK { + t.Error("surfaceModelMesh accepted an unusable FbsmFaces set; want ok=false so the caller boxes the element") + } + }) + } +} diff --git a/geometry/geometry.go b/geometry/geometry.go index a0a83b5..ba0138b 100644 --- a/geometry/geometry.go +++ b/geometry/geometry.go @@ -120,7 +120,15 @@ func tessellateItemDepth(item *step.Instance, unitScale float64, depth int) ([]f // overall reported Source stayed "brep" (since brep still won on the other // sibling items) — a few stray boxed sub-shells shift the whole element's // AABB by a few mm-cm without ever showing up as a Source mismatch. - if v, t, ok := shellBasedSurfaceModelMesh(item); ok { + if v, t, ok := surfaceModelMesh(item, attrSbsmBoundary); ok { + return scaleVerts(v, unitScale), t, SourceBrep + } + case item.IsA("IfcFaceBasedSurfaceModel"): + // FbsmFaces is a SET of IfcConnectedFaceSet — union their faces, the + // same traversal the shell-based case above does over IfcShell. + // Without this case duplex_a's 235 nested face sets are unreachable, + // and every element built from one becomes a box. + if v, t, ok := surfaceModelMesh(item, attrFbsmFaces); ok { return scaleVerts(v, unitScale), t, SourceBrep } case item.IsA("IfcBooleanClippingResult"), item.IsA("IfcBooleanResult"): diff --git a/geometry/testdata/synthetic/face_based_surface_model.ifc b/geometry/testdata/synthetic/face_based_surface_model.ifc new file mode 100644 index 0000000..ad866cb --- /dev/null +++ b/geometry/testdata/synthetic/face_based_surface_model.ifc @@ -0,0 +1,43 @@ +ISO-10303-21; +HEADER; +FILE_DESCRIPTION((''),'2;1'); +FILE_NAME('face_based_surface_model.ifc','',(''),(''),'','',''); +FILE_SCHEMA(('IFC4')); +ENDSEC; +DATA; +/* The same two faces as known_box.ifc, reached through an + IfcFaceBasedSurfaceModel -> IfcConnectedFaceSet pair instead of an + IfcFacetedBrep -> IfcClosedShell pair. duplex_a.ifc nests 235 + IfcConnectedFaceSets behind 40 of these, and before the dispatch case + existed every one of them was unreachable: the element fell back to its + OBB box. */ +#1=IFCPROJECT('0proj',$,'P',$,$,$,$,(#20),#10); +#10=IFCUNITASSIGNMENT((#11)); +#11=IFCSIUNIT(*,.LENGTHUNIT.,$,.METRE.); +#20=IFCGEOMETRICREPRESENTATIONCONTEXT($,'Model',3,1.E-05,#21,$); +#21=IFCAXIS2PLACEMENT3D(#22,$,$); +#22=IFCCARTESIANPOINT((0.,0.,0.)); +#30=IFCCARTESIANPOINT((10.,20.,5.)); +#31=IFCAXIS2PLACEMENT3D(#30,$,$); +#32=IFCLOCALPLACEMENT($,#31); +#40=IFCCARTESIANPOINT((0.,0.,0.)); +#41=IFCCARTESIANPOINT((1.,0.,0.)); +#42=IFCCARTESIANPOINT((1.,1.,0.)); +#43=IFCCARTESIANPOINT((0.,1.,0.)); +#44=IFCCARTESIANPOINT((0.,0.,1.)); +#45=IFCCARTESIANPOINT((1.,0.,1.)); +#46=IFCCARTESIANPOINT((1.,1.,1.)); +#47=IFCCARTESIANPOINT((0.,1.,1.)); +#50=IFCPOLYLOOP((#40,#41,#42,#43)); +#51=IFCFACEOUTERBOUND(#50,.T.); +#52=IFCFACE((#51)); +#53=IFCPOLYLOOP((#44,#45,#46,#47)); +#54=IFCFACEOUTERBOUND(#53,.T.); +#55=IFCFACE((#54)); +#60=IFCCONNECTEDFACESET((#52,#55)); +#61=IFCFACEBASEDSURFACEMODEL((#60)); +#62=IFCSHAPEREPRESENTATION(#20,'Body','SurfaceModel',(#61)); +#63=IFCPRODUCTDEFINITIONSHAPE($,$,(#62)); +#70=IFCBUILDINGELEMENTPROXY('0fbsm',$,'FaceSetBox',$,$,#32,#63,$); +ENDSEC; +END-ISO-10303-21; diff --git a/geometry/unhandled.go b/geometry/unhandled.go index f3dd2c8..4bf0795 100644 --- a/geometry/unhandled.go +++ b/geometry/unhandled.go @@ -18,6 +18,7 @@ var handledItemTypes = []string{ "IfcConnectedFaceSet", "IfcOpenShell", "IfcShellBasedSurfaceModel", + "IfcFaceBasedSurfaceModel", "IfcBooleanClippingResult", "IfcBooleanResult", } diff --git a/parity/testdata/baseline.json b/parity/testdata/baseline.json index 85e76e1..d26fe75 100644 --- a/parity/testdata/baseline.json +++ b/parity/testdata/baseline.json @@ -2,8 +2,8 @@ "duplex_a": { "total": 218, "extrude": 146, - "brep": 0, - "obb": 69, + "brep": 65, + "obb": 4, "empty": 3 }, "fzk_haus": {