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
260 stars 131 forks source link

XbimOccShape::WriteTriangulation is better to output vertex's value using double type, it impacts computational accuracy #302

Open ryanzll opened 3 years ago

ryanzll commented 3 years ago

XbimOccShape::WriteTriangulation is better to output vertex's value using double type, it impacts computational accuracy

Assemblies and versions affected:

Xbim.Geometry.Engine.dll version:5.1.404

Steps (or code) to reproduce the issue:

XbimOccShape.cpp, line 604

for each (XbimPoint3D p in points)
{
    binaryWriter->Write((float)p.X);
    binaryWriter->Write((float)p.Y);
    binaryWriter->Write((float)p.Z);
}

Expected behavior:

output XbimPoint3D with double type

Actual behavior or exception details:

output XbimPoint3D with float type

Additional Details

we are developing a software of quantity survey, numerical precision will impact calculation result, it is expected that at least 6 decimal digits. For float, there are about 7 decimal digits at most. if integer is a big number, there are less bit left for decimal.

martin1cerny commented 3 years ago

This will also double the storage size and memory consumption when this is handled in memory later on. Also, this really depends on the units and modelling strategy. When model is properly placed using local origin (for example placement of the site), this is usually not a problem. Obviously if the geometry is saved in IFC file as absolute coordinates from distant origin using millimeters, this becomes a problem. Other possible strategy is therefore to separate large displacement of any geometry into a simple translation (just [X,Y,Z]) to keep enough numerical space for the value.

ryanzll commented 3 years ago

This will also double the storage size and memory consumption when this is handled in memory later on. Also, this really depends on the units and modelling strategy. When model is properly placed using local origin (for example placement of the site), this is usually not a problem. Obviously if the geometry is saved in IFC file as absolute coordinates from distant origin using millimeters, this becomes a problem. Other possible strategy is therefore to separate large displacement of any geometry into a simple translation (just [X,Y,Z]) to keep enough numerical space for the value.

The fact is that, xbim store vertex's value with double type, and just use float type when output to memory stream. Furthermore, this operation is done one by one element, it will not consume much memory. I notice that new class WexBimMeshFace which is better than memory stream.

martin1cerny commented 3 years ago

WexBIM is internally using identical data structures for the triangulated geometry. But you are right, when we read it into memory, we use doubles. We only use singles for persistent storage. That makes it slightly tricky, because many people use Esent persistent models with processed geometry and we can't break this backwards compatibility. Feel free to submit a PR if it solves the backwards compatibility of persisted data.

ryanzll commented 3 years ago

Thanks for explaination. Before, I use BinaryReaderExtensions to get internal mesh data, it is straight for persistent storage but not straight for getting data by other program and may decrease performance. Some new interfaces similar WexBimMeshFace should be easy to use, without care about structure of memory stream, and don't break backwards compatibility.