From 9b3b15ebfebbd9a52e5cfb5b2658345581193499 Mon Sep 17 00:00:00 2001
From: Paul Lee <paullee0@gmail.com>
Date: Sun, 15 Jun 2025 10:33:26 +0800
Subject: [PATCH] [ArchRoof]  Improve subVolume generation

Fix #21633 : Holes in roof are causing troubles

FreeCAD Forum : Sketch based Arch_Roof and wall substraction
- https://forum.freecad.org/viewtopic.php?t=84389

Improved algorithm:
1. Extrusion of bottom faces in +Z.
2. The roof itself.
3. Extrusion of the top faces in +Z.

TODO:
Find better way to test and maybe to split suface point up and down and extrude separately
---
 src/Mod/BIM/ArchRoof.py | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/src/Mod/BIM/ArchRoof.py b/src/Mod/BIM/ArchRoof.py
index 6fb18227b4..4c323e24fb 100644
--- a/src/Mod/BIM/ArchRoof.py
+++ b/src/Mod/BIM/ArchRoof.py
@@ -762,17 +762,29 @@ class _Roof(ArchComponent.Component):
             # a Wall, but all portion of the wall above the roof solid would be
             # subtracted as well.
             #
-            # FC forum discussion : Sketch based Arch_Roof and wall substraction
+
+            # FC forum discussion, 2024.1.15 :
+            # Sketch based Arch_Roof and wall substraction
             # - https://forum.freecad.org/viewtopic.php?t=84389
             #
+            # Github issue #21633, 2025.5.29 :
+            # BIM: Holes in roof are causing troubles
+            # - https://github.com/FreeCAD/FreeCAD/issues/21633#issuecomment-2969640142
+
+
             faces = []
             solids = []
             for f in obj.Base.Shape.Faces:  # obj.Base.Shape.Solids.Faces
                 p = f.findPlane()  # Curve face (surface) seems return no Plane
                 if p:
-                    if p.Axis[2] < -1e-7:  # i.e. normal pointing below horizon
-                        faces.append(f)
+                    # See github issue #21633, all planes are added for safety
+                    #if p.Axis[2] < -1e-7:  # i.e. normal pointing below horizon
+                    faces.append(f)
                 else:
+                    # TODO 2025.6.15: See github issue #21633: Find better way
+                    #      to test and maybe to split suface point up and down
+                    #      and extrude separately
+                    
                     # Not sure if it is pointing towards and/or above horizon
                     # (upward or downward), or it is curve surface, just add.
                     faces.append(f)
@@ -785,6 +797,11 @@ class _Roof(ArchComponent.Component):
                 solid = f.extrude(Vector(0.0, 0.0, 1000000.0))
                 if not solid.isNull() and solid.isValid() and solid.Volume > 1e-3:
                     solids.append(solid)
+
+            # See github issue #21633: Solids are added for safety
+            for s in obj.Base.Shape.Solids:
+                solids.append(s)
+
             compound = Part.Compound(solids)
             compound.Placement = obj.Placement
             return compound