From 14dc20c0e57e1536ca56432fd7816fdc01b72091 Mon Sep 17 00:00:00 2001 From: TMHSDigital <154358121+TMHSDigital@users.noreply.github.com> Date: Wed, 7 Oct 2026 10:13:54 -0400 Subject: [PATCH] fix(templates): keep every mesh, handle shared and parented imports, bake modifier stacks in order The pipeline template exported only the largest mesh with exit 0. Its unit check could never fire, and LOD budgets were printed but never checked. Shared (instanced) mesh data made transform_apply raise. A child of a rotated glTF root passed the matrix_basis identity test and was grounded along a local axis. The headless template's non-first modifier_apply reversed the stack order, contrary to its README. The pipeline now isolates mesh data, unparents keeping world placement, applies transforms and joins every part. It checks the asset's extent (exit 8) and each LOD budget (exit 9), and exits 12 when the apply or join raises. The cleanup and export-preset skills and the three preset snippets get the same isolation and unparent guards. The headless template bakes the whole stack through new_from_object. check_example_rules now scans templates/ and flags object operators reached from a loop. Smoke covers the shared-mesh parented GLB, the modifier order, and the out-of-range extent. Signed-off-by: TMHSDigital Co-Authored-By: Claude Opus 5.5 --- .github/workflows/blender-smoke.yml | 45 ++++++ .../export-preset-axis/export_preset_axis.py | 4 +- skills/ai-mesh-cleanup/SKILL.md | 43 +++++- skills/engine-export-presets/SKILL.md | 14 +- snippets/export_preset_godot.py | 12 +- snippets/export_preset_unity.py | 12 +- snippets/export_preset_unreal.py | 12 +- .../ai-asset-pipeline-template/README.md | 40 ++++- .../ai-asset-pipeline-template/pipeline.py | 145 +++++++++++++----- .../headless-batch-script-template/README.md | 26 +++- .../headless-batch-script-template/script.py | 65 +++++--- tests/check_example_rules.py | 82 +++++++++- tests/smoke/check_glb_tris.py | 22 +++ tests/smoke/check_pipeline_glb.py | 72 +++++++++ tests/smoke/make_input.py | 11 ++ tests/smoke/make_pipeline_glb.py | 79 ++++++++-- 16 files changed, 580 insertions(+), 104 deletions(-) create mode 100644 tests/smoke/check_glb_tris.py create mode 100644 tests/smoke/check_pipeline_glb.py diff --git a/.github/workflows/blender-smoke.yml b/.github/workflows/blender-smoke.yml index 706b9b05..314add1d 100644 --- a/.github/workflows/blender-smoke.yml +++ b/.github/workflows/blender-smoke.yml @@ -218,6 +218,18 @@ jobs: [ "$code" -eq 2 ] || { echo "::error::expected exit 2 for no-mesh input, got $code"; exit 1; } echo "no-mesh exit code = $code (correct)" + - name: Headless template applies the new modifier after the existing stack (#468) + run: | + set -euo pipefail + # stack.blend: a cube with a live SUBSURF (levels 1). SUBSURF then + # TRIANGULATE is 48 tris; the reversed order a non-first + # modifier_apply produces is 72. + xvfb-run -a "$BLENDER" --background "$RUNNER_TEMP/out/stack.blend" \ + --python-exit-code 1 --python templates/headless-batch-script-template/script.py -- \ + --output "$RUNNER_TEMP/out/stack.glb" --apply-modifier TRIANGULATE + xvfb-run -a "$BLENDER" --background --python-exit-code 1 --python tests/smoke/check_glb_tris.py -- \ + "$RUNNER_TEMP/out/stack.glb" 48 + - name: Extension template passes Blender's own manifest validator run: | set -euo pipefail @@ -256,6 +268,39 @@ jobs: [ "$code" -eq 2 ] || { echo "::error::expected exit 2 for missing input, got $code"; exit 1; } echo "missing-input exit code = $code (correct)" + - name: Pipeline template keeps every part of a shared-mesh, parented GLB (#462, #467) + run: | + set -euo pipefail + # Root rotated 90 deg on X, scaled 2x, at z=3; Body plus two wheels + # sharing one mesh. lod0 must hold every part (world bbox == source), + # grounded on its world minimum Z, with no node rotation or scale. + mkdir -p "$RUNNER_TEMP/out/pipeline_parented" + xvfb-run -a "$BLENDER" --background --python-exit-code 1 --python tests/smoke/make_pipeline_glb.py -- \ + "$RUNNER_TEMP/out/parented_src.glb" --parented + xvfb-run -a "$BLENDER" --background --python-exit-code 1 --python templates/ai-asset-pipeline-template/pipeline.py -- \ + --input "$RUNNER_TEMP/out/parented_src.glb" \ + --outdir "$RUNNER_TEMP/out/pipeline_parented" \ + --preset unity \ + --lod-budgets 1024,256,64 \ + --collider convex + xvfb-run -a "$BLENDER" --background --python-exit-code 1 --python tests/smoke/check_pipeline_glb.py -- \ + "$RUNNER_TEMP/out/parented_src.glb" "$RUNNER_TEMP/out/pipeline_parented/lod0.glb" + + - name: Falsifier - pipeline template rejects a 2 km asset with exit 8 + run: | + set -euo pipefail + xvfb-run -a "$BLENDER" --background --python-exit-code 1 --python tests/smoke/make_pipeline_glb.py -- \ + "$RUNNER_TEMP/out/huge_src.glb" --radius 1000 + set +e + xvfb-run -a "$BLENDER" --background --python-exit-code 1 --python templates/ai-asset-pipeline-template/pipeline.py -- \ + --input "$RUNNER_TEMP/out/huge_src.glb" \ + --outdir "$RUNNER_TEMP/out/pipeline" \ + --preset unity + code=$? + set -e + [ "$code" -eq 8 ] || { echo "::error::expected exit 8 for out-of-range extent, got $code"; exit 1; } + echo "out-of-range extent exit code = $code (correct)" + - name: Headless render template runs (exit 0, PNG produced) run: | set -euo pipefail diff --git a/examples/export-preset-axis/export_preset_axis.py b/examples/export-preset-axis/export_preset_axis.py index 8367476f..9f93ead2 100644 --- a/examples/export-preset-axis/export_preset_axis.py +++ b/examples/export-preset-axis/export_preset_axis.py @@ -307,9 +307,11 @@ def principled(name, color, metallic, roughness): def apply_selected_mesh_transforms(): - # Same body as snippets/export_preset_unity.py: one operator call for the + # Core of snippets/export_preset_unity.py: one operator call for the # whole selection. transform_apply acts on selected_editable_objects, so # that is the key to override; selected_objects alone does not narrow it. + # The snippet's shared-mesh / parent prelude is left out: this scene + # builds single-user, unparented meshes. meshes = [o for o in bpy.context.selected_objects if o.type == "MESH"] if not meshes: return diff --git a/skills/ai-mesh-cleanup/SKILL.md b/skills/ai-mesh-cleanup/SKILL.md index 13a5bfb0..665fcf93 100644 --- a/skills/ai-mesh-cleanup/SKILL.md +++ b/skills/ai-mesh-cleanup/SKILL.md @@ -41,11 +41,34 @@ def scene_units_are_meters(scene): return abs(units.scale_length - 1.0) < 1e-6 +def isolate_mesh_data(objs): + # glTF instancing imports as several objects sharing one Mesh (users > 1). + # transform_apply refuses multi-user data, and origin_to_base() on shared + # data moves every other instance. One copy per object; the last user + # keeps the original. + for obj in objs: + if obj.data.users > 1: + obj.data = obj.data.copy() + + +def clear_parent_keep_transform(objs): + # glTF node trees import meshes under a root empty that is often rotated + # or scaled. Applying the child's own rotation leaves the root's in + # matrix_world, so local Z is still not world Z. + for obj in objs: + if obj.parent is not None: + world = obj.matrix_world.copy() + obj.parent = None + obj.matrix_world = world + bpy.context.view_layer.update() + + def rot_scale_is_identity(obj, tol=1e-6): # Rotation and scale both: origin_to_base() shifts along local Z, which is # world Z only once rotation is applied. A GLB node often carries a - # rotation with identity scale. - m = obj.matrix_basis.to_3x3() + # rotation with identity scale. Read matrix_world, not matrix_basis: a + # child of a rotated root has an identity basis. + m = obj.matrix_world.to_3x3() return all( abs(m[i][j] - (1.0 if i == j else 0.0)) < tol for i in range(3) @@ -68,7 +91,8 @@ def apply_transforms(objs): def origin_to_base(obj): - # Precondition: rotation and scale applied (local Z == world Z). + # Precondition: unparented, rotation and scale applied (local Z == world + # Z), and single-user data (isolate_mesh_data), or other instances move. mesh = obj.data n = len(mesh.vertices) flat = [0.0] * (n * 3) @@ -120,9 +144,14 @@ if not scene_units_are_meters(scene): scene.unit_settings.system = "METRIC" scene.unit_settings.scale_length = 1.0 -apply_transforms([o for o in imported_meshes() if not rot_scale_is_identity(o)]) +meshes = imported_meshes() +isolate_mesh_data(meshes) +clear_parent_keep_transform(meshes) +apply_transforms([o for o in meshes if not rot_scale_is_identity(o)]) ``` +The scene check above only catches a scene someone changed. A fresh or factory scene is always metric at 1.0, so it cannot see a prop written in centimeters as meters. Also check the geometry: after the apply, the largest bounding-box extent of a prop should be within a plausible range (the pipeline template uses 0.01 m to 100 m and exits non-zero outside it). + `export_apply=True` on glTF applies **modifiers**, not object scale. Unapplied object scale lands on the glTF node. Witness: [`examples/unapplied-scale-gltf/`](https://github.com/TMHSDigital/Blender-Developer-Tools/tree/main/examples/unapplied-scale-gltf). ### 3. Apply transforms @@ -131,6 +160,11 @@ apply_transforms([o for o in imported_meshes() if not rot_scale_is_identity(o)]) Gate the apply on rotation **and** scale (`rot_scale_is_identity`). Imported glTF nodes often carry a rotation with identity scale; skipping the apply then makes step 4 ground the mesh along its local Z, which moves it in world space (a cube rotated 90° on X at z=5 shifted by −1 in Y and Z). +Two import shapes break a naive apply, and both are ordinary glTF: + +- **Shared mesh data.** Instanced nodes import as several objects on one Mesh (`users == 2` after a round trip). `transform_apply` raises `RuntimeError: Cannot apply to a multi user` on both 4.5.11 and 5.2.1. Run `isolate_mesh_data` first. `transform_apply(isolate_users=True)` also exists on both lines (measured on 4.5.11 and 5.2.1), but it only helps the apply. `origin_to_base` rewrites vertices and still needs single-user data. +- **Parented meshes.** A child of a rotated or scaled root has an identity `matrix_basis`, so a basis check skips it and the root's rotation stays in `matrix_world`. Unparent with the world matrix kept, then test `matrix_world`. Measured with the pipeline template's parented smoke fixture on 4.5.11 and 5.2.1 (a root rotated 90° on X and scaled 2x at z=3). With the unparent step removed, LOD0 exported with its origin at z=3.0 while the geometry's minimum was z=2.0. + ### 4. Set origin Origin at the lowest Z of the mesh (sit-on-ground) via `foreach_get` / `foreach_set`, not a Python loop on `mesh.vertices`. This works in local space, so run it only after step 3. See [`examples/prop-origin-transform/`](https://github.com/TMHSDigital/Blender-Developer-Tools/tree/main/examples/prop-origin-transform) for origin-to-base plus `matrix_parent_inverse`. @@ -182,6 +216,7 @@ Draco, selected-only, explicit `export_yup`, and `export_apply=True` so the deci 5. **`bm.normal_update()` for flipped faces.** Use `recalc_face_normals`. 6. **Import then `bpy.ops.mesh.*` with no scale check.** Rule `validate-imported-mesh-scale`. 7. **Export with a live DECIMATE and `export_apply=False`.** The engine gets the dense mesh. Rule `no-unapplied-modifiers-on-export`. +8. **Assuming one imported object, single-user and unparented.** Picking the largest mesh drops every other part. Applying shared data raises, and testing `matrix_basis` misses a rotated parent. Isolate the data, unparent, apply, then join if the engine wants one asset. ## Version correctness diff --git a/skills/engine-export-presets/SKILL.md b/skills/engine-export-presets/SKILL.md index 3dd52ad2..ce5c15cc 100644 --- a/skills/engine-export-presets/SKILL.md +++ b/skills/engine-export-presets/SKILL.md @@ -33,17 +33,27 @@ Before any preset: meters in the scene (`scale_length == 1.0`), identity object ```python def apply_selected_mesh_transforms(): # One operator call for the whole selection. transform_apply reads - # selected_editable_objects, so that is the key to override; overriding - # selected_objects alone does not narrow it. + # selected_editable_objects, so that is the key to override. It refuses + # shared (glTF-instanced) mesh data, so copy it per object first, and + # unparent keeping the world placement, or a rotated root stays on the node. meshes = [o for o in bpy.context.selected_objects if o.type == "MESH"] if not meshes: return + for o in meshes: + if o.data.users > 1: + o.data = o.data.copy() + if o.parent is not None: + world = o.matrix_world.copy() + o.parent = None + o.matrix_world = world with bpy.context.temp_override( object=meshes[0], active_object=meshes[0], selected_editable_objects=meshes ): bpy.ops.object.transform_apply(location=False, rotation=True, scale=True) ``` +The two guards are for ordinary imports. Instanced glTF nodes come back as several objects sharing one Mesh, and `transform_apply` then raises `Cannot apply to a multi user` (4.5.11 and 5.2.1). A mesh under a rotated root has an identity `matrix_basis`. Applying it leaves the root's rotation in `matrix_world`, and `use_selection=True` writes that rotation onto the exported node. Copying the data means instances no longer share one mesh. That is the cost of baking transforms into vertices. + `use_selection=True` on every preset. Draco is opt-in on glTF; do not copy `gltf_draco_export.py` wholesale. ## glTF is the same for every engine diff --git a/snippets/export_preset_godot.py b/snippets/export_preset_godot.py index 9184903b..f7d7be81 100644 --- a/snippets/export_preset_godot.py +++ b/snippets/export_preset_godot.py @@ -18,11 +18,19 @@ def apply_selected_mesh_transforms(): # One operator call for the whole selection. transform_apply reads - # selected_editable_objects, so that is the key to override; overriding - # selected_objects alone does not narrow it. + # selected_editable_objects, so that is the key to override. It refuses + # shared (glTF-instanced) mesh data, so copy it per object first, and + # unparent keeping the world placement, or a rotated root stays on the node. meshes = [o for o in bpy.context.selected_objects if o.type == "MESH"] if not meshes: return + for o in meshes: + if o.data.users > 1: + o.data = o.data.copy() + if o.parent is not None: + world = o.matrix_world.copy() + o.parent = None + o.matrix_world = world with bpy.context.temp_override( object=meshes[0], active_object=meshes[0], selected_editable_objects=meshes ): diff --git a/snippets/export_preset_unity.py b/snippets/export_preset_unity.py index 7dada8be..53f57907 100644 --- a/snippets/export_preset_unity.py +++ b/snippets/export_preset_unity.py @@ -16,11 +16,19 @@ def apply_selected_mesh_transforms(): # One operator call for the whole selection. transform_apply reads - # selected_editable_objects, so that is the key to override; overriding - # selected_objects alone does not narrow it. + # selected_editable_objects, so that is the key to override. It refuses + # shared (glTF-instanced) mesh data, so copy it per object first, and + # unparent keeping the world placement, or a rotated root stays on the node. meshes = [o for o in bpy.context.selected_objects if o.type == "MESH"] if not meshes: return + for o in meshes: + if o.data.users > 1: + o.data = o.data.copy() + if o.parent is not None: + world = o.matrix_world.copy() + o.parent = None + o.matrix_world = world with bpy.context.temp_override( object=meshes[0], active_object=meshes[0], selected_editable_objects=meshes ): diff --git a/snippets/export_preset_unreal.py b/snippets/export_preset_unreal.py index 0721e4df..ce3bed2b 100644 --- a/snippets/export_preset_unreal.py +++ b/snippets/export_preset_unreal.py @@ -19,11 +19,19 @@ def apply_selected_mesh_transforms(): # One operator call for the whole selection. transform_apply reads - # selected_editable_objects, so that is the key to override; overriding - # selected_objects alone does not narrow it. + # selected_editable_objects, so that is the key to override. It refuses + # shared (glTF-instanced) mesh data, so copy it per object first, and + # unparent keeping the world placement, or a rotated root stays on the node. meshes = [o for o in bpy.context.selected_objects if o.type == "MESH"] if not meshes: return + for o in meshes: + if o.data.users > 1: + o.data = o.data.copy() + if o.parent is not None: + world = o.matrix_world.copy() + o.parent = None + o.matrix_world = world with bpy.context.temp_override( object=meshes[0], active_object=meshes[0], selected_editable_objects=meshes ): diff --git a/templates/ai-asset-pipeline-template/README.md b/templates/ai-asset-pipeline-template/README.md index 3a9448d0..29a00587 100644 --- a/templates/ai-asset-pipeline-template/README.md +++ b/templates/ai-asset-pipeline-template/README.md @@ -46,13 +46,25 @@ same way for every preset. 1. Parses script-side args after `--`. 2. Imports the GLB into an empty scene. -3. Checks scene units (metric meters), applies object rotation/scale, - sits the origin on the lowest Z, recalculates face normals, and - prints the evaluated triangle count. -4. Builds an LOD chain from `--lod-budgets`. -5. Optionally builds a convex hull or AABB box collider. -6. Exports each LOD (and the collider) under the chosen engine preset. -7. Returns explicit exit codes so a CI pipeline can detect failures. +3. Keeps **every** mesh in the file. Shared (instanced) mesh data gets one + copy per object, parents are cleared with the world placement kept, + rotation and scale are applied, and all parts are joined into one + object (`object.join`, which keeps material slots). A body plus + separate wheels ships as one asset with the wheels in place; nothing + is dropped. Split the parts in your own code before this step if your + engine wants them as separate assets. +4. Checks the joined asset's largest bounding-box extent against + 0.01 m to 100 m (`MIN_EXTENT_M` / `MAX_EXTENT_M`). A fresh scene is + always metric at scale 1.0, so this measures the imported geometry, + not the scene settings. A prop written in centimeters as meters + fails here instead of shipping 100x too large. +5. Sits the origin on the lowest world Z, recalculates face normals, + and prints the evaluated triangle count. +6. Builds an LOD chain from `--lod-budgets` and asserts that each LOD's + evaluated triangle count is at or under its budget. +7. Optionally builds a convex hull or AABB box collider. +8. Exports each LOD (and the collider) under the chosen engine preset. +9. Returns explicit exit codes so a CI pipeline can detect failures. Cleanup order follows `ai-mesh-cleanup`. LOD, collider, and export helpers are duplicated from the snippets named in `pipeline.py`'s @@ -74,6 +86,12 @@ as `export-preset-axis` number their own checks independently. | 5 | Import produced no mesh | | 6 | `outdir` is not a directory, or glTF export failed / wrote no file | | 7 | `--collider convex` produced a hull that is not closed (an edge without exactly two faces), e.g. a flat input | +| 8 | The joined asset's largest extent is outside 0.01 m to 100 m (wrong source units) | +| 9 | An LOD's evaluated triangle count is over its `--lod-budgets` entry | +| 12 | `transform_apply` or `object.join` raised `RuntimeError` (for example multi-user data that was not isolated) | + +10 and 11 are skipped on purpose. The repo's shared render gates use them +(framing and asset quality), so a template never reuses them. ## Expected environment @@ -91,6 +109,14 @@ as `export-preset-axis` number their own checks independently. - **Running without `--background`**. The script still works, but Blender opens a UI window and stays open after the script finishes. Use `--background` for unattended runs. +- **Instanced and parented glTF nodes**. Two nodes on one mesh import as + two objects sharing one Mesh (`users == 2`), and `transform_apply` + refuses that data. A mesh under a rotated root has an identity + `matrix_basis`, so the rotation lives only in `matrix_world`. The + script isolates and unparents before applying. If you remove that + step, a shared-mesh input exits 12. A parented input exits 0 but ships + with its origin off the ground: on the smoke fixture, z=3.0 against a + geometry minimum of z=2.0. - **Operators that need a 3D Viewport context**. Some operators only work when a `VIEW_3D` area exists. In headless mode, none does. Either rewrite using `bpy.data.*`, or fabricate a window+area via diff --git a/templates/ai-asset-pipeline-template/pipeline.py b/templates/ai-asset-pipeline-template/pipeline.py index 03e4091a..39ea3d21 100644 --- a/templates/ai-asset-pipeline-template/pipeline.py +++ b/templates/ai-asset-pipeline-template/pipeline.py @@ -18,7 +18,9 @@ # snippets/lod_chain.py (make_lod_chain; itself duplicates the above) # snippets/convex_hull_collider.py # snippets/export_preset_unity.py / export_preset_godot.py / export_preset_unreal.py -# Cleanup order follows skills/ai-mesh-cleanup/SKILL.md. +# Cleanup order follows skills/ai-mesh-cleanup/SKILL.md. Every mesh in the +# input is kept: parts are made single-user, unparented, applied and joined +# into one object before the LOD chain, so nothing is dropped. # Exit codes follow templates/headless-batch-script-template/script.py: # 0 success, 2+ distinct failure modes, argparse usage also exits 2. # @@ -96,18 +98,43 @@ def parse_budgets(text): return budgets -def scene_units_are_meters(scene): - units = scene.unit_settings - if units.system not in {"METRIC", "NONE"}: - return False - return abs(units.scale_length - 1.0) < 1e-6 +# Sanity range for the asset's largest world extent, in meters. A generated +# prop written in centimeters as meters lands ~100x too large; one written in +# meters as centimeters lands ~100x too small. read_factory_settings always +# leaves the scene metric at scale 1.0, so the scene settings cannot reveal +# this; only the imported geometry can. +MIN_EXTENT_M = 0.01 +MAX_EXTENT_M = 100.0 + + +def isolate_mesh_data(objs): + # glTF instancing imports as several objects sharing one Mesh. transform_apply + # refuses multi-user data ("Cannot apply to a multi user"), and rewriting + # shared vertices (origin_to_base) moves every other instance. Give each + # object its own copy; the last user keeps the original. + for obj in objs: + if obj.data.users > 1: + obj.data = obj.data.copy() + + +def clear_parent_keep_transform(objs): + # A glTF hierarchy imports its meshes under a root empty that may be + # rotated or scaled. Applying the child's own rotation leaves the root's in + # matrix_world, so local Z is still not world Z. Unparent, keeping the + # world placement, before applying. + for obj in objs: + if obj.parent is not None: + world = obj.matrix_world.copy() + obj.parent = None + obj.matrix_world = world + bpy.context.view_layer.update() def rot_scale_is_identity(obj, tol=1e-6): - # Rotation and scale both: origin_to_base() shifts along local Z, which is - # world Z only once rotation is applied. A GLB node often carries a - # rotation with identity scale. - m = obj.matrix_basis.to_3x3() + # Rotation and scale both, read from matrix_world: matrix_basis is identity + # on a child of a rotated root, which would skip the apply. origin_to_base() + # shifts along local Z, which is world Z only once this holds. + m = obj.matrix_world.to_3x3() return all( abs(m[i][j] - (1.0 if i == j else 0.0)) < tol for i in range(3) @@ -115,15 +142,48 @@ def rot_scale_is_identity(obj, tol=1e-6): ) -def apply_object_transform(obj): +def apply_transforms(objs): + # One operator call for the whole list. transform_apply acts on + # selected_editable_objects, so that is the key to override. + if not objs: + return with bpy.context.temp_override( - object=obj, active_object=obj, selected_editable_objects=[obj] + object=objs[0], active_object=objs[0], selected_editable_objects=list(objs) ): bpy.ops.object.transform_apply(location=False, rotation=True, scale=True) +def join_meshes(objs): + # Every part ships: one object.join folds the parts into objs[0], baking + # each part's world placement relative to it and keeping material slots. + if len(objs) == 1: + return objs[0] + active = objs[0] + with bpy.context.temp_override( + object=active, + active_object=active, + selected_objects=list(objs), + selected_editable_objects=list(objs), + ): + bpy.ops.object.join() + return active + + +def largest_extent_m(obj, scene): + # Precondition: rotation and scale applied, so local extents are world ones. + mesh = obj.data + n = len(mesh.vertices) + if n == 0: + return 0.0 + flat = [0.0] * (n * 3) + mesh.vertices.foreach_get("co", flat) + extent = max(max(flat[a::3]) - min(flat[a::3]) for a in range(3)) + return extent * scene.unit_settings.scale_length + + def origin_to_base(obj): - # Precondition: rotation and scale applied (local Z == world Z). + # Precondition: rotation and scale applied (local Z == world Z) and the + # mesh single-user (isolate_mesh_data), or other instances move. mesh = obj.data n = len(mesh.vertices) if n == 0: @@ -254,19 +314,6 @@ def box_collider(obj, name=None): return collider -def apply_selected_mesh_transforms(): - # Duplicated from snippets/export_preset_unity.py. One operator call for - # the whole selection; transform_apply reads selected_editable_objects, - # so that is the key to override (selected_objects alone does not narrow it). - meshes = [o for o in bpy.context.selected_objects if o.type == "MESH"] - if not meshes: - return - with bpy.context.temp_override( - object=meshes[0], active_object=meshes[0], selected_editable_objects=meshes - ): - bpy.ops.object.transform_apply(location=False, rotation=True, scale=True) - - def select_only(obj): for other in bpy.data.objects: other.select_set(False) @@ -279,7 +326,9 @@ def export_preset(filepath, preset, draco): # it that way (Unreal converts to centimeters itself), so every preset is # the same export. `preset` stays as the hook for engine-specific import # hints, such as Godot's "-convcolonly" collider name suffix. - apply_selected_mesh_transforms() + # The snippets' apply_selected_mesh_transforms() prelude is not repeated + # here: main() already isolated, unparented and applied every part, and + # the LODs and collider copy that identity matrix_world. bpy.ops.export_scene.gltf( filepath=filepath, export_format="GLB", @@ -336,16 +385,29 @@ def main(): print(f"Found {len(meshes)} mesh object(s): {[o.name for o in meshes]}") + # Shared mesh data and parent hierarchies are how glTF instancing and node + # trees import; both must be resolved before any transform is applied. + isolate_mesh_data(meshes) + clear_parent_keep_transform(meshes) + try: + apply_transforms([o for o in meshes if not rot_scale_is_identity(o)]) + hero = join_meshes(meshes) + except RuntimeError as exc: + print(f"ERROR: transform apply or join failed: {exc}", file=sys.stderr) + return 12 + print(f"Joined {len(meshes)} part(s) into {hero.name}") + scene = bpy.context.scene - if not scene_units_are_meters(scene): - scene.unit_settings.system = "METRIC" - scene.unit_settings.scale_length = 1.0 - print("Set scene units to metric meters") - - hero = max(meshes, key=evaluated_triangle_count) - if not rot_scale_is_identity(hero): - apply_object_transform(hero) - print(f"Applied object scale/rotation on {hero.name}") + extent = largest_extent_m(hero, scene) + print(f"largest_extent_m={extent:.4f}") + if not MIN_EXTENT_M <= extent <= MAX_EXTENT_M: + print( + f"ERROR: largest extent {extent:.4f} m is outside " + f"[{MIN_EXTENT_M}, {MAX_EXTENT_M}] m; check the source units", + file=sys.stderr, + ) + return 8 + origin_to_base(hero) recalc_normals(hero) src_tris = evaluated_triangle_count(hero) @@ -353,9 +415,14 @@ def main(): lods = make_lod_chain(hero, budgets) for lod, budget in zip(lods, budgets): - print( - f"{lod.name} budget={budget} evaluated_tris={evaluated_triangle_count(lod)}" - ) + tris = evaluated_triangle_count(lod) + print(f"{lod.name} budget={budget} evaluated_tris={tris}") + if tris > budget: + print( + f"ERROR: {lod.name} has {tris} triangles, over its budget of {budget}", + file=sys.stderr, + ) + return 9 collider = None if args.collider == "convex": diff --git a/templates/headless-batch-script-template/README.md b/templates/headless-batch-script-template/README.md index b51de627..6cecaf2c 100644 --- a/templates/headless-batch-script-template/README.md +++ b/templates/headless-batch-script-template/README.md @@ -30,10 +30,12 @@ it is forwarded to `script.py` as `sys.argv`. 1. Parses script-side args after `--`. 2. Iterates every mesh object in the loaded `.blend`. -3. If `--apply-modifier` was passed, adds that modifier and applies it - in place. The application step uses `bpy.context.temp_override` - rather than the deprecated context-dict-passing form, so the script - works on Blender 4.5 LTS and 5.x. +3. If `--apply-modifier` was passed, appends that modifier to the end of + each mesh's stack. It then bakes the **whole** stack in stack order + through the data API: `bpy.data.meshes.new_from_object` on the + evaluated object, one depsgraph evaluation for every mesh, with no + operator per object. The new modifier runs after any existing ones. + Every modifier on the object is applied, not only the new one. 4. Exports the scene to a `.glb` at the given output path. 5. Returns explicit exit codes (0 success, 2-4 different failure modes) so a CI pipeline can detect failures. @@ -71,10 +73,18 @@ is legal; argparse usage is `2`). Not a repo-wide table. work when a `VIEW_3D` area exists. In headless mode, none does. Either rewrite using `bpy.data.*`, or fabricate a window+area via `temp_override` (advanced; see the `headless-batch-scripting` skill). -- **Modifier application order**. The script adds modifiers at the end - of the existing modifier stack and applies them, so they run after - any pre-existing modifiers. If the input file already has modifiers, - this may not produce what you expect. +- **Modifier application order**. Do not swap the bake for + `bpy.ops.object.modifier_apply(modifier=new.name)`. When the new + modifier is not first in the stack, that operator evaluates it against + the **base** mesh, prints only `Info: Applied modifier was not first, + result may not be as expected`, and leaves the earlier modifiers live. + `export_apply=True` then runs those afterwards, so the order is + reversed. Measured on 4.5.11 and 5.2.1 with a cube carrying a live + SUBSURF (levels 1) and `--apply-modifier TRIANGULATE`, the operator + path exported 72 triangles, which is TRIANGULATE first and then + SUBSURF. The stack bake gives 26 verts and 48 triangles, which is + SUBSURF first and then TRIANGULATE. CI checks this case + (`tests/smoke/check_glb_tris.py`). ## Extending the template diff --git a/templates/headless-batch-script-template/script.py b/templates/headless-batch-script-template/script.py index 55cfd164..ca86d5a6 100644 --- a/templates/headless-batch-script-template/script.py +++ b/templates/headless-batch-script-template/script.py @@ -9,7 +9,9 @@ # Anything before `--` is consumed by Blender itself. # # This template demonstrates the safe headless batch pattern: -# - bpy.data.* for direct manipulation (no UI required) +# - bpy.data.* for direct manipulation (no UI required), including the +# modifier bake: Mesh.new_from_object on the evaluated object, not +# bpy.ops.object.modifier_apply per object in a loop # - bpy.context.temp_override(...) only when an operator is genuinely needed # - explicit exit codes so a CI pipeline can detect failures # @@ -53,21 +55,39 @@ def parse_args(argv): return parser.parse_args(script_args) -def add_and_apply_modifier(obj, modifier_type, subsurf_levels=2): - """Add a modifier to obj and apply it. - - Modifier application is one of the few cases where bpy.ops is the - canonical path; bpy.data does not expose an apply method. We use - temp_override to set the active object cleanly, instead of the - deprecated context-dict-passing form. - """ +def add_modifier(obj, modifier_type, subsurf_levels=2): + """Append a modifier to the END of obj's stack, after any existing ones.""" modifier = obj.modifiers.new(name=modifier_type, type=modifier_type) if modifier_type == "SUBSURF": modifier.levels = subsurf_levels modifier.render_levels = subsurf_levels + return modifier - with bpy.context.temp_override(object=obj, active_object=obj): - bpy.ops.object.modifier_apply(modifier=modifier.name) + +def bake_modifier_stack(objs): + """Replace each object's mesh with its evaluated stack, in stack order. + + Not bpy.ops.object.modifier_apply: on a modifier that is not first in the + stack it evaluates that modifier against the BASE mesh and leaves the + earlier ones live ("Applied modifier was not first"), so export_apply then + runs them afterwards and the order is reversed. new_from_object on the + evaluated object bakes the whole stack in its real order, through the data + API, with one depsgraph evaluation for every object (no operator per + object in a loop). Each object gets its own new mesh, so shared mesh data + is never rewritten under another user. + """ + depsgraph = bpy.context.evaluated_depsgraph_get() + for obj in objs: + baked = bpy.data.meshes.new_from_object( + obj.evaluated_get(depsgraph), + preserve_all_data_layers=True, + depsgraph=depsgraph, + ) + old = obj.data + obj.modifiers.clear() + obj.data = baked + if old.users == 0: + bpy.data.meshes.remove(old) def main(): @@ -82,15 +102,20 @@ def main(): if args.apply_modifier: for obj in mesh_objects: - try: - add_and_apply_modifier(obj, args.apply_modifier, args.subsurf_levels) - print(f"Applied {args.apply_modifier} to {obj.name}") - except RuntimeError as exc: - print( - f"ERROR: failed to apply {args.apply_modifier} to {obj.name}: {exc}", - file=sys.stderr, - ) - return 3 + add_modifier(obj, args.apply_modifier, args.subsurf_levels) + try: + bake_modifier_stack(mesh_objects) + except RuntimeError as exc: + print( + f"ERROR: failed to apply {args.apply_modifier}: {exc}", + file=sys.stderr, + ) + return 3 + for obj in mesh_objects: + print( + f"Applied {args.apply_modifier} to {obj.name}: " + f"{len(obj.data.vertices)} verts, {len(obj.data.polygons)} faces" + ) try: bpy.ops.export_scene.gltf( diff --git a/tests/check_example_rules.py b/tests/check_example_rules.py index 108a9b3e..078b0df5 100644 --- a/tests/check_example_rules.py +++ b/tests/check_example_rules.py @@ -3,7 +3,8 @@ Agents copy these scripts (bmesh-gear is the anatomy every new example starts from), so a script that breaks a rule teaches the anti-pattern. Two -rules are checked statically over examples/ and showcase/: +rules are checked statically over examples/, showcase/ and templates/ +(templates also get prefer-data-over-ops-in-loops, see below): - use-foreach-set-for-bulk-data: a `for x in <...>.polygons` or `for x in <...>.vertices` loop that assigns to an attribute of `x`. Those two @@ -31,7 +32,79 @@ def scripts(root: Path) -> list[Path]: - return sorted([*root.glob("examples/*/*.py"), *root.glob("showcase/*/*.py")]) + return sorted([*root.glob("examples/*/*.py"), *root.glob("showcase/*/*.py"), + *template_scripts(root)]) + + +# --- templates/ scan and prefer-data-over-ops-in-loops (#468) --------------- +# +# Templates are copy-paste starters, so they get the two rules above plus +# prefer-data-over-ops-in-loops: a `for` loop whose body reaches a +# bpy.ops.object.* call, directly or through functions defined in the same +# file. The headless template applied a modifier per object that way. Examples +# are not held to it: some loop per part over operators with no data-API +# equivalent (uv.smart_project). Exempt a loop with a marker on the `for` line +# or the line above: +# +# # ops-loop-exempt: one export per LOD file; export has no data API + +OPS_MARKER = "# ops-loop-exempt:" +OBJECT_OPS = "bpy.ops.object." + + +def template_scripts(root: Path) -> list[Path]: + return sorted(root.glob("templates/*/*.py")) + + +def _object_ops_calls(node: ast.AST) -> list[str]: + return [ast.unparse(c.func) for c in ast.walk(node) + if isinstance(c, ast.Call) and ast.unparse(c.func).startswith(OBJECT_OPS)] + + +def _reaches_object_ops(tree: ast.Module) -> dict[str, str]: + """Same-file function name -> the bpy.ops.object call it reaches.""" + funcs = {f.name: f for f in ast.walk(tree) if isinstance(f, ast.FunctionDef)} + reach: dict[str, str] = {} + changed = True + while changed: + changed = False + for name, fn in funcs.items(): + if name in reach: + continue + direct = _object_ops_calls(fn) + hit = direct[0] if direct else next( + (reach[c.func.id] for c in ast.walk(fn) if isinstance(c, ast.Call) + and isinstance(c.func, ast.Name) and c.func.id in reach), None) + if hit: + reach[name] = hit + changed = True + return reach + + +def check_ops_in_loops(path: Path, root: Path) -> list[str]: + rel = path.relative_to(root).as_posix() + src = path.read_text(encoding="utf-8") + lines = src.split("\n") + tree = ast.parse(src) + reach = _reaches_object_ops(tree) + errors = [] + for loop in ast.walk(tree): + if not isinstance(loop, ast.For): + continue + here = lines[loop.lineno - 1] + above = lines[loop.lineno - 2] if loop.lineno >= 2 else "" + if OPS_MARKER in here or above.strip().startswith(OPS_MARKER): + continue + body = ast.Module(body=loop.body, type_ignores=[]) + hits = _object_ops_calls(body) + [ + f"{c.func.id}() -> {reach[c.func.id]}" for c in ast.walk(body) + if isinstance(c, ast.Call) and isinstance(c.func, ast.Name) and c.func.id in reach] + if hits: + errors.append(f"{rel}:{loop.lineno}: loop over {ast.unparse(loop.iter)} calls " + f"{hits[0]} per iteration; use bpy.data / bmesh or one " + f"operator call for the whole set (rule " + f"prefer-data-over-ops-in-loops) or mark '{OPS_MARKER} '") + return errors def writes_loop_var(loop: ast.For) -> bool: @@ -89,6 +162,8 @@ def check(root: Path = ROOT) -> list[str]: errors = [] for path in scripts(root): errors += check_file(path, root) + for path in template_scripts(root): + errors += check_ops_in_loops(path, root) return errors @@ -98,7 +173,8 @@ def main() -> int: print(f"::error::{e}", file=sys.stderr) if not errors: print(f"example rules: {len(scripts(ROOT))} scripts follow use-foreach-set and " - "active_object guards") + f"active_object guards; {len(template_scripts(ROOT))} templates have no " + "object operator in a loop") return 1 if errors else 0 diff --git a/tests/smoke/check_glb_tris.py b/tests/smoke/check_glb_tris.py new file mode 100644 index 00000000..056efdd6 --- /dev/null +++ b/tests/smoke/check_glb_tris.py @@ -0,0 +1,22 @@ +"""Assert a GLB's total triangle count (headless template modifier order, #468). + + blender --background --python check_glb_tris.py -- FILE.glb EXPECTED_TRIS + +Exit 3 when the re-imported triangle count differs. For stack.blend (a cube +with a live SUBSURF levels 1) run through script.py --apply-modifier +TRIANGULATE, the existing stack first then the new modifier gives 48 tris; +the reversed order that a non-first modifier_apply produces gives 72. +""" +import sys + +import bpy + +path, expected = sys.argv[sys.argv.index("--") + 1:][:2] +bpy.ops.wm.read_factory_settings(use_empty=True) +bpy.ops.import_scene.gltf(filepath=path.replace("\\", "/")) +tris = 0 +for obj in bpy.data.objects: + if obj.type == "MESH": + tris += len(obj.data.loop_triangles) +print(f"{path}: {tris} triangles (expected {expected})") +sys.exit(0 if tris == int(expected) else 3) diff --git a/tests/smoke/check_pipeline_glb.py b/tests/smoke/check_pipeline_glb.py new file mode 100644 index 00000000..c5d5f4cc --- /dev/null +++ b/tests/smoke/check_pipeline_glb.py @@ -0,0 +1,72 @@ +"""Assert the ai-asset-pipeline template kept every part of a GLB in place. + + blender --background --python check_pipeline_glb.py -- SOURCE.glb LOD0.glb + +Re-imports both files and compares them in world space (#462, #467): + + 3 LOD0 has a different number of mesh objects than one (parts not joined) + 4 LOD0's world bbox differs from the source's (a part was dropped or moved) + 5 LOD0's origin is not on its world minimum Z (grounded along a local axis) + 6 LOD0's node carries rotation or scale (transforms not applied) + +The fixture (make_pipeline_glb.py --parented) stays under every LOD budget, so +LOD0 is undecimated and its bbox must match the source exactly. +""" +import sys + +import bpy + +TOL = 1e-4 + + +def import_glb(path): + bpy.ops.wm.read_factory_settings(use_empty=True) + bpy.ops.import_scene.gltf(filepath=path.replace("\\", "/")) + bpy.context.view_layer.update() + return [o for o in bpy.data.objects if o.type == "MESH"] + + +def world_points(objs): + return [o.matrix_world @ v.co for o in objs for v in o.data.vertices] + + +def bbox(points): + return [min(p[a] for p in points) for a in range(3)] + [ + max(p[a] for p in points) for a in range(3) + ] + + +def main(): + src_path, lod_path = sys.argv[sys.argv.index("--") + 1:][:2] + src = import_glb(src_path) + src_box = bbox(world_points(src)) + print(f"source: {len(src)} mesh object(s), world bbox {[round(c, 4) for c in src_box]}") + + lod = import_glb(lod_path) + if len(lod) != 1: + print(f"FAIL: lod0 has {len(lod)} mesh objects, expected 1", file=sys.stderr) + return 3 + obj = lod[0] + points = world_points(lod) + lod_box = bbox(points) + print(f"lod0: {obj.name}, world bbox {[round(c, 4) for c in lod_box]}") + drift = max(abs(a - b) for a, b in zip(src_box, lod_box)) + if drift > TOL: + print(f"FAIL: lod0 world bbox differs from source by {drift:.4f}", file=sys.stderr) + return 4 + origin_z = obj.matrix_world.translation.z + print(f"lod0 origin z={origin_z:.4f}, world min z={lod_box[2]:.4f}") + if abs(origin_z - lod_box[2]) > TOL: + print("FAIL: lod0 origin is not on its world minimum Z", file=sys.stderr) + return 5 + m = obj.matrix_world.to_3x3() + off = max(abs(m[i][j] - (1.0 if i == j else 0.0)) for i in range(3) for j in range(3)) + if off > TOL: + print(f"FAIL: lod0 node has rotation/scale (max off-identity {off:.4f})", file=sys.stderr) + return 6 + print("OK: lod0 keeps every part, grounded, transforms applied") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/smoke/make_input.py b/tests/smoke/make_input.py index 1db1560c..48321790 100644 --- a/tests/smoke/make_input.py +++ b/tests/smoke/make_input.py @@ -23,3 +23,14 @@ bpy.ops.wm.read_factory_settings(use_empty=True) bpy.ops.wm.save_as_mainfile(filepath=empty) print(f"saved empty {empty}") +# a cube with a LIVE SUBSURF (levels 1) for the modifier-order path (#468): +# --apply-modifier TRIANGULATE must run after it (26 verts, 48 tris), not +# before it (38 verts, 36 quads -> 72 tris once glTF triangulates) +stack = out.replace("input.blend","stack.blend") +bpy.ops.wm.read_factory_settings(use_empty=True) +me = bpy.data.meshes.new("Cube"); bm = bmesh.new() +bmesh.ops.create_cube(bm, size=2.0); bm.to_mesh(me); bm.free() +o = bpy.data.objects.new("Cube", me); bpy.context.collection.objects.link(o) +o.modifiers.new("Subsurf", "SUBSURF").levels = 1 +bpy.ops.wm.save_as_mainfile(filepath=stack) +print(f"saved stack {stack}") diff --git a/tests/smoke/make_pipeline_glb.py b/tests/smoke/make_pipeline_glb.py index f5acd594..28b59ab3 100644 --- a/tests/smoke/make_pipeline_glb.py +++ b/tests/smoke/make_pipeline_glb.py @@ -1,21 +1,72 @@ +"""Write a fixture GLB for templates/ai-asset-pipeline-template/pipeline.py. + + blender --background --python make_pipeline_glb.py -- OUT.glb [--radius R] [--parented] + +Default: one UV sphere of radius R (1.0). --radius 1000 is the out-of-range +units fixture (pipeline exit 8). + +--parented: the shape glTF instancing and node trees import as (#462, #467). +A Root empty, rotated 90 deg about X and scaled 2x at z=3, parents a Body box +and two wheels, WheelL and WheelR, that share one mesh (users=2) and carry +different rotations. tests/smoke/check_pipeline_glb.py then asserts the +pipeline's lod0.glb keeps every part (world bbox == source) with its origin +on the world minimum Z. +""" +import argparse +import math +import sys + import bmesh import bpy -import sys -out = sys.argv[sys.argv.index("--") + 1 :][0] + +def mesh_from(name, build): + me = bpy.data.meshes.new(name) + bm = bmesh.new() + try: + build(bm) + bm.to_mesh(me) + finally: + bm.free() + return me + + +def link(name, data, parent=None): + obj = bpy.data.objects.new(name, data) + bpy.context.collection.objects.link(obj) + obj.parent = parent + return obj + + +args = argparse.ArgumentParser() +args.add_argument("out") +args.add_argument("--radius", type=float, default=1.0) +args.add_argument("--parented", action="store_true") +args = args.parse_args(sys.argv[sys.argv.index("--") + 1:]) + bpy.ops.wm.read_factory_settings(use_empty=True) -me = bpy.data.meshes.new("Fixture") -bm = bmesh.new() -try: - bmesh.ops.create_uvsphere(bm, u_segments=48, v_segments=24, radius=1.0) - bm.to_mesh(me) -finally: - bm.free() -obj = bpy.data.objects.new("Fixture", me) -bpy.context.collection.objects.link(obj) -obj.select_set(True) -bpy.context.view_layer.objects.active = obj -path = out.replace("\\", "/") +if args.parented: + root = link("Root", None) + root.location = (0.0, 0.0, 3.0) + root.rotation_euler = (math.radians(90.0), 0.0, 0.0) + root.scale = (2.0, 2.0, 2.0) + link("Body", mesh_from("BodyMesh", lambda bm: bmesh.ops.create_cube(bm, size=1.0)), root) + wheel = mesh_from("WheelMesh", lambda bm: bmesh.ops.create_cone( + bm, cap_ends=True, segments=12, radius1=0.3, radius2=0.3, depth=0.2)) + left = link("WheelL", wheel, root) + left.location = (0.9, 0.0, -0.3) + left.rotation_euler = (0.0, math.radians(90.0), 0.0) + right = link("WheelR", wheel, root) + right.location = (-0.9, 0.0, -0.3) + right.rotation_euler = (0.0, math.radians(-90.0), 0.4) + assert wheel.users == 2, wheel.users +else: + link("Fixture", mesh_from("Fixture", lambda bm: bmesh.ops.create_uvsphere( + bm, u_segments=48, v_segments=24, radius=args.radius))) + +for obj in bpy.data.objects: + obj.select_set(True) +path = args.out.replace("\\", "/") bpy.ops.export_scene.gltf( filepath=path, export_format="GLB",