-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[rmodels] Optional GPU skinning #4321
Conversation
I checked and currently this does break the examples and the existing model rendering when no shader is supplied. I'm investigating why now. |
…examples. Added gpu skinning on drawing of instanced meshes.
The issue appeared to be inserting the new shader uniform locations into the middle of the shader locations enum. Perhaps someone with more experience can move them into a more logical position but I'm not sure right now what may be hard-coded in raylib which relies on the current locations and needs updating. I will also try to prepare a small example for the examples folder. |
Okay I added a small example |
@orangeduck Hi! Thank you very much for this improvement! It has actually been requested many times by raylib community! The main concern it was not added before was the lack of support for UBOs by some OpenGL versions but as per my understanding, this solution can work with any OpenGL version. Just let me some days to review it more carefully, specifically the In any case, I'm really confident on merging this improvement, after all you are probably the developer that has pushed further raylib 3d models animations! |
@raysan5 Brilliant, thank you! Yes I think this can work on any OpenGL version. I believe the main limitation to this approach is the The other downside is of course that for animated models that use CPU skinning you will now pay the additional VRAM cost for uploading the Thanks again! |
Well, that can be addressed with a Also, in case it is enabled + we are on a limited platform (OpenGL ES 2.0) + an animated model loaded has >MAX_GPU_BONE_COUNT, a warning TraceLog() message can be shown, just in case... |
src/rmodels.c
Outdated
|
||
model.meshes[i].boneCount = model.boneCount; | ||
model.meshes[i].boneMatrices = RL_CALLOC(model.meshes[i].boneCount, sizeof(Matrix)); | ||
for (int j = 0; j < model.meshes[i].boneCount; j++) |
Check notice
Code scanning / CodeQL
Declaration hides variable Note
line 6349
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.
@orangeduck Hi Daniel, it seems this could be a potential issue, this variable should be probably k
.
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.
Thanks! I took a look and I think it does look safe to use j
here I just need to remove the variable declaration as all the iterator variables are declared at the top of the function.
@orangeduck About to merge this great improvement, just a small issue detected by the automatic analysis system. |
@orangeduck Thank you very much for this great improvement! 😄 |
This adds a form of optional GPU skinning to raylib that doesn't interfere with the existing CPU skinning method, but does expose enough information for a user to write their own GPU skinning in shader code if they want.
Essentially the change works as follows:
boneIds
andboneWeights
vertex data are now uploaded to the GPU along with the rest of the vertex data.boneMatrices
array which is uploaded to the shader as a new uniform and is used to contain the skinning bone transformation matrices for that mesh. These matrices can be updated using theUpdateModelAnimationBoneMatrices
function which essentially acts as a replacement to theUpdateModelAnimation
function when you want to do GPU skinning.And that is about it. The normal CPU skinning continues to work as intended where users call
UpdateModelAnimation
. However with this update users can instead callUpdateModelAnimationBoneMatrices
and write their own shader which does the skinning. That might look as follows:I have tested this in my own little demo but I have not yet run through to see if it breaks anything else or to check it works with all the existing demos. That is still to do!
I know your intention is to keep raylib simple and minimal and this update is a little hacky in some senses so obviously feel free to close it if you think it is not the right approach or not useful. I just thought I would share it in case you think it might be valuable.
Many thanks in advance!
Dan