From 355cca36791a0953c5ff19e6f8cb0627a4e4d4b1 Mon Sep 17 00:00:00 2001
From: Andrej730 <azhilenkov@gmail.com>
Date: Tue, 9 Jul 2024 17:25:01 +0500
Subject: [PATCH] Improve styles handling (partially support non-shading
 styles/fix bugs)

1) Previously create_shape (was failing with "RuntimeError: Unexpected topology") or iterator (was hanging indefinetely trying initialize with a IfcSlab from #4898) were breaking if element had a material that had IfcSurfaceStyle with just IfcExternallyDefinedSurfaceStyle and no IfcSurfaceStyleShading.

Now get_surface_style is returning IfcSurfaceStyle as first element of the pair even if there was no IfcSurfaceStyleShading - so it can be used to create a `taxonomy::style::ptr` with it to indicate that geometry is still referring to some style.

2) Added logs for unsupported presentation styles (non-IfcSurfaceStyles) to both `mapping::map_impl(const IfcSchema::IfcStyledItem* inst)` and `mapping::map_impl(const IfcSchema::IfcMaterial* material)`. Also `map_impl` will not break now if it will meet IfcCurveStyle.
---
 src/ifcgeom/mapping/mapping.cpp | 38 +++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/src/ifcgeom/mapping/mapping.cpp b/src/ifcgeom/mapping/mapping.cpp
index 2fc0c062f8..e1ea7db0e9 100644
--- a/src/ifcgeom/mapping/mapping.cpp
+++ b/src/ifcgeom/mapping/mapping.cpp
@@ -399,9 +399,10 @@ namespace {
         }
 #endif
 
+        IfcSchema::IfcSurfaceStyle *surface_style = nullptr;
         for (auto& style : prs_styles) {
             if (style->declaration().is(IfcSchema::IfcSurfaceStyle::Class())) {
-                IfcSchema::IfcSurfaceStyle* surface_style = (IfcSchema::IfcSurfaceStyle*)style;
+                surface_style = (IfcSchema::IfcSurfaceStyle*)style;
                 if (surface_style->Side() != IfcSchema::IfcSurfaceSide::IfcSurfaceSide_NEGATIVE) {
                     auto styles_elements = surface_style->Styles();
                     for (auto mt = styles_elements->begin(); mt != styles_elements->end(); ++mt) {
@@ -412,8 +413,7 @@ namespace {
                 }
             }
         }
-
-        return std::make_pair<IfcSchema::IfcSurfaceStyle*, T*>(nullptr, nullptr);
+        return std::make_pair(surface_style, nullptr);
     }
 
     bool process_colour(IfcSchema::IfcColourRgb* colour, double* rgb) {
@@ -473,7 +473,16 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcMaterial* material) {
             styles->push((**it).Items()->as<IfcSchema::IfcStyledItem>());
         }
         if (styles->size() == 1) {
-            return map(*styles->begin());
+            IfcSchema::IfcStyledItem *styled_item = *styles->begin();
+            auto mapped_item = map(styled_item);
+            if (mapped_item) {
+                return mapped_item;
+            }
+            // Check if it's failed or just some unsupported case.
+            if (failed_on_purpose_.find(styled_item) == failed_on_purpose_.end()) {
+                return nullptr;
+            }
+            Logger::Warning("Skipping unsupported material style for material: ", material);
         }
     }
 
@@ -500,25 +509,32 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcStyledItem* inst) {
     IfcSchema::IfcSurfaceStyleShading* shading = style_pair.second;
 
     if (style == nullptr) {
-        // @todo we should probably log something that the kind of style,
-        // such as IfcCurveStyle, in this collections are unsupported.
+        // E.g. IfcCurveStyle is skipped as unsupported.
+        Logger::Warning("Only IfcSurfaceStyle is supported, couldn't find it in IfcStyledItem: ", inst);
         failed_on_purpose_.insert(inst);
         return nullptr;
     }
 
-    static taxonomy::colour white = taxonomy::colour(1., 1., 1.);
-
     taxonomy::style::ptr surface_style = taxonomy::make<taxonomy::style>();
-
     surface_style->instance = style;
     if (settings_.get<settings::UseMaterialNames>().get() && style->Name()) {
         surface_style->name = *style->Name();
     } else {
         std::ostringstream oss;
-        oss << shading->declaration().name() << "-" << shading->data().id();
+        if (shading) {
+            oss << shading->declaration().name() << "-" << shading->data().id();
+        } else {
+            oss << "-";
+        }
         surface_style->name = oss.str();
     }
-    
+
+    if (shading == nullptr) {
+        // E.g. IfcSurface style has only IfcExternallyDefinedSurfaceStyle.
+        return surface_style;
+    }
+
+    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];