glTF skinning and morphing animations #184

kvark · 2018-02-22T16:25:10Z

This is a revival, rebase, and a substantial rewrite of #151

cc @alteous - if you want to check out how the bones are traversed without caching the world transforms

Update Cargo.toml

Remove support for CatmullRomSpline interpolation

Update shader comment

alteous · 2018-02-22T18:46:38Z

data/shaders/pbr_vs.glsl

@@ -1,59 +1,101 @@
-


I'm not sure who removed this but beware of possible copyright violation.

alteous · 2018-02-22T18:48:59Z

src/render/mod.rs

+            }
+
+            let mut skeletons = Vec::new();
+            for w in hub.walk(&scene.first_child) {


alteous · 2018-02-22T18:49:25Z

src/render/mod.rs

@@ -729,7 +868,7 @@ impl Renderer {

            for w in hub.walk(&scene.first_child) {


alteous · 2018-02-22T18:49:47Z

src/render/mod.rs

@@ -789,39 +928,37 @@ impl Renderer {
        }

        for w in hub.walk(&scene.first_child) {


alteous

It seems you are traversing the scene graph (hub.walk) three times every frame. IIRC, you were pretty adamant about not traversing it more than once in the original PR.

kvark · 2018-02-22T19:02:43Z

@alteous thanks for taking a look!

I'm not sure who removed this but beware of possible copyright violation.

does the shader code match anything licensed? I thought you wrote it, and then I changed it more. If there are still lines under license, I'd like to rewrite them anyway.

It seems you are traversing the scene graph (hub.walk) three times every frame. IIRC, you were pretty adamant about not traversing it more than once in the original PR.

You might have misunderstood me then. I said previously that ideally in some not far future we'd want to only traverse the graph once and collect all the render data into appropriate buckets for GPU. That doesn't in any way force this change to go crazy out of scope, especially since there were already multiple passes over the graph before it.

And that ideal scenario was brought up as an argument to not cache those world transforms (or anything, really, we are almost functional!). As you can see, bone transforms are fitting the scheme just fine.

alteous · 2018-02-22T21:33:56Z

The majority of the shader code was taken from https://github.com/KhronosGroup/glTF-WebGL-PBR/tree/master/shaders. I adapted the code for use with gfx changed its style to reflect ours.

kvark · 2018-02-23T02:55:42Z

@alteous yeah, you did good. I double checked the shaders and see no concerns about them.
Also, added a commit that bumps the version.

vitvakatu

Brief review

vitvakatu · 2018-02-23T07:54:30Z

.travis.yml

@@ -23,7 +23,8 @@ notifications:
    on_start: never

 script:
-  - cargo build
+  - cargo check


Do we really need check? I think cargo test is enough.

vitvakatu · 2018-02-23T08:01:55Z

examples/aviator/main.rs

@@ -19,7 +19,7 @@ const COLOR_BROWN_DARK: three::Color = 0x23190f;
 const COLOR_BLUE: three::Color = 0x68c3c0;

 fn main() {
-    env_logger::init().unwrap();
+    env_logger::init();


What about enabling logging for every example?

would be nice, but requires some sort of example boilerplate, and creeps out of scope for this PR

vitvakatu · 2018-02-23T08:07:14Z

src/animation.rs

@@ -484,12 +489,14 @@ impl ActionData {
                        ))
                    };
                    let update = frame_start_value.slerp(frame_end_value, s);
+                    use Object;


Let's import it globally.

kvark

@vitvakatu comments addressed

kvark · 2018-02-23T14:36:51Z

examples/aviator/main.rs

@@ -19,7 +19,7 @@ const COLOR_BROWN_DARK: three::Color = 0x23190f;
 const COLOR_BLUE: three::Color = 0x68c3c0;

 fn main() {
-    env_logger::init().unwrap();
+    env_logger::init();


would be nice, but requires some sort of example boilerplate, and creeps out of scope for this PR

kvark · 2018-02-23T14:37:05Z

src/animation.rs

@@ -484,12 +489,14 @@ impl ActionData {
                        ))
                    };
                    let update = frame_start_value.slerp(frame_end_value, s);
+                    use Object;


kvark · 2018-02-23T14:37:15Z

.travis.yml

@@ -23,7 +23,8 @@ notifications:
    on_start: never

 script:
-  - cargo build
+  - cargo check


vitvakatu

@kvark looks good to me

alteous · 2018-02-24T11:17:42Z

src/material.rs

@@ -19,7 +19,7 @@ pub mod basic {
    /// Renders triangle meshes with a solid color or texture.
    #[derive(Clone, Hash, Debug, PartialEq, Eq)]
    pub struct Basic {
-        /// Solid color applied in the absense of `map`.
+        /// Solid color applied in the absence of `map`.


kvark · 2018-02-24T12:12:18Z

Thanks! Bors r=vitvakatu

…

On Feb 24, 2018, at 06:17, David Harvey-Macaulay ***@***.***> wrote: @alteous commented on this pull request. In src/material.rs: > @@ -19,7 +19,7 @@ pub mod basic { /// Renders triangle meshes with a solid color or texture. #[derive(Clone, Hash, Debug, PartialEq, Eq)] pub struct Basic { - /// Solid color applied in the absense of `map`. + /// Solid color applied in the absence of `map`. TIL. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

@alteous

184: glTF skinning and morphing animations r=vitvakatu a=kvark This is a revival, rebase, and a substantial rewrite of #151 cc @alteous - if you want to check out how the bones are traversed without caching the world transforms ![three-rs-skinning](https://user-images.githubusercontent.com/107301/36550245-d878073e-17c2-11e8-87ca-68299dfff775.png)

bors · 2018-02-24T12:33:39Z

Build succeeded

continuous-integration/travis-ci/push

alteous and others added 18 commits February 22, 2018 09:30

Add Bone and Skeleton structures

77803ef

Add optional skeletal data to SubNode::Visual

93de8b7

Add joint matrices to shaders

3096c03

Add joint matrix calculation

1e80c4b

Complete mesh skinning implementation

464d33f

Add pipework for morph target implementation

78e21d3

Update Cargo.toml

Use samplerBuffer for joint matrices

a888735

Fix use of deprecated methods

fa807b8

Remove support for CatmullRomSpline interpolation

Add uniform buffer for morph target displacements

30ebb2d

Split joints and morph targets from base geometry

efa93da

Add displacements

92daec5

Update shader comment

Implement morth targets

e14cdeb

PBR shader refactor

1978cc5

Bring back geometry::Shape

faded39

Actually fix morphing

1aa84d8

Make vertex skinning work

ccd40bd

Use 3 vectors per bone

65f339c

Post-rebase fallout fixes

d5e0232

kvark requested a review from vitvakatu February 22, 2018 16:25