From fe1cb9e92bb8a5cb1970565142db59e7ba91907f Mon Sep 17 00:00:00 2001 From: Otso Alho Date: Tue, 13 Mar 2018 11:00:23 +0200 Subject: [PATCH 01/11] default to blank material with IfcMaterial name --- src/ifcgeom/IfcGeomRenderStyles.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ifcgeom/IfcGeomRenderStyles.cpp b/src/ifcgeom/IfcGeomRenderStyles.cpp index c47f10a236..09c211a58b 100644 --- a/src/ifcgeom/IfcGeomRenderStyles.cpp +++ b/src/ifcgeom/IfcGeomRenderStyles.cpp @@ -131,7 +131,8 @@ const IfcGeom::SurfaceStyle* IfcGeom::Kernel::get_style(const IfcSchema::IfcMate } } } - return 0; + IfcGeom::SurfaceStyle material_style = IfcGeom::SurfaceStyle(material->id(), material->Name()); + return &(style_cache[material->id()] = material_style); } static std::map default_materials; From e7a8a97639d178dbf0a93a3b6349c627f0842ded Mon Sep 17 00:00:00 2001 From: Michael Sundqvist Date: Wed, 28 Mar 2018 17:33:46 +0300 Subject: [PATCH 02/11] If material name is empty when writing collada file, name it as a missing material. Logger::Notice() objects that do not have a material assigned. --- src/ifcconvert/ColladaSerializer.cpp | 10 ++++++++++ src/ifcgeom/IfcGeomFunctions.cpp | 3 +++ 2 files changed, 13 insertions(+) diff --git a/src/ifcconvert/ColladaSerializer.cpp b/src/ifcconvert/ColladaSerializer.cpp index 1347aa4f5e..6191f2136d 100644 --- a/src/ifcconvert/ColladaSerializer.cpp +++ b/src/ifcconvert/ColladaSerializer.cpp @@ -313,6 +313,11 @@ void ColladaSerializer::ColladaExporter::ColladaMaterials::ColladaEffects::write { std::string material_name = (serializer->settings().get(SerializerSettings::USE_MATERIAL_NAMES) ? material.original_name() : material.name()); + + if (material_name.empty()) { + material_name = "missing-material-" + material.name(); + } + collada_id(material_name); openEffect(material_name + "-fx"); COLLADASW::EffectProfile effect(mSW); @@ -360,6 +365,11 @@ void ColladaSerializer::ColladaExporter::ColladaMaterials::write() { BOOST_FOREACH(const IfcGeom::Material& material, materials) { std::string material_name = (serializer->settings().get(SerializerSettings::USE_MATERIAL_NAMES) ? material.original_name() : material.name()); + + if (material_name.empty()) { + material_name = "missing-material-" + material.name(); + } + std::string material_name_unescaped = material_name; // workaround double-escaping that would occur in addInstanceEffect() IfcUtil::sanitate_material_name(material_name_unescaped); collada_id(material_name); diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index 53013da932..d33c48e267 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -1455,6 +1455,9 @@ IfcGeom::BRepElement

* IfcGeom::Kernel::create_brep_for_representation_and_pro } } } + else { + Logger::Notice("Object '" + product->Name() + "' (guid " + product->GlobalId() + " ) has no material!"); + } if (material_style_applied) { representation_id_builder << "-material-" << single_material->entity->id(); From 6f3eb84f66a9b176177dd91d4eabcd27e3533db2 Mon Sep 17 00:00:00 2001 From: Michael Sundqvist Date: Thu, 29 Mar 2018 01:03:12 +0300 Subject: [PATCH 03/11] Fix instance_material missing material names as well. --- src/ifcconvert/ColladaSerializer.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/ifcconvert/ColladaSerializer.cpp b/src/ifcconvert/ColladaSerializer.cpp index 6191f2136d..9ff5171ac9 100644 --- a/src/ifcconvert/ColladaSerializer.cpp +++ b/src/ifcconvert/ColladaSerializer.cpp @@ -103,6 +103,11 @@ void ColladaSerializer::ColladaExporter::ColladaGeometries::write( COLLADASW::Triangles triangles(mSW); std::string material_name = (serializer->settings().get(SerializerSettings::USE_MATERIAL_NAMES) ? materials[previous_material_id].original_name() : materials[previous_material_id].name()); + + if (material_name.empty()) { + material_name = "missing-material-" + materials[previous_material_id].name(); + } + collada_id(material_name); triangles.setMaterial(material_name); triangles.setCount((unsigned long)num_triangles); @@ -160,6 +165,11 @@ void ColladaSerializer::ColladaExporter::ColladaGeometries::write( COLLADASW::Lines lines(mSW); std::string material_name = (serializer->settings().get(SerializerSettings::USE_MATERIAL_NAMES) ? materials[it->first].original_name() : materials[it->first].name()); + + if (material_name.empty()) { + material_name = "missing-material-" + materials[it->first].name(); + } + collada_id(material_name); lines.setMaterial(material_name); lines.setCount((unsigned long)it->second.size()); @@ -228,7 +238,7 @@ void ColladaSerializer::ColladaExporter::ColladaScene::add( BOOST_FOREACH(std::string material_name, material_ids) { /// @todo This is done 6 times in this file, try to perform this once and be done with the material naming for the export. collada_id(material_name); - COLLADASW::InstanceMaterial material (material_name, "#" + material_name); + COLLADASW::InstanceMaterial material(material_name, "#" + material_name); instanceGeometry.getBindMaterial().getInstanceMaterialList().push_back(material); } instanceGeometry.add(); @@ -407,6 +417,11 @@ void ColladaSerializer::ColladaExporter::write(const IfcGeom::TriangulationEleme } std::string material_name = (serializer->settings().get(SerializerSettings::USE_MATERIAL_NAMES) ? material.original_name() : material.name()); + + if (material_name.empty()) { + material_name = "missing-material-" + material.name(); + } + collada_id(material_name); material_references.push_back(material_name); } From 1c83b2db537585d0e82c504c5e7e5a32ce48dbcd Mon Sep 17 00:00:00 2001 From: Michael Sundqvist Date: Wed, 4 Apr 2018 14:43:43 +0300 Subject: [PATCH 04/11] Remove object name from no material log message, raise level to warning --- src/ifcgeom/IfcGeomFunctions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index d33c48e267..489a95b1d6 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -1456,7 +1456,7 @@ IfcGeom::BRepElement

* IfcGeom::Kernel::create_brep_for_representation_and_pro } } else { - Logger::Notice("Object '" + product->Name() + "' (guid " + product->GlobalId() + " ) has no material!"); + Logger::Warning("Object '" + product->GlobalId() + "' has no material!"); } if (material_style_applied) { From 06589ca1c68a89ad821b5f37b25688cb040de2d6 Mon Sep 17 00:00:00 2001 From: Michael Sundqvist Date: Thu, 31 May 2018 16:07:03 +0300 Subject: [PATCH 05/11] Refactor ColladaSerializer.cpp, encode material names only once, decode them before passing to OpenCollada functions that take URI parameters --- src/ifcconvert/ColladaSerializer.cpp | 99 +++++++++++----------------- src/ifcconvert/ColladaSerializer.h | 10 ++- 2 files changed, 44 insertions(+), 65 deletions(-) diff --git a/src/ifcconvert/ColladaSerializer.cpp b/src/ifcconvert/ColladaSerializer.cpp index 9ff5171ac9..78531a2a52 100644 --- a/src/ifcconvert/ColladaSerializer.cpp +++ b/src/ifcconvert/ColladaSerializer.cpp @@ -60,11 +60,7 @@ void ColladaSerializer::ColladaExporter::ColladaGeometries::addFloatSource(const source.finish(); } -void ColladaSerializer::ColladaExporter::ColladaGeometries::write( - const std::string &mesh_id, const std::string& /*default_material_name*/, const std::vector& positions, - const std::vector& normals, const std::vector& faces, const std::vector& edges, - const std::vector material_ids, const std::vector& materials, - const std::vector& uvs) +void ColladaSerializer::ColladaExporter::ColladaGeometries::write(std::string mesh_id, std::string, std::vector positions, std::vector normals, std::vector faces, std::vector edges, std::vector material_ids, std::vector materials, std::vector uvs, std::vector material_references) { openMesh(mesh_id); @@ -101,14 +97,8 @@ void ColladaSerializer::ColladaExporter::ColladaGeometries::write( const size_t num_triangles = std::distance(index_range_start, it) / 3; if ((previous_material_id != current_material_id && num_triangles > 0) || (it == faces.end())) { COLLADASW::Triangles triangles(mSW); - std::string material_name = (serializer->settings().get(SerializerSettings::USE_MATERIAL_NAMES) - ? materials[previous_material_id].original_name() : materials[previous_material_id].name()); - if (material_name.empty()) { - material_name = "missing-material-" + materials[previous_material_id].name(); - } - - collada_id(material_name); + std::string material_name = material_references[previous_material_id]; triangles.setMaterial(material_name); triangles.setCount((unsigned long)num_triangles); int offset = 0; @@ -163,15 +153,7 @@ void ColladaSerializer::ColladaExporter::ColladaGeometries::write( for (linelist_t::const_iterator it = linelist.begin(); it != linelist.end(); ++it) { COLLADASW::Lines lines(mSW); - std::string material_name = (serializer->settings().get(SerializerSettings::USE_MATERIAL_NAMES) - ? materials[it->first].original_name() : materials[it->first].name()); - - if (material_name.empty()) { - material_name = "missing-material-" + materials[it->first].name(); - } - - collada_id(material_name); - lines.setMaterial(material_name); + lines.setMaterial(material_references[it->first]); lines.setCount((unsigned long)it->second.size()); int offset = 0; lines.getInputList().push_back(COLLADASW::Input(COLLADASW::InputSemantic::VERTEX, "#" + mesh_id + COLLADASW::LibraryGeometries::VERTICES_ID_SUFFIX, offset)); @@ -234,11 +216,13 @@ void ColladaSerializer::ColladaExporter::ColladaScene::add( node.start(); node.addMatrix(matrix_array); COLLADASW::InstanceGeometry instanceGeometry(mSW); - instanceGeometry.setUrl ("#" + geom_name); + instanceGeometry.setUrl("#" + geom_name); BOOST_FOREACH(std::string material_name, material_ids) { - /// @todo This is done 6 times in this file, try to perform this once and be done with the material naming for the export. - collada_id(material_name); - COLLADASW::InstanceMaterial material(material_name, "#" + material_name); + // Unescape to avoid double escaping beucase OpenCollada's material URI parameter escapes XML internally + std::string unescaped = material_name; + IfcUtil::unescape_xml(unescaped); + + COLLADASW::InstanceMaterial material(material_name, "#" + unescaped); instanceGeometry.getBindMaterial().getInstanceMaterialList().push_back(material); } instanceGeometry.add(); @@ -319,17 +303,9 @@ void ColladaSerializer::ColladaExporter::ColladaScene::write() { } } -void ColladaSerializer::ColladaExporter::ColladaMaterials::ColladaEffects::write(const IfcGeom::Material& material) +void ColladaSerializer::ColladaExporter::ColladaMaterials::ColladaEffects::write(const IfcGeom::Material &material, const std::string &material_uri) { - std::string material_name = (serializer->settings().get(SerializerSettings::USE_MATERIAL_NAMES) - ? material.original_name() : material.name()); - - if (material_name.empty()) { - material_name = "missing-material-" + material.name(); - } - - collada_id(material_name); - openEffect(material_name + "-fx"); + openEffect(material_uri + "-fx"); COLLADASW::EffectProfile effect(mSW); effect.setShaderType(COLLADASW::EffectProfile::LAMBERT); if (material.hasDiffuse()) { @@ -361,11 +337,29 @@ void ColladaSerializer::ColladaExporter::ColladaMaterials::ColladaEffects::close void ColladaSerializer::ColladaExporter::ColladaMaterials::add(const IfcGeom::Material& material) { if (!contains(material)) { - effects.write(material); + std::string material_name = (serializer->settings().get(SerializerSettings::USE_MATERIAL_NAMES) + ? material.original_name() : material.name()); + + if (material_name.empty()) { + material_name = "missing-material-" + material.name(); + } + + collada_id(material_name); + + effects.write(material, material_name); materials.push_back(material); + material_uris.push_back(material_name); } } +std::string ColladaSerializer::ColladaExporter::ColladaMaterials::getMaterialUri(const IfcGeom::Material& material) { + std::vector::iterator it = std::find(materials.begin(), materials.end(), material); + long index = std::distance(materials.begin(), it); + + std::string material_uri = material_uris.at(static_cast(index)); + return material_uri; +} + bool ColladaSerializer::ColladaExporter::ColladaMaterials::contains(const IfcGeom::Material& material) { return std::find(materials.begin(), materials.end(), material) != materials.end(); } @@ -373,18 +367,13 @@ bool ColladaSerializer::ColladaExporter::ColladaMaterials::contains(const IfcGeo void ColladaSerializer::ColladaExporter::ColladaMaterials::write() { effects.close(); BOOST_FOREACH(const IfcGeom::Material& material, materials) { - std::string material_name = (serializer->settings().get(SerializerSettings::USE_MATERIAL_NAMES) - ? material.original_name() : material.name()); - - if (material_name.empty()) { - material_name = "missing-material-" + material.name(); - } - - std::string material_name_unescaped = material_name; // workaround double-escaping that would occur in addInstanceEffect() - IfcUtil::sanitate_material_name(material_name_unescaped); - collada_id(material_name); + std::string material_name = getMaterialUri(material); openMaterial(material_name); - addInstanceEffect("#" + material_name_unescaped + "-fx"); + + // Unescape to avoid double escaping beucase OpenCollada's addInstanceEffect escapes XML internally + IfcUtil::unescape_xml(material_name); + + addInstanceEffect("#" + material_name + "-fx"); closeMaterial(); } closeLibrary(); @@ -412,21 +401,13 @@ void ColladaSerializer::ColladaExporter::write(const IfcGeom::TriangulationEleme std::vector material_references; BOOST_FOREACH(const IfcGeom::Material& material, mesh.materials()) { - if (!materials.contains(material)) { - materials.add(material); - } - std::string material_name = (serializer->settings().get(SerializerSettings::USE_MATERIAL_NAMES) - ? material.original_name() : material.name()); + materials.add(material); - if (material_name.empty()) { - material_name = "missing-material-" + material.name(); - } - - collada_id(material_name); + std::string material_name = materials.getMaterialUri(material); material_references.push_back(material_name); } - DeferredObject deferred(name, representation_id, o->type(), o->transformation(), mesh.verts(), mesh.normals(), + DeferredObject deferred(name, representation_id, o->type(), o->transformation(), mesh.verts(), mesh.normals(), mesh.faces(), mesh.edges(), mesh.material_ids(), mesh.materials(), material_references, mesh.uvs()); if (serializer->settings().get(SerializerSettings::USE_ELEMENT_HIERARCHY)) { @@ -482,7 +463,7 @@ void ColladaSerializer::ColladaExporter::endDocument() { continue; } geometries_written.insert(it->representation_id); - geometries.write(it->representation_id, it->type, it->vertices, it->normals, it->faces, it->edges, it->material_ids, it->materials, it->uvs); + geometries.write(it->representation_id, it->type, it->vertices, it->normals, it->faces, it->edges, it->material_ids, it->materials, it->uvs, it->material_references); } geometries.close(); diff --git a/src/ifcconvert/ColladaSerializer.h b/src/ifcconvert/ColladaSerializer.h index a0ca6325c2..dbee52260e 100644 --- a/src/ifcconvert/ColladaSerializer.h +++ b/src/ifcconvert/ColladaSerializer.h @@ -69,11 +69,7 @@ private: {} void addFloatSource(const std::string& mesh_id, const std::string& suffix, const std::vector& floats, const char* coords = "XYZ"); - void write(const std::string &mesh_id, const std::string& default_material_name, - const std::vector& positions, const std::vector& normals, - const std::vector& faces, const std::vector& edges, - const std::vector material_ids, const std::vector& materials, - const std::vector& uvs); + void write(std::string mesh_id, std::string default_material_name, std::vector positions, std::vector normals, std::vector faces, std::vector edges, std::vector material_ids, std::vector materials, std::vector uvs, std::vector material_references); void close(); ColladaSerializer *serializer; }; @@ -115,11 +111,12 @@ private: explicit ColladaEffects(COLLADASW::StreamWriter& stream) : COLLADASW::LibraryEffects(&stream) {} - void write(const IfcGeom::Material& material); + void write(const IfcGeom::Material &material, const std::string &material_uri); void close(); ColladaSerializer *serializer; }; std::vector materials; + std::vector material_uris; public: explicit ColladaMaterials(COLLADASW::StreamWriter& stream, ColladaSerializer *_serializer) : COLLADASW::LibraryMaterials(&stream) @@ -127,6 +124,7 @@ private: , effects(stream) {} void add(const IfcGeom::Material& material); + std::string getMaterialUri(const IfcGeom::Material& material); bool contains(const IfcGeom::Material& material); void write(); ColladaSerializer *serializer; From 327be7030f3cf09d4535ed5f5364f9fe396bee18 Mon Sep 17 00:00:00 2001 From: Stinkfist0 Date: Tue, 2 Oct 2018 16:29:40 +0300 Subject: [PATCH 06/11] ColladaSerializer minor tweaks and to-dos. --- src/ifcconvert/ColladaSerializer.cpp | 21 +++++++++++++-------- src/ifcconvert/ColladaSerializer.h | 8 +++++++- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/ifcconvert/ColladaSerializer.cpp b/src/ifcconvert/ColladaSerializer.cpp index 78531a2a52..e0c3eb169c 100644 --- a/src/ifcconvert/ColladaSerializer.cpp +++ b/src/ifcconvert/ColladaSerializer.cpp @@ -60,7 +60,12 @@ void ColladaSerializer::ColladaExporter::ColladaGeometries::addFloatSource(const source.finish(); } -void ColladaSerializer::ColladaExporter::ColladaGeometries::write(std::string mesh_id, std::string, std::vector positions, std::vector normals, std::vector faces, std::vector edges, std::vector material_ids, std::vector materials, std::vector uvs, std::vector material_references) +void ColladaSerializer::ColladaExporter::ColladaGeometries::write( + const std::string &mesh_id, const std::string &/**<@todo 'default_material_name' unused, remove? */, + const std::vector& positions, const std::vector& normals, + const std::vector& faces, const std::vector& edges, + const std::vector& material_ids, const std::vector& /**<@todo 'materials' unused, remove? */, + const std::vector& uvs, const std::vector& material_references) { openMesh(mesh_id); @@ -217,7 +222,7 @@ void ColladaSerializer::ColladaExporter::ColladaScene::add( node.addMatrix(matrix_array); COLLADASW::InstanceGeometry instanceGeometry(mSW); instanceGeometry.setUrl("#" + geom_name); - BOOST_FOREACH(std::string material_name, material_ids) { + BOOST_FOREACH(const std::string &material_name, material_ids) { // Unescape to avoid double escaping beucase OpenCollada's material URI parameter escapes XML internally std::string unescaped = material_name; IfcUtil::unescape_xml(unescaped); @@ -303,7 +308,8 @@ void ColladaSerializer::ColladaExporter::ColladaScene::write() { } } -void ColladaSerializer::ColladaExporter::ColladaMaterials::ColladaEffects::write(const IfcGeom::Material &material, const std::string &material_uri) +void ColladaSerializer::ColladaExporter::ColladaMaterials::ColladaEffects::write( + const IfcGeom::Material &material, const std::string &material_uri) { openEffect(material_uri + "-fx"); COLLADASW::EffectProfile effect(mSW); @@ -354,10 +360,8 @@ void ColladaSerializer::ColladaExporter::ColladaMaterials::add(const IfcGeom::Ma std::string ColladaSerializer::ColladaExporter::ColladaMaterials::getMaterialUri(const IfcGeom::Material& material) { std::vector::iterator it = std::find(materials.begin(), materials.end(), material); - long index = std::distance(materials.begin(), it); - - std::string material_uri = material_uris.at(static_cast(index)); - return material_uri; + ptrdiff_t index = std::distance(materials.begin(), it); + return material_uris.at(index); } bool ColladaSerializer::ColladaExporter::ColladaMaterials::contains(const IfcGeom::Material& material) { @@ -463,7 +467,8 @@ void ColladaSerializer::ColladaExporter::endDocument() { continue; } geometries_written.insert(it->representation_id); - geometries.write(it->representation_id, it->type, it->vertices, it->normals, it->faces, it->edges, it->material_ids, it->materials, it->uvs, it->material_references); + geometries.write(it->representation_id, it->type, it->vertices, it->normals, it->faces, it->edges, + it->material_ids, it->materials, it->uvs, it->material_references); } geometries.close(); diff --git a/src/ifcconvert/ColladaSerializer.h b/src/ifcconvert/ColladaSerializer.h index dbee52260e..8c435f81a0 100644 --- a/src/ifcconvert/ColladaSerializer.h +++ b/src/ifcconvert/ColladaSerializer.h @@ -69,7 +69,13 @@ private: {} void addFloatSource(const std::string& mesh_id, const std::string& suffix, const std::vector& floats, const char* coords = "XYZ"); - void write(std::string mesh_id, std::string default_material_name, std::vector positions, std::vector normals, std::vector faces, std::vector edges, std::vector material_ids, std::vector materials, std::vector uvs, std::vector material_references); + /// @todo pass simply DeferredObject? + void write( + const std::string &mesh_id, const std::string &default_material_name, + const std::vector& positions, const std::vector& normals, + const std::vector& faces, const std::vector& edges, + const std::vector& material_ids, const std::vector& materials, + const std::vector& uvs, const std::vector& material_references); void close(); ColladaSerializer *serializer; }; From d3092090dfda363301641619a631f4bd72c0eaff Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Mon, 10 Dec 2018 17:15:39 +0100 Subject: [PATCH 07/11] Substitute tiny trimmed curves with linear approximation when deflection within tolerance --- src/ifcgeom/IfcGeomShapes.cpp | 18 ++++++++-- src/ifcgeom/IfcGeomWires.cpp | 67 ++++++++++++++++++++++++++++++----- 2 files changed, 74 insertions(+), 11 deletions(-) diff --git a/src/ifcgeom/IfcGeomShapes.cpp b/src/ifcgeom/IfcGeomShapes.cpp index 17acda26e7..d41025c659 100644 --- a/src/ifcgeom/IfcGeomShapes.cpp +++ b/src/ifcgeom/IfcGeomShapes.cpp @@ -983,8 +983,22 @@ bool IfcGeom::Kernel::convert(const IfcSchema::IfcSweptDiskSolid* l, TopoDS_Shap { gp_Pnt directrix_origin; gp_Vec directrix_tangent; - TopExp_Explorer exp(wire, TopAbs_EDGE); - TopoDS_Edge edge = TopoDS::Edge(exp.Current()); + + TopoDS_Edge edge; + + // Find first edge + TopoDS_Vertex v0, v1; + TopExp::Vertices(wire, v0, v1); + TopTools_IndexedDataMapOfShapeListOfShape map; + TopExp::MapShapesAndAncestors(wire, TopAbs_VERTEX, TopAbs_EDGE, map); + TopTools_ListOfShape edges; + if (map.FindFromKey(v0, edges) && edges.Extent() == 1) { + edge = TopoDS::Edge(edges.First()); + } else { + Logger::Error("Unable to locate first edge of:", l->Directrix()->entity); + return false; + } + double u0, u1; Handle(Geom_Curve) crv = BRep_Tool::Curve(edge, u0, u1); crv->D1(u0, directrix_origin, directrix_tangent); diff --git a/src/ifcgeom/IfcGeomWires.cpp b/src/ifcgeom/IfcGeomWires.cpp index f84fa867bb..da868e7af0 100644 --- a/src/ifcgeom/IfcGeomWires.cpp +++ b/src/ifcgeom/IfcGeomWires.cpp @@ -417,6 +417,31 @@ bool IfcGeom::Kernel::convert(const IfcSchema::IfcCompositeCurve* l, TopoDS_Wire return true; } +namespace { + + /* + Below is code to deduce the formula below in SageMath + + | R, b = var('R b') + | + | Bxy = R * cos(b), R * sin(b) + | Cxy = R * cos(b/2), R * sin(b/2) + | + | def dot(v, w): + | return v[0] * w[0] + v[1] * w[1] + | + | def norm(v): + | l = sqrt(v[0]^2 + v[1]^2) + | return v[0] / l, v[1] / l + | + | (R - R*dot(norm(Cxy), norm(Bxy))).full_simplify() + */ + + double deflection_for_approximating_circle(double radius, double param) { + return -radius * cos(1 / 2 * param)*cos(param) - radius * sin(1 / 2 * param)*sin(param) + radius; + } +} + bool IfcGeom::Kernel::convert(const IfcSchema::IfcTrimmedCurve* l, TopoDS_Wire& wire) { IfcSchema::IfcCurve* basis_curve = l->BasisCurve(); bool isConic = basis_curve->is(IfcSchema::Type::IfcConic); @@ -449,7 +474,8 @@ bool IfcGeom::Kernel::convert(const IfcSchema::IfcTrimmedCurve* l, TopoDS_Wire& bool has_flts[2] = {false,false}; bool has_pnts[2] = {false,false}; - BRepBuilderAPI_MakeWire w; + TopoDS_Edge e; + for ( IfcEntityList::it it = trims1->begin(); it != trims1->end(); it ++ ) { IfcUtil::IfcBaseClass* i = *it; if ( i->is(IfcSchema::Type::IfcCartesianPoint) ) { @@ -486,15 +512,15 @@ bool IfcGeom::Kernel::convert(const IfcSchema::IfcTrimmedCurve* l, TopoDS_Wire& TopoDS_Vertex v2 = BRepBuilderAPI_MakeVertex(pnts[1]); FTol.SetTolerance(v1, getValue(GV_PRECISION), TopAbs_VERTEX); FTol.SetTolerance(v2, getValue(GV_PRECISION), TopAbs_VERTEX); - BRepBuilderAPI_MakeEdge e (curve,v1,v2); - if ( ! e.IsDone() ) { - BRepBuilderAPI_EdgeError err = e.Error(); + BRepBuilderAPI_MakeEdge me (curve,v1,v2); + if (!me.IsDone()) { + BRepBuilderAPI_EdgeError err = me.Error(); if ( err == BRepBuilderAPI_PointProjectionFailed ) { Logger::Message(Logger::LOG_WARNING,"Point projection failed for:",l->entity); trim_cartesian_failed = true; } } else { - w.Add(e.Edge()); + e = me.Edge(); } } @@ -519,15 +545,38 @@ bool IfcGeom::Kernel::convert(const IfcSchema::IfcTrimmedCurve* l, TopoDS_Wire& } } if ( isConic && ALMOST_THE_SAME(fmod(flts[1]-flts[0],M_PI*2.),0.) ) { - w.Add(BRepBuilderAPI_MakeEdge(curve)); + e = BRepBuilderAPI_MakeEdge(curve).Edge(); } else { - BRepBuilderAPI_MakeEdge e (curve,flts[0],flts[1]); - w.Add(e.Edge()); + BRepBuilderAPI_MakeEdge me (curve,flts[0],flts[1]); + e = me.Edge(); } } else if ( trim_cartesian_failed && (has_pnts[0] && has_pnts[1]) ) { - w.Add(BRepBuilderAPI_MakeEdge(pnts[0],pnts[1])); + e = BRepBuilderAPI_MakeEdge(pnts[0], pnts[1]).Edge(); } + if (isConic) { + // Tiny circle segnments can cause issues later on, for example + // when the comp curve is used as the sweeping directrix. + double a, b; + Handle(Geom_Curve) crv = BRep_Tool::Curve(e, a, b); + double radius = -1.; + if (crv->DynamicType() == STANDARD_TYPE(Geom_Circle)) { + radius = Handle(Geom_Circle)::DownCast(crv)->Radius(); + } else if (crv->DynamicType() == STANDARD_TYPE(Geom_Ellipse)) { + // The formula above is for circles, but probably good enough + radius = Handle(Geom_Ellipse)::DownCast(crv)->MajorRadius(); + } + if (radius > 0. && deflection_for_approximating_circle(radius, b - a) < getValue(GV_PRECISION)) { + TopoDS_Vertex v0, v1; + TopExp::Vertices(e, v0, v1); + e = TopoDS::Edge(BRepBuilderAPI_MakeEdge(v0, v1).Edge().Oriented(e.Orientation())); + Logger::Warning("Subsituted edge with linear approximation", l->entity); + } + } + + BRepBuilderAPI_MakeWire w; + w.Add(e); + if (w.IsDone()) { wire = w.Wire(); From d39c7bd6414297f3b478b1c52170b28b27b1a120 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Mon, 10 Dec 2018 17:26:41 +0100 Subject: [PATCH 08/11] Postfix integers --- src/ifcgeom/IfcGeomWires.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ifcgeom/IfcGeomWires.cpp b/src/ifcgeom/IfcGeomWires.cpp index da868e7af0..8fe677d4b6 100644 --- a/src/ifcgeom/IfcGeomWires.cpp +++ b/src/ifcgeom/IfcGeomWires.cpp @@ -438,7 +438,7 @@ namespace { */ double deflection_for_approximating_circle(double radius, double param) { - return -radius * cos(1 / 2 * param)*cos(param) - radius * sin(1 / 2 * param)*sin(param) + radius; + return -radius * std::cos(1. / 2. * param) * std::cos(param) - radius * std::sin(1. / 2. * param) * std::sin(param) + radius; } } From 336601a7ae6dd18d363501de96a91cc480daad82 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Tue, 11 Dec 2018 10:55:03 +0100 Subject: [PATCH 09/11] Missing FindFromKey() overload on older versions of occt --- src/ifcgeom/IfcGeomShapes.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ifcgeom/IfcGeomShapes.cpp b/src/ifcgeom/IfcGeomShapes.cpp index d41025c659..1f263cf7dd 100644 --- a/src/ifcgeom/IfcGeomShapes.cpp +++ b/src/ifcgeom/IfcGeomShapes.cpp @@ -991,9 +991,8 @@ bool IfcGeom::Kernel::convert(const IfcSchema::IfcSweptDiskSolid* l, TopoDS_Shap TopExp::Vertices(wire, v0, v1); TopTools_IndexedDataMapOfShapeListOfShape map; TopExp::MapShapesAndAncestors(wire, TopAbs_VERTEX, TopAbs_EDGE, map); - TopTools_ListOfShape edges; - if (map.FindFromKey(v0, edges) && edges.Extent() == 1) { - edge = TopoDS::Edge(edges.First()); + if (map.Contains(v0) && map.FindFromKey(v0).Extent() == 1) { + edge = TopoDS::Edge(map.FindFromKey(v0).First()); } else { Logger::Error("Unable to locate first edge of:", l->Directrix()->entity); return false; From a8f4f0270a5ef550fc384d593b0b9bc17d740b0b Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Tue, 11 Dec 2018 13:01:28 +0100 Subject: [PATCH 10/11] Don't double quote boost dependency dir #515 --- win/build-deps.cmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/win/build-deps.cmd b/win/build-deps.cmd index d0078dea78..afc47cb8f5 100644 --- a/win/build-deps.cmd +++ b/win/build-deps.cmd @@ -158,7 +158,7 @@ set BOOST_VERSION=1.67.0 set BOOST_VER=%BOOST_VERSION:.=_% :: DEPENDENCY_NAME is used for logging and DEPENDENCY_DIR for saving from some redundant typing set DEPENDENCY_NAME=Boost %BOOST_VERSION% -set DEPENDENCY_DIR="%DEPS_DIR%\boost_%BOOST_VER%" +set DEPENDENCY_DIR=%DEPS_DIR%\boost_%BOOST_VER% set BOOST_LIBRARYDIR=%DEPENDENCY_DIR%\stage\%VS_PLATFORM%\lib :: NOTE Also zip download exists, if encountering problems with 7z for some reason. set ZIP_EXT=7z From a0f37cf9ea0059c73783f1995b633e811a846373 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Wed, 12 Dec 2018 16:12:38 +0100 Subject: [PATCH 11/11] Fix some warnings --- cmake/CMakeLists.txt | 2 +- src/ifcgeom/IfcGeomIterator.h | 2 +- src/ifcgeom/IfcGeomWires.cpp | 6 ++---- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/cmake/CMakeLists.txt b/cmake/CMakeLists.txt index 01d7d3467c..4221827b10 100644 --- a/cmake/CMakeLists.txt +++ b/cmake/CMakeLists.txt @@ -413,7 +413,7 @@ IF(MSVC) ENDIF() ENDFOREACH() ElSE() - add_definitions(-Wall -Wextra) + add_definitions(-Wall -Wextra -Wno-maybe-uninitialized) if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") add_definitions(-Wno-tautological-constant-out-of-range-compare) endif() diff --git a/src/ifcgeom/IfcGeomIterator.h b/src/ifcgeom/IfcGeomIterator.h index c48e934a5c..f452845b8c 100644 --- a/src/ifcgeom/IfcGeomIterator.h +++ b/src/ifcgeom/IfcGeomIterator.h @@ -155,8 +155,8 @@ namespace IfcGeom { Iterator(const IteratorSettings& settings, IfcParse::IfcFile* file, std::vector& filters) : settings(settings) , ifc_file(file) - , owns_ifc_file(false) , filters_(filters) + , owns_ifc_file(false) { _initialize(); } diff --git a/src/ifcgeom/IfcGeomWires.cpp b/src/ifcgeom/IfcGeomWires.cpp index 8fe677d4b6..884efbec35 100644 --- a/src/ifcgeom/IfcGeomWires.cpp +++ b/src/ifcgeom/IfcGeomWires.cpp @@ -128,7 +128,7 @@ namespace { Handle(Geom_Curve) crv = BRep_Tool::Curve(e, _, __); const bool is_line = crv->DynamicType() == STANDARD_TYPE(Geom_Line); const bool is_circle = crv->DynamicType() == STANDARD_TYPE(Geom_Circle); - all_linear = all_linear && all_linear; + all_linear = all_linear && is_line; single_circle = first && is_circle; } @@ -209,7 +209,7 @@ namespace { double dist = p1.Distance(p2); - // Distance is within 2p, this is fine + // Distance is within tolerance, this is fine if (dist < p_) { mw_.Add(w1); goto check; @@ -403,8 +403,6 @@ bool IfcGeom::Kernel::convert(const IfcSchema::IfcCompositeCurve* l, TopoDS_Wire BRepBuilderAPI_MakeWire w; TopoDS_Vertex wire_first_vertex, wire_last_vertex, edge_first_vertex, edge_last_vertex; - const double precision_sq_2 = 2 * getValue(GV_PRECISION) * getValue(GV_PRECISION); - TopTools_ListIteratorOfListOfShape it(converted_segments); IfcEntityList::ptr profile = l->entity->getInverse(IfcSchema::Type::IfcProfileDef, -1);