-
-
Notifications
You must be signed in to change notification settings - Fork 36.1k
SkinnedMesh support on Editor #8422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
8a8abbe
Support SkinnedMesh on Editor
takahirox 42bf0e7
Merge branch 'dev' into EditorSkinnedMeshSupport
takahirox 19cf981
Reflect mrdoob's comment for SkinnedMesh support on Editor
takahirox da67bcb
Update how to judge if Editor Loader creates SkinnedMesh
takahirox File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrdoob
What do you think of this idea?
I suppose this could be a bit too complex and there would be any better/simpler ways...
(Maybe the root issue is that SkinnedMesh here has two bones sets in its children and
this idea could be kinda just workaround)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds a bit confusing...
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So let me make the issue more specific with an example.
Imagine that there is a geometry with bones which
hold default pose bone parameters as just Objects.
You create SkinnedMesh instance from that geometry.
Bone instances are generated in SkinnedMesh constructor
and they'll be children of SkinnedMesh with forming of parent-child structure
and also will be bones of SkinnesMesh.skeleton.
You add Mesh to Bone B.
You let SkinnedMesh do skinning.
After that you create json with toJSON() and then
re-create SkinnedMesh with ObjectLoader.parse(json).
(This represents auto-save and auto-load of Editor)
The issue that SkinnedMesh which you get has two bone sets.
One is made from json.geometry.bones and represents default pose (Bone A, B, C).
Another one is made from json.object(SkinnedMesh).children and represents updated bones (Bone A', B', C')
What we want is the updated one.
So my idea is that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrdoob
So, what do you think of this.
Should I write comment in more detail?
Or should I consider any simpler ways?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/ping @mrdoob
Is my idea too complicated to be merged?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we still convert node-based bones into skeleton bones during load time if I am not mistaken. If we were to only use skeleton-bones by default, we could likely speed up load time.
I could see it useful to have a function to convert skeleton-bones to nodes and vice versa, but I would only keep one representation at a time to avoid issues -- although as I said modifying the bones in a game engine will likely invalidate all saved animations for that character -- which is likely a bad thing. Character's bones should be modified in the content creation tool that is also creating all your animations (e.g. Maya, Blender, etc.) so that they are properly synchronized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it comes down to what the intended scope of Three.js and the editor is supposed to be.
Authoring software will most probably do a better job when it comes to editing, OTOH I guess it could be very useful (e.g. for debugging) to have a GUI-way to explore an exported animation, maybe even to change it in some ways.
Looking at all the trouble with the exporters lately, going "baked only" may make it more difficult to get right even if it means fewer code, but I could be wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a necessity. There is the Skeleton helper for this.
We need to get the exporters/workflow fixed or ThreeJS will be considered to be unserious/unstable/unusable compared to its competitors. To avoid that issue is to put one's head in the sand -- that is a huge issue that if it isn't fixed will hurt ThreeJS tremendously. The most important is Blender, because it is a free tool, the second most important should be the FBX importer (for the commercial tools 3DS Max, Maya, etc.).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are three markets that ThreeJS target in my opinion: those learning to code 3D (where ThreeJS is very dominant) and those creating 3D projects (there is a lot of competition here) and those using ThreeJS as the renderer for other tools (such as our usage.) The exporters do not need to be fixed to target those learning to code 3D, they are happy with OBJ/MTL imports for the most part. For most third party tools using ThreeJS as the renderer, they also do not need to be fixed, these other tools have their own loaders -- these users of ThreeJS Iview as low value to the project as they tend not to be contributors back to the project though because they view the other users of ThreeJS as potential competitors. But those creating serious 3D projects need to be able to rely on their artists (who want to work in their content creation tool of choice since that is where they are productive) creating content that just works in ThreeJS. If there are non-trivial post export steps that need to be done to get content to work, when it will just work in Unity3D or other tools, then you are putting ThreeJS at a disadvantage, and a serious one at that. A great model to follow is Blend4Web - it is nearly perfect WYSIWYG when moving from Blender to WebGL both in terms of materials, lights, animations, meshes, bones, skinning, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bhouston
Yes, that one lets you see whether it worked, but not necessarily what went wrong. An online tool (like the Three editor) that lets you load a model from a support request in order to see names & hierarchy might come useful, I figure.
Absolutely. If all others fail can still use Blender as a converter and at least won't have to pay an extra fee on top of the mess. I'm following that topic closely - ready to step in when required. The code quality is very workable - well-structured and not bad at all.