-
-
Notifications
You must be signed in to change notification settings - Fork 21.3k
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
Do not use NOTIFICATION_PHYSICS_PROCESS
for 2D Editor
#87886
Do not use NOTIFICATION_PHYSICS_PROCESS
for 2D Editor
#87886
Conversation
It might have been done this way so that the method is only called once if multiple objects are selected. If you are snapping hundreds of objects at once in a complex scene, it may have a noticeable performance impact. |
Ok I think I have an explanation, after going through git blame which I should have done in the first place. 2D being on physics ticks has been the case since the dawn of time, and I don't see any reason for it to continue to be. 3D on the other hand... Was apparently to fix a snapping and an instantiating issue when 'Run on Separate Thread' is used. This isn't ideal because it ties viewport updates in the editor to physics ticks, but I think this is now beyond the scope of the PR to fix and can be looked at later. I'm going to descope 3D and make this a 2D only update. I've looked through the code and found no raycast/collision related code in 2D, so I think this is a safe update. |
3d96935
to
88b1103
Compare
NOTIFICATION_PHYSICS_PROCESS
for 2D Editor
@@ -3917,7 +3917,7 @@ void CanvasItemEditor::_notification(int p_what) { | |||
EditorRunBar::get_singleton()->connect("stop_pressed", callable_mp(this, &CanvasItemEditor::_update_override_camera_button).bind(false)); | |||
} break; | |||
|
|||
case NOTIFICATION_PHYSICS_PROCESS: { | |||
case NOTIFICATION_PROCESS: { | |||
EditorNode::get_singleton()->get_scene_root()->set_snap_controls_to_pixels(GLOBAL_GET("gui/common/snap_controls_to_pixels")); |
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.
It's a pre-existing issue, but re-setting this every frame (and thus re-querying the project settings for it) seems quite wasteful.
I would move it to the constructor, and connect to ProjectSettings.settings_changed
to update.
Thanks! |
Edit: 3D descoped, see comments below.
Fixes: #87885
The editor plugins shouldn't use physics process for thing unrelated to physics for reasons explained in the attached issue.
This PR also removes unnecessary method to snap to floor that was part of a physics process2024-02-02.22-35-12.mp4