Skip to content

Commit

Permalink
only normalize weights found in gltf file, don't normalize defaults
Browse files Browse the repository at this point in the history
  • Loading branch information
schell committed Aug 24, 2024
1 parent 51743cf commit 031ee81
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 36 deletions.
33 changes: 33 additions & 0 deletions DEVLOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,39 @@ Ok, I made a little progress there. I think what I'll do is checkout the last co
good, write a test to output some of the Fox's vertices, joint matrices and skinning matrices and then
use those as data in a unit test on my bugfix branch. I probably should have started here.

...

Turns out it doesn't compile at that commit, I know it did at the time, but it looks like one of the
dependencies is borked. Instead of spending a bunch of time debugging _that problem_, I'll just port
over some of the functions.

...

First I'm going to do a sanity check to ensure that `NestedTransform` is updating the entire hierarchy
correctly.

Yup, that's fine.

...

I found another glb model with distortion - CesiumMan.

I found another glb model with distortion - RobotExpressive. It's actually a really cute robot
with some great animations in it. Good find. Even better is that I found a three.js issue that
seems to detail [the same problem I'm
having](https://github.com/mrdoob/three.js/issues/15319) - I hope.

...

THAT WAS THE KEY.

The solution was to normalize the weights before collecting the vertices.

This fixes the Fox model, and the cute robot model, but the cute robot model still has an odd
artifact on its thumb during animation.

This does *not* fix CesiumMan, so I'll have to investigate that separately.

## Thu Aug 22, 2024

### Distorted Fox, continued
Expand Down
1 change: 1 addition & 0 deletions crates/renderling/src/slab/cpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ impl<Buffer: IsBuffer> SlabAllocator<Buffer> {
HybridArray::new(self, values)
}

/// Return the ids of all sources that require updating.
pub fn get_updated_source_ids(&self) -> FxHashSet<usize> {
// UNWRAP: panic on purpose
let mut update_set = self.update_queue.write().unwrap();
Expand Down
20 changes: 20 additions & 0 deletions crates/renderling/src/stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,27 @@ mod test {
} = c.get_global_transform();
let global_transform = (translation, rotation, scale);
let legacy_transform = legacy_get_world_transform(&c);
assert_eq!(legacy_transform, global_transform);

c.modify(|t| t.translation = Vec3::ONE);

let all_updates = slab.get_updated_source_ids();
assert_eq!(
std::collections::HashSet::from_iter([
a.get_notifier_index(),
b.get_notifier_index(),
c.get_notifier_index()
]),
all_updates
);

let Transform {
translation,
rotation,
scale,
} = c.get_global_transform();
let global_transform = (translation, rotation, scale);
let legacy_transform = legacy_get_world_transform(&c);
assert_eq!(legacy_transform, global_transform);
}
}
5 changes: 5 additions & 0 deletions crates/renderling/src/stage/cpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -945,6 +945,11 @@ impl NestedTransform {
nested
}

#[cfg(test)]
pub(crate) fn get_notifier_index(&self) -> usize {
self.notifier_index
}

fn mark_dirty(&self) {
// UNWRAP: safe because it's unbounded
self.notify.try_send(self.notifier_index).unwrap();
Expand Down
69 changes: 34 additions & 35 deletions crates/renderling/src/stage/gltf_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,11 +399,27 @@ impl GltfPrimitive {
}
log::debug!(" joints: {all_joints:?}");

const UNWEIGHTED_WEIGHTS: [f32; 4] = [1.0, 0.0, 0.0, 0.0];
let mut logged_weights_not_f32 = false;
let weights = reader
.read_weights(0)
.into_iter()
.flat_map(|ws| ws.into_f32())
.chain(std::iter::repeat([f32::MAX; 4]))
.flat_map(|ws| {
if !logged_weights_not_f32 {
match ws {
gltf::mesh::util::ReadWeights::U8(_) => log::warn!("weights are u8"),
gltf::mesh::util::ReadWeights::U16(_) => log::warn!("weights are u16"),
gltf::mesh::util::ReadWeights::F32(_) => {}
}
logged_weights_not_f32 = true;
}
ws.into_f32().map(|weights| {
// normalize the weights
let sum = weights[0] + weights[1] + weights[2] + weights[3];
weights.map(|w| w / sum)
})
})
.chain(std::iter::repeat(UNWEIGHTED_WEIGHTS))
.take(positions.len());
let vs = joints.into_iter().zip(weights);
let vs = colors.zip(vs);
Expand All @@ -412,22 +428,9 @@ impl GltfPrimitive {
let vs = uv1s.zip(vs);
let vs = uv0s.into_iter().zip(vs);
let vs = positions.into_iter().zip(vs);
let mut logged_not_normalized = false;
let mut unnormalized_weight_vertices_count = 0;
let vertices = vs
.map(
|(position, (uv0, (uv1, (normal, (tangent, (color, (joints, mut weights)))))))| {
let weight_sum: f32 = weights.iter().sum();
let are_normalized = 1.0 - weight_sum <= f32::EPSILON;
if !are_normalized {
unnormalized_weight_vertices_count += 1;
if !logged_not_normalized {
log::warn!("weights are not normalized: {weights:?}");
logged_not_normalized = true;
weights = weights.map(|w| w / weight_sum);
log::warn!(" normalized to {weights:?}");
}
}
|(position, (uv0, (uv1, (normal, (tangent, (color, (joints, weights)))))))| {
Vertex {
position,
color,
Expand All @@ -443,16 +446,12 @@ impl GltfPrimitive {
.collect::<Vec<_>>();
let vertices = stage.new_array(vertices);
log::debug!("{} vertices, {:?}", vertices.len(), vertices.array());
if logged_not_normalized {
log::debug!(
" {unnormalized_weight_vertices_count} of which had unnormalized joint weights"
);
}
let indices = stage.new_array(indices);
log::debug!("{} indices, {:?}", indices.len(), indices.array());
let gltf::mesh::Bounds { min, max } = primitive.bounding_box();
let min = Vec3::from_array(min);
let max = Vec3::from_array(max);

Self {
vertices,
indices,
Expand Down Expand Up @@ -1338,20 +1337,6 @@ mod test {
},
);

let slab = futures_lite::future::block_on(stage.read(
ctx.get_device(),
ctx.get_queue(),
Some("stage slab"),
..,
))
.unwrap();

assert_eq!(1, doc.skins.len());
let skin = doc.skins[0].skin.get();
for joint_index in 0..skin.joints.len() {
skin.get_joint_matrix(, , )
}

// let mut animator = doc
// .animations
// .get(0)
Expand All @@ -1370,5 +1355,19 @@ mod test {
// ..Default::default()
// },
// );

// let slab = futures_lite::future::block_on(stage.read(
// ctx.get_device(),
// ctx.get_queue(),
// Some("stage slab"),
// ..,
// ))
// .unwrap();

// assert_eq!(1, doc.skins.len());
// let skin = doc.skins[0].skin.get();
// for joint_index in 0..skin.joints.len() {
// // skin.get_joint_matrix(, , )
// }
}
}
8 changes: 7 additions & 1 deletion crates/renderling/src/stage/gltf_support/anime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,7 @@ pub struct Animator {
pub nodes: rustc_hash::FxHashMap<usize, NestedTransform>,
/// The animation that will apply to the nodes.
pub animation: Animation,
erred_unsupported_morph_targets: bool,
}

impl Animator {
Expand Down Expand Up @@ -732,7 +733,12 @@ impl Animator {
t.scale = scale;
});
}
TweenProperty::MorphTargetWeights(_) => todo!(),
TweenProperty::MorphTargetWeights(_) => {
if !self.erred_unsupported_morph_targets {
log::error!("model has morph targets, which are currently unsupported");
self.erred_unsupported_morph_targets = true;
}
}
}
} else {
log::warn!("node {node_index} isn't in the animator's list of nodes");
Expand Down

0 comments on commit 031ee81

Please sign in to comment.