From b5a61753a5ffc2ad55bcdf829b99168b9512182e Mon Sep 17 00:00:00 2001 From: Cristian Ritter Date: Thu, 15 Aug 2024 13:56:31 -0300 Subject: [PATCH 01/10] Adding --surface-color in conversion settings to force the use of surface color instead of diffuse color. Issue #5075 --- src/ifcgeom/ConversionSettings.h | 7 +++++++ src/ifcgeom/mapping/mapping.cpp | 6 +++++- src/ifcgeom/taxonomy.h | 13 +++++++++++-- src/serializers/GltfSerializer.cpp | 4 ++-- 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/ifcgeom/ConversionSettings.h b/src/ifcgeom/ConversionSettings.h index f49941ec4b..611c615001 100644 --- a/src/ifcgeom/ConversionSettings.h +++ b/src/ifcgeom/ConversionSettings.h @@ -331,6 +331,13 @@ namespace ifcopenshell { static constexpr bool defaultvalue = false; }; + struct SurfaceColour : public SettingBase { + static constexpr const char* const name = "surface-colour"; + static constexpr const char* const description = + "Prioritizes the surface color instead of using diffuse."; + static constexpr bool defaultvalue = false; + }; + enum PiecewiseStepMethod { MAXSTEPSIZE, MINSTEPS }; diff --git a/src/ifcgeom/mapping/mapping.cpp b/src/ifcgeom/mapping/mapping.cpp index 1fbd73d0c7..00cbacaba4 100644 --- a/src/ifcgeom/mapping/mapping.cpp +++ b/src/ifcgeom/mapping/mapping.cpp @@ -565,10 +565,14 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcStyledItem* inst) { return surface_style; } + surface_style->use_surface_color = settings_.get().get(); + static taxonomy::colour white = taxonomy::colour(1., 1., 1.); double rgb[3]; if (process_colour(shading->SurfaceColour(), rgb)) { - surface_style->diffuse.components() << rgb[0], rgb[1], rgb[2]; + surface_style->surface.components() << rgb[0], rgb[1], rgb[2]; + } else { + surface_style->surface = white; } if (auto rendering_style = shading->as()) { diff --git a/src/ifcgeom/taxonomy.h b/src/ifcgeom/taxonomy.h index 3b77273240..9b02e7a2fc 100644 --- a/src/ifcgeom/taxonomy.h +++ b/src/ifcgeom/taxonomy.h @@ -305,8 +305,10 @@ typedef item const* ptr; std::string name; colour diffuse; + colour surface; colour specular; double specularity, transparency; + bool use_surface_color; void print(std::ostream& o, int indent = 0) const; @@ -321,8 +323,15 @@ typedef item const* ptr; // @todo equality implementation based on values? bool operator==(const style& other) const { return instance == other.instance; } - style() : specularity(std::numeric_limits::quiet_NaN()), transparency(std::numeric_limits::quiet_NaN()) {} - style(const std::string& name) : name(name), specularity(std::numeric_limits::quiet_NaN()), transparency(std::numeric_limits::quiet_NaN()) {} + style() : specularity(std::numeric_limits::quiet_NaN()), transparency(std::numeric_limits::quiet_NaN()), use_surface_color(false) {} + style(const std::string& name) : name(name), specularity(std::numeric_limits::quiet_NaN()), transparency(std::numeric_limits::quiet_NaN()), use_surface_color(false) {} + + colour get_color() const { + if (use_surface_color && surface) { + return surface; + } + return diffuse; + } bool has_specularity() const { return !std::isnan(specularity); diff --git a/src/serializers/GltfSerializer.cpp b/src/serializers/GltfSerializer.cpp index c5b4d4c776..980788e1c2 100644 --- a/src/serializers/GltfSerializer.cpp +++ b/src/serializers/GltfSerializer.cpp @@ -97,9 +97,9 @@ int GltfSerializer::writeMaterial(const ifcopenshell::geometry::taxonomy::style: std::array base; base.fill(1.0); - if (style->diffuse) { + if (style->get_color()) { for (int i = 0; i < 3; ++i) { - base[i] = style->diffuse.ccomponents()(i); + base[i] = style->get_color().ccomponents()(i); } } if (style->transparency == style->transparency) { From 876f0594594a6a45d6a1648d53b62d8a0d4e19cf Mon Sep 17 00:00:00 2001 From: Cristian Ritter Date: Thu, 15 Aug 2024 14:18:13 -0300 Subject: [PATCH 02/10] remove surface white if doesn't have surface colour Signed-off-by: Cristian Ritter --- src/ifcgeom/mapping/mapping.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/ifcgeom/mapping/mapping.cpp b/src/ifcgeom/mapping/mapping.cpp index 00cbacaba4..50d7fad3f6 100644 --- a/src/ifcgeom/mapping/mapping.cpp +++ b/src/ifcgeom/mapping/mapping.cpp @@ -565,15 +565,13 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcStyledItem* inst) { return surface_style; } - surface_style->use_surface_color = settings_.get().get(); + surface_style->use_surface_color = settings_.get().get(); - static taxonomy::colour white = taxonomy::colour(1., 1., 1.); - double rgb[3]; - if (process_colour(shading->SurfaceColour(), rgb)) { - surface_style->surface.components() << rgb[0], rgb[1], rgb[2]; - } else { - surface_style->surface = white; - } + static taxonomy::colour white = taxonomy::colour(1., 1., 1.); + double rgb[3]; + if (process_colour(shading->SurfaceColour(), rgb)) { + surface_style->surface.components() << rgb[0], rgb[1], rgb[2]; + } if (auto rendering_style = shading->as()) { if (rendering_style->DiffuseColour() && process_colour(rendering_style->DiffuseColour(), rgb)) { From 0efafada1d67f8cf63b07bde8d545bde3d1ecdee Mon Sep 17 00:00:00 2001 From: Cristian Ritter Date: Mon, 19 Aug 2024 18:41:45 -0300 Subject: [PATCH 03/10] fixed missing SurfaceColour on SettingsContainer --- src/ifcgeom/ConversionSettings.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ifcgeom/ConversionSettings.h b/src/ifcgeom/ConversionSettings.h index 611c615001..1dce283315 100644 --- a/src/ifcgeom/ConversionSettings.h +++ b/src/ifcgeom/ConversionSettings.h @@ -447,7 +447,7 @@ namespace ifcopenshell { }; class IFC_GEOM_API Settings : public SettingsContainer< - std::tuple + std::tuple > {}; } From cd3df94a4bbb362d3ac50fb62f5f7439109ce89f Mon Sep 17 00:00:00 2001 From: Cristian Ritter Date: Mon, 19 Aug 2024 18:44:43 -0300 Subject: [PATCH 04/10] add surface and use_surface_color on style class --- src/ifcgeom/taxonomy.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ifcgeom/taxonomy.h b/src/ifcgeom/taxonomy.h index 9b02e7a2fc..91239c8bc2 100644 --- a/src/ifcgeom/taxonomy.h +++ b/src/ifcgeom/taxonomy.h @@ -316,7 +316,7 @@ typedef item const* ptr; virtual kinds kind() const { return STYLE; } virtual size_t calc_hash() const { - auto v = std::make_tuple(static_cast(STYLE), name, diffuse.hash(), specular.hash(), specularity, transparency); + auto v = std::make_tuple(static_cast(STYLE), name, diffuse.hash(), surface.hash(), specular.hash(), specularity, transparency, use_surface_color); return boost::hash{}(v); } From d714eca4eec6e8f06491351817b708b073fb9c28 Mon Sep 17 00:00:00 2001 From: Cristian Ritter Date: Tue, 20 Aug 2024 11:44:20 -0300 Subject: [PATCH 05/10] replacing diffuse to get_color method --- src/ifcgeom/IfcGeomRepresentation.cpp | 2 +- src/ifcgeom/SurfaceStyle.cpp | 4 ++++ src/ifcgeomserver/IfcGeomServer.cpp | 4 ++-- src/serializers/WavefrontObjSerializer.cpp | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/ifcgeom/IfcGeomRepresentation.cpp b/src/ifcgeom/IfcGeomRepresentation.cpp index 89eb2bdd6b..2cf05fd9de 100644 --- a/src/ifcgeom/IfcGeomRepresentation.cpp +++ b/src/ifcgeom/IfcGeomRepresentation.cpp @@ -149,7 +149,7 @@ IfcGeom::Representation::Serialization::Serialization(const BRep& brep) int sid = -1; if (it->hasStyle()) { - const auto& clr = it->Style().diffuse.ccomponents(); + const auto& clr = it->Style().get_color().ccomponents(); surface_styles_.push_back(clr(0)); surface_styles_.push_back(clr(1)); surface_styles_.push_back(clr(2)); diff --git a/src/ifcgeom/SurfaceStyle.cpp b/src/ifcgeom/SurfaceStyle.cpp index 3d862b4825..69310f3929 100644 --- a/src/ifcgeom/SurfaceStyle.cpp +++ b/src/ifcgeom/SurfaceStyle.cpp @@ -89,6 +89,10 @@ void IfcGeom::set_default_style_file(const std::string& json_file) { boost::optional diffuse = material.get_child_optional("diffuse"); default_materials[name]->diffuse = read_colour_component(diffuse); + // @todo Is it necessary to get the surface too? + // boost::optional surface = material.get_child_optional("surface"); + // default_materials[name]->surface = read_colour_component(surface); + boost::optional specular = material.get_child_optional("specular"); default_materials[name]->specular = read_colour_component(specular); diff --git a/src/ifcgeomserver/IfcGeomServer.cpp b/src/ifcgeomserver/IfcGeomServer.cpp index 15cd7e546b..fdb349a0fb 100644 --- a/src/ifcgeomserver/IfcGeomServer.cpp +++ b/src/ifcgeomserver/IfcGeomServer.cpp @@ -354,8 +354,8 @@ protected: std::vector > > diffuse_color_array; for (auto it = geom->geometry().materials().begin(); it != geom->geometry().materials().end(); ++it) { const auto& mat = **it; - if (mat.diffuse) { - const auto& color = mat.diffuse.ccomponents(); + if (mat.get_color()) { + const auto& color = mat.get_color().ccomponents(); diffuse_color_array.push_back(std::array{ static_cast(color(0)), static_cast(color(1)), diff --git a/src/serializers/WavefrontObjSerializer.cpp b/src/serializers/WavefrontObjSerializer.cpp index 0414659b98..300fceeb88 100644 --- a/src/serializers/WavefrontObjSerializer.cpp +++ b/src/serializers/WavefrontObjSerializer.cpp @@ -67,7 +67,7 @@ void WaveFrontOBJSerializer::writeMaterial(const ifcopenshell::geometry::taxonom mtl_stream.stream << "newmtl " << material_name << "\n"; { - auto& diffuse = style.diffuse.ccomponents(); + auto& diffuse = style.get_color().ccomponents(); mtl_stream.stream << "Kd " << diffuse(0) << " " << diffuse(1) << " " << diffuse(2) << "\n"; } if (style.specular) { From 371b4b6079412efc9658013af766966e505e6115 Mon Sep 17 00:00:00 2001 From: Cristian Ritter Date: Tue, 20 Aug 2024 11:45:29 -0300 Subject: [PATCH 06/10] removing unnecessary use_surface_color on calc_hash --- src/ifcgeom/taxonomy.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ifcgeom/taxonomy.h b/src/ifcgeom/taxonomy.h index 91239c8bc2..6399f513aa 100644 --- a/src/ifcgeom/taxonomy.h +++ b/src/ifcgeom/taxonomy.h @@ -316,7 +316,7 @@ typedef item const* ptr; virtual kinds kind() const { return STYLE; } virtual size_t calc_hash() const { - auto v = std::make_tuple(static_cast(STYLE), name, diffuse.hash(), surface.hash(), specular.hash(), specularity, transparency, use_surface_color); + auto v = std::make_tuple(static_cast(STYLE), name, diffuse.hash(), surface.hash(), specular.hash(), specularity, transparency); return boost::hash{}(v); } From 7d50491a998687f3567345886cefe61210070fa5 Mon Sep 17 00:00:00 2001 From: Cristian Ritter Date: Tue, 20 Aug 2024 12:16:19 -0300 Subject: [PATCH 07/10] checking if diffuse has value, if not, use surface Signed-off-by: Cristian Ritter --- src/ifcgeom/taxonomy.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ifcgeom/taxonomy.h b/src/ifcgeom/taxonomy.h index 6399f513aa..e9b2a5afe1 100644 --- a/src/ifcgeom/taxonomy.h +++ b/src/ifcgeom/taxonomy.h @@ -327,7 +327,7 @@ typedef item const* ptr; style(const std::string& name) : name(name), specularity(std::numeric_limits::quiet_NaN()), transparency(std::numeric_limits::quiet_NaN()), use_surface_color(false) {} colour get_color() const { - if (use_surface_color && surface) { + if ((use_surface_color && surface) || !diffuse) { return surface; } return diffuse; From ae2933eea51da2a37348265e4cd0d98a5d883d6e Mon Sep 17 00:00:00 2001 From: csritter <73347739+csritter@users.noreply.github.com> Date: Mon, 26 Aug 2024 01:08:06 -0300 Subject: [PATCH 08/10] prevent a copy Co-authored-by: Thomas Krijnen --- src/ifcgeom/taxonomy.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ifcgeom/taxonomy.h b/src/ifcgeom/taxonomy.h index e9b2a5afe1..ceea149091 100644 --- a/src/ifcgeom/taxonomy.h +++ b/src/ifcgeom/taxonomy.h @@ -326,7 +326,7 @@ typedef item const* ptr; style() : specularity(std::numeric_limits::quiet_NaN()), transparency(std::numeric_limits::quiet_NaN()), use_surface_color(false) {} style(const std::string& name) : name(name), specularity(std::numeric_limits::quiet_NaN()), transparency(std::numeric_limits::quiet_NaN()), use_surface_color(false) {} - colour get_color() const { + const colour& get_color() const { if ((use_surface_color && surface) || !diffuse) { return surface; } From 9fceec35cce09d5e657bb3a404dc25cabc4c2e04 Mon Sep 17 00:00:00 2001 From: Cristian Ritter Date: Mon, 26 Aug 2024 10:17:50 -0300 Subject: [PATCH 09/10] add diffuse on surface, if has no surface color Signed-off-by: Cristian Ritter --- src/ifcgeom/mapping/mapping.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ifcgeom/mapping/mapping.cpp b/src/ifcgeom/mapping/mapping.cpp index 50d7fad3f6..c69db2dbad 100644 --- a/src/ifcgeom/mapping/mapping.cpp +++ b/src/ifcgeom/mapping/mapping.cpp @@ -577,6 +577,9 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcStyledItem* inst) { if (rendering_style->DiffuseColour() && process_colour(rendering_style->DiffuseColour(), rgb)) { const taxonomy::colour& old_diffuse = surface_style->diffuse ? surface_style->diffuse : white; surface_style->diffuse = taxonomy::colour(old_diffuse.r() * rgb[0], old_diffuse.g() * rgb[1], old_diffuse.b() * rgb[2]); + if (!surface_style->surface) { + surface_style->surface = surface_style->diffuse; + } } if (rendering_style->DiffuseTransmissionColour()) { // Not supported From 56d19039e3fdb6cbd16718d4cbcb2b683587dcad Mon Sep 17 00:00:00 2001 From: Cristian Ritter Date: Mon, 26 Aug 2024 15:34:50 -0300 Subject: [PATCH 10/10] always apply in diffuse color, if doens't have surface color Signed-off-by: Cristian Ritter --- src/ifcgeom/mapping/mapping.cpp | 4 +--- src/ifcgeom/taxonomy.h | 6 +++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/ifcgeom/mapping/mapping.cpp b/src/ifcgeom/mapping/mapping.cpp index c69db2dbad..5430e2aecb 100644 --- a/src/ifcgeom/mapping/mapping.cpp +++ b/src/ifcgeom/mapping/mapping.cpp @@ -571,15 +571,13 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcStyledItem* inst) { double rgb[3]; if (process_colour(shading->SurfaceColour(), rgb)) { surface_style->surface.components() << rgb[0], rgb[1], rgb[2]; + surface_style->diffuse = surface_style->surface; } if (auto rendering_style = shading->as()) { if (rendering_style->DiffuseColour() && process_colour(rendering_style->DiffuseColour(), rgb)) { const taxonomy::colour& old_diffuse = surface_style->diffuse ? surface_style->diffuse : white; surface_style->diffuse = taxonomy::colour(old_diffuse.r() * rgb[0], old_diffuse.g() * rgb[1], old_diffuse.b() * rgb[2]); - if (!surface_style->surface) { - surface_style->surface = surface_style->diffuse; - } } if (rendering_style->DiffuseTransmissionColour()) { // Not supported diff --git a/src/ifcgeom/taxonomy.h b/src/ifcgeom/taxonomy.h index ceea149091..4dbb2dc6f8 100644 --- a/src/ifcgeom/taxonomy.h +++ b/src/ifcgeom/taxonomy.h @@ -327,10 +327,10 @@ typedef item const* ptr; style(const std::string& name) : name(name), specularity(std::numeric_limits::quiet_NaN()), transparency(std::numeric_limits::quiet_NaN()), use_surface_color(false) {} const colour& get_color() const { - if ((use_surface_color && surface) || !diffuse) { + if (use_surface_color && surface) { return surface; - } - return diffuse; + } + return diffuse; } bool has_specularity() const {