From 81a9d0e747d8a545bc68b55ad58e3fa0a3183a29 Mon Sep 17 00:00:00 2001 From: paul <40677073+paullee0@users.noreply.github.com> Date: Mon, 1 Oct 2018 08:02:52 +0800 Subject: [PATCH 01/14] Changing opencollada version to "v1.6.63" Discussion: https://github.com/IfcOpenShell/IfcOpenShell/issues/480 Summary: "... Compiling OpenCOLLADA fail on fedora 27... changing opencollada version to "v1.6.63" (since some time now they do proper tagging over there, the commit we track now is from 19 Dec 2013, probably worth updating as newer compilers might rightfully complain)... Probably best to rm -rf ... IfcOpenShell/build/Linux/x86_64/build/OpenCOLLADA before restarting." --- nix/build-all.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nix/build-all.py b/nix/build-all.py index 7a96d2fbaf..b97e003d24 100644 --- a/nix/build-all.py +++ b/nix/build-all.py @@ -248,7 +248,8 @@ CMAKE_VERSION_2=CMAKE_VERSION[:CMAKE_VERSION.rindex('.')] OCE_LOCATION="https://github.com/tpaviot/oce/archive/OCE-%s.tar.gz" % (OCE_VERSION,) BOOST_LOCATION="http://downloads.sourceforge.net/project/boost/boost/%s/boost_%s.tar.bz2" % (BOOST_VERSION, BOOST_VERSION_UNDERSCORE) OPENCOLLADA_LOCATION="https://github.com/KhronosGroup/OpenCOLLADA.git" -OPENCOLLADA_COMMIT="f99d59e73e565a41715eaebc00c7664e1ee5e628" +#OPENCOLLADA_COMMIT="f99d59e73e565a41715eaebc00c7664e1ee5e628" +OPENCOLLADA_COMMIT="v1.6.63" # Helper functions From 882d27fe252dfd4a8ed96c3f716710d454a50785 Mon Sep 17 00:00:00 2001 From: Stinkfist0 Date: Tue, 25 Sep 2018 10:29:48 +0300 Subject: [PATCH 02/14] Use /permissive- introduced in VS 2017 to enforce standards-conformance. https://docs.microsoft.com/en-us/cpp/build/reference/permissive-standards-conformance?view=vs-2017 --- cmake/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmake/CMakeLists.txt b/cmake/CMakeLists.txt index ad286ec85e..01d7d3467c 100644 --- a/cmake/CMakeLists.txt +++ b/cmake/CMakeLists.txt @@ -396,6 +396,10 @@ IF(MSVC) # Disable overeager and false positives causing C4458 ("declaration of 'indentifier' hides class member"), at least for now. ADD_DEFINITIONS(-wd4458) ENDIF() + # Enforce standards-conformance on VS > 2015, older Boost versions fail to compile with this + if (MSVC_VERSION GREATER 1900 AND (Boost_MAJOR_VERSION GREATER 1 OR Boost_MINOR_VERSION GREATER 66)) + add_definitions(-permissive-) + endif() # Link against the static VC runtime # TODO Make this configurable FOREACH(flag CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE CMAKE_CXX_FLAGS_MINSIZEREL From dab3382b29803956b3cb6561f18346a85f44ce8d Mon Sep 17 00:00:00 2001 From: Jean Niklas L'orange Date: Thu, 20 Sep 2018 15:58:24 +0200 Subject: [PATCH 03/14] Implement support for custom default materials The support for custom default materials is implemented as a JSON file passed via the command line. The default material specification will override the defaults, except the toplevel default value. --- src/ifcconvert/IfcConvert.cpp | 14 +++++++-- src/ifcgeom/IfcGeomRenderStyles.cpp | 49 +++++++++++++++++++++++++++++ src/ifcgeom/IfcGeomRenderStyles.h | 3 +- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/src/ifcconvert/IfcConvert.cpp b/src/ifcconvert/IfcConvert.cpp index cd9fba7f75..f1a4beb457 100644 --- a/src/ifcconvert/IfcConvert.cpp +++ b/src/ifcconvert/IfcConvert.cpp @@ -34,6 +34,7 @@ #include "../ifcconvert/SvgSerializer.h" #include "../ifcgeom/IfcGeomIterator.h" +#include "../ifcgeom/IfcGeomRenderStyles.h" #include #include @@ -178,7 +179,8 @@ int main(int argc, char** argv) inclusion_traverse_filter include_traverse_filter; exclusion_filter exclude_filter; exclusion_traverse_filter exclude_traverse_filter; - std::string filter_filename; + std::string filter_filename; + std::string default_material_filename; po::options_description geom_options("Geometry options"); geom_options.add_options() @@ -259,7 +261,11 @@ int main(int argc, char** argv) "Specifies a filter file that describes the used filtering criteria. Supported formats " "are '--include=arg GlobalId ...' and 'include arg GlobalId ...'. Spaces and tabs can be used as delimiters." "Multiple filters of same type with different values can be inserted on their own lines. " - "See --include, --include+, --exclude, and --exclude+ for more details."); + "See --include, --include+, --exclude, and --exclude+ for more details.") + ("default-material-file", po::value(&default_material_filename), + "Specifies a material file that describes the material object types will have" + "if an object does not have any specified material in the IFC file."); + std::string bounds, offset_str; #ifdef HAVE_ICU @@ -491,6 +497,10 @@ int main(int argc, char** argv) } } + if (!default_material_filename.empty()) { + IfcGeom::set_default_style(default_material_filename); + } + /// @todo Clean up this filter code further. std::vector used_filters; if (include_filter.type != geom_filter::UNUSED) { used_filters.push_back(include_filter); } diff --git a/src/ifcgeom/IfcGeomRenderStyles.cpp b/src/ifcgeom/IfcGeomRenderStyles.cpp index 39c0d4ef37..04875679f5 100644 --- a/src/ifcgeom/IfcGeomRenderStyles.cpp +++ b/src/ifcgeom/IfcGeomRenderStyles.cpp @@ -17,10 +17,16 @@ * * ********************************************************************************/ +#include +#include +#include + #include #include "IfcGeom.h" +namespace pt = boost::property_tree; + bool process_colour(IfcSchema::IfcColourRgb* colour, double* rgb) { if (colour != 0) { rgb[0] = colour->Red(); @@ -170,6 +176,49 @@ void InitDefaultMaterials() { default_materials_initialized = true; } +void IfcGeom::set_default_style(const std::string& json_file) { + if (default_materials_initialized) { + default_materials.clear(); + } + + pt::ptree root; + pt::read_json(json_file, root); + + for (pt::ptree::value_type &material_pair : root) { + std::string name = material_pair.first; + default_materials.insert(std::make_pair(name, IfcGeom::SurfaceStyle(name))); + + pt::ptree material = material_pair.second; + boost::optional 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(); + i++; + } + default_materials[name].Diffuse().reset(IfcGeom::SurfaceStyle::ColorComponent(rgb[0], rgb[1], rgb[2])); + } + boost::optional 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(); + i++; + } + default_materials[name].Specular().reset(IfcGeom::SurfaceStyle::ColorComponent(rgb[0], rgb[1], rgb[2])); + } + boost::optional specular_roughness = material.get_optional("specular-roughness"); + if (specular_roughness) { + default_materials[name].Specularity().reset(1.0 / specular_roughness.get()); + } + default_materials[name].Transparency() = material.get_optional("transparency"); + } + + default_materials_initialized = true; +} + const IfcGeom::SurfaceStyle* IfcGeom::get_default_style(const std::string& s) { if (!default_materials_initialized) InitDefaultMaterials(); std::map::const_iterator it = default_materials.find(s); diff --git a/src/ifcgeom/IfcGeomRenderStyles.h b/src/ifcgeom/IfcGeomRenderStyles.h index 39a67136dd..03c34e2702 100644 --- a/src/ifcgeom/IfcGeomRenderStyles.h +++ b/src/ifcgeom/IfcGeomRenderStyles.h @@ -96,7 +96,8 @@ namespace IfcGeom { boost::optional& Specularity() { return specularity; } }; - IFC_GEOM_API const SurfaceStyle* get_default_style(const std::string& ifc_type); + IFC_GEOM_API const SurfaceStyle* get_default_style(const std::string& ifc_type); + IFC_GEOM_API void set_default_style(const std::string& json_file); } #endif From a6d3619449817c46e6bedce345d8c00e0611baa7 Mon Sep 17 00:00:00 2001 From: Jean Niklas L'orange Date: Thu, 11 Oct 2018 12:51:17 +0200 Subject: [PATCH 04/14] Fix bug where default material was not initialised We kind-of sort-of do double work here, but I think it is better to always assume that InitDefaultMaterials when setting the defaults, to avoid too many code paths. --- src/ifcgeom/IfcGeomRenderStyles.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ifcgeom/IfcGeomRenderStyles.cpp b/src/ifcgeom/IfcGeomRenderStyles.cpp index 04875679f5..cf0fac335b 100644 --- a/src/ifcgeom/IfcGeomRenderStyles.cpp +++ b/src/ifcgeom/IfcGeomRenderStyles.cpp @@ -177,9 +177,10 @@ void InitDefaultMaterials() { } void IfcGeom::set_default_style(const std::string& json_file) { - if (default_materials_initialized) { - default_materials.clear(); + if (!default_materials_initialized) { + InitDefaultMaterials(); } + default_materials.clear(); pt::ptree root; pt::read_json(json_file, root); @@ -216,7 +217,6 @@ void IfcGeom::set_default_style(const std::string& json_file) { default_materials[name].Transparency() = material.get_optional("transparency"); } - default_materials_initialized = true; } const IfcGeom::SurfaceStyle* IfcGeom::get_default_style(const std::string& s) { From 7e9e56cd2c0137644668139b7951421e79442ec1 Mon Sep 17 00:00:00 2001 From: Jean Niklas L'orange Date: Thu, 11 Oct 2018 14:43:27 +0200 Subject: [PATCH 05/14] Support setting default "fallback" material style Specifying a fallback default material style is added by specifying a style for "*" in the --default-material-file. --- src/ifcgeom/IfcGeomRenderStyles.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/ifcgeom/IfcGeomRenderStyles.cpp b/src/ifcgeom/IfcGeomRenderStyles.cpp index cf0fac335b..f3499af235 100644 --- a/src/ifcgeom/IfcGeomRenderStyles.cpp +++ b/src/ifcgeom/IfcGeomRenderStyles.cpp @@ -177,9 +177,7 @@ void InitDefaultMaterials() { } void IfcGeom::set_default_style(const std::string& json_file) { - if (!default_materials_initialized) { - InitDefaultMaterials(); - } + if (!default_materials_initialized) InitDefaultMaterials(); default_materials.clear(); pt::ptree root; @@ -217,6 +215,16 @@ void IfcGeom::set_default_style(const std::string& json_file) { default_materials[name].Transparency() = material.get_optional("transparency"); } + // Is "*" present? If yes, remove it and make it the default style. + std::map::const_iterator it = default_materials.find("*"); + if (it != default_materials.end()) { + IfcGeom::SurfaceStyle star = it->second; + default_material.Diffuse() = star.Diffuse(); + default_material.Specular() = star.Specular(); + default_material.Specularity() = star.Specularity(); + default_material.Transparency() = star.Transparency(); + default_materials.erase(it); + } } const IfcGeom::SurfaceStyle* IfcGeom::get_default_style(const std::string& s) { @@ -224,7 +232,10 @@ const IfcGeom::SurfaceStyle* IfcGeom::get_default_style(const std::string& s) { std::map::const_iterator it = default_materials.find(s); if (it == default_materials.end()) { default_materials.insert(std::make_pair(s, IfcGeom::SurfaceStyle(s))); - default_materials[s].Diffuse().reset(*default_material.Diffuse()); + default_materials[s].Diffuse() = default_material.Diffuse(); + default_materials[s].Specular() = default_material.Specular(); + default_materials[s].Specularity() = default_material.Specularity(); + default_materials[s].Transparency() = default_material.Transparency(); it = default_materials.find(s); } const IfcGeom::SurfaceStyle& surface_style = it->second; From 9c1660a84b16a4714a6dafd27025bd2430946ca1 Mon Sep 17 00:00:00 2001 From: Jean Niklas L'orange Date: Thu, 11 Oct 2018 16:07:59 +0200 Subject: [PATCH 06/14] 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 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 read_colour_component(const boost::optional 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(); + 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 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(); - 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 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(); - 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("specular-roughness")); } - boost::optional specular_roughness = material.get_optional("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("transparency"); } - default_materials[name].Transparency() = material.get_optional("transparency"); } // Is "*" present? If yes, remove it and make it the default style. From e39835d25e5aeec99bea44f0a204d0f381cc0289 Mon Sep 17 00:00:00 2001 From: Jean Niklas L'orange Date: Fri, 12 Oct 2018 10:32:25 +0200 Subject: [PATCH 07/14] Make API more consistent --- src/ifcconvert/IfcConvert.cpp | 2 +- src/ifcgeom/IfcGeomRenderStyles.cpp | 2 +- src/ifcgeom/IfcGeomRenderStyles.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ifcconvert/IfcConvert.cpp b/src/ifcconvert/IfcConvert.cpp index db97f70967..158d605173 100644 --- a/src/ifcconvert/IfcConvert.cpp +++ b/src/ifcconvert/IfcConvert.cpp @@ -499,7 +499,7 @@ int main(int argc, char** argv) if (!default_material_filename.empty()) { try { - IfcGeom::set_default_style(default_material_filename); + IfcGeom::set_default_style_file(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; diff --git a/src/ifcgeom/IfcGeomRenderStyles.cpp b/src/ifcgeom/IfcGeomRenderStyles.cpp index 9c10ca499a..c47f10a236 100644 --- a/src/ifcgeom/IfcGeomRenderStyles.cpp +++ b/src/ifcgeom/IfcGeomRenderStyles.cpp @@ -195,7 +195,7 @@ boost::optional read_colour_component(con return IfcGeom::SurfaceStyle::ColorComponent(rgb[0], rgb[1], rgb[2]); } -void IfcGeom::set_default_style(const std::string& json_file) { +void IfcGeom::set_default_style_file(const std::string& json_file) { if (!default_materials_initialized) InitDefaultMaterials(); default_materials.clear(); diff --git a/src/ifcgeom/IfcGeomRenderStyles.h b/src/ifcgeom/IfcGeomRenderStyles.h index 03c34e2702..1ffe03149b 100644 --- a/src/ifcgeom/IfcGeomRenderStyles.h +++ b/src/ifcgeom/IfcGeomRenderStyles.h @@ -97,7 +97,7 @@ namespace IfcGeom { }; IFC_GEOM_API const SurfaceStyle* get_default_style(const std::string& ifc_type); - IFC_GEOM_API void set_default_style(const std::string& json_file); + IFC_GEOM_API void set_default_style_file(const std::string& json_file); } #endif From c420cc1abb3382b39dd134144256d35a9b765500 Mon Sep 17 00:00:00 2001 From: Stinkfist0 Date: Mon, 8 Oct 2018 10:38:27 +0300 Subject: [PATCH 08/14] IfcFile::Init: return false if unexpected schema encountered. --- src/ifcparse/IfcParse.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ifcparse/IfcParse.cpp b/src/ifcparse/IfcParse.cpp index b7cfd58b46..665440ffa9 100644 --- a/src/ifcparse/IfcParse.cpp +++ b/src/ifcparse/IfcParse.cpp @@ -1267,7 +1267,10 @@ bool IfcFile::Init(IfcParse::IfcSpfStream* s) { } if (schemas.size() != 1 || schemas[0] != IfcSchema::Identifier) { - Logger::Message(Logger::LOG_ERROR, std::string("File schema encountered different from expected '") + IfcSchema::Identifier + "'"); + Logger::Message(Logger::LOG_ERROR, "File schema encountered (" + + boost::algorithm::join(schemas, ", ") + ") different from expected " + + std::string(IfcSchema::Identifier) + "."); + return false; } boost::circular_buffer token_stream(3, Token()); From 2024a24e31a062b86eecca953d256a6d57cdade9 Mon Sep 17 00:00:00 2001 From: Stinkfist0 Date: Mon, 8 Oct 2018 10:39:27 +0300 Subject: [PATCH 09/14] IfcConvert: proper "exit routine" if initialization of input fails when trying to create geometry files. --- src/ifcconvert/IfcConvert.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ifcconvert/IfcConvert.cpp b/src/ifcconvert/IfcConvert.cpp index 158d605173..6d40194ea8 100644 --- a/src/ifcconvert/IfcConvert.cpp +++ b/src/ifcconvert/IfcConvert.cpp @@ -595,6 +595,7 @@ int main(int argc, char** argv) if (use_element_hierarchy && output_extension != ".dae") { std::cerr << "[Error] --use-element-hierarchy can be used only with .dae output.\n"; + /// @todo Lots of duplicate error-and-exit code. write_log(!quiet); print_usage(); delete serializer; @@ -628,6 +629,9 @@ int main(int argc, char** argv) time(&start); if (!init_input_file(input_filename, ifc_file, no_progress || quiet, mmap)) { + write_log(!quiet); + delete serializer; + std::remove(output_temp_filename.c_str()); /**< @todo Windows Unicode support */ return EXIT_FAILURE; } From 58759cf365bb8d575c77f4e26c5488690a9df35f Mon Sep 17 00:00:00 2001 From: Stinkfist0 Date: Mon, 8 Oct 2018 16:36:48 +0300 Subject: [PATCH 10/14] IfcConvert: prevent memory leaks and clean up code by utilizing shared_ptr. --- src/ifcconvert/IfcConvert.cpp | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/ifcconvert/IfcConvert.cpp b/src/ifcconvert/IfcConvert.cpp index 6d40194ea8..4659875800 100644 --- a/src/ifcconvert/IfcConvert.cpp +++ b/src/ifcconvert/IfcConvert.cpp @@ -40,6 +40,7 @@ #include #include +#include #include #include @@ -556,7 +557,7 @@ int main(int argc, char** argv) settings.set_deflection_tolerance(deflection_tolerance); settings.precision = precision; - GeometrySerializer* serializer; + boost::shared_ptr serializer; /**< @todo use std::unique_ptr when possible */ if (output_extension == ".obj") { // Do not use temp file for MTL as it's such a small file. const std::string mtl_filename = change_extension(output_filename, "mtl"); @@ -564,25 +565,25 @@ int main(int argc, char** argv) Logger::Notice("Using world coords when writing WaveFront OBJ files"); settings.set(IfcGeom::IteratorSettings::USE_WORLD_COORDS, true); } - serializer = new WaveFrontOBJSerializer(output_temp_filename, mtl_filename, settings); + serializer = boost::make_shared(output_temp_filename, mtl_filename, settings); #ifdef WITH_OPENCOLLADA } else if (output_extension == ".dae") { - serializer = new ColladaSerializer(output_temp_filename, settings); + serializer = boost::make_shared(output_temp_filename, settings); #endif } else if (output_extension == ".stp") { - serializer = new StepSerializer(output_temp_filename, settings); + serializer = boost::make_shared(output_temp_filename, settings); } else if (output_extension == ".igs") { IGESControl_Controller::Init(); // work around Open Cascade bug - serializer = new IgesSerializer(output_temp_filename, settings); + serializer = boost::make_shared(output_temp_filename, settings); } else if (output_extension == ".svg") { settings.set(IfcGeom::IteratorSettings::DISABLE_TRIANGULATION, true); - serializer = new SvgSerializer(output_temp_filename, settings); + serializer = boost::make_shared(output_temp_filename, settings); if (vmap.count("section-height") != 0) { Logger::Notice("Overriding section height"); - static_cast(serializer)->setSectionHeight(section_height); + static_cast(serializer.get())->setSectionHeight(section_height); } if (bounding_width.is_initialized() && bounding_height.is_initialized()) { - static_cast(serializer)->setBoundingRectangle(bounding_width.get(), bounding_height.get()); + static_cast(serializer.get())->setBoundingRectangle(bounding_width.get(), bounding_height.get()); } } else { std::cerr << "[Error] Unknown output filename extension '" + output_extension + "'\n"; @@ -591,14 +592,11 @@ int main(int argc, char** argv) return EXIT_FAILURE; } - // NOTE After this point, make sure to delete serializer upon application exit. - if (use_element_hierarchy && output_extension != ".dae") { std::cerr << "[Error] --use-element-hierarchy can be used only with .dae output.\n"; /// @todo Lots of duplicate error-and-exit code. write_log(!quiet); print_usage(); - delete serializer; std::remove(output_temp_filename.c_str()); /**< @todo Windows Unicode support */ return EXIT_FAILURE; } @@ -619,7 +617,6 @@ int main(int argc, char** argv) } if (!serializer->ready()) { - delete serializer; std::remove(output_temp_filename.c_str()); /**< @todo Windows Unicode support */ write_log(!quiet); return EXIT_FAILURE; @@ -630,7 +627,6 @@ int main(int argc, char** argv) if (!init_input_file(input_filename, ifc_file, no_progress || quiet, mmap)) { write_log(!quiet); - delete serializer; std::remove(output_temp_filename.c_str()); /**< @todo Windows Unicode support */ return EXIT_FAILURE; } @@ -640,7 +636,6 @@ int main(int argc, char** argv) /// @todo It would be nice to know and print separate error prints for a case where we found no entities /// and for a case we found no entities that satisfy our filtering criteria. Logger::Error("No geometrical entities found"); - delete serializer; std::remove(output_temp_filename.c_str()); /**< @todo Windows Unicode support */ write_log(!quiet); return EXIT_FAILURE; @@ -663,7 +658,6 @@ int main(int argc, char** argv) if (center_model) { if (site_local_placement || building_local_placement) { Logger::Error("Cannot use --center-model together with --{site,building}-local-placement"); - delete serializer; return EXIT_FAILURE; } @@ -678,7 +672,6 @@ int main(int argc, char** argv) } else { if (sscanf(offset_str.c_str(), "%lf;%lf;%lf", &offset[0], &offset[1], &offset[2]) != 3) { std::cerr << "[Error] Invalid use of --model-offset\n"; - delete serializer; std::remove(output_temp_filename.c_str()); /**< @todo Windows Unicode support */ print_options(serializer_options); return EXIT_FAILURE; @@ -752,7 +745,8 @@ int main(int argc, char** argv) } serializer->finalize(); - delete serializer; + // Make sure the dtor is explicitly run here (e.g. output files are closed before renaming them). + serializer.reset(); // Renaming might fail (e.g. maybe the existing file was open in a viewer application) // Do not remove the temp file as user can salvage the conversion result from it. From fbc0215ef1b437348ec16beaccd46ffdfa9fd808 Mon Sep 17 00:00:00 2001 From: Stinkfist0 Date: Tue, 21 Aug 2018 13:52:09 +0300 Subject: [PATCH 11/14] IfcGeomIterator: optimize filtering by performing it prior to geometry reuse inspections. Improves the iteration speed immensely, especially on very large files when only a fraction of the content is wanted to be converted. --- src/ifcgeom/IfcGeomIterator.h | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/ifcgeom/IfcGeomIterator.h b/src/ifcgeom/IfcGeomIterator.h index 499f042c48..111f5ae84d 100644 --- a/src/ifcgeom/IfcGeomIterator.h +++ b/src/ifcgeom/IfcGeomIterator.h @@ -416,6 +416,18 @@ namespace IfcGeom { if (!ifcproducts) { ifcproducts = IfcSchema::IfcProduct::list::ptr(new IfcSchema::IfcProduct::list); IfcSchema::IfcProduct::list::ptr unfiltered_products = kernel.products_represented_by(representation); + // Filter the products based on the set of entities and/or names being included or excluded for processing. + for (IfcSchema::IfcProduct::list::it jt = unfiltered_products->begin(); jt != unfiltered_products->end(); ++jt) { + IfcSchema::IfcProduct* prod = *jt; + if (boost::all(filters_, filter_match(prod))) { + ifcproducts->push(prod); + } + } + + if (ifcproducts->size() == 0) { + _nextShape(); + continue; + } geometry_reuse_ok_for_current_representation_ = reuse_ok_(unfiltered_products); @@ -450,14 +462,6 @@ namespace IfcGeom { continue; } - // Filter the products based on the set of entities and/or names being included or excluded for processing. - for (IfcSchema::IfcProduct::list::it jt = unfiltered_products->begin(); jt != unfiltered_products->end(); ++jt) { - IfcSchema::IfcProduct* prod = *jt; - if (boost::all(filters_, filter_match(prod))) { - ifcproducts->push(prod); - } - } - ifcproduct_iterator = ifcproducts->begin(); } From ffa282af9d50023a484db327e766ebf6e75d3c6d Mon Sep 17 00:00:00 2001 From: Stinkfist0 Date: Thu, 23 Aug 2018 10:33:43 +0300 Subject: [PATCH 12/14] IfcGeomIterator: check the filtered products, not unfiltered ones, for reusability. Speeds up things significantly and also fixes geometry reusability in many cases. --- src/ifcgeom/IfcGeomIterator.h | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/ifcgeom/IfcGeomIterator.h b/src/ifcgeom/IfcGeomIterator.h index 111f5ae84d..ec153c715b 100644 --- a/src/ifcgeom/IfcGeomIterator.h +++ b/src/ifcgeom/IfcGeomIterator.h @@ -405,18 +405,17 @@ namespace IfcGeom { for (;;) { IfcSchema::IfcRepresentation* representation; - // Have we reached the end of our list of representations? if ( representation_iterator == representations->end() ) { representations.reset(); - return 0; + return 0; // reached the end of our list of representations } representation = *representation_iterator; - // Has the list of IfcProducts for this representation been initialized? if (!ifcproducts) { + // Init. the list of filtered IfcProducts for this representation ifcproducts = IfcSchema::IfcProduct::list::ptr(new IfcSchema::IfcProduct::list); IfcSchema::IfcProduct::list::ptr unfiltered_products = kernel.products_represented_by(representation); - // Filter the products based on the set of entities and/or names being included or excluded for processing. + // Include only the desired products for processing. for (IfcSchema::IfcProduct::list::it jt = unfiltered_products->begin(); jt != unfiltered_products->end(); ++jt) { IfcSchema::IfcProduct* prod = *jt; if (boost::all(filters_, filter_match(prod))) { @@ -429,7 +428,7 @@ namespace IfcGeom { continue; } - geometry_reuse_ok_for_current_representation_ = reuse_ok_(unfiltered_products); + geometry_reuse_ok_for_current_representation_ = reuse_ok_(ifcproducts); IfcSchema::IfcRepresentationMap::list::ptr maps = representation->RepresentationMap(); @@ -447,13 +446,12 @@ namespace IfcGeom { } } + // Check if this represenation has (or will be) processed as part its mapped representation bool representation_processed_as_mapped_item = false; - - IfcSchema::IfcRepresentation* representation_mapped_to = kernel.representation_mapped_to(representation); + IfcSchema::IfcRepresentation* representation_mapped_to = kernel.representation_mapped_to(representation); if (representation_mapped_to) { - // Check if this representation has (or will be) processed as part its mapped representation - representation_processed_as_mapped_item = ok_mapped_representations->contains(representation_mapped_to) || - reuse_ok_(kernel.products_represented_by(representation_mapped_to)); + representation_processed_as_mapped_item = geometry_reuse_ok_for_current_representation_ || + ok_mapped_representations->contains(representation_mapped_to); } if (representation_processed_as_mapped_item) { From 342cce89d70a5ded5fb1bf52d87383bfce5021ca Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Tue, 30 Oct 2018 09:05:18 +0100 Subject: [PATCH 13/14] #487 perform fix after sewing if not valid --- src/ifcgeom/IfcGeomFunctions.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index 8148f78680..f77d8e0e19 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -324,7 +324,18 @@ bool IfcGeom::Kernel::create_solid_from_faces(const TopTools_ListOfShape& face_l try { builder.Perform(); shape = builder.SewedShape(); - valid_shell = BRepCheck_Analyzer(shape).IsValid() != 0 && count(shape, TopAbs_SHELL) > 0; + + { + BRepCheck_Analyzer ana(shape); + if (!ana.IsValid()) { + ShapeFix_Shape sfs(shape); + sfs.Perform(); + shape = sfs.Shape(); + } + } + + BRepCheck_Analyzer ana(shape); + valid_shell = ana.IsValid() != 0 && count(shape, TopAbs_SHELL) > 0; } catch (const Standard_Failure& e) { if (e.GetMessageString() && strlen(e.GetMessageString())) { Logger::Error(e.GetMessageString()); From e64d8750e14e64a01f4a1b115f07bc6c9afe0b72 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Tue, 30 Oct 2018 12:15:35 +0100 Subject: [PATCH 14/14] Use all representations if no representations found through context check --- src/ifcgeom/IfcGeomIterator.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/ifcgeom/IfcGeomIterator.h b/src/ifcgeom/IfcGeomIterator.h index ec153c715b..c48e934a5c 100644 --- a/src/ifcgeom/IfcGeomIterator.h +++ b/src/ifcgeom/IfcGeomIterator.h @@ -273,10 +273,15 @@ namespace IfcGeom { } if (representations->size() == 0) { - Logger::Message(Logger::LOG_ERROR, "No geometries found"); - return false; + Logger::Message(Logger::LOG_ERROR, "No representations encountered in relevant contexts, using all"); + representations = ifc_file->entitiesByType(); } + if (representations->size() == 0) { + Logger::Message(Logger::LOG_ERROR, "No representations encountered, aborting"); + return false; + } + representation_iterator = representations->begin(); ifcproducts.reset();