[rmodels] Smarter use of GPU and CPU skinning - #5921
Conversation
Have the model animation update functions check the current shader to see if they are going to do GPU skinning or not. Lazy load the CPU animation buffers so they don't get allocated if the user is doing GPU skinning. This change allows both GPU and CPU skinning in the same program.
|
This is a refactor of PR #5902 made after feedback. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
raysan5
changed the title
| @@ -322,7 +322,7 @@ | |||
| #endif | |||
| #ifndef SUPPORT_GPU_SKINNING | |||
| // GPU skinning disabled by default, some GPUs do not support more than 8 VBOs | |||
There was a problem hiding this comment.
this comment should probably be updated, too
| #define SUPPORT_GPU_SKINNING 0 | ||
| // GPU skinning enabled by default for GL3.3+, | ||
| // Some GPUs do not support more than 8 VBOs so you may need to disable it if you have issues loading animated models | ||
| #define SUPPORT_GPU_SKINNING 1 |
There was a problem hiding this comment.
I prefer to avoid enabling this by default. CPU solution is widely supported but GPU can fail in some platforms.
There was a problem hiding this comment.
This does not disable CPU skinning. Would you be ok with default enable for GL 3.3+?
There was a problem hiding this comment.
If GPU skinning can not be enabled on target platform, does it fallback to CPU skinning? I'd like to keep the conditionals at minimum in config.h.
There was a problem hiding this comment.
I have a plan for how to do this. I will test it in a fork.
Is there any hard info on devices that that the smaller number of vertex attributes? or is it just an ambiguous "some" ?
There was a problem hiding this comment.
My research on the OpenGL spec.
OpenGL 3.0 - 4.6: Requires a minimum of 16 attributes
| RLAPI ModelAnimation *LoadModelAnimations(const char *fileName, int *animCount); // Load model animations from file | ||
| RLAPI void UpdateModelAnimation(Model model, ModelAnimation anim, float frame); // Update model animation pose (vertex buffers and bone matrices) | ||
| RLAPI void UpdateModelAnimationEx(Model model, ModelAnimation animA, float frameA, ModelAnimation animB, float frameB, float blend); // Update model animation pose, blending two animations | ||
| RLAPI void UpdateModelAnimationBones(Model model, ModelAnimation anim, float frame); // Update model animation pose (bones only) |
There was a problem hiding this comment.
Could those functions be kept internal to rmodels, I prefer to expose the minimum required functions for users.
There was a problem hiding this comment.
It is very useful for people who want to do GPU skinning only.
This was a function in 5.5, but it was removed for 6.0 for no clear reason (incorrectly in my option).
I am simply restoring it due to it's utility.
There was a problem hiding this comment.
I removed them because I considered most users don't need them, that was the reason to add the models_animation_blend_custom example, for users that need that level of control.
| blend = Clamp(blend, 0.0f, 1.0f); | ||
| if (currentFrame >= anim.keyframeCount) currentFrame = currentFrame%anim.keyframeCount; | ||
| if (nextFrame >= anim.keyframeCount) nextFrame = nextFrame%anim.keyframeCount; | ||
| if (currentFrame >= anim.keyframeCount) currentFrame = currentFrame % anim.keyframeCount; |
There was a problem hiding this comment.
Please, could you review formatting to follow raylib conventions.
| // Compute runtime bone matrix from model current pose | ||
| //----------------------------------------------------------------------------------- | ||
| Transform *bindPoseTransform = &model.skeleton.bindPose[boneIndex]; | ||
| Transform* bindPoseTransform = &model.skeleton.bindPose[boneIndex]; |
There was a problem hiding this comment.
Please, could you review formatting to follow raylib conventions.
| static void UpdateModelAnimationVertexBuffers(Model model); | ||
|
|
||
| // Lazy allocation of CPU animation buffers for vertex positions and normals, used for software skinning | ||
| static void AllocateMeshCPUAnimBuffers(Mesh* mesh) |
There was a problem hiding this comment.
As far it is only used once, I prefert to avoid an extra function, code can be used inside function.
There was a problem hiding this comment.
Yes I can make that change. I just didn't want to make the outer function much larger.
4 participants
This PR enables both GPU and CPU skinning to be used at the same time in the same build of raylib.
It uses the state of the shader instead of the build flag at load time to know if it should use the CPU animation buffers or not.
The update functions now lazy allocate the CPU side animation buffers only when they are needed (this has a side effect of saving memory on animatable models that are never drawn animated).
The update code also checks if the shader is going to use the bones, and if so, skips the CPU side buffers.
I also split the update functions into two parts, one that does just the bone math and one that does it all. This allows the user to update just the bones if they want, and not even try to do the CPU buffers.
This all allows animation to always 'just work' with a correct fallback to CPU is GPU is not supported, and only allocating the CPU side buffers if they are needed.