-
Notifications
You must be signed in to change notification settings - Fork 420
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
Add skinning support for gfx-replay and legacy replay rendering. #2120
Conversation
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.
Good progress! I left comments. Let me know when I should look again!
@@ -150,9 +158,9 @@ bool fromJsonValue(const JsonGenericValue& obj, | |||
metadata::attributes::ObjectInstanceShaderType& x) { | |||
std::string shaderTypeToUseString; | |||
// read as string | |||
bool shaderTypeSucceess = fromJsonValue(obj, shaderTypeToUseString); | |||
bool shaderTypeSuccess = fromJsonValue(obj, shaderTypeToUseString); |
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 for fixing all the spelling typos!
@@ -945,5 +938,40 @@ void BulletPhysicsManager::removeRigidConstraint(int constraintId) { | |||
} | |||
} | |||
|
|||
void BulletPhysicsManager::instantiateSkinnedModel( |
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.
@aclegg3 maybe you wanna take a look at changes to BulletPhysicsManager.
/** | ||
* @brief The next available unique ID for instantiated rigs. | ||
*/ | ||
int _nextRigInstanceID = 0; |
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.
In the future we could revisit this so that it managed IDs similar to how PhysicsManager does, reusing freed IDs instead of just incrementing, on the off chance that the RigManager may persist through billions of training cycles.
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.
LGTM. Always nice to see some things move out of ResourceManager that can.
FYI - Tests were passing prior to rebasing to main due to our CI image losing support.
The last run was green. |
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.
LGTM!
Motivation and Context
This changeset adds support for:
Capture from the SIRo sandbox tool, which renders using the classic replay renderer:
fetch-2023-09-29_19.20.31.mp4
How it works
Overview
This changeset introduces a new structure,
Rig
, which contains a node for each bone of a skinned model. During asset instantiation, theRig
is associated to the model instance so that it can be posed during rendering.Therefore, the skinned model is animated by modifying transforms of the
Rig
bone nodes. This is currently done in either of those ways:rigUpdate.pose
in gfx-replay keyframes.Gfx-replay
Two new keyframe fields are introduced:
Additionally, RenderAssetCreationInfo now has a rigId field.
How Has This Been Tested
Types of changes
Checklist