Skip to content

Ensure frame_post_draw signal disconnect properly #39

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.

Sign up for GitHub

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

Merged
unvermuthet merged 2 commits into from
Dec 27, 2025
Merged

Ensure frame_post_draw signal disconnect properly #39

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1a5b3b0
Ensure frame_post_draw signal disconnect properly
cyberpsyche Dec 17, 2025
149fa6f
delete the comment line in viewport_texture_router.cpp
cyberpsyche Dec 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/viewport_texture_router.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/

#include "viewport_texture_router.hpp"
#include "ndi.hpp"

#include <godot_cpp/classes/rendering_server.hpp>
#include <godot_cpp/classes/viewport_texture.hpp>
Expand All @@ -18,6 +19,8 @@ ViewportTextureRouter::ViewportTextureRouter() {
}

ViewportTextureRouter::~ViewportTextureRouter() {
// Ensure the disconnection from the frame_post_draw signal if it's connected
SIGNAL_DISCONNECT(RenderingServer::get_singleton(), "frame_post_draw", callable_mp(this, &ViewportTextureRouter::request_textures));
Comment on lines +22 to +23
Copy link
Owner

@unvermuthet unvermuthet Dec 20, 2025

Choose a reason for hiding this comment

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

I'm a bit nervous about this. The ViewportTextureRouter is a core-level engine singleton. This basically lives as long as the whole engine, so I don't see a practical reason. I think by the time this is called, all NDIOutputs should be gone. And so, the last viewport will have been removed, and the signal will have been disconnected. I guess the only way this causes harm is if the RenderingServer is already gone when this is called. Did you test the impact? Any console errors when closing the editor or game? If this isn't part of fixing the error from #38, I'd prefer to leave it as is.

}

void ViewportTextureRouter::add_viewport(Viewport *viewport) {
Expand Down Expand Up @@ -46,7 +49,7 @@ void ViewportTextureRouter::remove_viewport(Viewport *viewport) {

// Last viewport removed, disconnect from the frame_post_draw signal
if (vps.size() == 0) {
RenderingServer::get_singleton()->disconnect("frame_post_draw", callable_mp(this, &ViewportTextureRouter::request_textures));
SIGNAL_DISCONNECT(RenderingServer::get_singleton(), "frame_post_draw", callable_mp(this, &ViewportTextureRouter::request_textures));
}

print_verbose("NDI: ViewportTextureRouter registrations: ", vps);
Expand Down