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",