Merge branch 'master' into v0.6.0

# Conflicts:
#	src/ifcgeom/IfcGeomIteratorImplementation.h

cmake/CMakeLists.txt

@@ -444,7 +444,7 @@ IF(MSVC)
 	ENDFOREACH()
 	ENDIF()
 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()

src/ifcgeom/IfcGeomFunctions.cpp

@@ -1493,6 +1493,9 @@ IfcGeom::BRepElement<P, PP>* IfcGeom::Kernel::create_brep_for_representation_and
 			}
 		}
 	}
+    else {
+        Logger::Warning("Object '" + product->GlobalId() + "' has no material!");
+    }
 
 	if (material_style_applied) {
 		representation_id_builder << "-material-" << single_material->data().id();

src/ifcgeom/IfcGeomIteratorImplementation.h

@@ -724,8 +724,8 @@ namespace IfcGeom {
 		MAKE_TYPE_NAME(IteratorImplementation_)(const IteratorSettings& settings, IfcParse::IfcFile* file, const std::vector<IfcGeom::filter_t>& filters)
 			: settings(settings)
 			, ifc_file(file)
-			, owns_ifc_file(false)
 			, filters_(filters)
+			, owns_ifc_file(false)
 		{
 			_initialize();
 		}

src/ifcgeom/IfcGeomRenderStyles.cpp

@@ -135,5 +135,6 @@ 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);
 }

src/ifcgeom/IfcGeomShapes.cpp

@@ -993,8 +993,21 @@ 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);
+		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());
+			return false;
+		}
+
 		double u0, u1;
 		Handle(Geom_Curve) crv = BRep_Tool::Curve(edge, u0, u1);
 		crv->D1(u0, directrix_origin, directrix_tangent);

src/ifcgeom/IfcGeomWires.cpp

@@ -130,7 +130,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;
 		}
 
@@ -211,7 +211,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;
@@ -405,8 +405,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->data().getInverse(&IfcSchema::IfcProfileDef::Class(), -1);
@@ -419,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 * std::cos(1. / 2. * param) * std::cos(param) - radius * std::sin(1. / 2. * param) * std::sin(param) + radius;
+	}
+}
+
 bool IfcGeom::Kernel::convert(const IfcSchema::IfcTrimmedCurve* l, TopoDS_Wire& wire) {
 	IfcSchema::IfcCurve* basis_curve = l->BasisCurve();
 	bool isConic = basis_curve->declaration().is(IfcSchema::IfcConic::Class());
@@ -451,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->declaration().is(IfcSchema::IfcCartesianPoint::Class()) ) {
@@ -488,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);
 				trim_cartesian_failed = true;
 			}
 		} else {
-			w.Add(e.Edge());
+			e = me.Edge();
 		}
 	}
 
@@ -521,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);
+		}
+	}
+
+	BRepBuilderAPI_MakeWire w;
+	w.Add(e);
+
 	if (w.IsDone()) {
 		wire = w.Wire();
 

src/serializers/ColladaSerializer.cpp

@@ -61,10 +61,11 @@ void ColladaSerializer::ColladaExporter::ColladaGeometries::addFloatSource(const
 }
 
 void ColladaSerializer::ColladaExporter::ColladaGeometries::write(
-    const std::string &mesh_id, const std::string& /*default_material_name*/, const std::vector<real_t>& positions,
-    const std::vector<real_t>& normals, const std::vector<int>& faces, const std::vector<int>& edges,
-    const std::vector<int> material_ids, const std::vector<IfcGeom::Material>& materials,
-    const std::vector<real_t>& uvs)
+    const std::string &mesh_id, const std::string &/**<@todo 'default_material_name' unused, remove? */,
+    const std::vector<real_t>& positions, const std::vector<real_t>& normals,
+    const std::vector<int>& faces, const std::vector<int>& edges,
+    const std::vector<int>& material_ids, const std::vector<IfcGeom::Material>& /**<@todo 'materials' unused, remove? */,
+    const std::vector<real_t>& uvs, const std::vector<std::string>& material_references)
 {
 	openMesh(mesh_id);
 	
@@ -101,9 +102,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());
-            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;
@@ -158,10 +158,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());
-        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));
@@ -224,11 +221,13 @@ void ColladaSerializer::ColladaExporter::ColladaScene::add(
 	node.start();
 	node.addMatrix(matrix_array);
 	COLLADASW::InstanceGeometry instanceGeometry(mSW);
-	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);
+	instanceGeometry.setUrl("#" + geom_name);
+    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);
+
+        COLLADASW::InstanceMaterial material(material_name, "#" + unescaped);
 		instanceGeometry.getBindMaterial().getInstanceMaterialList().push_back(material);
 	}
 	instanceGeometry.add();
@@ -309,12 +308,10 @@ 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());
-    collada_id(material_name);
-    openEffect(material_name + "-fx");
+    openEffect(material_uri + "-fx");
 	COLLADASW::EffectProfile effect(mSW);
 	effect.setShaderType(COLLADASW::EffectProfile::LAMBERT);
 	if (material.hasDiffuse()) {
@@ -346,11 +343,27 @@ 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<IfcGeom::Material>::iterator it = std::find(materials.begin(), materials.end(), material);
+	ptrdiff_t index = std::distance(materials.begin(), it);
+	return material_uris.at(index);
+}
+
 bool ColladaSerializer::ColladaExporter::ColladaMaterials::contains(const IfcGeom::Material& material) {
 	return std::find(materials.begin(), materials.end(), material) != materials.end();
 }
@@ -358,13 +371,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());
-        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();
@@ -392,16 +405,13 @@ void ColladaSerializer::ColladaExporter::write(const IfcGeom::TriangulationEleme
 
 	std::vector<std::string> 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());
-		collada_id(material_name);
+		materials.add(material);
+
+		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)) {
@@ -474,7 +484,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);
+		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();
 

src/serializers/ColladaSerializer.h

@@ -69,11 +69,13 @@ private:
 			{}
             void addFloatSource(const std::string& mesh_id, const std::string& suffix,
                 const std::vector<real_t>& floats, const char* coords = "XYZ");
-            void write(const std::string &mesh_id, const std::string& default_material_name,
+            /// @todo pass simply DeferredObject?
+            void write(
+                const std::string &mesh_id, const std::string &default_material_name,
                 const std::vector<real_t>& positions, const std::vector<real_t>& normals,
                 const std::vector<int>& faces, const std::vector<int>& edges,
-                const std::vector<int> material_ids, const std::vector<IfcGeom::Material>& materials,
-                const std::vector<real_t>& uvs);
+                const std::vector<int>& material_ids, const std::vector<IfcGeom::Material>& materials,
+                const std::vector<real_t>& uvs, const std::vector<std::string>& material_references);
 			void close();
             ColladaSerializer *serializer;
 		};
@@ -115,11 +117,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<IfcGeom::Material> materials;
+			std::vector<std::string> material_uris;
 		public:
 			explicit ColladaMaterials(COLLADASW::StreamWriter& stream, ColladaSerializer *_serializer)
 				: COLLADASW::LibraryMaterials(&stream)
@@ -127,6 +130,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;

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