From 6cc61d02580a877f02e262eb8a341d61ebf1daea Mon Sep 17 00:00:00 2001
From: louis-e <44675238+louis-e@users.noreply.github.com>
Date: Mon, 6 Apr 2026 21:27:24 +0200
Subject: [PATCH] Address review: use shared constant and skip bitmap
 allocation when unused

- Make BUILDING_PASSAGE_HEIGHT pub(crate) and use it in buildings_interior.rs
  instead of hardcoded literal 4 (Copilot comment 4)
- Add CoordinateBitmap::new_empty() zero-allocation constructor
- Skip bitmap allocation in collect_building_passage_coords when no
  tunnel=building_passage ways exist (Copilot comment 5)
---
 src/element_processing/buildings.rs             | 17 ++++-------------
 src/element_processing/highways.rs              | 13 +++++++++++++
 .../subprocessor/buildings_interior.rs          |  3 ++-
 src/floodfill_cache.rs                          | 12 ++++++++++++
 4 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/src/element_processing/buildings.rs b/src/element_processing/buildings.rs
index 216938b..3d2d4cf 100644
--- a/src/element_processing/buildings.rs
+++ b/src/element_processing/buildings.rs
@@ -55,7 +55,7 @@ pub(crate) struct HolePolygon {
 /// Height (in blocks above ground floor) of a building-passage archway.
 /// Walls and floors below this height are removed at tunnel=building_passage
 /// highway coordinates, creating a ground-level opening through the building.
-const BUILDING_PASSAGE_HEIGHT: i32 = 4;
+pub(crate) const BUILDING_PASSAGE_HEIGHT: i32 = 4;
 
 /// Accent block options for building decoration
 const ACCENT_BLOCK_OPTIONS: [Block; 6] = [
@@ -3607,8 +3607,7 @@ pub fn generate_buildings(
         // passage coordinate, place a wall column from ground to passage ceiling.
         // This creates the left/right corridor walls inside the archway.
         if !building_passages.is_empty() {
-            let passage_height =
-                BUILDING_PASSAGE_HEIGHT.min(config.building_height);
+            let passage_height = BUILDING_PASSAGE_HEIGHT.min(config.building_height);
             let abs = config.abs_terrain_offset;
             for &(x, z) in &cached_floor_area {
                 if building_passages.contains(x, z) {
@@ -3620,17 +3619,9 @@ pub fn generate_buildings(
                     || building_passages.contains(x, z - 1)
                     || building_passages.contains(x, z + 1);
                 if adjacent_to_passage {
-                    for y in (config.start_y_offset + 1)
-                        ..=(config.start_y_offset + passage_height)
+                    for y in (config.start_y_offset + 1)..=(config.start_y_offset + passage_height)
                     {
-                        editor.set_block_absolute(
-                            config.wall_block,
-                            x,
-                            y + abs,
-                            z,
-                            None,
-                            None,
-                        );
+                        editor.set_block_absolute(config.wall_block, x, y + abs, z, None, None);
                     }
                 }
             }
diff --git a/src/element_processing/highways.rs b/src/element_processing/highways.rs
index dc01837..3be269b 100644
--- a/src/element_processing/highways.rs
+++ b/src/element_processing/highways.rs
@@ -988,6 +988,19 @@ pub fn collect_building_passage_coords(
     xzbbox: &XZBBox,
     scale: f64,
 ) -> CoordinateBitmap {
+    // Quick scan: skip bitmap allocation entirely when there are no passage ways.
+    let has_any = elements.iter().any(|e| {
+        if let ProcessedElement::Way(w) = e {
+            w.tags.get("tunnel").map(|v| v.as_str()) == Some("building_passage")
+                && w.tags.contains_key("highway")
+        } else {
+            false
+        }
+    });
+    if !has_any {
+        return CoordinateBitmap::new_empty();
+    }
+
     let mut bitmap = CoordinateBitmap::new(xzbbox);
 
     for element in elements {
diff --git a/src/element_processing/subprocessor/buildings_interior.rs b/src/element_processing/subprocessor/buildings_interior.rs
index 29d9d83..a0a2db8 100644
--- a/src/element_processing/subprocessor/buildings_interior.rs
+++ b/src/element_processing/subprocessor/buildings_interior.rs
@@ -1,4 +1,5 @@
 use crate::block_definitions::*;
+use crate::element_processing::buildings::BUILDING_PASSAGE_HEIGHT;
 use crate::floodfill_cache::CoordinateBitmap;
 use crate::world_editor::WorldEditor;
 use std::collections::HashSet;
@@ -369,7 +370,7 @@ pub fn generate_building_interior(
                 // Skip interior blocks in building-passage zones on floors
                 // that fall within the archway opening.
                 if building_passages.contains(x, z)
-                    && floor_y < start_y_offset + 4.min(building_height)
+                    && floor_y < start_y_offset + BUILDING_PASSAGE_HEIGHT.min(building_height)
                 {
                     continue;
                 }
diff --git a/src/floodfill_cache.rs b/src/floodfill_cache.rs
index 3cb8f2d..b576aad 100644
--- a/src/floodfill_cache.rs
+++ b/src/floodfill_cache.rs
@@ -58,6 +58,18 @@ impl CoordinateBitmap {
         }
     }
 
+    /// Creates a zero-size bitmap that contains nothing and allocates no memory.
+    pub fn new_empty() -> Self {
+        Self {
+            bits: Vec::new(),
+            min_x: 0,
+            min_z: 0,
+            width: 0,
+            height: 0,
+            count: 0,
+        }
+    }
+
     /// Converts (x, z) coordinate to bit index, returning None if out of bounds.
     #[inline]
     fn coord_to_index(&self, x: i32, z: i32) -> Option<usize> {