Refactor road assembly module

This commit is contained in:
2026-08-03 13:39:05 +08:00
parent 64517993e9
commit 0f3c1ee635
12 changed files with 315 additions and 39 deletions

View File

@@ -0,0 +1,4 @@
{"file": ".trellis/spec/blender/testing.md", "reason": "Defines pure test command and bpy-layer parity expectations for validation."}
{"file": ".trellis/spec/guides/artifact-parity-guide.md", "reason": "Defines compare contract and expected no-diff result for refactors."}
{"file": ".trellis/spec/pipeline/layer-registry.md", "reason": "Defines ROAD_LAYERS order and warning behavior to review after extraction."}
{"file": ".trellis/spec/blender/module-structure.md", "reason": "Defines dependency direction and safe import boundaries to review after new module creation."}

View File

@@ -0,0 +1,74 @@
# Design
## Architecture
`roads.py` will join the bpy-layer feature modules under
`blender/osmassets/`. It may import `os`, `json`, `MeshBatch`, `add_polyline`,
and geometry helpers required by current road assembly:
```python
from osmassets.geom import clip_polygon, feature_in_bounds, geometry_rings
from osmassets.mesh import MeshBatch, add_polyline
```
The module will expose two functions:
```python
def assemble_geojson_layer(path, layer_id, projector, collection, material, z):
...
def assemble_osm_fallback(ways, projector, collection, material):
...
```
`assemble_geojson_layer()` returns the same count as the current
`add_geojson_layer()`. `assemble_osm_fallback()` returns no count because the
current fallback also leaves `road_counts` unchanged.
## Boundaries
- `generate_scene.py` keeps catalog ownership: `road_mats`, `ROAD_LAYERS`
iteration, `check_layers()` warnings, `road_counts`, scene metadata, and
stdout markers.
- `roads.py` owns only object/mesh construction for already chosen road inputs.
- `catalog.py` and `scripts/lib/scene-layers.js` remain untouched; road layer
collection/order is not part of this task.
- No new pure-Python logic is introduced. Existing geometry helpers are reused.
## Data Flow
1. `generate_scene.py` creates `road_mats` from `catalog.ROAD_LAYERS` and
`catalog.road_material_specs()`.
2. If `geojson_dir` exists, `generate_scene.py` runs `catalog.check_layers()`
and prints warnings exactly as before.
3. For each `catalog.ROAD_LAYERS` entry, `generate_scene.py` calls
`roads.assemble_geojson_layer(...)` and stores the returned count under the
layer id in `road_counts`.
4. If `road_counts.get("road_surface", 0) == 0`, `generate_scene.py` calls
`roads.assemble_osm_fallback(...)` with OSM ways and the road surface
material.
5. `generate_scene.py` writes `road_counts` into scene metadata and `SCENE_DONE`.
## Compatibility
The move must preserve road object names, mesh names, geometry clipping,
feature counting, layer iteration order, warning behavior, fallback trigger,
fallback widths, z values, material assignments, and metadata JSON. Cesium
export consumes the resulting `.blend`, so parity should see no non-ignored
contract difference.
## Trade-Offs
`catalog.check_layers()` intentionally stays in `generate_scene.py`. Moving it
into `roads.py` would mix cross-language catalog validation with object
assembly and increase the blast radius of this refactor.
The full feature registry remains deferred. Roads are the last high-risk
assembly extraction; registry design should be evaluated after this commit when
the remaining `generate_scene.py` responsibilities are clearer.
## Rollback
Rollback is mechanical: move `assemble_geojson_layer()` and
`assemble_osm_fallback()` logic back into `generate_scene.py`, restore the
direct call sites, remove the `roads.py` import, and delete the module.

View File

@@ -0,0 +1,5 @@
{"file": ".trellis/spec/blender/module-structure.md", "reason": "Defines bpy-layer module boundaries and feature module responsibilities for extracting roads."}
{"file": ".trellis/spec/blender/asset-generation.md", "reason": "Defines MeshBatch usage, material order sensitivity, and deterministic asset-generation constraints."}
{"file": ".trellis/spec/pipeline/layer-registry.md", "reason": "Defines ROAD_LAYERS / SCENE_LAYERS cross-language ordering contract and check_layers warning behavior."}
{"file": ".trellis/spec/guides/artifact-parity-guide.md", "reason": "Defines mandatory before/after parity validation for pure Blender scene refactors."}
{"file": ".trellis/spec/guides/cross-layer-thinking-guide.md", "reason": "Defines cross-layer road layer and stage-output contract risks."}

View File

@@ -0,0 +1,46 @@
# Implementation Plan
## Checklist
1. Capture a parity baseline before product-code edits.
2. Add `blender/osmassets/roads.py` with current GeoJSON road layer assembly
and OSM fallback logic.
3. Import `roads.py` in `blender/generate_scene.py`.
4. Remove inline `add_geojson_layer()` from `generate_scene.py`.
5. Replace the GeoJSON road layer call site with
`roads.assemble_geojson_layer(...)`.
6. Replace the OSM fallback loop with `roads.assemble_osm_fallback(...)`.
7. Run Python validation:
- `python3 -m py_compile blender/osmassets/roads.py`
- `python3 -m py_compile blender/generate_scene.py`
- `python3 -m unittest blender/tests/test_pure.py`
8. Capture parity after the refactor.
9. Compare before/after parity snapshots and inspect any non-ignored diff.
10. Update `.trellis/spec` and `docs/changelog.md` only for durable status or
convention changes discovered during implementation.
11. Commit, archive the task, and record the journal entry.
## Validation Commands
```bash
node scripts/parity.js capture road-module-before --stages blender,cesium
python3 -m py_compile blender/osmassets/roads.py
python3 -m py_compile blender/generate_scene.py
python3 -m unittest blender/tests/test_pure.py
node scripts/parity.js capture road-module-after --stages blender,cesium
node scripts/parity.js compare road-module-before road-module-after
```
## Risk Points
- `ROAD_LAYERS` order is load-bearing. Do not reorder, filter, or copy it into
a second list.
- `road_counts` must remain owned by `generate_scene.py`; fallback roads must
still leave it unchanged.
- `Layer catalog warning:` print text must remain unchanged because it is the
only runtime warning for JS/Python layer drift.
- `Road_<layer_id>` and `OSM_Road_<way_id>` names affect parity digests.
- `clip_polygon()` and `feature_in_bounds()` behavior must be reused exactly;
do not replace with new geometry logic.
- Parity is mandatory because this is a pure refactor of bpy-layer scene
generation.

View File

@@ -0,0 +1,93 @@
# Extract road module
## Goal
Move Blender road assembly out of `blender/generate_scene.py` into a dedicated
bpy-layer module, keeping generated `.blend` / `.glb` / metadata structure
unchanged. This completes the P2 feature-module extraction except for the
deferred full `features/` registry.
## Background
- `.trellis/spec/guides/artifact-parity-guide.md` records P2 as partial:
`water.py`, `grass.py`, `scrub.py`, `tree.py`, `fountain.py`, and
`building.py` are already split out, while roads still live in
`generate_scene.py`.
- `blender/generate_scene.py:148` defines `add_geojson_layer()`, which reads
one osm2streets GeoJSON road layer, filters by bounds, projects coordinates,
clips polygons, batches them as `Road_<layer>`, and returns a feature count.
- `blender/generate_scene.py:656` creates `road_mats` by zipping
`catalog.ROAD_LAYERS` with `catalog.road_material_specs()`.
- `blender/generate_scene.py:741` runs `catalog.check_layers(geojson_dir)` and
prints `Layer catalog warning:` messages before iterating `ROAD_LAYERS`.
- `blender/generate_scene.py:750` falls back to simple OSM highway polylines
when `road_counts["road_surface"]` is zero.
- `blender/generate_scene.py:866` stores `scene["road_feature_counts"]`; the
same `road_counts` object is emitted in `SCENE_DONE` as `road_features`.
## Requirements
1. Add a new bpy-layer module at `blender/osmassets/roads.py`.
2. Move the current roads assembly behavior into the new module:
- GeoJSON layer mesh assembly currently in `add_geojson_layer()`;
- simple OSM highway fallback currently in the `road_surface == 0` block.
3. Keep `generate_scene.py` responsible for:
- creating `road_mats` from `catalog.ROAD_LAYERS` and
`catalog.road_material_specs()`;
- running `catalog.check_layers()` and printing existing warning text;
- owning `road_counts`;
- writing `scene["road_feature_counts"]`;
- writing the existing `SCENE_DONE` JSON shape.
4. Preserve existing road object names and mesh names:
- `Road_<layer_id>`
- `OSM_Road_<way_id>`
5. Preserve existing GeoJSON behavior:
- missing layer file returns count `0`;
- feature is skipped when `feature_in_bounds()` is false;
- rings come from `geometry_rings()`;
- projected rings are clipped with `clip_polygon()`;
- only rings with at least three points are batched and counted.
6. Preserve existing OSM fallback behavior:
- fallback triggers only when `road_counts.get("road_surface", 0) == 0`;
- only ways with a `highway` tag and at least two coordinates are drawn;
- widths remain `secondary: 7.0`, `residential: 5.5`, `service: 3.5`,
default `4.0`;
- fallback material remains `road_mats["road_surface"]`;
- fallback z remains `0.03`;
- fallback does not add to `road_counts`.
7. Do not introduce a full `features/` registry in this task.
8. Do not modify `ROAD_LAYERS`, `SCENE_LAYERS`, material definitions,
`MATERIALS` order, GeoJSON schema, z-index/z-height values, stdout markers,
parity ignore lists, or known unrelated defects D1-D3.
## Acceptance Criteria
- [ ] `blender/osmassets/roads.py` contains road assembly behavior and imports
only bpy-layer-safe dependencies.
- [ ] `blender/generate_scene.py` imports and calls the new roads module; no
inline `add_geojson_layer()` remains there.
- [ ] `road_counts`, `scene["road_feature_counts"]`, and `SCENE_DONE`
`road_features` remain unchanged.
- [ ] Existing `Layer catalog warning:` print behavior remains in
`generate_scene.py`.
- [ ] `python3 -m py_compile blender/osmassets/roads.py` passes.
- [ ] `python3 -m py_compile blender/generate_scene.py` passes.
- [ ] `python3 -m unittest blender/tests/test_pure.py` passes.
- [ ] A before/after parity comparison is run for the Blender/Cesium stages and
any non-ignored diff is either absent or explicitly explained as
expected. For this refactor, the expected result is no contract diff.
## Out Of Scope
- Full `features/` registry implementation.
- Any change to osm2streets road splitting, `SCENE_LAYERS`, or QGIS pipeline
behavior.
- Any change to `catalog.ROAD_LAYERS`, material names, color values, or order.
- Any change to fallback highway widths, z values, object names, or counts.
- Turning `catalog.check_layers()` warnings into errors.
- Moving `road_mats` creation or `road_feature_counts` scene metadata out of
`generate_scene.py`.
## Open Questions
None.

View File

@@ -0,0 +1,26 @@
{
"id": "extract-road-module",
"name": "extract-road-module",
"title": "Extract road module",
"description": "Move Blender road assembly out of generate_scene.py into a dedicated bpy-layer module without changing generated artifacts.",
"status": "in_progress",
"dev_type": null,
"scope": null,
"package": null,
"priority": "P2",
"creator": "dingkang",
"assignee": "dingkang",
"createdAt": "2026-08-03",
"completedAt": null,
"branch": null,
"base_branch": "main",
"worktree_path": null,
"commit": null,
"pr_url": null,
"subtasks": [],
"children": [],
"parent": null,
"relatedFiles": [],
"notes": "",
"meta": {}
}