Skip to content

Commit

Permalink
lots of bug fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
schell committed May 12, 2024
1 parent 29e1e8e commit a002c7a
Show file tree
Hide file tree
Showing 11 changed files with 257 additions and 239 deletions.
13 changes: 7 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ crabslab = { path = "../crabslab/crates/crabslab", version = "0.4.6", default-fe
dagga = "0.2.1"
env_logger = "0.10.0"
futures-lite = "1.13"
gltf = { git = 'https://github.com/schell/gltf.git', branch ="feature/channel-sampler-index", features = ["KHR_lights_punctual", "KHR_materials_unlit", "KHR_materials_emissive_strength", "extras"] }
gltf = { git = 'https://github.com/schell/gltf.git', branch ="feature/channel-sampler-index", features = ["KHR_lights_punctual", "KHR_materials_unlit", "KHR_materials_emissive_strength", "extras", "extensions"] }
image = "0.24"
log = "0.4"
naga = { version = "0.19", features = ["spv-in", "wgsl-out", "wgsl-in", "msl-out"] }
pretty_assertions = "1.4.0"
proc-macro2 = { version = "1.0", features = ["span-locations"] }
glam = { version = "0.24.2", default-features = false }
serde_json = "1.0.117"
snafu = "0.7"
syn = { version = "2.0.49", features = ["full", "extra-traits", "parsing"] }
wasm-bindgen = "0.2"
Expand Down
50 changes: 50 additions & 0 deletions DEVLOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,55 @@
# devlog

## Sun May 12, 2024

### More Fox Skinning

I've noticed that some GLTF models (like `CesiumMan`) cause the uber-shader to barf.
I haven't figured out what feature in those models is causing it yet. It may or may not
be related to the fox skinning problem.

### Box!

A very simple GLTF file fails to render. It's the `Box.glb` model.

![box should look like this](https://github.com/KhronosGroup/glTF-Sample-Models/raw/main/2.0/Box/screenshot/screenshot.png)

Ooh, now upon visiting the [Khronos sample models repo](https://github.com/KhronosGroup/glTF-Sample-Models?tab=readme-ov-file)
I find that it (the repo) has been deprecated in favor of [another](https://github.com/KhronosGroup/glTF-Sample-Assets).

Anyway - this is a fundamentally simple GLTF model so something must have regressed in `renderling`...

#### Investigation

* Turns out there are buffer writes happening each frame, which is weird because the `Box.glb` model doesn't
include animation.
* When I trace it out it looks like the camera's view has NaN values.
* Looks like after adding a `debug_assert!` I can see that the camera's calculated radius (the
distance at which the camera rotates around the model) is `inf`...
* That's because after loading, the model's bounding box is `[inf, inf, inf] [-inf, -inf, -inf]`...
* And calculation of the bounding box only takes into consideration the nodes in the scene and
doesn't include those node's children...

After updating the bounding box calculation to take child nodes into consideration the problem
is fixed.

<img width="450" alt="Screenshot 2024-05-12 at 10 52 21 AM" src="https://github.com/schell/renderling/assets/24942/9d3618d3-60bb-47c4-9a37-4b7a71952277">

* But there are still two `Transform` writes per frame when there should be none.
* I can't see any other place in the example app where those transforms are being updated.
* I recently redid the way `NestedTransform` do their updates, so I'll look there.
* There's nothing modifying those transforms...
* Ah, but each update source is being polled for updates each frame, and NestedTransforms
always give their global transform as an update regardless if it changed.
* I'm changing the update sources to be a set, and the `SlabAllocator` only checks those sources
that have sent in an "update" signal on its notification channel. This also means we only check
sources for strong counts when this "update" signal comes in, so those sources need to send the
signal on Drop. All in all though this should be a nice optimization.
* ...but alas, after the update I get the grey screen of death again, which means something's not
right...
* Turns out it was because `Gpu::new` was calling `SlabAllocator::next_update_k` twice, using one
for its `notifier_index` and then using the other for the first notification.

## Sat May 11, 2024

### Skinning a Fox
Expand Down
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ Shaders are written in Rust, via `rust-gpu`.
* Tight integration with GLTF:
- Loading scenes, nodes, animations etc
- Includes tools for controlling animations

- Supported extensions:
* KHR_lights_punctual
* KHR_materials_unlit
* KHR_materials_emissive_strength
* Image based lighting + analytical lighting

* Good documentation
Expand Down
62 changes: 34 additions & 28 deletions crates/example/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub struct App {
animations_conflict: bool,

// look at
eye: Vec3,
center: Vec3,
// distance from the origin
radius: f32,
// anglular position on a circle `radius` away from the origin on x,z
Expand Down Expand Up @@ -112,7 +112,7 @@ impl App {
loads: Arc::new(Mutex::new(HashMap::default())),
last_frame_instant: now(),
radius,
eye: Vec3::ZERO,
center: Vec3::ZERO,
phi,
theta,
left_mb_down,
Expand All @@ -131,13 +131,19 @@ impl App {

pub fn update_camera_view(&self) {
let UVec2 { x: w, y: h } = self.stage.get_size();
let camera_position = Self::camera_position(self.radius, self.phi, self.theta);
let camera = Camera::new(
Mat4::perspective_infinite_rh(std::f32::consts::FRAC_PI_4, w as f32 / h as f32, 0.01),
Mat4::look_at_rh(
Self::camera_position(self.radius, self.phi, self.theta),
self.eye,
Vec3::Y,
),
Mat4::look_at_rh(camera_position, self.center, Vec3::Y),
);
debug_assert!(
camera.view.is_finite(),
"camera view is borked w:{w} h:{h} camera_position: {camera_position} center: {} \
radius: {} phi: {} theta: {}",
self.center,
self.radius,
self.phi,
self.theta
);
if self.camera.get() != camera {
self.camera.set(camera);
Expand Down Expand Up @@ -182,18 +188,17 @@ impl App {

let scene = doc.default_scene.unwrap_or(0);
log::info!("Displaying scene {scene}");
let nodes = doc
.scenes
.get(scene)
.map(Vec::clone)
.unwrap_or_else(|| (0..doc.nodes.len()).collect());
//log::trace!(" nodes:");
for node_index in nodes.iter() {
// UNWRAP: safe because we know the node exists
let node = doc.nodes.get(*node_index).unwrap();
let nodes = doc.nodes_in_scene(scene).flat_map(|n| {
n.children
.iter()
.filter_map(|i| doc.nodes.get(*i))
.chain(std::iter::once(n))
});
log::trace!(" nodes:");
for node in nodes {
let tfrm = Mat4::from(node.global_transform());
let decomposed = Transform::from(tfrm);
//log::trace!(" {} {:?} {decomposed:?}", node.index, node.name);
log::trace!(" {} {:?} {decomposed:?}", node.index, node.name);
if let Some(mesh_index) = node.mesh {
// UNWRAP: safe because we know the node exists
for primitive in doc.meshes.get(mesh_index).unwrap().primitives.iter() {
Expand All @@ -205,6 +210,17 @@ impl App {
}
}

log::trace!("Bounding box: {min} {max}");
let halfway_point = min + ((max - min).normalize() * ((max - min).length() / 2.0));
let length = min.distance(max);
let radius = length * 1.25;

self.radius = radius;
self.center = halfway_point;
self.last_frame_instant = now();

self.update_camera_view();

if doc.animations.is_empty() {
log::trace!(" animations: none");
} else {
Expand All @@ -222,7 +238,7 @@ impl App {
has_conflicting_animations || !animated_nodes.is_disjoint(&target_nodes);
animated_nodes.extend(target_nodes);

//log::trace!(" {i} {:?} {}s", a.name, a.length_in_seconds());
log::trace!(" {i} {:?} {}s", a.name, a.length_in_seconds());
// for (t, tween) in a.tweens.iter().enumerate() {
// log::trace!(
// " tween {t} targets node {} {}",
Expand All @@ -239,16 +255,6 @@ impl App {
}
self.animations_conflict = has_conflicting_animations;
self.document = Some(doc);

let halfway_point = min + ((max - min).normalize() * ((max - min).length() / 2.0));
let length = min.distance(max);
let radius = length * 1.25;

self.radius = radius;
self.eye = halfway_point;
self.last_frame_instant = now();

self.update_camera_view();
}

pub fn tick_loads(&mut self) {
Expand Down
1 change: 1 addition & 0 deletions crates/example/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ fn main() {
env_logger::Builder::default()
.filter_module("example", log::LevelFilter::Trace)
.filter_module("renderling", log::LevelFilter::Debug)
.filter_module("renderling::stage::cpu", log::LevelFilter::Debug)
.filter_module("renderling::slab", log::LevelFilter::Debug)
.filter_module(
"renderling::stage::gltf_support::anime",
Expand Down
3 changes: 2 additions & 1 deletion crates/renderling/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ shaders = [
"tonemapping_fragment",
"tonemapping_vertex"
]
gltf = ["dep:gltf"]
gltf = ["dep:gltf", "dep:serde_json"]
sdf = []
tutorial = [
"tutorial_passthru_fragment",
Expand Down Expand Up @@ -110,6 +110,7 @@ image = {workspace = true, features = ["hdr"]}
log = {workspace = true}
rustc-hash = "1.1"
send_wrapper = "0.6"
serde_json = {workspace = true, optional = true}
snafu = {workspace = true}
wgpu = { workspace = true, features = ["spirv"] }
winit = { workspace = true, optional = true }
Expand Down
2 changes: 1 addition & 1 deletion crates/renderling/src/pbr/light.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Stage lighting.
use crabslab::{Id, SlabItem};
use glam::{Mat4, Vec3, Vec4};
use glam::{Vec3, Vec4};

use crate::transform::Transform;

Expand Down
Loading

0 comments on commit a002c7a

Please sign in to comment.