From 048db115bdc3f707eba3f56e9dbb7ce998e52bd6 Mon Sep 17 00:00:00 2001
From: Jean Niklas L'orange <jeannikl@hypirion.com>
Date: Thu, 11 Oct 2018 16:07:59 +0200
Subject: [PATCH] Catch and inform about errors

The changes in how specular roughness and transparency is read in was
done to ensure type safety. If you added

    "transparency": "0.8"

to the file, then that would be silently ignored. In the new style, it
instead errors out if the type cannot be converted to a double.
---
 src/ifcconvert/IfcConvert.cpp       | 12 ++++++--
 src/ifcgeom/IfcGeomRenderStyles.cpp | 48 ++++++++++++++++-------------
 2 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/src/ifcconvert/IfcConvert.cpp b/src/ifcconvert/IfcConvert.cpp
index f1a4beb457..db97f70967 100644
--- a/src/ifcconvert/IfcConvert.cpp
+++ b/src/ifcconvert/IfcConvert.cpp
@@ -497,9 +497,15 @@ int main(int argc, char** argv)
         }
     }
 
-    if (!default_material_filename.empty()) {
-      IfcGeom::set_default_style(default_material_filename);
-    }
+		if (!default_material_filename.empty()) {
+			try {
+				IfcGeom::set_default_style(default_material_filename);
+			} catch (const std::exception& e) {
+				std::cerr << "[Error] Could not read default material file " << default_material_filename << ":" << std::endl;
+				std::cerr << e.what() << std::endl;
+				return EXIT_FAILURE;
+			}
+		}
 
     /// @todo Clean up this filter code further.
     std::vector<geom_filter> used_filters;
diff --git a/src/ifcgeom/IfcGeomRenderStyles.cpp b/src/ifcgeom/IfcGeomRenderStyles.cpp
index f3499af235..9c10ca499a 100644
--- a/src/ifcgeom/IfcGeomRenderStyles.cpp
+++ b/src/ifcgeom/IfcGeomRenderStyles.cpp
@@ -176,6 +176,25 @@ void InitDefaultMaterials() {
 	default_materials_initialized = true;
 }
 
+boost::optional<IfcGeom::SurfaceStyle::ColorComponent> read_colour_component(const boost::optional<pt::ptree&> list) {
+	if (!list) {
+		return boost::none;
+	}
+	double rgb[3];
+	int i = 0;
+	for (pt::ptree::value_type &colour : list.get()) {
+		if (3 <= i) {
+			throw std::runtime_error("rgb array over 3 elements large");
+		}
+		rgb[i] = colour.second.get_value<double>();
+		i++;
+	}
+	if (i != 3) {
+		throw std::runtime_error("rgb array less than 3 elements large (was " + std::to_string(i) + ")");
+	}
+	return IfcGeom::SurfaceStyle::ColorComponent(rgb[0], rgb[1], rgb[2]);
+}
+
 void IfcGeom::set_default_style(const std::string& json_file) {
   if (!default_materials_initialized) InitDefaultMaterials();
   default_materials.clear();
@@ -189,30 +208,17 @@ void IfcGeom::set_default_style(const std::string& json_file) {
 
     pt::ptree material = material_pair.second;
     boost::optional<pt::ptree&> diffuse = material.get_child_optional("diffuse");
-    if (diffuse) {
-      double rgb[3];
-      int i = 0;
-      for (pt::ptree::value_type &colour : diffuse.get()) {
-        rgb[i] = colour.second.get_value<double>();
-        i++;
-      }
-      default_materials[name].Diffuse().reset(IfcGeom::SurfaceStyle::ColorComponent(rgb[0], rgb[1], rgb[2]));
-    }
+    default_materials[name].Diffuse() = read_colour_component(diffuse);
+
     boost::optional<pt::ptree&> specular = material.get_child_optional("specular");
-    if (specular) {
-      double rgb[3];
-      int i = 0;
-      for (pt::ptree::value_type &colour : specular.get()) {
-        rgb[i] = colour.second.get_value<double>();
-        i++;
-      }
-      default_materials[name].Specular().reset(IfcGeom::SurfaceStyle::ColorComponent(rgb[0], rgb[1], rgb[2]));
+    default_materials[name].Specular() = read_colour_component(specular);
+
+    if (material.get_child_optional("specular-roughness")) {
+      default_materials[name].Specularity().reset(1.0 / material.get<double>("specular-roughness"));
     }
-    boost::optional<double> specular_roughness = material.get_optional<double>("specular-roughness");
-    if (specular_roughness) {
-      default_materials[name].Specularity().reset(1.0 / specular_roughness.get());
+    if (material.get_child_optional("transparency")) {
+      default_materials[name].Transparency() = material.get<double>("transparency");
     }
-    default_materials[name].Transparency() = material.get_optional<double>("transparency");
   }
 
 	// Is "*" present? If yes, remove it and make it the default style.