Skip to content

[3.x] Physics Interpolation - Reduce unnecessary VisualServer updates#104854

Merged
lawnjelly merged 1 commit intogodotengine:3.xfrom
lawnjelly:fti_reduce_server_updates
Apr 7, 2025
Merged

[3.x] Physics Interpolation - Reduce unnecessary VisualServer updates#104854
lawnjelly merged 1 commit intogodotengine:3.xfrom
lawnjelly:fti_reduce_server_updates

Conversation

@lawnjelly
Copy link
Copy Markdown
Member

With the new SceneTreeFTI, most xforms are updated to the server externally by the FTI system, so it is no longer necessary to update the server on each NOTIFICATION_TRANSFORM_CHANGED.

Each call to VisualServer::get_singleton()->instance_set_transform() incurs some expense, so it is sensible to reduce these when they are superfluous (as a later update will be made by SceneTreeFTI immediately prior to rendering).

Notes

  • I didn't put this in the main PR both to keep it clear and free from excessive optimization, and because I wanted to look at this in isolation. There is the potential for regressions here so we can easily revert or bisect a self contained PR.
  • With the old FTI I had previously looked at preventing propagation into non-interpolated branches ([3.x] Physics Interpolation - reduce xforms in non-interpolated nodes #103331) but with the new technique this is probably no longer appropriate, not least because we need to set DIRTY_GLOBAL_INTERPOLATED.
  • Most of these NOTIFICATION_TRANSFORM_CHANGED will be no-ops for VisualInstances in this situation (which isn't ideal), however we need to (for now) allow any derived classes the opportunity to respond to the notification. This can possibly be further optimized in the future.

With the new `SceneTreeFTI`, most xforms are updated to the server externally by the FTI system, so it is no longer necessary to update the server on each `NOTIFICATION_TRANSFORM_CHANGED`.
Copy link
Copy Markdown
Member

@rburing rburing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine given the frame/tick update calls recently added to SceneTree.

@lawnjelly lawnjelly merged commit 4d6605b into godotengine:3.x Apr 7, 2025
14 checks passed
@lawnjelly
Copy link
Copy Markdown
Member Author

Thanks!

@lawnjelly lawnjelly deleted the fti_reduce_server_updates branch April 7, 2025 06:26
@lawnjelly
Copy link
Copy Markdown
Member Author

lawnjelly commented Apr 7, 2025

Ah as I suspected there was a risk of regression here and I'm seeing a visual change in Wrought Flesh, I'll see if I can pin it down and do a fix.

Likely a loading case where we need to call notify_change.

UPDATE:
Ah, no I've got it, SceneTreeFTI is currently set to ignore the notify change for non-interpolated nodes (and this PR removes sending them directly to the server, so they never get sent sometimes lol 😊 ). Will fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants