Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

normalize joint weights #10539

Merged
merged 3 commits into from
Jan 27, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 23 additions & 12 deletions crates/bevy_render/src/mesh/mesh/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ impl Mesh {
attribute: MeshVertexAttribute,
values: impl Into<VertexAttributeValues>,
) {
let mut values = values.into();
let values = values.into();
let values_format = VertexFormat::from(&values);
if values_format != attribute.format {
panic!(
Expand All @@ -215,17 +215,6 @@ impl Mesh {
);
}

// validate attributes
if attribute.id == Self::ATTRIBUTE_JOINT_WEIGHT.id {
let VertexAttributeValues::Float32x4(ref mut values) = values else {
unreachable!() // we confirmed the format above
};
for value in values.iter_mut().filter(|v| *v == &[0.0, 0.0, 0.0, 0.0]) {
// zero weights are invalid
value[0] = 1.0;
}
}

self.attributes
.insert(attribute.id, MeshAttributeData { attribute, values });
}
Expand Down Expand Up @@ -621,6 +610,28 @@ impl Mesh {
pub fn morph_target_names(&self) -> Option<&[String]> {
self.morph_target_names.as_deref()
}

/// Normalize joint weights so they sum to 1.
pub fn normalize_joint_weights(&mut self) {
if let Some(joints) = self.attribute_mut(Self::ATTRIBUTE_JOINT_WEIGHT) {
let VertexAttributeValues::Float32x4(ref mut joints) = joints else {
panic!("unexpected joint weight format");
};

for weights in joints.iter_mut() {
let sum: f32 = weights.iter().sum();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be worth verifying the weights are positive first? If someone accidentally uses with_inserted_attribute with a mixture of positive and negative floats, this will do weird things.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

negative weights are technically not allowed in the spec, whereas not summing to precisely 1 is allowed due to rounding issues.

but sure, no reason not to. i arbitrarily chose to clamp negatives to zero, i guess other treatments might be better (like if all are negative, maybe you should just flip the sign?) but since we're in invalid-data territory there's probably no perfect answer. if someone wants a different treatment they can always write their own normalize function.

if sum == 0.0 {
// zero weights are invalid
weights[0] = 1.0;
} else {
let recip = sum.recip();
for weight in weights.iter_mut() {
*weight *= recip;
}
}
}
}
}
}

#[derive(Debug, Clone)]
Expand Down