Extend skinning for >4 bones per vertex #6772
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
can you sign the CLA btw -- I know it's moot, but this way we can commit the changes. |
|
Would you be able to rebase the change so it builds on main again? thanks! |
|
Also please add a line in |
|
@poweifeng @bejado I think we're super close to being able to land this PR, I'd like a one last round of going over it, since it's such a large change. Thank you! |
Will take another look |
| for (auto iBonePair = mBonePairs.begin(); iBonePair != mBonePairs.end(); ++iBonePair) { | ||
| auto primitiveIndex = iBonePair->first; | ||
| auto bonePairsForPrimitive = iBonePair->second; | ||
| if (bonePairsForPrimitive.size()) { |
There was a problem hiding this comment.
nit: invert the condition and do a continue to reduce nesting
if (!bonePairsForPimiritve.size()) {
continue;
}
There was a problem hiding this comment.
That's clearer, thank you. Done
| auto bonePairsForPrimitive = iBonePair->second; | ||
| if (bonePairsForPrimitive.size()) { | ||
| size_t vertexCount = mEntries[primitiveIndex].vertices->getVertexCount(); | ||
| std::unique_ptr<uint16_t[]> skinJoints( |
There was a problem hiding this comment.
Done, sorry I forgot to reply, thank you
Texture size calculation fixed Memory allocation for texture data fixed
pixelflinger
force-pushed
the
skinning
branch
from
87ec534 to
0ca2928
Compare
|
@fvbj the android/ios (GLES shader issue) and windows builds are failing (C++ issue), FYI. |
# Conflicts: # NEW_RELEASE_NOTES.md
|
@pixelflinger Everything looks good, what can I do to finalize the PR? Thank you. |
|
@poweifeng @romainguy @bejado @fvbj I'm planning to merge this on Monday after we branch the release. Let me know if there are any concerns. |
|
@fvbj it looks like several targets are not building. |
|
The failures seem due to a reliance on an implicit conversion: |
uint is not a standard type, use size_t instead
|
@fvbj can you rebase one more time, hopefully there won't be conflicts tomorrow morning when I try to merge it. Thanks! |
# Conflicts: # NEW_RELEASE_NOTES.md
* Add skinning and morphing samples to check functionality * Implement skinning for more than four bones pair vertex The API allows defining an unlimited number of bone indices and weights of primitives. Data is defined in building process of the renderable manager. Backward compatibility with the original solution. Skinning of vertices is calculated on GPU, data is transferred to the vertex shader in the texture.
Improve bone skinning for more than four bones per vertex