bwasty / gltf-viewer

glTF 2.0 Viewer written in Rust
The Unlicense
188 stars 31 forks source link

Adding camera bounds cause incorrect bounding box estimation #47

Closed ox closed 6 years ago

ox commented 6 years ago

https://github.com/bwasty/gltf-viewer/blob/847bf5f05af17a4937fff10195fc9c00e58299f5/src/render/node.rs#L109-L113

I stumbled onto this TODO after trying to take a screenshot of a model with a camera, but using bounding box estimation. It seems that the camera bounds are being taken into account when trying to set a nice camera position, causing the bounding box to be incorrectly estimated.

Here is the screenshot produced by gltf-viewer: heart-render-1080

And here is the scene view in Houdini, notice the small camera dot in the bottom left: screenshot from 2018-08-10 13-40-41

I'm attempting to get the camera node's bounds to not count in the scene bounds, but my Rust knowledge is very limited. I hacked together this diff that seems to resolve the problem:

diff --git a/src/render/node.rs b/src/render/node.rs
index a1dc13c..80a487f 100644
--- a/src/render/node.rs
+++ b/src/render/node.rs
@@ -106,6 +106,10 @@ impl Node {
             self.bounds = mesh.bounds
                 .transform(&self.final_transform);
         }
+        else if let Some(ref _camera) = self.camera {
+            // Skip updating bounds for Cameras, since they don't have bounds.
+        }
         else if self.children.is_empty() {
             // Cameras (others?) have neither mesh nor children. Their position is the origin
             // TODO!: are there other cases? Do bounds matter for cameras?

Is this the correct solution? Do you have any other ideas about how to fix this?

bwasty commented 6 years ago

Looks good - a slight simplification would be else if self.camera.is_some().

ox commented 6 years ago

awesome. should I make a PR or...?

bwasty commented 6 years ago

Yes, that would be great :)

ox commented 6 years ago

when could I expect this change to make it onto crates.io? I'd love to be able to just cargo install with a version rather than with a git url and sha.

bwasty commented 6 years ago

Here it is: https://crates.io/crates/gltf-viewer :)

ox commented 6 years ago

wow, thank you for the quick reply and publish! I really appreciate it!