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

XbimGeometry producing flipped triangles #96

Open Kuraperunat opened 6 years ago

Kuraperunat commented 6 years ago

Here's yet another case of trying to create triangulated meshes using Xbim. It's working perfectly except for some, seemingly random ifc objects the triangle indices are always flipped. I've tried looking at all the existing code and fiddled around with the parameters and the transformations but I can't figure out what's causing it.

Here's an example of the issue: https://i.imgur.com/GrwMAv9.jpg On the left you see a mesh generated by another program, and on the right you see the result of Xbim. The triangle indices of Xbim are flipped (e.g. 0,1,2 is for some reason 2,1,0). This happens with many different ifc files and in seemingly totally random places.

Here's the relevant part of my code where I triangulate the model.

using (var model = IfcStore.Open(path)) {

    var ctx = new Xbim3DModelContext(model);
    ctx.CreateContext();

    using (var geomRead = model.GeometryStore.BeginRead()) {

        var scale = model.ModelFactors.OneMetre;

        var prodIds = new HashSet<int>();
        foreach (var product in model.Instances.OfType<IIfcProduct>()) {
            if (product is IIfcFeatureElement) continue;
            prodIds.Add(product.EntityLabel);
        }

        var toIgnore = new short[4];
        toIgnore[0] = model.Metadata.ExpressTypeId("IFCOPENINGELEMENT");
        toIgnore[1] = model.Metadata.ExpressTypeId("IFCPROJECTIONELEMENT");
        if (model.IfcSchemaVersion == Xbim.Common.Step21.IfcSchemaVersion.Ifc4) {
            toIgnore[2] = model.Metadata.ExpressTypeId("IFCVOIDINGFEATURE");
            toIgnore[3] = model.Metadata.ExpressTypeId("IFCSURFACEFEATURE");
        }

        foreach (var geometry in geomRead.ShapeGeometries) {

            if (geometry.ShapeData.Length <= 0) //no geometry to display so don't write out any products for it
                continue;
            var instances = geomRead.ShapeInstancesOfGeometry(geometry.ShapeLabel);

            var xbimShapeInstances = instances.Where(
                si => !toIgnore.Contains(si.IfcTypeId) &&
                si.RepresentationType ==
                XbimGeometryRepresentationType.OpeningsAndAdditionsIncluded &&
                prodIds.Contains(si.IfcProductLabel)).ToList();

            if (!xbimShapeInstances.Any()) continue;

            XbimShapeTriangulation tr;
            using (var ms = new MemoryStream(((IXbimShapeGeometryData)geometry).ShapeData))
                using (var br = new BinaryReader(ms))
                    tr = br.ReadShapeTriangulation();

            foreach (IXbimShapeInstanceData xbimShapeInstance in xbimShapeInstances) {

                var styleId = xbimShapeInstance.StyleLabel > 0
                    ? xbimShapeInstance.StyleLabel
                    : xbimShapeInstance.IfcTypeId * -1;

                var instanceTransform = ((XbimShapeInstance)xbimShapeInstance).Transformation;
                var trTransformed = tr.Transform(instanceTransform);

                var offset = vertices.Count;

                for (int k = 0; k < trTransformed.Vertices.Count; k++) {
                    var v = trTransformed.Vertices[k];
                    vertices.Add(new Vector3((float)(v.X / scale), (float)(v.Y / scale), (float)(v.Z / scale)));
                }

                for (int k = 0; k < trTransformed.Faces.Count; k++) {
                    var face = trTransformed.Faces[k];
                    var indices = face.Indices;
                    for (int z = 0; z < indices.Count; z += 3) {
                        int t0 = indices[z + 0], t1 = indices[z + 1], t2 = indices[z + 2];
                        triangles.Add(offset + t0);
                        triangles.Add(offset + t1);
                        triangles.Add(offset + t2);
                    }
                }
            }
        }
    }
}
shole commented 6 years ago

i can reproduce/illustrate this in XbimXplorer without third party code by changing SurfaceLayerStyler.cs method GetNewStyleMesh() to be

protected static WpfMeshGeometry3D GetNewStyleMesh(WpfMaterial wpfMaterial, Model3DGroup >tmpTransparentsGroup,
       Model3DGroup tmpOpaquesGroup) {
       XbimColour col = new XbimColour("fff", 1f, 0, 0, 1f); // make a red backface material
       WpfMaterial backmat = new WpfMaterial();
       backmat.CreateMaterial(col);
       if (wpfMaterial.IsTransparent) { // but let's ignore transparencies
           backmat = wpfMaterial;
       }
       var mg = new WpfMeshGeometry3D(wpfMaterial, backmat);
       mg.WpfModel.SetValue(FrameworkElement.TagProperty, mg);
       mg.BeginUpdate();
       if (wpfMaterial.IsTransparent)
           tmpTransparentsGroup.Children.Add(mg);
       else
           tmpOpaquesGroup.Children.Add(mg);
       return mg;
   }

This will paint backfaces in red, like this; https://i.imgur.com/WnBUjHo.jpg This is a healthy ifc model which imports through ifcopenshell correctly.

shole commented 6 years ago

After a full day of compiling, I tested this with the latest common develop branch code (Geometry 4.0.15-V0030) and the issue remains. https://i.imgur.com/oifQZYj.jpg As you can see the flipped triangles are not the same, so it's an issue in the meshing somewhere. (Separate issue; The develop version also adds a nonexistent down-facing square in this IFC which does not exist in the model. Some other IFCs do not add this.)

shole commented 6 years ago

After studying a bit I think this issue is twofold;

  1. Inverted transforms are not resolved This is often used to mirror elements Well illustrated by the chairs in the test model SampleHouse.ifc https://i.imgur.com/12Pp3NY.jpg This should be a relatively easy fix; just flip faces if transform scale sum is negative. ( if ( sign(scale.x)sign(scale.y)sign(scale.z) < 0 ) flipTriangles=true; )

  2. Something goes awry in non-primitive 3d meshes. The roofing on my example model in previous posts is not primitives but triangle mesh. It's split into multiple ceiling elements which are solid boxes in shape, but it starts flipping the faces at seemingly random positions - not the whole elements like in case 1 above. It's not a thing with the model as this happens with other non-primitive meshes. Here's some random letter flipping https://i.imgur.com/JlNRamT.jpg - the primitive based building in this model is almost entirely correct. Opening the files give a number of these warnings from XbimGeometryCreator in the log, so I think the issue might be in the ifc format parser somewhere;

    GeomEngine: #8999221=IfcCompositeCurve [Invalid part of a composite curve found. It has been discarded] GeomEngine: #6537656=IfcPolygonalBoundedHalfSpace [Incorrectly defined PolygonalBoundary #8999221. It has been ignored]

I know at some point these models were processed with Solibri IFC optimizer (which is standard practice), if that makes a difference. But essentially all industry standard ifc readers still open them correctly. By contrast opening a model without freeflowing non-primitive triangles does not give these warnings and i only see issues caused by case 1 with inverted transforms.

SteveLockley commented 5 years ago

Hi Kuraperunat, I am aware of this problem, its been long standing I am afraid, but I am trying to resolve it now. Do you have any Ifc files that I could use to reproduce you scenarios please? Snippets can be extracted using the Xbim.Extract Utility if you cannot share a whole model

SBRK commented 5 years ago

I'm having the same problem. With Xbim.Essentials and Xbim.Geometry v4, no problem. With v5, I'm seeing random flipped faces.

Wolgraph commented 5 years ago

Hi. I've had the same problem with an ifc project of mine. I couldn't fix the issue withing the code, so I managed to write a class to check and unify the normals and fix the face orientation. Give it a look. https://github.com/Wolgraph/GeometryEditor

bekraft commented 4 years ago

Just found a candidate for solution of this. I've had the same trouble while importing meshes to Unity. The problem seems to be related to planar meshing algorithm and orientation alignment. It wasn't stable enough.

Wolgraph commented 4 years ago

Just found a candidate for solution of this. I've had the same trouble while importing meshes to Unity. The problem seems to be related to planar meshing algorithm and orientation alignment. It wasn't stable enough.

I managed to create a C# class which check and fix tris’ orientation. https://github.com/Wolgraph/GeometryEditor It is designed for Unity

bekraft commented 4 years ago

Yes, I saw your solution. The current patch does the same, since it`s the only way to handle this type of checks stable manner (at least I don't know any other). But I don't like to fix it only for Unity. It's more a general issue with the custom tessellator in XbimEssentials.

Wolgraph commented 4 years ago

I forgot I already pasted the link. I'm sorry. If I had time I would have wrote it for general solution, too, using structures or multi-arrays. Glad to ear you've found a solution. Good job!

magnus-anderholm-sweco-se commented 3 years ago

Hi. We are experiencing the exact same problem.

I believe I can provide a stripped down model with a lot of similar geometry that shows the issue. stairs.zip I have used a customized version of XbimXplorer with back face culling enabled to generate the images. (basically just edited the code and ensured that back faces are not drawn)

So first load the provided zip file in a XbimXplorer with back face culling enabled. Then locate the stairs in image below incorrect stairs

All stairs should look like this stair-ok

However some seem to have inverted winding order stairs  nok

As mentioned before. Other viewers on the market does not seem to suffer from this. We have so far tested Solibri, IFC++ and Bimvision. Do note that we don't really use XBimXplorer ourselves. We actually extract the geometries from IFC files and show them in a game engine. However since the problem is visible in XBimXplorer I figured it was the most appropriate way to provide a better chance of debugging. If there is anything else I can do to simplify the process please let me know. I can definitely provide more samples if that is required. If I had the skills I would attempt to solve it myself and contribute to this fantastic library. However my linear algebra knowledge is even older +15 years. Sadly I have probably forgotten more than I ever knew on the subject.

I will gladly help in testing and verifying any potential bugfixes that may or may not come out of this. I have a huge amount of IFC files at my disposal that I can test on. (sorry I cannot share them though)

bekraft commented 3 years ago

So, are you using the XbimGeometry project to extract the geometry? In my case, some of the inverted meshes were already wrong in the source IFC. There's already a fix, which attempts to invert meshes. But it's not yet merged to the develop branch. The current approach (and of the fix,too) has some limitations. If those meshes transmitted as separate non-closed triangulated meshes (i.e. each face as a separate mesh), a more sophisticated method is needed to sew the partial meshes by their common edges into a single mesh. Other viewers just "hide" this issue as they draw also the backfaces. So depth-ordering (and therefore the graphics engine) solves the issue on their behalf.

magnus-anderholm-sweco-se commented 3 years ago

Well we are not using XbimGeometry explicitly. I just assumed (incorrectly?) that it is used somewhere in the internals of XBIM when we load IFC and later on extract the geometries with code that looks something like this.

XBim3dModelContext ctx = .....
var shapeGeometry = ctx.ShapeGeometry(shapeInstance);
using (var stream = new MemoryStream(shapeGeometry.ShapeData, false))
using (var reader = new BinaryReader(stream)) {
      var triangulation = reader.ReadShapeTriangulation();
      triangulation.Write(ourOwnBinaryStream)
}

Our engine can read the format that XbimShapeTriangulation.Write() writes to the binary stream so that is get it into the game engine at a later stage.

I'm aware of how some viewers hide the problem, as you say. However in this particular case I don't believe that is why it looks ok in other viewers. The IFC++ example viewer has options to control both front and back face culling. By default it runs with back face culling enabled and then we cannot see the issue. However if we turn on front face culling instead .Then all stairs look exactly like they do when they are incorrect in XBimXplorer. Here's what it looks with backface culling in IFC++ (correct) ifc++ backface

Here's how it looks with frontface culling in IFC++ (incorrect)

ifc++ frontface

bekraft commented 3 years ago

Your example is using IfcClosedShells. When the context is built, XbimGeometry is involved. So you are using it implicitly. I have tried to read your example using the patched version (https://github.com/xBimTeam/XbimEssentials/pull/299 ). The source of trouble is located in the XbimEssentials project. The geometry shown is correct and no warnings are logged. Means that the original IFC export is correct. Have you tried to use the develop branch (just to exclude other other bug sources)?

magnus-anderholm-sweco-se commented 3 years ago

Nope. I haven't tried that yet. I will do it next week (hopefully monday). I think its unlikely that it will work. But it is better to be 100% sure. Big thanks for taking time and checking the model.

magnus-anderholm-sweco-se commented 3 years ago

I can build latest master alright (same IFC geometry bug though) but the develop branch refuses to build on my machine. Seems like the required nuget packages are simply not available on the myget xbim_develop source. I'm not sure how to proceed. I'm probaby missing something but I cannot find anything in the docs indicating any other buildsteps than checking out the branch and building. Nuget should handle the rest (and get the dev packages from myget xbim_develop). Have double checked the nuget.config but it seems alright.

magnus-anderholm-sweco-se commented 3 years ago

Had a look at the code in XBimEssentials. Am I correct in assuming that the issue probably lies somewhere in XBimTesselator and/or XBimTriangulatedMesh?

bekraft commented 3 years ago

Yes. It's actually the way how the final orientation of the overall mesh is determined. The current logic (tries) to place/compute a point on some extreme coordinates at the boundary and assumes the opposite direction to be internal (as far I remember). But this only works with compact and (better convex) figures. If you have sparse, spread and non-convex bodies, you likely get into trouble. Therefore the new way first tries to close the figure topologically and if successfully done, tries to compute the enclosed volume. It an indication of the orientation. If negative, you're inside out. If it not closed or intentionally not closable (i.e. triangle meshes of landscape or whatever), it doesn't do anything and hopes that the original mesh is oriented the right way (crossing the virtual fingers ;-). I've already found exports, where this doesn't work since the exporting platform incorrectly states that the mesh is closed and stores the normals in reverse direction. So there's no solution for all. But the PR is more stable than the current solution (and btw returns also the enclosed volumes as indicators of closed shells)