Feature/model node animation#136
Open
Roxeena wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for addressing model nodes by name and applying custom transforms to internal nodes, including cache/binary format updates to persist node names.
Changes:
- Bumps model/cache versions and serializes/deserializes node names.
- Stores Assimp node names in
ModelNode. - Adds custom node transform APIs and applies them during rendering.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/io/model/modelreaderbinary.cpp |
Reads node names from updated binary model files while retaining older format support. |
src/io/model/modelreaderassimp.cpp |
Populates ModelNode names from Assimp nodes. |
src/io/model/modelnode.cpp |
Stores node names and custom transforms on model nodes. |
src/io/model/modelgeometry.cpp |
Persists node names in cache files and applies custom node transforms during rendering. |
include/ghoul/io/model/modelnode.h |
Exposes node name and custom transform accessors/mutators. |
include/ghoul/io/model/modelgeometry.h |
Adds public APIs for updating custom node transforms and override behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1197
to
+1200
| for (io::ModelNode& node : _nodes) { | ||
| if (node.name() == nodeName) { | ||
| node.updateCustomTransform(customTransform); | ||
| return; |
Member
Author
There was a problem hiding this comment.
I'm not sure if we want to custom transform to affect the bounding radius... what do others think?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This makes it possible to add a transformation to an internal model node for a 3D model. This also requires all nodes to store the name, therefor the caching format has been updated. The user can choose to either replace the existing transformation with the custom one or add it on top of the existing one with a parameter.