feat: Vertex skinning #545

Rhuagh · Jan 17, 2018

This change is

@Rhuagh maybe you want to try to make it with new renderer instead?
It's almost ready.

omni-viral · Jan 18, 2018

@Rhuagh maybe you want to try to make it with new renderer instead?
It's almost ready.

Tbh, most of the code is independent of the renderer, it's just a vertex shader, a single component, and some wiring. I will fix that when the new renderer lands.

Rhuagh · Jan 18, 2018

Tbh, most of the code is independent of the renderer, it's just a vertex shader, a single component, and some wiring. I will fix that when the new renderer lands.

I will squash after reviews are done.

Rhuagh · Jan 19, 2018

I will squash after reviews are done.

Xaeroxe · Jan 20, 2018

Xaeroxe requested changes Jan 20, 2018

View changes

This looks great! Thank you! I have a couple nits, and some questions about the gltf example.

What's the licensing on the monster + animation? Who made them? What are our legal obligations in using it?
When I run the example and press space bar the monster jumps to a new position before animating, and facing the wrong direction. What do we need to do to make him animate in place?

Xaeroxe · Jan 20, 2018

amethyst_gltf/src/format/mod.rs

+    })
+}
+
+fn _flip_check(uv: [f32; 2], flip_v: bool) -> [f32; 2] {


method isn't public so the leading underscore naming scheme is redundant and inconsistent with the rest of the code.

Yeah, sorry about that, I'm currently working mostly with python at work, and there you need to put _ at the start of functions for them to be private.. I forgot that here I was working with a real language.

Xaeroxe · Jan 20, 2018

amethyst_input/src/button.rs

@@ -8,14 +8,18 @@ use super::local_virtual_key_code::LocalVirtualKeyCode;
 pub enum Button {
     /// Virtual Keyboard keys, use this when the letter on the key matters
     /// more than the position of the key.
-    Key(#[serde(with = "LocalVirtualKeyCode")] VirtualKeyCode),
+    Key(#[serde(with = "LocalVirtualKeyCode")]
+    VirtualKeyCode),


This is strange. It's not following the same formatting as a nearly identical enum variant just a few lines below.

Yeah, I didn't understand why it did that either

Xaeroxe · Jan 20, 2018

amethyst_renderer/src/pass/flat/interleaved.rs

@@ -116,6 +107,8 @@ where
                     model: *global.as_ref(),
                 });
 
+            println!("{:?}", vertex_args);


Woops, looks like this was left in accidentally.

Xaeroxe · Jan 20, 2018

amethyst_renderer/src/pass/flat/separate.rs

 use tex::Texture;
 use types::{Encoder, Factory};
 use vertex::{Position, Separate, TexCoord, VertexFormat};
 
 /// Draw mesh without lighting
 #[derive(Derivative, Clone, Debug, PartialEq)]
 #[derivative(Default(bound = "Self: Pass"))]
-pub struct DrawFlatSeparate;
+pub struct DrawFlatSeparate {
+    skinning: bool,


This is a good first step towards composable passes. Thank you!

The model is from the official gltf sample models and is free to use for testing and educational purposes.

The model jumps to a new position because that's how the animation is defined in the gltf. I tested it with 2 separate gltf viewers and they both gave the same results.

There's a problem here though which I forgot about, the only pass that works atm is the flat one because for some reason the gfx version we use have a 4 vertex attribute limit. I will look into that tonight.

Rhuagh · Jan 20, 2018

The model is from the official gltf sample models and is free to use for testing and educational purposes.

The model jumps to a new position because that's how the animation is defined in the gltf. I tested it with 2 separate gltf viewers and they both gave the same results.

There's a problem here though which I forgot about, the only pass that works atm is the flat one because for some reason the gfx version we use have a 4 vertex attribute limit. I will look into that tonight.

The last issue is fixed in gfx-rs/gfx#1750.

Rhuagh · Jan 20, 2018

The last issue is fixed in gfx-rs/gfx#1750.

omni-viral · Jan 20, 2018

omni-viral reviewed Jan 20, 2018

View changes

I like it!

omni-viral · Jan 20, 2018

amethyst_renderer/src/skinning.rs

+    combo: AnimatedVertexBufferCombination,
+    renderer: &mut Renderer,
+) -> ::error::Result<Mesh> {
+    build_mesh_with_some!(


I always glad to see code can be reused 😄

omni-viral · Jan 20, 2018

amethyst_renderer/src/pass/pbm/separate.rs

+        };
+        if self.skinning {
+            builder
+                .with_raw_vertex_buffer(


I should check that such boilerplate are taken care of in new renderer.

Need to wait for gfx to be released with the fix before this can be pushed through.

Rhuagh · Jan 20, 2018

Need to wait for gfx to be released with the fix before this can be pushed through.

This is a great PR, I just marked some nits. I reviewed all the files, but I didn't check out the repo and run the examples.

Reviewed 22 of 40 files at r1, 3 of 9 files at r2, 3 of 4 files at r3, 18 of 18 files at r4.
Review status: all files reviewed at latest revision, 11 unresolved discussions.

amethyst_animation/src/skinning/systems.rs, line 34 at r4 (raw file):

            .join()
            .map(|(joint, _)| joint.skin)
            .collect::<HashSet<Entity>>();

I don't think this collect is necessary.

amethyst_gltf/src/format/mod.rs, line 147 at r4 (raw file):

    // TODO: morph targets, cameras
    // TODO: KHR_materials_common extension
    debug!("Loading nodes");

Could include name here.

amethyst_renderer/src/lib.rs, line 95 at r4 (raw file):

mod config;
#[macro_use]
mod macros;

Local macros should go before the other modules, because that's necessary for using them. Once you want to use a macro from e.g. cam or color, you'll just get a confusing error. Thus, I think putting it above all the other modules is a good practice to avoid this.x

amethyst_renderer/src/skinning.rs, line 57 at r4 (raw file):

    combo: AnimatedVertexBufferCombination,
    renderer: &mut Renderer,
) -> ::error::Result<Mesh> {

nit: Can use an import here.

amethyst_renderer/src/skinning.rs, line 79 at r4 (raw file):

    /// Create a new combo mesh creator with the given combo
    pub fn new(combo: AnimatedVertexBufferCombination) -> Self {
        Self { combo }

Meh, I think using Self for the construction of a struct looks confusing. But I think that's just me :)

amethyst_renderer/src/system.rs, line 57 at r4 (raw file):

            Ok(pipe) => Ok(Self::new(pipe, renderer)),
            Err(err) => {
                error!("Failed creating pipeline: {}", err);

Is the error message better now? Maybe we can include both the Display and the Debug output.

amethyst_renderer/src/pass/shaded_util.rs, line 42 at r4 (raw file):

}

unsafe impl Pod for DirectionalLightPod {}

I'm wondering, could Pod be replaced with Copy? Not related with this PR, so marking this as "Satisfied".

amethyst_renderer/src/pass/shaded_util.rs, line 54 at r4 (raw file):

        .join()
        .filter_map(|light| {
            if let Light::Point(ref light) = *light {

It might be good to have point, directional, .. methods which just return an Option. I think I saw this pattern before.

amethyst_renderer/src/pass/util.rs, line 67 at r4 (raw file):

}

pub(crate) fn add_textures(

Ah, this is great :)

amethyst_renderer/src/pass/flat/interleaved.rs, line 110 at r3 (raw file):

Previously, Xaeroxe (Jacob Kiesel) wrote…

Woops, looks like this was left in accidentally.

I think this is resolved, please mark it as "Satisfied" in that case @Xaeroxe

amethyst_renderer/src/pass/flat/separate.rs, line 25 at r3 (raw file):

Previously, Xaeroxe (Jacob Kiesel) wrote…

This is a good first step towards composable passes. Thank you!

🎉

Comments from Reviewable

torkleyy · Jan 21, 2018

This is a great PR, I just marked some nits. I reviewed all the files, but I didn't check out the repo and run the examples.

Reviewed 22 of 40 files at r1, 3 of 9 files at r2, 3 of 4 files at r3, 18 of 18 files at r4.
Review status: all files reviewed at latest revision, 11 unresolved discussions.

amethyst_animation/src/skinning/systems.rs, line 34 at r4 (raw file):

            .join()
            .map(|(joint, _)| joint.skin)
            .collect::<HashSet<Entity>>();

I don't think this collect is necessary.

amethyst_gltf/src/format/mod.rs, line 147 at r4 (raw file):

    // TODO: morph targets, cameras
    // TODO: KHR_materials_common extension
    debug!("Loading nodes");

Could include name here.

amethyst_renderer/src/lib.rs, line 95 at r4 (raw file):

mod config;
#[macro_use]
mod macros;

Local macros should go before the other modules, because that's necessary for using them. Once you want to use a macro from e.g. cam or color, you'll just get a confusing error. Thus, I think putting it above all the other modules is a good practice to avoid this.x

amethyst_renderer/src/skinning.rs, line 57 at r4 (raw file):

    combo: AnimatedVertexBufferCombination,
    renderer: &mut Renderer,
) -> ::error::Result<Mesh> {

nit: Can use an import here.

amethyst_renderer/src/skinning.rs, line 79 at r4 (raw file):

    /// Create a new combo mesh creator with the given combo
    pub fn new(combo: AnimatedVertexBufferCombination) -> Self {
        Self { combo }

Meh, I think using Self for the construction of a struct looks confusing. But I think that's just me :)

amethyst_renderer/src/system.rs, line 57 at r4 (raw file):

            Ok(pipe) => Ok(Self::new(pipe, renderer)),
            Err(err) => {
                error!("Failed creating pipeline: {}", err);

Is the error message better now? Maybe we can include both the Display and the Debug output.

amethyst_renderer/src/pass/shaded_util.rs, line 42 at r4 (raw file):

}

unsafe impl Pod for DirectionalLightPod {}

I'm wondering, could Pod be replaced with Copy? Not related with this PR, so marking this as "Satisfied".

amethyst_renderer/src/pass/shaded_util.rs, line 54 at r4 (raw file):

        .join()
        .filter_map(|light| {
            if let Light::Point(ref light) = *light {

It might be good to have point, directional, .. methods which just return an Option. I think I saw this pattern before.

amethyst_renderer/src/pass/util.rs, line 67 at r4 (raw file):

}

pub(crate) fn add_textures(

Ah, this is great :)

amethyst_renderer/src/pass/flat/interleaved.rs, line 110 at r3 (raw file):

Previously, Xaeroxe (Jacob Kiesel) wrote…

Woops, looks like this was left in accidentally.

I think this is resolved, please mark it as "Satisfied" in that case @Xaeroxe

amethyst_renderer/src/pass/flat/separate.rs, line 25 at r3 (raw file):

Previously, Xaeroxe (Jacob Kiesel) wrote…

This is a good first step towards composable passes. Thank you!

🎉

Comments from Reviewable

Review status: all files reviewed at latest revision, 9 unresolved discussions.

amethyst_animation/src/skinning/systems.rs, line 34 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

I don't think this collect is necessary.

How would I make it only return unique entities in the for loop then ?

amethyst_gltf/src/format/mod.rs, line 147 at r4 (raw file):

name
How do you mean ? The name of the asset is included in the first debug line above.

amethyst_renderer/src/lib.rs, line 95 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Local macros should go before the other modules, because that's necessary for using them. Once you want to use a macro from e.g. cam or color, you'll just get a confusing error. Thus, I think putting it above all the other modules is a good practice to avoid this.x

Done.

amethyst_renderer/src/skinning.rs, line 57 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

nit: Can use an import here.

Done.

amethyst_renderer/src/system.rs, line 57 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Is the error message better now? Maybe we can include both the Display and the Debug output.

Yeah, the issue is that the other error won't give me anything from the inner errors, so I needed to print it closer to the source to get the actual gfx error message :/

amethyst_renderer/src/pass/shaded_util.rs, line 54 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

It might be good to have point, directional, .. methods which just return an Option. I think I saw this pattern before.

I'll defer that for later

Comments from Reviewable

Rhuagh · Jan 21, 2018

Review status: all files reviewed at latest revision, 9 unresolved discussions.

amethyst_animation/src/skinning/systems.rs, line 34 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

I don't think this collect is necessary.

How would I make it only return unique entities in the for loop then ?

amethyst_gltf/src/format/mod.rs, line 147 at r4 (raw file):

name
How do you mean ? The name of the asset is included in the first debug line above.

amethyst_renderer/src/lib.rs, line 95 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Local macros should go before the other modules, because that's necessary for using them. Once you want to use a macro from e.g. cam or color, you'll just get a confusing error. Thus, I think putting it above all the other modules is a good practice to avoid this.x

Done.

amethyst_renderer/src/skinning.rs, line 57 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

nit: Can use an import here.

Done.

amethyst_renderer/src/system.rs, line 57 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Is the error message better now? Maybe we can include both the Display and the Debug output.

Yeah, the issue is that the other error won't give me anything from the inner errors, so I needed to print it closer to the source to get the actual gfx error message :/

amethyst_renderer/src/pass/shaded_util.rs, line 54 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

It might be good to have point, directional, .. methods which just return an Option. I think I saw this pattern before.

I'll defer that for later

Comments from Reviewable

Review status: all files reviewed at latest revision, 9 unresolved discussions.

amethyst_renderer/src/skinning.rs, line 79 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Meh, I think using Self for the construction of a struct looks confusing. But I think that's just me :)

Well, possibly for the construction, for returning Self i much prefer this way, because it means I don't have to write a gazillion type parameters all the time.

Comments from Reviewable

Rhuagh · Jan 21, 2018

Review status: all files reviewed at latest revision, 9 unresolved discussions.

amethyst_renderer/src/skinning.rs, line 79 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Meh, I think using Self for the construction of a struct looks confusing. But I think that's just me :)

Well, possibly for the construction, for returning Self i much prefer this way, because it means I don't have to write a gazillion type parameters all the time.

Comments from Reviewable

Reviewed 2 of 2 files at r5.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

amethyst_gltf/src/format/mod.rs, line 147 at r4 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

name
How do you mean ? The name of the asset is included in the first debug line above.

Ah right. I prefer to include the name or some id because with parallel code the output can easily mix. But I guess it's fine as is.

amethyst_renderer/src/skinning.rs, line 57 at r4 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

Done.

Err..you didn't use the imported item though :)

amethyst_renderer/src/skinning.rs, line 79 at r4 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

Well, possibly for the construction, for returning Self i much prefer this way, because it means I don't have to write a gazillion type parameters all the time.

Yeah, I prefer Self for specifying the return type and the real name for constructing the value.

amethyst_renderer/src/system.rs, line 57 at r4 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

Yeah, the issue is that the other error won't give me anything from the inner errors, so I needed to print it closer to the source to get the actual gfx error message :/

OK. Maybe the ll errors are better, let's just leave it as is for now.

Comments from Reviewable

torkleyy · Jan 21, 2018

Reviewed 2 of 2 files at r5.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

amethyst_gltf/src/format/mod.rs, line 147 at r4 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

name
How do you mean ? The name of the asset is included in the first debug line above.

Ah right. I prefer to include the name or some id because with parallel code the output can easily mix. But I guess it's fine as is.

amethyst_renderer/src/skinning.rs, line 57 at r4 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

Done.

Err..you didn't use the imported item though :)

amethyst_renderer/src/skinning.rs, line 79 at r4 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

Well, possibly for the construction, for returning Self i much prefer this way, because it means I don't have to write a gazillion type parameters all the time.

Yeah, I prefer Self for specifying the return type and the real name for constructing the value.

amethyst_renderer/src/system.rs, line 57 at r4 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

Yeah, the issue is that the other error won't give me anything from the inner errors, so I needed to print it closer to the source to get the actual gfx error message :/

OK. Maybe the ll errors are better, let's just leave it as is for now.

Comments from Reviewable

Review status: all files reviewed at latest revision, 4 unresolved discussions.

amethyst_gltf/src/format/mod.rs, line 147 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Ah right. I prefer to include the name or some id because with parallel code the output can easily mix. But I guess it's fine as is.

I can do that. Is there a good way to add a prefix for the scope with log?

amethyst_renderer/src/skinning.rs, line 57 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Err..you didn't use the imported item though :)

Ah, i used it only one place...

Comments from Reviewable

Rhuagh · Jan 21, 2018

Review status: all files reviewed at latest revision, 4 unresolved discussions.

amethyst_gltf/src/format/mod.rs, line 147 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Ah right. I prefer to include the name or some id because with parallel code the output can easily mix. But I guess it's fine as is.

I can do that. Is there a good way to add a prefix for the scope with log?

amethyst_renderer/src/skinning.rs, line 57 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Err..you didn't use the imported item though :)

Ah, i used it only one place...

Comments from Reviewable

Xaeroxe · Jan 21, 2018

Xaeroxe approved these changes Jan 21, 2018

View changes

Review status: 39 of 46 files reviewed at latest revision, 3 unresolved discussions.

amethyst_gltf/src/format/mod.rs, line 147 at r4 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

I can do that. Is there a good way to add a prefix for the scope with log?

Havent found a good way to add an id without having to send it down through the function hierarchy.

amethyst_renderer/src/skinning.rs, line 57 at r4 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

Ah, i used it only one place...

Done.

amethyst_renderer/src/skinning.rs, line 79 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Yeah, I prefer Self for specifying the return type and the real name for constructing the value.

Done.

Comments from Reviewable

Rhuagh · Jan 23, 2018

Review status: 39 of 46 files reviewed at latest revision, 3 unresolved discussions.

amethyst_gltf/src/format/mod.rs, line 147 at r4 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

I can do that. Is there a good way to add a prefix for the scope with log?

Havent found a good way to add an id without having to send it down through the function hierarchy.

amethyst_renderer/src/skinning.rs, line 57 at r4 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

Ah, i used it only one place...

Done.

amethyst_renderer/src/skinning.rs, line 79 at r4 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Yeah, I prefer Self for specifying the return type and the real name for constructing the value.

Done.

Comments from Reviewable

This is ready, gfx-core has been released.

Rhuagh · Jan 24, 2018

This is ready, gfx-core has been released.

r? @torkleyy @omni-viral

Rhuagh · Jan 25, 2018

r? @torkleyy @omni-viral

Reviewed 9 of 9 files at r6.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

amethyst_renderer/src/skinning.rs, line 79 at r4 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

Done.

OK, thank you!

Comments from Reviewable

torkleyy · Jan 25, 2018

Reviewed 9 of 9 files at r6.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

amethyst_renderer/src/skinning.rs, line 79 at r4 (raw file):

Previously, Rhuagh (Simon Rönnberg) wrote…

Done.

OK, thank you!

Comments from Reviewable

torkleyy · Jan 25, 2018

torkleyy approved these changes Jan 25, 2018

View changes

bors try

try

Build succeeded

bors · Jan 25, 2018

try

Build succeeded

omni-viral · Jan 25, 2018

omni-viral approved these changes Jan 25, 2018

View changes

Ship it!

@Xaeroxe the specific section was gltf file which isn't a code. My statement was stupid enough to delete it. But you somehow managed to read it although it existed less than a minute 😄

omni-viral · Jan 25, 2018

@Xaeroxe the specific section was gltf file which isn't a code. My statement was stupid enough to delete it. But you somehow managed to read it although it existed less than a minute 😄

😄 the Roxe is always watching!

I got an email about it and happened to be looking at my phone haha

Xaeroxe · Jan 25, 2018

😄 the Roxe is always watching!

I got an email about it and happened to be looking at my phone haha

bors r+

Rhuagh · Jan 25, 2018

bors r+

Build succeeded

bors · Jan 25, 2018

Build succeeded

Rhuagh added type: feature project: rendering status: working project: animation project: gltf labels Jan 17, 2018

feat: Vertex skinning

7511ac2

Rhuagh added status: ready and removed status: working labels Jan 18, 2018

Rhuagh changed the title from [WIP] feat: Vertex skinning to feat: Vertex skinning Jan 18, 2018

Rhuagh added status: working and removed status: ready labels Jan 20, 2018

Rhuagh added some commits Jan 18, 2018

fix: Quaternion animation data was loaded incorrectly from GLTF files…
…. Also adds possibility to flip v coordinates for OpenGL based pipelines

0a7e06c

refactor: Reduce code duplication in passes

b76797f

Rhuagh and others added some commits Jan 20, 2018

feat: Add optional translation from bounding volume centroid to origi…
…n when loading gltf assets

5ceb7e7

fix: Reduce allocations for skin matrix computation

Loading status checks…

854ab4a

Rhuagh added status: ready and removed status: working labels Jan 24, 2018

bors bot added a commit that referenced this pull request Jan 25, 2018

Try #545:

Loading status checks…

d8bd58a

Rhuagh deleted the Rhuagh:vertex-skinning branch Jan 25, 2018

amethyst/amethyst

Join GitHub today

feat: Vertex skinning #545

Conversation

Rhuagh commented Jan 17, 2018 • edited by torkleyy Edited 2 times torkleyy edited Jan 17, 2018 (most recent) Rhuagh edited Jan 17, 2018

Rhuagh added type: feature project: rendering status: working project: animation project: gltf labels Jan 17, 2018

This comment has been minimized.

omni-viral commented Jan 18, 2018

This comment has been minimized.

Rhuagh commented Jan 18, 2018

Rhuagh added status: ready and removed status: working labels Jan 18, 2018

Rhuagh changed the title from [WIP] feat: Vertex skinning to feat: Vertex skinning Jan 18, 2018

This comment has been minimized.

Rhuagh commented Jan 19, 2018

Xaeroxe requested changes Jan 20, 2018 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Rhuagh commented Jan 20, 2018

This comment has been minimized.

Rhuagh commented Jan 20, 2018

omni-viral reviewed Jan 20, 2018 View changes

This comment has been minimized.

This comment has been minimized.

Rhuagh added status: working and removed status: ready labels Jan 20, 2018

This comment has been minimized.

Rhuagh commented Jan 20, 2018

Rhuagh added some commits Jan 18, 2018

This comment has been minimized.

torkleyy commented Jan 21, 2018

This comment has been minimized.

Rhuagh commented Jan 21, 2018

This comment has been minimized.

Rhuagh commented Jan 21, 2018

This comment has been minimized.

torkleyy commented Jan 21, 2018

This comment has been minimized.

Rhuagh commented Jan 21, 2018

Xaeroxe approved these changes Jan 21, 2018 View changes

This comment has been minimized.

Rhuagh commented Jan 23, 2018

Rhuagh and others added some commits Jan 20, 2018

Rhuagh added status: ready and removed status: working labels Jan 24, 2018

This comment has been minimized.

Rhuagh commented Jan 24, 2018

This comment has been minimized.

Rhuagh commented Jan 25, 2018

This comment has been minimized.

torkleyy commented Jan 25, 2018

torkleyy approved these changes Jan 25, 2018 View changes

bors bot added a commit that referenced this pull request Jan 25, 2018

This comment has been minimized.

bors bot Jan 25, 2018

try

Build succeeded

bors bot commented Jan 25, 2018

try

Build succeeded

omni-viral approved these changes Jan 25, 2018 View changes

This comment has been minimized.

omni-viral commented Jan 25, 2018 • edited Edited 1 time omni-viral edited Jan 25, 2018 (most recent)

This comment has been minimized.

Xaeroxe commented Jan 25, 2018

This comment has been minimized.

Rhuagh commented Jan 25, 2018

bors bot added a commit that referenced this pull request Jan 25, 2018

This comment has been minimized.

bors bot Jan 25, 2018

Build succeeded

bors bot commented Jan 25, 2018

Build succeeded

Hide details View details bors bot merged commit 854ab4a into amethyst:develop Jan 25, 2018 4 checks passed

4 checks passed

Rhuagh deleted the Rhuagh:vertex-skinning branch Jan 25, 2018

Rhuagh commented Jan 17, 2018 •

edited by torkleyy

Edited 2 times

torkleyy edited Jan 17, 2018 (most recent)

Rhuagh edited Jan 17, 2018

Xaeroxe requested changes Jan 20, 2018

View changes

omni-viral reviewed Jan 20, 2018

View changes

Xaeroxe approved these changes Jan 21, 2018

View changes

torkleyy approved these changes Jan 25, 2018

View changes

omni-viral approved these changes Jan 25, 2018

View changes

omni-viral commented Jan 25, 2018 •

edited

Edited 1 time

omni-viral edited Jan 25, 2018 (most recent)

bors bot merged commit `854ab4a` into amethyst:develop Jan 25, 2018
4 checks passed