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

StackOverflowException when trying to create geometry #445

Open
ChernyshevDS opened this issue Sep 7, 2023 · 10 comments
Open

StackOverflowException when trying to create geometry #445

ChernyshevDS opened this issue Sep 7, 2023 · 10 comments

Comments

@ChernyshevDS
Copy link

ChernyshevDS commented Sep 7, 2023

Hello. I've got a problem loading a specific IFC model (which I can not share, unfortunately): calling Xbim3DModelContext.CreateContext has thrown a StackOverflowException. In attempt to find the cause I've built a debug version of Xbim.Geometry.Engine, but the StackOverflowException disappeared and regular exception appeared instead.

I was able to find a place where it was generated - it was Xbim.Geometry.Engine.XbimCompound.cpp in method TopoDS_Shape XbimCompound::InitFaces(IEnumerable<IIfcFace^>^ ifcFaces, IIfcRepresentationItem^ theItem, ILogger^ logger). Here is the fragment (comment was there, line 1531):

try
{
    gp_Dir outerNormal = XbimWire::NormalDir(outerLoop); //this can throw an exception if the wire is nonsense (line) and should just be dropped
    
    ...
    <more code>
}
catch (const std::exception&)
{
    XbimGeometryCreator::LogDebug(logger, ifcFace, "Outer loop is not a bounded area,  face ignored");
    continue;
}

Problem is that XbimWire::NormalDir did not throw an std::exception, but Standard_ConstructionError from OpenCascade which is not a descendant of std::exception. The exception was caused by the attempt to construct an instance of gp_Dir class (direction vector) with all arguments set to zero. Clearly there is an issue with geometry in my model, but I'd prefer to open a model with some errors than not be able to open it at all.

The comment in the fragment above indicates that this situation is not unexpected, so I guess it'd be fine to just add another catch(Standard_Failure) clause with the same log-and-continue behaviour.

UPD: It seems that XbimWire::NormalDir behaves differently in Debug and Release mode (see comment in XbimWire.h, line 81 on branch XbimGeometry_develop_5.1.666), so the StackOverflowException was still an issue in Release mode. I had to add a check for NaN values in outerNormal like this:

try
{
    gp_Dir outerNormal = XbimWire::NormalDir(outerLoop); //this can throw an exception if the wire is nonsense (line) and should just be dropped
    if (XbimConvert::IsInvalid(outerNormal, 0))
    {
        XbimGeometryCreator::LogDebug(logger, ifcFace, "Outer loop normal could not be found,  face ignored");
        continue;
    }

    ...
}
catch (Standard_Failure sf)
{
    XbimGeometryCreator::LogDebug(logger, ifcFace, "Outer loop normal could not be found,  face ignored");
    continue;
}
catch (const std::exception&)
{
    XbimGeometryCreator::LogDebug(logger, ifcFace, "Outer loop is not a bounded area,  face ignored");
    continue;
}

XbimConvert::IsInvalid is picked from XbimGeometry_develop_5.1.666 branch. So far, this fix has allowed me to finally open my model.

Assemblies and versions affected:

XbimGeometry 5.1.403, commit 24a3db9

@ChernyshevDS ChernyshevDS changed the title Unhandled exception when trying to create geometry StackOverflowException when trying to create geometry Sep 7, 2023
@andyward
Copy link
Member

It would be great if you can supply a test for this. I know you can't share the model, but if you know the element triggering the issue, you can use the Simplify capability in Xplorer (View->Developer Windows->IFC Stripping) to extract the single element to a new model.

image

@ChernyshevDS
Copy link
Author

@andyward Thanks for the reply. I didn't know that Xplorer has this feature, that's cool. I finally managed to extract the problematic element, and I'd say that was not as easy as I thought. It seems that the problem is not with the element itself, but with associated opening in it, so i had to include the corresponding IFCRELVOIDSELEMENT. Anyway, the stripped file is in the attachment.
01-23-01_main deck_supports under the deck.ifc.stripped.ifc.txt

@ChernyshevDS
Copy link
Author

ChernyshevDS commented Sep 12, 2023

I'd like to share some of my findings, may be they will help. I tried to find out why XbimWire::NormalDir throws an exception.
The method accepts the wire and iterates over its vertices to calculate the normal. I've noticed that iterating with BRepTools_WireExplorer returns only 2 vertices while the wire object is built with IfcPolyLoop with 3 vertices, but my cpp-fu is't strong enough to debug the WireExplorer itself.

@ChernyshevDS
Copy link
Author

ChernyshevDS commented Sep 18, 2023

I guess, I've found the source of my problem: incorrect model 🤦‍♂️ I've bumped into this discussion: #281 and figured that the fix I've made was already implemented in a dev branch in 2021, but never made it into master branch, which was a base for my own fork of Xbim.

Another important thing I've got from this discussion is that model precision parameter defined in IfcGeometricRepresentationContext is crucial for correct mesh generation and it may be incorrectly set by some IFC exporters. I was struggling with some mesh artifacts for a long time, and decreasing this parameter has immediately fixed them. My models were generated using ProStructures v10, and I'm not sure if this is a bug or just incorrect setting somewhere.

@andyward
Copy link
Member

Definitely worth checking out the v6 GE (netcore branch) if you have problematic models - it has quite a few fixes around precision. As you noticed it's key to a lot of the geometry issues. It's also on a later 7.7 OpenCascade version. It's also a lot easier to work with as OCC imported via nuget rather than compiled in.

@ChernyshevDS
Copy link
Author

ChernyshevDS commented Sep 19, 2023

Thanks, I should try it. I've chosen the 5.1.430 version a long time ago (don't remember exactly why, I can recall some issues with dependencies), and I decided to make my own fork and patch it as needed. I seems a bit strange to me that fixes are received into dev branch, but master stays abandoned for 2 years. As for netcore branch I'm a bit hesitant to switch on using a dev branch as I'm not sure if it's stable enough.

@jomijomi
Copy link

Yeah, regarding precision I think it is good practice to add a check before calling CreateContext. Just in case...

//Make sure we get at least 1/100 mm
//(mf is model.ModelFactors)
double sufficientPrecision = mf.OneMilliMetre / 100.0;
if(mf.Precision > sufficientPrecision)
{
           mf.Precision = sufficientPrecision;
}

@ChernyshevDS
Copy link
Author

@jomijomi I'm not sure that this is a correct approach. I guess, it may introduce some errors when loading georeferenced models which typically have large coordinates. @andyward maybe you have some insight, why can't I set the precision to 1 nanometer and be done with it? What are the consequences?

@andyward
Copy link
Member

@martin1cerny or @SteveLockley is probably a better authority on this than me - especially on WCS & geo-referencing.

But having a precision of nanometers when dealing with large coordinates is going to create all kinds of issues with Float/Double precision?

@jomijomi
Copy link

Yes, the ideal situation is of course to respect the precision/accuracy from the BIM-modelling or processing software. On that I fully agree!! However, I’ve noticed that I tend to encounter far more IFCs with obvious wrong precision/tolerance than I should, so this has almost been the standard setting in my app… Should state that this is mostly for housing/building projects, and not infrastructure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants