Skip to content
This repository was archived by the owner on Mar 30, 2019. It is now read-only.

Conversation

@jwollen
Copy link
Contributor

@jwollen jwollen commented Jun 3, 2014

I recently did some work on the model pipeline and would like to contribute some of it to the toolkit.

The branch currently contains just the minimum fixes and changes needed to support skeletal animation.
Before doing anything major, I'd like to discuss what you would like to see, and what you feel is out of the scope of the toolkit.

A few pending changes, that I would like to introduce next, include

  1. Separate data (Model) from state and rendering logic (ModelInstance). This would also make Model.Clone() unnecessary and integrate well with scene graphs.
  2. Move transformation related logic into a Skeleton (and SkeletonInstance) class.
  3. Facilitate separation of meshes, skeletons and animations. Make it possible to apply animations and meshes from different models to matching skeletons.
  4. Remove parameter setup and effect specific code from Model. This could be done using some kind of configurable EffectBindingSystem, that automatically sets parameters when effects are applied.
  5. Support animation playback. Should we provide a basic animation library or just a few helpers?

I'm not yet sure what to do on the compiler side of things, beyond some refactoring. Assimp poses some problems, when it comes to more advanced stuff (e.g. morph targets).
I feel like I would need to ditch it eventually, so I was debating whether I'm going to write a separate import library.
It would basically be a managed import-only alternative to Assimp, supporting only a few major formats and being more extensible and easier to maintain.
I might as well base it on SharpDX and the ModelCompiler and we could decide later if it fits the toolkit.

I also added a SkeletalAnimation sample. Right now it references the Dude.fbx model, which Assimp doesn't import correctly, so the animation is a bit messed up. I'm going to add a model that is better suited soon.

@ArtiomCiumac
Copy link
Contributor

Everything works fine (except the weird animations in the sample you mentioned) - I am ready to merge this. It is a really good contribution.

Separate data (Model) from state and rendering logic (ModelInstance). This would also make Model.Clone() unnecessary and integrate well with scene graphs.

Good idea, would help in case if the same model is used multiple times.

Move transformation related logic into a Skeleton (and SkeletonInstance) class.
Facilitate separation of meshes, skeletons and animations. Make it possible to apply animations and meshes from different models to matching skeletons.

This is a more difficult task, but Toolkit would definitely benefit from such functionality.

Remove parameter setup and effect specific code from Model. This could be done using some kind of configurable EffectBindingSystem, that automatically sets parameters when effects are applied.

I think we can leave this for later, as it may require more time to implement and debug.

Support animation playback. Should we provide a basic animation library or just a few helpers?

We can start with few helpers, but then we can add more functionality as needed.

Keep in mind that Toolkit is not designed to be a full game engine - it is just a set of tools for getting started quickly, so the things should be easy to use but they should allow access to low-level structures for advanced usage.

I'm not yet sure what to do on the compiler side of things, beyond some refactoring. Assimp poses some problems, when it comes to more advanced stuff (e.g. morph targets).
I feel like I would need to ditch it eventually, so I was debating whether I'm going to write a separate import library.

If Assimp poses some limitations - we would definitely benefit from a separate library, however it can take much longer time to implement. Maybe it is worth to investigate first the state of Assimp and Assimp.NET - if there are any updated versions that fix these issues.

@jwollen, do you want to keep it open to add additional changesets or would prefer to open a separate request?
@xoofx, do you have any comments?

@xoofx
Copy link
Member

xoofx commented Jun 4, 2014

Thanks @jwollen for all the work on this! I agree with @ArtiomCiumac comments.

Regarding the pull request, I would just rename SrtTransform to CompositeTransform. Also, could the ModelAnimationController in the sample be used as the basis for an animation GameSystem integrated into the Game assembly that would provide this basic controller without having to duplicate it? (Like the BasicEffect for effects)

Otherwise the changes look small to support basic skinning, so that's a good indication.

@jwollen
Copy link
Contributor Author

jwollen commented Jun 5, 2014

I would just rename SrtTransform to CompositeTransform

Done.

Also, could the ModelAnimationController in the sample be used as the basis for an animation GameSystem

Sounds good. Doing that as soon as I have the skeleton-related changes in place.

do you want to keep it open to add additional changesets or would prefer to open a separate request?

Feel free to merge when you see fit. The current state seems pretty solid. I'll open new pull requests for major updates. I'll also open one for the samples project later.

@xoofx
Copy link
Member

xoofx commented Jun 5, 2014

Feel free to merge when you see fit. The current state seems pretty solid. I'll open new pull requests for major updates. I'll also open one for the samples project later.

Great, we will just wait for the AnimationSystem to merge this first set.

@ArtiomCiumac
Copy link
Contributor

@jwollen, what is the state of this request? The code looks fine for me and I can merge it. Does it have any known issues?

jwollen added 14 commits June 27, 2014 12:32
Conflicts:
	Source/Toolkit/SharpDX.Toolkit.Graphics/SharpDX.Toolkit.Graphics.csproj
	Source/Toolkit/SharpDX.Toolkit/Graphics/ModelData.cs
…ted in mesh space (in default pose), instead of bone space.
…y, since Assimp does not provide the actual bind pose)
Conflicts:
	Source/Toolkit/SharpDX.Toolkit.Game/SharpDX.Toolkit.Game.csproj
…ind poses didn't achieve the actual intent to

1. Effortlessly support additive animations
2. Share skinning matrices between multiple models

Conflicts:
	Source/Toolkit/SharpDX.Toolkit/Graphics/ModelData.cs
Conflicts:
	Source/Toolkit/SharpDX.Toolkit/Graphics/ModelData.cs
@jwollen
Copy link
Contributor Author

jwollen commented Jun 27, 2014

Sorry for the delay. Resolved merge conflicts and should be good to go. No known issues.
I also opened a pull request for the sample (sharpdx/SharpDX-Samples#4).

@ArtiomCiumac ArtiomCiumac merged commit 82f6b03 into sharpdx:master_integration Jun 27, 2014
@ArtiomCiumac
Copy link
Contributor

Everything seems fine.
Thank you for this great contribution!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants