Skip to content
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

157 provide example for a human asset analog to vehicle example #158

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ClemensLinnhoff
Copy link
Collaborator

Describe your changes

Add human model (without helper bones and with full hands (no fingers as bones))

Issue ticket number and link

Fixes #157

Mention a member

@ipg-sig

Checklist before requesting a review

  • I have performed a self-review of my code/documentation.
  • My changes generate no new warnings during the documentation generation.

Signed-off-by: ClemensLinnhoff <[email protected]>
Signed-off-by: ClemensLinnhoff <[email protected]>
@ipg-sig
Copy link

ipg-sig commented Nov 7, 2024

@ClemensLinnhoff

Should the included animation look like this or wasn´t it intended?
Anim

@ClemensLinnhoff
Copy link
Collaborator Author

ClemensLinnhoff commented Nov 7, 2024

Should the included animation look like this or wasn´t it intended?

I think we had some animations in there (running, jumping or similar). But they were not intended to be exported as part of this example. Feel free to remove them.

@LudwigFriedmann
Copy link
Collaborator

@ClemensLinnhoff Is this PR ready for review?

@ClemensLinnhoff
Copy link
Collaborator Author

No, this is still a draft. This was just my initial upload and @ipg-sig might need to adapt the model to the new definitions.

@ClemensLinnhoff
Copy link
Collaborator Author

I think there is something wrong with the node hierarchy. I imported the glTF into Blender and the hierarchy looks like this:

image

Looks like the hierarchy was flattened. Was that done during export maybe?

@ClemensLinnhoff
Copy link
Collaborator Author

Otherwise it looks great. I like the example of the wrist band as an accessory.

@LudwigFriedmann
Copy link
Collaborator

It may be the case that blender and/or the gltf exporter considers nodes which are linked to the armature as children.
I can export my previous human model and see if I experience similar results

@ipg-sig ipg-sig force-pushed the 157-provide-example-for-a-human-asset-analog-to-vehicle-example branch from 606b272 to 401dc2d Compare November 20, 2024 14:56
@ClemensLinnhoff
Copy link
Collaborator Author

I checked the updated human example and the hierarchy is still flattened in Blender. But maybe this is a Blender bug?
Because this is an excerpt of the current glTF file and here the group nodes do have children.

{
	"mesh":0,
	"name":"Accessorie",
	"skin":0
},
{
	"children":[
		31
	],
	"name":"Grp_Accessories"
},
{
	"mesh":1,
	"name":"Human",
	"skin":0
},
{
	"children":[
		33
	],
	"name":"Grp_Body"
},
{
	"mesh":2,
	"name":"Vest",
	"skin":0
},
{
	"children":[
		35
	],
	"name":"Grp_Clothing"
},

@ClemensLinnhoff
Copy link
Collaborator Author

And I get the following report from a glTF validator:

image

@ClemensLinnhoff
Copy link
Collaborator Author

On the other hand, I imported it to this editor: https://www.gltfeditor.com/

Here the structure looks correct:

image

@ClemensLinnhoff
Copy link
Collaborator Author

I also asked our 3D artist and he says it should not be possible to have empty nodes with children in an armature. The meshes should directly be part of it, at least in Blender.

@ClemensLinnhoff
Copy link
Collaborator Author

I also asked our 3D artist and he says it should not be possible to have empty nodes with children in an armature. The meshes should directly be part of it, at least in Blender.

Okay we tried around a bit. So it does seem to work. But it is an import bug in Blender that flattens the hierarchy. Anyways this seems to be quite an exotic solution having empty nodes as part of an armature.

@ipg-sig
Copy link

ipg-sig commented Nov 21, 2024

@ClemensLinnhoff Thank you very much! I have tried different combinations in Blender, but it always ended in the flattened hierarchy after reimporting the exported files (all 3 file formats).

To avoid it, we could also use the group names as an Prefix for the meshes, so we can get rid of the additional group nodes.
Example:

  • Accessories_Wristband
  • Body_Human
  • Clothing_Vest
  • Hair_Short

But I think this should be discussed in the group next week?
I can fix it in advance for next week, but I will not be able to be present at the next meeting.

@ClemensLinnhoff
Copy link
Collaborator Author

@ClemensLinnhoff Thank you very much! I have tried different combinations in Blender, but it always ended in the flattened hierarchy after reimporting the exported files (all 3 file formats).

To avoid it, we could also use the group names as an Prefix for the meshes, so we can get rid of the additional group nodes. Example:

  • Accessories_Wristband
  • Body_Human
  • Clothing_Vest
  • Hair_Short

But I think this should be discussed in the group next week? I can fix it in advance for next week, but I will not be able to be present at the next meeting.

Yes, this came also to my mind. Just have a prefix for the meshes instead of the empty nodes.

@LudwigFriedmann
Copy link
Collaborator

LudwigFriedmann commented Nov 21, 2024

I can confirm flattening of the hierachy happens for my example, as well (even though I've made sure this option is disabled within the export dialog).

Blend:
pedestrian_blend

glTF:
pedestrian_gltf

@ClemensLinnhoff
Copy link
Collaborator Author

We created an issue in the Blender project and already got an answer from another user and from a blender organization member who do not think this is a bug since the behavior of the mesh is still correct even if the hierarchy is flattened. The issue was already closed:
https://projects.blender.org/blender/blender/issues/130677

So if Blender is not supporting this and will not support this, I think we should flatten the hierarchy in the definitions as well, as @ipg-sig proposed.

@ipg-sig ipg-sig force-pushed the 157-provide-example-for-a-human-asset-analog-to-vehicle-example branch from 401dc2d to c603d0c Compare November 22, 2024 14:55
@ipg-sig ipg-sig self-assigned this Nov 22, 2024
@ipg-sig
Copy link

ipg-sig commented Nov 22, 2024

Model and exported files re updated @ClemensLinnhoff .

With OpenUSD the "Grp_Root" was always renamed to "root" after import, but I am not sure if this is another Blender OpenUSD Bug. FBX and GLTF work fine now. maybe someone can check it again with OpenUSD?
The .blend file is also updated.

Copy link
Collaborator Author

@ClemensLinnhoff ClemensLinnhoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good, just one minor remark.

"buffers":[
{
"byteLength":1629064,
"uri":"Human_Example.bin"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file name is wrong here. This would be a simple fix, if you are sure you uploaded the correct files. Just want to make sure.

@ClemensLinnhoff ClemensLinnhoff marked this pull request as ready for review November 22, 2024 17:29
@ClemensLinnhoff
Copy link
Collaborator Author

@ipg-sig feel free to add yourself as co-author the the metadata of the asset file :)

@LudwigFriedmann
Copy link
Collaborator

LudwigFriedmann commented Nov 25, 2024

Model and exported files re updated @ClemensLinnhoff .

With OpenUSD the "Grp_Root" was always renamed to "root" after import, but I am not sure if this is another Blender OpenUSD Bug. FBX and GLTF work fine now. maybe someone can check it again with OpenUSD? The .blend file is also updated.

I wasn't able to reproduce this behavior in Blender 4.1.1.
"Grp_Root" kept its name over an export and re-import as USD.
image

@ClemensLinnhoff
Copy link
Collaborator Author

Model and exported files re updated @ClemensLinnhoff .
With OpenUSD the "Grp_Root" was always renamed to "root" after import, but I am not sure if this is another Blender OpenUSD Bug. FBX and GLTF work fine now. maybe someone can check it again with OpenUSD? The .blend file is also updated.

I wasn't able to reproduce this behavior in Blender 4.1.1. "Grp_Node" kept its name over an export and re-import as USD.

I can somewhat reproduce it. After reimporting the pedestrian USD file, I get the following structure:

image

@ClemensLinnhoff
Copy link
Collaborator Author

But that seems to be a pedestrian issue, as it works for the example vehicle. This also explains, why it works in @LudwigFriedmann s example:

image

@ClemensLinnhoff
Copy link
Collaborator Author

During usd export in Blender you can set the name of the root prim. The default values is /root.

image

But you can simply remove that and then the root node it Grp_Root, as specified. I tried an online usd viewer and this is the hierarchy there:

image

@ClemensLinnhoff
Copy link
Collaborator Author

The is a neutral bone in the example, that is not in the structure definition. Do we need the neutral bone?

@ClemensLinnhoff
Copy link
Collaborator Author

Sometimes the x-axis is pointing forwards, sometime backwards. Can we unify this?

@ipg-sig
Copy link

ipg-sig commented Dec 2, 2024

Sometimes the x-axis is pointing forwards, sometime backwards. Can we unify this?

We could unify it, but we would lose the ability to mirror the animation properly. For example: If I have mirroring enabled and rotate the shoulder, one would rotate forwards and the other backwards. As it is now, it is also the standard rotation, if you symmetrize the bones.
In my opinion, it is therefore more beneficial to keep it that way so we are able to properly mirror bones and keyframes for future animations..

@ClemensLinnhoff
Copy link
Collaborator Author

Sometimes the x-axis is pointing forwards, sometime backwards. Can we unify this?

We could unify it, but we would lose the ability to mirror the animation properly. For example: If I have mirroring enabled and rotate the shoulder, one would rotate forwards and the other backwards. As it is now, it is also the standard rotation, if you symmetrize the bones. In my opinion, it is therefore more beneficial to keep it that way so we are able to properly mirror bones and keyframes for future animations..

We tried this out, but this only worked in the shoulders.
In any case, you can get the same result by pasting the key frames flipped (e.g. Shift + Ctrl + v in Blender).

We also had a look at the Rigify default figure, there, also the orientations are the same for left an right. The z-axis is pointing forward in the arms and backwards in the legs, but same for left and right.

image

@ipg-sig
Copy link

ipg-sig commented Dec 2, 2024

We tried this out, but this only worked in the shoulders.

Thats a little bit strange. It should work for all mirrored bones (full arms, legs and eyes). I noticed that the Upper_Arm is named incorrectly on the right side, therefore the mirroring doesn´t work as expected, but works on all other bones for me.

We also had a look at the Rigify default figure, there, also the orientations are the same for left an right.

For the legs it is possible to keep them in the same orientation (same for the eyes).
Flipping the keyframes works of course too, but could be more complex if someone wants to transform the bones inside the simulation. If we keep it like that, the right side is always mirrored with the orientation and we don´t have a mix. But I don´t have a strong opinion on that. So if you like, we can also discuss it in the group, if there are any preferences :)

@ClemensLinnhoff
Copy link
Collaborator Author

I also don't have a strong opinion. @MatthiasThDs do you have an opinion about this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide example for a human asset analog to vehicle example
3 participants