physics: retire avian's collider mass-property pass, 181 ms off a world load
`WorldPlugins` has disabled `MassPropertyPlugin` for as long as the world has had colliders, with the reason written beside it: there are no dynamic bodies, so no mass is ever read. The pass kept running anyway. `ColliderBackendPlugin`, which we keep for `update_collider_scale`, registers `update_collider_mass_properties` itself (`collision/collider/backend.rs:232`), and that system's query filters `Without<Sensor>` and nothing about the body type — so every collider that *changed* paid `Collider::mass_properties`, a full parry volume integral over the trimesh, for a value with no reader. Terrain streams its colliders as trimeshes by the tile, so the cost lands in a world load. Measured on a cold Elwynn vista with `WOW_SYSTEM_TIMING=1`, before and after, same viewpoint: before run_fixed_main_schedule worst 183.0 ms 200.9 ms/s after run_fixed_main_schedule worst 1.3 ms 25.9 ms/s `update_collider_mass_properties` no longer appears in the report at all, and its 181.3 ms worst run was the whole of that schedule's spike — the nesting is what identified it. A run condition rather than another `.disable()`, because the system cannot be removed individually and shares a `.chain()` with `update_collider_scale`. `MassPropertySystems` is `pub` but not in avian's prelude, hence the full path. The invariant the whole physics trim rests on now has a wall of its own: `tests/no_dynamic_bodies_wall.rs` fails if `RigidBody::Dynamic` appears in the workspace. This bug was exactly an invariant that lived only in prose — the `.disable()` comment stated it correctly while a system slipped through the gap between two plugins — so the assert is the point. Verified by planting a dynamic body and watching it fail, then reverting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
Claude Opus 5
@@ -154,6 +154,26 @@ impl Plugin for WorldFoundation {
|
||||
.init_resource::<crate::collision::ColliderEpoch>()
|
||||
.init_resource::<crate::collision::MoverTraceExclusions>()
|
||||
.add_systems(First, crate::collision::track_collider_removals);
|
||||
// Disabling `MassPropertyPlugin` above does **not** retire every mass-property system:
|
||||
// `ColliderBackendPlugin`, which we keep for `update_collider_scale`, registers
|
||||
// `update_collider_mass_properties` itself (`collision/collider/backend.rs:232`) into this
|
||||
// set. Its query filters `Without<Sensor>` and nothing else — not the body type — so every
|
||||
// collider that *changes* pays `Collider::mass_properties`, a full parry volume integral
|
||||
// over the trimesh, for a mass no dynamic body will ever read. Measured on a cold Elwynn
|
||||
// vista with `WOW_SYSTEM_TIMING=1`: **one run of 181 ms**, which is also the whole of that
|
||||
// second's `run_fixed_main_schedule` worst (183 ms). Terrain streams trimesh colliders by
|
||||
// the tile, so the spike lands squarely in the world-load hitches.
|
||||
//
|
||||
// Retiring the set is safe for the same reason `MassPropertyPlugin` is off: the workspace
|
||||
// has 23 `RigidBody::Static`, 4 `Kinematic` and **0 `Dynamic`**, and reads no mass
|
||||
// component anywhere (`ColliderMassProperties`, `ComputedMass`, `ColliderDensity`,
|
||||
// `CenterOfMass`, `AngularInertia`: no consumers). Re-enable with the plugin when a dynamic
|
||||
// body enters the world — the comment on `.disable::<MassPropertyPlugin>()` says the same.
|
||||
app.configure_sets(
|
||||
FixedPostUpdate,
|
||||
avian3d::dynamics::rigid_body::mass_properties::MassPropertySystems::UpdateColliderMassProperties
|
||||
.run_if(|| false),
|
||||
);
|
||||
// No pre-step transform propagation: movers write `Position`/`Rotation` directly, so a
|
||||
// second whole-world sweep buys nothing; an opening door's hull reaches physics a frame
|
||||
// late. `WOW_PHYS_PREPROP=1` restores avian's default.
|
||||
|
||||
@@ -0,0 +1,124 @@
|
||||
//! The dynamic-body wall: the world has **no `RigidBody::Dynamic`**, and most of the physics
|
||||
//! trimming in `world_plugins.rs` is correct only while that holds.
|
||||
//!
|
||||
//! `WorldPlugins` disables `MassPropertyPlugin`, `ForcePlugin`, `IntegratorPlugin`, `SolverPlugin`,
|
||||
//! `XpbdSolverPlugin`, `SolverBodyPlugin`, `CcdPlugin`, `IslandPlugin`, `JointPlugin` and
|
||||
//! `PhysicsInterpolationPlugin`, runs `SubstepCount(1)` instead of avian's 6, and retires
|
||||
//! `MassPropertySystems::UpdateColliderMassProperties`. Every one of those is justified by the same
|
||||
//! sentence — *"the world has no dynamic bodies"* — and each would fail **silently**, as wrong
|
||||
//! motion rather than a compile error, the day one appears.
|
||||
//!
|
||||
//! **Why this is a test and not a comment.** It already went wrong once in the other direction:
|
||||
//! `.disable::<MassPropertyPlugin>()` was in place with exactly the right reasoning written beside
|
||||
//! it, while `update_collider_mass_properties` kept running, because `ColliderBackendPlugin` — kept
|
||||
//! for `update_collider_scale` — registers that system itself. Nothing caught it until a
|
||||
//! `WOW_SYSTEM_TIMING=1` run showed one run of **181 ms** integrating trimesh volumes for a mass no
|
||||
//! body reads. An invariant asserted only in prose is an invariant nothing checks.
|
||||
//!
|
||||
//! If you are here because this test failed: you added a dynamic body, which is a legitimate thing
|
||||
//! to do. Re-enable the plugins that body needs, drop the run condition on
|
||||
//! `MassPropertySystems::UpdateColliderMassProperties`, reconsider `SubstepCount`, and add the
|
||||
//! kind to [`ALLOWED`] with the reason.
|
||||
//!
|
||||
//! Scope: this guards the *body kind* only. That no code reads a mass property today was checked by
|
||||
//! hand when the run condition landed (`ColliderMassProperties`, `ComputedMass`, `CenterOfMass`,
|
||||
//! `AngularInertia`: no readers anywhere); it is not asserted here, because a grep for those names
|
||||
//! fires on prose and on unrelated geometry code, and a brittle wall is worse than none.
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
/// This test's own file name, the one file excluded from the scan.
|
||||
const SELF: &str = "no_dynamic_bodies_wall.rs";
|
||||
|
||||
/// Body kinds the world may name, and why. `Dynamic` is deliberately absent.
|
||||
const ALLOWED: &[(&str, &str)] = &[
|
||||
(
|
||||
"Static",
|
||||
"terrain, WMOs, doodads and one-sided collision: never integrated",
|
||||
),
|
||||
(
|
||||
"Kinematic",
|
||||
"transports and attached hulls: they write Position/Rotation directly",
|
||||
),
|
||||
];
|
||||
|
||||
#[test]
|
||||
fn the_world_names_no_dynamic_rigid_body() {
|
||||
let root = workspace_root();
|
||||
let mut offenders: Vec<String> = Vec::new();
|
||||
for path in workspace_sources(&root) {
|
||||
// This file names the forbidden kind itself — in the header, in the scan below and in the
|
||||
// failure message — so it is the one file the wall cannot read.
|
||||
if path.file_name().is_some_and(|f| f == SELF) {
|
||||
continue;
|
||||
}
|
||||
let Ok(text) = std::fs::read_to_string(&path) else {
|
||||
continue;
|
||||
};
|
||||
for (n, line) in text.lines().enumerate() {
|
||||
// Prose may discuss the kind — the `.disable()` comments in `world_plugins.rs` exist to
|
||||
// explain exactly why there is none — and commented-out code is not active. Only code
|
||||
// counts.
|
||||
if line.trim_start().starts_with("//") {
|
||||
continue;
|
||||
}
|
||||
if line.contains("RigidBody::Dynamic") {
|
||||
let rel = path.strip_prefix(&root).unwrap_or(&path);
|
||||
offenders.push(format!("{}:{}", rel.display(), n + 1));
|
||||
}
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
offenders.is_empty(),
|
||||
"RigidBody::Dynamic appears in the workspace, but `WorldPlugins` is trimmed for a world \
|
||||
without one (no mass properties, no solver, no integrator, no CCD, one substep). Read \
|
||||
this file's header before deleting the assert.\nFound at:\n {}\nAllowed kinds: {}",
|
||||
offenders.join("\n "),
|
||||
ALLOWED
|
||||
.iter()
|
||||
.map(|(k, why)| format!("{k} ({why})"))
|
||||
.collect::<Vec<_>>()
|
||||
.join(", "),
|
||||
);
|
||||
}
|
||||
|
||||
fn workspace_root() -> PathBuf {
|
||||
PathBuf::from(env!("CARGO_MANIFEST_DIR"))
|
||||
.parent()
|
||||
.and_then(Path::parent)
|
||||
.expect("crates/benilla-world → the workspace root")
|
||||
.to_path_buf()
|
||||
}
|
||||
|
||||
/// Every `.rs` under each crate's `src/`, `examples/` and `tests/`.
|
||||
fn workspace_sources(root: &Path) -> Vec<PathBuf> {
|
||||
let mut out = Vec::new();
|
||||
for crate_dir in std::fs::read_dir(root.join("crates"))
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.flatten()
|
||||
{
|
||||
for sub in ["src", "examples", "tests"] {
|
||||
let dir = crate_dir.path().join(sub);
|
||||
if dir.is_dir() {
|
||||
collect_rs(&dir, &mut out);
|
||||
}
|
||||
}
|
||||
}
|
||||
out.sort();
|
||||
out
|
||||
}
|
||||
|
||||
fn collect_rs(dir: &Path, out: &mut Vec<PathBuf>) {
|
||||
let mut stack = vec![dir.to_path_buf()];
|
||||
while let Some(d) = stack.pop() {
|
||||
for e in std::fs::read_dir(&d).into_iter().flatten().flatten() {
|
||||
let p = e.path();
|
||||
if p.is_dir() {
|
||||
stack.push(p);
|
||||
} else if p.extension().is_some_and(|x| x == "rs") {
|
||||
out.push(p);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user