Lookup building footprint in tree generation #711
Conversation
|
⏱️ Benchmark run finished in 0m 33s 📈 Compared against baseline: 30s 🟢 Generation time is unchanged. 📅 Last benchmark: 2026-01-11 01:05:28 UTC You can retrigger the benchmark by commenting |
There was a problem hiding this comment.
Pull request overview
This pull request adds building footprint lookup functionality to prevent trees from spawning inside buildings. The implementation uses a memory-efficient bitmap structure that stores building coordinates using only 1 bit per coordinate, reducing memory usage significantly compared to HashSet-based approaches.
Changes:
- Introduced
BuildingFootprintBitmapstruct for memory-efficient coordinate storage - Added
collect_building_footprintsmethod to gather building coordinates from flood fill cache - Modified
Tree::createto accept optional building footprint bitmap and skip generation within buildings - Updated all tree generation call sites across natural, leisure, and landuse modules to pass building footprints
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/floodfill_cache.rs | Implements BuildingFootprintBitmap struct with bitmap operations and adds collect_building_footprints method |
| src/element_processing/tree.rs | Adds building_footprints parameter to Tree::create and implements early return for buildings |
| src/element_processing/natural.rs | Threads building_footprints parameter through all tree generation calls |
| src/element_processing/leisure.rs | Threads building_footprints parameter through all tree generation calls |
| src/element_processing/landuse.rs | Threads building_footprints parameter through all tree generation calls |
| src/data_processing.rs | Collects building footprints after precomputing flood fills and passes them to element processors |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/floodfill_cache.rs
Outdated
| if let Some(cached) = self.way_cache.get(&member.way.id) { | ||
| for &(x, z) in cached { | ||
| footprints.set(x, z); |
There was a problem hiding this comment.
All relation members are collected as building footprints, including inner ways. Inner ways in building relations typically represent holes or courtyards. Since building generation only processes outer members, this creates an inconsistency where trees won't spawn in courtyards even though they're not filled with building blocks. Consider filtering to only include outer members to allow natural features in courtyards.
| if let Some(cached) = self.way_cache.get(&member.way.id) { | |
| for &(x, z) in cached { | |
| footprints.set(x, z); | |
| // Only treat outer members as building footprints. | |
| // Inner members typically represent courtyards/holes and | |
| // should not be marked as solid building area. | |
| if member.role == "outer" { | |
| if let Some(cached) = self.way_cache.get(&member.way.id) { | |
| for &(x, z) in cached { | |
| footprints.set(x, z); | |
| } |
|
retrigger-benchmark |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.