Fix building multipolygon ring assembly and roof rendering#749
Conversation
- Merge open outer-way segments into closed rings for building multipolygon relations (same pattern as water_areas), fixing empty flood fills that produced wall-only outlines - Preserve building multipolygon member ways unclipped in osm_parser so ring assembly can operate on complete geometry - Rewrite generate_roof_only_structure to respect height tags (min_height, building:min_level, layer) and roof:shape, using dome renderer for curved shells - Map circular and spherical roof:shape values to RoofType::Dome - Route building:part=roof elements to the roof-only generator before the building tag match
|
⏱️ Benchmark run finished in 0m 33s 📈 Compared against baseline: 30s 🟢 Generation time is unchanged. 📅 Last benchmark: 2026-02-10 20:34:02 UTC You can retrigger the benchmark by commenting |
There was a problem hiding this comment.
Pull request overview
Fixes missing filled geometry for complex building multipolygon relations by preserving unclipped member ways for ring assembly, and improves rendering for roof-only building parts by using height/shape/material tags for placement and block choice.
Changes:
- Preserve unclipped member ways for building multipolygon relations to enable proper ring merging before clipping.
- Assemble outer member way segments into closed rings for multipolygon building relations and render them via the standard building pipeline.
- Rework roof-only generation to use tag-derived vertical placement, roof shape selection, and terrain-aware absolute placement; route
building:part=roofcorrectly.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/osm_parser.rs |
Keeps unclipped member ways for building multipolygon relations so ring assembly can occur before clipping. |
src/element_processing/buildings.rs |
Adds relation ring assembly for outline-only buildings; reworks roof-only rendering and routes building:part=roof correctly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Extend roof-only pillars from ground level (terrain-aware) to slab height instead of from start_y_offset, preventing floating roofs when raised by layer/min_height tags - Use synthetic IDs with bit-63 flag + relation ID + ring index for merged ways, avoiding FloodFillCache and deterministic RNG collisions with real way IDs
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Use relative base (0) for pillar_base when terrain is disabled, matching slab_y's coordinate system; previously used args.ground_level (absolute) which made the range empty - Clip assembled outer rings to the world bounding box after merge_way_segments, preventing oversized flood fills and out-of-bounds block placement - Pass xzbbox to generate_building_from_relation for post-merge clipping
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Use 16-bit ring index mask (0xFFFF) instead of 8-bit, supporting up to 65536 rings per relation - Add comment explaining why base_height uses start_y_offset directly in roof-only dome path (no walls underneath, unlike from_roof_area)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Fixes rendering of complex multipolygon building relations (e.g. Sydney Opera House) that split their outline across many short way segments. Previously these produced wall-only outlines with no filled floors, ceilings, or roofs.
Changes
Ring assembly for building multipolygon relations
osm_parser.rs: Added building multipolygon relations to thekeep_unclippedcondition so member ways are preserved for ring assembly (matching the existing water/boundary pattern).buildings.rs→generate_building_from_relation: Rewrote the!has_partsbranch to collect outer member node lists, merge open segments into closed rings viamerge_way_segments(), close nearly-closed rings, discard degenerate ones, then build syntheticProcessedWayobjects with relation tags for downstream processing.Roof rendering improvements
generate_roof_only_structure: Rewritten to be height- and shape-aware:min_height,building:min_level, andlayertags for vertical placement instead of using a hardcoded Y=5.roof:shapeand dispatches dome/hipped/pyramidal shapes to the existinggenerate_dome_roof()renderer.material/roof:material(glass) andcolour/building:colour(white → quartz).set_block_absolutefor correct terrain-aware placement.parse_roof_type: Added"circular"and"spherical"mappings toRoofType::Dome.generate_buildings: Added early return forbuilding:part=roofbefore thebuildingtag match, so roof shell parts are correctly routed to the roof-only generator.Impact
building:part=roof/building=roofcode paths.generate_roof_only_structureis isolated from the normal building style/preset pipeline.