Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Still not rendering IfcTriangulatedFaceSet #167

Closed
guenter1holzeder opened this issue Mar 6, 2019 · 12 comments
Closed

Still not rendering IfcTriangulatedFaceSet #167

guenter1holzeder opened this issue Mar 6, 2019 · 12 comments
Labels
awaiting closure Issues that should be closed if we don't hear back on in a week bug Confirmed bug - system not working as intended
Milestone

Comments

@guenter1holzeder
Copy link
Contributor

A sample wall with IfcTriangulatedFaceSet is not rendered as a shape.
But it can be converted to a XbimSolid via XbimGeometryEngine and XbimShellset correctly.

I played with several constellations develope/master but with no positive result.

image

image

IfcTriangulatedFaceSet.zip

@andyward
Copy link
Member

andyward commented Mar 6, 2019

Is it related to #145? There was a recent PR #151 that got merged. Have you got the very latest?

@guenter1holzeder
Copy link
Contributor Author

I used both 5.0.163 (Master) and 5.0.169-develop

@andyward
Copy link
Member

andyward commented Mar 6, 2019

Thanks. 169 includes that PR, so looks like this is a bug.

@andyward andyward added the bug Confirmed bug - system not working as intended label Mar 6, 2019
@bekraft
Copy link
Contributor

bekraft commented Mar 8, 2019

The main reason here is, that the implementation ignores the "PnIndex" attribute. It maps between the local index and the provided index of the IfcCartesianPointList3D given by "Coordinates". There's an argument-out-of range exception on the log

==== 13:53	5	Error
Xbim.ModelGeometry.Scene.Xbim3DModelContext
GeomScene: #48="IfcTriangulatedFaceSet" ["Failed to create shape geometry, Der Index lag außerhalb des Bereichs. Er darf nicht negativ und kleiner als die Sammlung sein.
Parametername: index"]

But somehow the given IFC model does not correspond to the latest IFC4 spec. It roughly says, that the given 2D list of normals should have the same length as the provided coordinate indices. It seems that the model exporter has written less normals than coordinates (6 vs. 8). "PnIndex" refers also to one of 6 indexes. So my guess is, that the exporter's "PnIndex" refers to the normals and maps it onto the vertices - which is indeed not the meaning of the IFC4 spec (see IfcTriangulatedFaceSet). Am I wrong? On the other hand, the way it has been exported makes more sense to me ...

@SteveLockley
Copy link
Member

Guenter, I am not sure where the issue starts here, it looks like you have used xbim extract to strip some objects out. Do you have the original file so we can start from base zero?

@SteveLockley
Copy link
Member

SteveLockley commented Apr 15, 2019

This is really odd, as Bekraft points out the PnIndex should be a L[1:?] of integers but your file has a L[1:?] L[3:3] of integers

I would have thought this just plain wrong except one of the samples on the IFC schema site does a similar thing http://www.buildingsmart-tech.org/ifc/IFC4/Add2/html/ . We build our code from the express schema and that says it should be a list of integers.
To compound things we are not even processing PnIndex in the geometry engine, other than when parsing. This is becasue it did not exist until Addendum 2 (they snook it in)
I think the best thing is that we assume the sample file above is wrong and the schema is correct. If you can fix your IFC file to comply with the schema, i.e. List of Integers that point to the the coordinate index to be used. Then we need to implement it :-)

@guenter1holzeder
Copy link
Contributor Author

From a practical view I must say - other IFC viewers seem to have no problem with this. When you are in a B2B situation, no modification on IFC files can be done (by the end user). So it's sink or swim for them.

I think the question is, how tolerant an application should be on schema variances?

@SteveLockley
Copy link
Member

SteveLockley commented Apr 15, 2019

I understand and am not trying to be fascist with this in anyway. We are in fact ignoring this data, which is most likely what other tools are doing as this was not in add1.
Wasn't sure if you were the author of the file, but I guess it is graphisoft. I will have a chat with the guys who deal with parsing and see if we can swallow some of these complaints and take a softer approach

@guenter1holzeder
Copy link
Contributor Author

In our business we're getting only final models from different producers (CADs). If we get a dickey IFC file we have to deal with it :-)

@bekraft
Copy link
Contributor

bekraft commented Apr 15, 2019

I've tried also to find some other examples. But even the IFC spec doesn't provide some for this special case. Also the figures of spec page itself are somehow inconsistent. So figure 347 refers to Add1 whilst figure 348 refers to Add2. Don't know how to handle this ... Taking to graphisoft or others might be the best (since their exporters should be certified against whatever ;-)

@SteveLockley
Copy link
Member

SteveLockley commented Apr 15, 2019

Had a good look and I think the PnIndex is fundamentally defined incorrectly in the IFC standard. I think the PnIndex should be the indices of the normals not the x,y,z coordinates as described in the Ifc Docs. I will make this assumption and amend the code and inform BuildingSmart. That said the Graphisoft output is not in accordance with the schema either, one or the other will need to be fixed, but xBIM will handle the current output and the correct output.

@andyward andyward added the in progress Being worked on label May 8, 2019
@SteveLockley SteveLockley added awaiting closure Issues that should be closed if we don't hear back on in a week and removed in progress Being worked on labels May 17, 2019
@guenter1holzeder
Copy link
Contributor Author

Well done

@andyward andyward added this to the 5.1.0 milestone Jun 2, 2019
guenter1holzeder added a commit to OrcaIfc/XbimGeometry that referenced this issue Oct 29, 2019
* Test added for Radian Parameters over PI in value

* Updated MemoryModel package to handle radian conversion factors correctly Github xBimTeam#92 issue

* Fixing issue xBimTeam#145

using filtering of representation items instead of finding first only

* Some corrections for fix xBimTeam#145

Evaluation wasn't triggered because there was no consumer of stream

* Fix for imprecisely defined planar wires github issue xBimTeam#73
Fix for SurfaceBaseModels being defined as multiple solids

* Extruded area solids with compound profiles treated to return a solid set

* Fixing issue xBimTeam#158

Second operand has never been added to difference build chain.
Refactored IfcBooleanClippingResult to XbimSolidSet conversion.

* Experimental: Changed ProjectTarget to 8.1 which should support Win 7SP1 clients

* Added targetver to set WINVER as per
https://docs.microsoft.com/en-us/cpp/porting/modifying-winver-and-win32-winnt?view=vs-2017

This is to support Win 7SP1 runtimes.

* Update azure-pipelines.yml for Azure Pipelines

Removed tests

* removed target.h

* Updated Nuget dependencies

* Geometry engine unreferenced after tests completed

* Workaround for boolean precision removed xBimTeam#166

* Fix for excessively large solid extrusions xBimTeam#160

* Fix for Ifc4 Add2 IfcTriangulatedFaceSet not rendering  xBimTeam#167

* Fix for inaccurately defined IfcPolyloops that are not planar

* Faceted Face set re-implemented to support tolerances better

* Threading Model changed to avoid race conditions, boolean cut ops optimised for non-intersecting arguments

* Composite curve reimplemented to correctly handle polylines as edges

* IndexedPolyCurve implemented as curve

* Fix for orientation of trimmed composite curve segments that are reversed

* IfcSweptDiskSolidPolygonal fixed for closed directrix.
XbimWire trim fixed for incorrect parameterisation

* Composite Curve as wire added

* IfcSectionedSpine fixed incorrect orientation error

* Non-Intersecting cuts not returning the body shape resolved

* Erro in booleans caused by Solids created from two coincidental faces fixed

* Trimming of compound curve wire fix

* Support for IIfcPolygonalFaceSet commenced

* Support for 2D Polylines as curves added

* Closed Shell chnaged to allow faulty solids with zero volume

* Undone zero volume closed shells

* Support for IfcPolygonalFaceSet completed.
Very large facesetsare meshed rather than OCCed to prevent extremely slow performance

* Reinstate tests now stabilty issue fixed

* Closed Seep for swept disk solid fixed

* Removed last traces of the large file

* Adding axis tags to logging output for convenience

* Adding fix for zero grid bounding box in CreateGrid

assuming that the bounding box has a zero Z-extent. Does this hold?

* Fix for unsupported curve type issue xBimTeam#180

* SweptDiskSolid fixed to align sweep with directrix direction

* Interval Points for Wire fixed to include last point
Composite curve wires now allow partial curves where segments are badly specified, a warning is issued rather than a failure error

* Support for composite curves containing multi-edge segments added

* Continue IfcGrid construction on exception

* Fixing pipe maker tolerance issue

* Half Space solid reimplemented with OCC MakeHalfSpaceSolid
Polygonally bound half space amended to construct semi-infinite prism
Fuzzy value for booleans increased to 6*tolerance
Max faces to sew increased to 3000

* Grid creation fixed for errors on building lines with near precision thickness

* Polygonal bounded half space corrected for potential boolean hangs with large extrema.
Edge start and end points fixed to handle null vertices

* Empty profile definition handled, polygonally bounded half space extrusion limited to prevent booleans hanging, compositve curve creation handles incorrect reverse segment definition

* References update to Master Nuget dependencies
Warnings fixed

* No change

* Minor readme update for VS versions

* Added extra tests to help with failing CI test

* Updated to 5.1 RTM essentials

* Added changelog for 5.1

* Update build for 5.1

* Fixed changelog typo

* Added changelog for 5.1

* Update build for 5.1

* Fixed changelog typo

* Updated to 5.1 version data.
Removed FileVersion so stamped by build

* Updated to latest pre-release 5.1 dependencies

* Updated latest RTM dependencies
(with fileversions)

* Updated 5.1 dependencies (xBimTeam#187)

* Updated latest RTM dependencies
(with fileversions)

* Release 5.1 with 5.1 AssemblyVersion (xBimTeam#188)

* Added changelog for 5.1

* Update build for 5.1

* Fixed changelog typo

* Updated to 5.1 version data.
Removed FileVersion so stamped by build

* Updated to latest pre-release 5.1 dependencies

* Updated latest RTM dependencies
(with fileversions)

* Confirmed release number

* Regression causing incorrect clipping of HalfSpaceSolids fixed

* Partial fix for incorrectly parameterised Polyline trims, further work is required to correctly implement Composite and Polyline curve trims when the BIM authros start to correctly write out the parameter values

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Extended CI build timeout from default (60 minutes) to 120 minutes

* Trying to fix pipeline with explicit definition of the job name

* Trying to fix pipeline with extended timeout

* Trying to fix the pipeline

* Creation of solid sets from IfcPolygonalFaceSet added

* Create Method for IfcPolygonalFaceSet amended to respect closed shells

* Updated Essentials to Release

* Attempt to fix large coordinates by reducing them and only keeping the transformation.

* Updated Tessellator

* Updated tessellator fixes shape bounding box position. Fixed transformation of the product bbox for region computation.

* Updated Essentials

* Added explicit test runsettings to help with failing tests

* Update azure-pipelines.yml for Azure Pipelines

Add runsettings

* Refactored tests to use paths relative to deployed items.

* Only deploy the native DLL for the current platform

Means that consumers only get the native DLL appropriate to their current platform build (x86/x64)
Fixes xBimTeam#216

* Better management of trims for IIfcSweptDiskSolid

Typically used for rebars.

* Tests was expecting wrong number of faces.

* Resumed caching to help see files in Xplorer.

When /caching is issued as parameter, the files are persisted as in xbim
format so they can be seen in xplorer to check the results of meshing.

* Tolerant of missing Z coordinate for some 3D pts.

* More cases of Z missing tolerance.

by code analysis, given a bug with unusual IFC files.

* Cutting wires of IIfcCompositeCurve only if needed

* Closed profile definitions with no outer profile caught and ignored

* Fix for OCC infinite loop in ShapeHealing
Time out added to ShapeHealing and Boolean Performs

* All shape healing functions protected with a timeout

* Fixed local tests runs

Wasn't running tests in VS test runner due to "An exception occurred while invoking executor 'executor://mstestadapter/v2': Could not load file or assembly 'System.IO,"

Since we introduced DeploymentItem, we were impacted by microsoft/testfx#295

* Copy both native files when building for AnyCPU

Broken fixing xBimTeam#216

* Added Test for Advanced Brep failure

* Added additional guard to be more tolerant when processing models without valid Contexts supplied.

Should address xBimTeam/XbimWindowsUI#94

* More graceful handling of representations without a ContextOfItems. These are in violation of the specs but we should support them better.

E.g. 2x3_Arboleda_Bldg-Arch.ifc

* Going for green.
Ignore an unimplemented test in this branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting closure Issues that should be closed if we don't hear back on in a week bug Confirmed bug - system not working as intended
Projects
None yet
Development

No branches or pull requests

4 participants