-
-
Notifications
You must be signed in to change notification settings - Fork 22.8k
FTI - global_transform_interpolated()
on demand for invisible nodes
#107308
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
FTI - global_transform_interpolated()
on demand for invisible nodes
#107308
Conversation
can confirm this fixes my game and my test cases |
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.
A different solution might be to add a flag for visible nodes to interpolate even if they're invisible. That would save additional processing if they're queried each frame anyway.
Still, in this case, it would be questionable what to do when the interpolated position is queried if the node wasn't set to calculate interpolated frames. So we may need this implementation here anyway.
I did consider this first, but if you think about it, the logic for doing this automatically is quite difficult. We also don't want to put the burden of this on the user (if you were thinking manual user flag, as I had considered it in the past). Bug potentialYou would have to know which invisible chain to calculate ahead of time, deal with logic for turning this on and off, it's pretty complex and bug prone. We've had to deal with similar before for client side FTI, and it was a bit of a nightmare .. one of the benefits of the new system is we removed all these bugs. On demand needs to be availableEssentially the user can call Performance considerations
I'm not sure it will actually save processing (unless it is called multiple times currently, because result not cached), it just shifts it from on demand to Calculating on demand is far simpler conceptually, and should only be done rarely anyway ( Different PR / ProposalImo exploring the approach you mention of getting the Essentially my thoughts are:
|
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.
Looks good to me for the moment. There's some things that could be better, but as discussed that depends on other PRs moving forward. The state of the code is better after the PR than before, so let's merge.
Thanks! |
Fixes #107235
Forward port of #107307
Explanation
For visible nodes, the interpolated transform is already cached and calculated in
SceneTreeFTI
. However, invisible nodes do not calculate interpolated transforms (in order to save processing), thus getting their interpolated transform directly is not possible.Instead we add mechanism to calculate this interpolated transform on the fly. This will be more expensive than for visible nodes, but can be handy in some situations.
Notes
_is_vi_visible()
toVisualInstances
is getting a little tiresome now, so I have a follow up PR (Pre-calculateis_visible_in_tree()
#107330) to change the calculation and caching ofis_visible_in_tree()
, but that should be considered as a separate issue, and this PR will run fine without it.