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

Pose Animation not working correctly with Skeleton Animation #439

Open
silicium42 opened this issue Apr 6, 2024 · 2 comments
Open

Pose Animation not working correctly with Skeleton Animation #439

silicium42 opened this issue Apr 6, 2024 · 2 comments

Comments

@silicium42
Copy link
Contributor

System Information

  • Ogre Version: 2.3/master branch
  • Operating System / Platform: Windows 10
  • RenderSystem: Vulkan
  • GPU: AMD RX 5700

Detailled description

Pose animations do not work when a SubMesh uses skeleton animations with more than 4 bones. In my case that manifested as all offsets of the pose (i.e. posePos in the vertex shader) being set to zero and therefor no transformation from poses being applied to the mesh by the shader. As far as I can see this is due to the following vertex shader code in 800.VertexShader_piece_vs.any which calculates an incorrect baseVertexID:

@property( hlms_pose )
@piece( PoseTransform )
	// Pose data starts after all 3x4 bone matrices
	uint poseDataStart = (worldMaterialIdx[inVs_drawId].x >> 9u) @property( hlms_skeleton ) + @value(hlms_bones_per_vertex)u * 3u@end ;

	float4 poseData = readOnlyFetch( worldMatBuf, int( poseDataStart ) );

	@property( syntax != hlsl )
		@property( syntax != metal )
			uint baseVertexID = floatBitsToUint( poseData.x );
		@end
		uint vertexID = uint( inVs_vertexId )- baseVertexID;
	@else
		uint vertexID = inVs_vertexId;
	@end

This code assumes the location of the baseVertexID is offset by the maximum number of bone weights per vertex the SubMesh uses ( i.e. hlms_bones_per_vertex) which is often, but not always, incorrect. It looks like the code should use the number of bones that affect the SubMesh (i.e. the size of the Renderables mBlendIndexToBoneIndexMap), which is unfortunately not part of the HlmsBaseProp properties.

Testing conditions

I found this bug in version 2.3 originally and have just tested it with the master branch where it is still present. I don't know a lot about the state of pose animation in Ogre-Next, but i hope someone can tell me how it is supposed to work. Since i used the Sample_V2Mesh for my tests of exporting meshes with skeleton and pose animation from blender, i had to make a few modifications to the v1 import process to allow for poses to work at all. Unfortunately i could not get poses to work with v1 Entities at all ( the mPoseData of the SubMeshes contained a NULL pointer). Nevertheless i am convinced if they were to work ( if anyone could tell me how that would be nice), the same problem would affect them as well, since it stems from the vertex shader.

I have made a simple test object from the blender default cube in two versions, where both have a pose named "Size" that scales them uniformly but one of them has six bones assigned to the submeshes and one only four. I will attach a zip with both versions here in xml format.
test_cube.zip

Quick Fix

I have found a quick and dirty fix that adds the aforementioned property to HlmsBaseProp and changes the 800.VertexShader_piece_vs.any accordingly:
OgreHlms.h:

@ -1042,6 +1042,7 @@ namespace Ogre
        static const IdString Pose;
        static const IdString PoseHalfPrecision;
        static const IdString PoseNormals;
+       static const IdString NumBones;

        static const IdString Normal;
        static const IdString QTangent;

OgreHlms.cpp:

@ -91,6 +91,7 @@ namespace Ogre
    const IdString HlmsBaseProp::Pose = IdString( "hlms_pose" );
    const IdString HlmsBaseProp::PoseHalfPrecision = IdString( "hlms_pose_half" );
    const IdString HlmsBaseProp::PoseNormals = IdString( "hlms_pose_normals" );
+   const IdString HlmsBaseProp::NumBones = IdString( "hlms_num_bones" );

    const IdString HlmsBaseProp::Normal = IdString( "hlms_normal" );
    const IdString HlmsBaseProp::QTangent = IdString( "hlms_qtangent" );
@ -2703,7 +2704,8 @@ namespace Ogre
        mT[kNoTid].setProperties.clear();

        setProperty( kNoTid, HlmsBaseProp::Skeleton, renderable->hasSkeletonAnimation() );

+       if( renderable->hasSkeletonAnimation() )
+           setProperty( kNoTid, HlmsBaseProp::NumBones, static_cast<Ogre::RenderableAnimated *>( renderable )->getBlendIndexToBoneIndexMap()->size() );
        setProperty( kNoTid, HlmsBaseProp::Pose, renderable->getNumPoses() );
        setProperty( kNoTid, HlmsBaseProp::PoseHalfPrecision, renderable->getPoseHalfPrecision() );
        setProperty( kNoTid, HlmsBaseProp::PoseNormals, renderable->getPoseNormals() );

800.VertexShader_piece_vs.any :

@ -120,7 +120,7 @@
@property( hlms_pose )
@piece( PoseTransform )
	// Pose data starts after all 3x4 bone matrices
-	uint poseDataStart = (worldMaterialIdx[inVs_drawId].x >> 9u) @property( hlms_skeleton ) + @value(hlms_bones_per_vertex)u * 3u@end ;
+	uint poseDataStart = (worldMaterialIdx[inVs_drawId].x >> 9u) @property( hlms_skeleton ) + @value(hlms_num_bones)u * 3u@end ;

	float4 poseData = readOnlyFetch( worldMatBuf, int( poseDataStart ) );
@darksylinc
Copy link
Member

Your fix appears to be the right way! (well, there are various possible ways, but this is one of them)

The only detail is that when you do:

if( renderable->hasSkeletonAnimation() )
           setProperty( kNoTid, HlmsBaseProp::NumBones, static_cast<Ogre::RenderableAnimated *>( renderable )->getBlendIndexToBoneIndexMap()->size() )

We would have to also check if it's using pose animations. Otherwise that snippet will force us to create & compile one shader / PSO for each mesh that has a different number of bones and that would be very bad for performance and stutters.

Your fix still means that each mesh using poses w/ a different number of bones will get a different shader / PSO, but that should be much less frequent than plain skeletal animation.

Perhaps an alternative solution could send the number of bones dynamically (so that the same shader / PSO can be reused) if this happens to be problem; but until then your solution is fine.

I don't know a lot about the state of pose animation in Ogre-Next

Unfortunately it hasn't received the love and attention that it should have :(

Though AFAIK v1 pose animations should be working (it hasn't changed since the original Ogre 1.x implementation), but you have to force OgreNext to use SW skinning for the whole thing (which is slow).

@silicium42
Copy link
Contributor Author

We would have to also check if it's using pose animations. Otherwise that snippet will force us to create & compile one shader / PSO for each mesh that has a different number of bones and that would be very bad for performance and stutters.

Your fix still means that each mesh using poses w/ a different number of bones will get a different shader / PSO, but that should be much less frequent than plain skeletal animation.

Thanks! i have implemented this fix although i have yet to test more than one meshe at the same time using poses.

Perhaps an alternative solution could send the number of bones dynamically (so that the same shader / PSO can be reused) if this happens to be problem; but until then your solution is fine.

I thought about that as well, but i didn't know if there were any concerns with alignment, so i opted for the easier fix.

Unfortunately it hasn't received the love and attention that it should have :(

Though AFAIK v1 pose animations should be working (it hasn't changed since the original Ogre 1.x implementation), but you have to force OgreNext to use SW skinning for the whole thing (which is slow).

That is somewhat sad to hear. When i'm done with testing i could provide a patch for this bug and the importing of poses from v1 meshes. I'm not really sure whether to base it on the v2-3 or master branch, so any advice is appreciated.

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

No branches or pull requests

2 participants