Repository: godotengine/godot (language: cpp)

Issue:
Race Condition in `EmbeddedProcess::_try_embed_process`
### Tested versions

4.4-dev7

### System information

Godot v4.4.dev (f2d97462a) - Windows 11 (build 26100) - Multi-window, 2 monitors - Direct3D 12 (Forward+) - dedicated NVIDIA GeForce RTX 4080 (NVIDIA; 32.0.15.6614) - Intel(R) Core(TM) i9-14900K (32 threads)

### Issue description

There seems to be a race condition somewhere when embedding the game. When running things normally the Game tab shows nothing around 80% of the time. Resizing and none of the buttons in the game tab change anything. If I put a break point on https://github.com/godotengine/godot/blob/bdf625bd54958c737fa6b7213b07581cc91059ad/editor/plugins/embedded_process.cpp#L189 and wait a bit the window gets correctly shown all the time. One other thing I noticed was that if I put godot on my second monitor and launched the game it would show the game on my primary monitor where it should be drawn when it would have been embedded in the editor when the embedding "fails".

I wasn't able to narrow this down further yet but the error branches are never entered even if the Game tab does not display anything. 

### Steps to reproduce

- enable "Embed Game on Next Play"
- hit play

### Minimal reproduction project (MRP)

-


Discussion / hints:
@0x0ACB thanks for testing and reporting this issue.
Unfortunately, I'm not able to reproduce on Windows 11 with 2 monitors.
Is "Make Game Window Floating" enabled? I guess not, just want to be sure.
Did you try with an empty project?
Are you in single window mode?

The fact that the window appears on the wrong monitor is troubing. It seems that the embedding control is not able to get the correct embedded control global position. Can you check the values returned by `get_screen_embedded_window_rect()`? Are the coords correct?

OK so I had a bit more time to do some more digging and I also remembered a similar issue I had in the past when using godot as an overaly. The y coordinate for the window is calculated incorrectly. My two monitors are stacked vertically not horizontally and it seems that godot has trouble calculating the correct global position.

When embedding is successful with godot running on the bottom monitor or godot is running on my top monitor and the embedding fails. The global position of the game window ends up as: `(1870, 1603)`. When godot is running on the bottom monitor and embedding fails the global position of the game is: `(1767, 3031)`.

> "Make Game Window Floating"

I had it disabled for my latest tests but the not showing up on monitor 1 issue does exist whether it is enabled or not. With it enabled the floating window will appear on the bottom monitor though if the godot editor is on the top.

> Did you try with an empty project?

I can reproduce the issue with an empty project as well yes.

> Are you in single window mode?

no

> Can you check the values returned by get_screen_embedded_window_rect()? Are the coords correct?

The coords do seem to be correct however the actual position of the game window does not match the result of `get_screen_embedded_window_rect()`. The call to `SetPosition` in `DisplayServerWindows::embed_process` also applies the same coordinates to the window whether embedding succeeds or fails. What I did notices was that even though `SetPosition` never returns `0/FALSE` the window position of the game does not update when embedding failed. I also noticed that when embedding fails the game is not closed when closing the editor.


I can offer you the code and workarounds I am using for my Overlay class I wrote for godot which has fixes for the issue of appearing on the wrong monitor or not correctly being overlayed on top of the target window maybe it helps. Of particular note is the need to hide the window before applying the styles and then showing the window again since otherwise the calls to `SetWindowLong(PtrW)` might not take effect immediately. Also I noticed that you are using `SetParent` to set the parent window. This has some issues when compared to `SetWindowLongPtrW(m_engineWnd, GWLP_HWNDPARENT, std::bit_cast<LONG_PTR>(m_targetWindow));` in my experience because it attaches the two input queues for the windows together when they run on different threads.

```C++
void overlay::OverlayWIN32::match_size() const
{
    print_verbose("Matching window");
    RECT rect;
    ERR_FAIL_COND(GetClientRect(m_targetWindow, &rect) == FALSE);
    MapWindowPoints(m_targetWindow, nullptr, reinterpret_cast<POINT *>(&rect), 2);

    const HMONITOR hMonitor = MonitorFromWindow(m_targetWindow, MONITOR_DEFAULTTONEAREST);
    MONITORINFO mi = { sizeof(mi) };
    GetMonitorInfoW(hMonitor, &mi);

    const Rect2i screen_rect(mi.rcMonitor.left, mi.rcMonitor.top, mi.rcMonitor.right - mi.rcMonitor.left, mi.rcMonitor.bottom - mi.rcMonitor.top);
    const Rect2i window_rect(rect.left, rect.top, rect.right - rect.left, rect.bottom - rect.top);
    print_verbose("Screen rect: " + screen_rect + " Window rect: " + window_rect);

    if(window_rect.position == screen_rect.position && window_rect.size == screen_rect.size)
    {
        // prevent fullscreen
        print_verbose("Preventing fullscreen");
        rect.left += 1;
        rect.top += 1;
        rect.right -= 1;
        rect.bottom -= 1;
    }
    ERR_FAIL_COND(SetWindowPos(m_engineWnd, nullptr, rect.left, rect.top, rect.right - rect.left, rect.bottom - rect.top, SWP_NOZORDER | SWP_NOACTIVATE | SWP_ASYNCWINDOWPOS) == FALSE);
}


void overlay::OverlayWIN32::attach()
{
    ERR_FAIL_COND(m_attached);
    m_attached = true;

    const auto display = DisplayServer::get_singleton();
    m_engineWnd = std::bit_cast<HWND>(display->window_get_native_handle(DisplayServer::HandleType::WINDOW_HANDLE));

    ShowWindow(m_engineWnd, SW_HIDE);

    bool hasWindow = false;
    if(m_targetWindow)
    {
        DWORD pid = 0;
        if (GetWindowThreadProcessId(m_targetWindow, &pid) != 0)
        {
            if (pid == client::get_instance()->get_process_id())
            {
                hasWindow = true;
            }
        }
    }

    if(hasWindow)
    {
        print_line("Reattaching to existing window");
        MessageQueue::get_singleton()->push_callable(callable_mp(this, &OverlayWIN32::deferred_attach));
        return;
    }

    std::thread([this]()
    {
        print_line("Waiting for window");
        std::optional<ipc::actions::window_created> windowCreated;
        while (!windowCreated)
        {
            windowCreated = ipc::shared_memory::get_instance()->receive_action<ipc::actions::window_created>();
            std::this_thread::sleep_for(std::chrono::milliseconds(100));
        }
        m_targetWindow = std::bit_cast<HWND>(windowCreated->hwnd);
        MessageQueue::get_singleton()->push_callable(callable_mp(this, &OverlayWIN32::deferred_attach));
    }).detach();
}


void overlay::OverlayWIN32::deferred_attach()
{
    print_line("Attaching");

    const auto display = DisplayServer::get_singleton();
    m_engineWnd = std::bit_cast<HWND>(display->window_get_native_handle(DisplayServer::HandleType::WINDOW_HANDLE));

    // disable all kinds of input buffering
    Input::get_singleton()->set_use_accumulated_input(false);
    Input::get_singleton()->set_agile_input_event_flushing(false);

    // Attach to the window
    display->window_set_flag(DisplayServer::WINDOW_FLAG_BORDERLESS, true);
    display->window_set_flag(DisplayServer::WINDOW_FLAG_TRANSPARENT, true);
    SceneTree::get_singleton()->get_root()->set_transparent_background(true);

    auto exstyle = GetWindowLongW(m_engineWnd, GWL_EXSTYLE);
    exstyle = exstyle & ~WS_EX_APPWINDOW & ~WS_EX_TOOLWINDOW;
    exstyle = exstyle | WS_EX_LAYERED | WS_EX_TRANSPARENT | WS_EX_NOACTIVATE;
    SetWindowLongW(m_engineWnd, GWL_EXSTYLE, exstyle);
    SetWindowLongPtrW(m_engineWnd, GWLP_HWNDPARENT, std::bit_cast<LONG_PTR>(m_targetWindow));

    match_size();

    // create input handler window
    static const char* cls_name = "overlay_message";
    UnregisterClassA(cls_name, nullptr);
    WNDCLASSEX wx = { 0 };
    wx.cbSize = sizeof(WNDCLASSEX);
    wx.lpfnWndProc = &OverlayWIN32::StaticMessageWndProc;
    wx.hInstance = nullptr;
    wx.lpszClassName = cls_name;
    CRASH_COND(!RegisterClassExA(&wx));
    m_messageWnd = CreateWindowExA(0, wx.lpszClassName, cls_name, 0, 0, 0, 0, 0, HWND_MESSAGE, nullptr, nullptr, nullptr);
    CRASH_COND(!m_messageWnd);

    ipc::write::window_info windowInfo;
    windowInfo.godot_hwnd = std::bit_cast<uint64_t>(m_engineWnd);
    windowInfo.message_hwnd = std::bit_cast<uint64_t>(m_messageWnd);
    windowInfo.want_input = false;
    display->cursor_set_shape(DisplayServer::CURSOR_BUSY);
    display->cursor_set_shape(DisplayServer::CURSOR_ARROW);
    windowInfo.cursor = reinterpret_cast<uint64_t>(GetCursor());
    ipc::shared_memory::get_instance()->set_window_info(windowInfo);

    ShowWindow(m_engineWnd, SW_SHOW);
    SetActiveWindow(m_engineWnd);

    m_wantInput = true;
    print_line("Attached");
    emit_signal(SNAME("attached"));
}
```
Thanks a lot for that, I'll take take a closer look to your answer and try to fix the issue!

Fix patch:
diff --git a/platform/linuxbsd/x11/display_server_x11.cpp b/platform/linuxbsd/x11/display_server_x11.cpp
index 540726fa6dc7..f03cd296e078 100644
--- a/platform/linuxbsd/x11/display_server_x11.cpp
+++ b/platform/linuxbsd/x11/display_server_x11.cpp
@@ -5718,7 +5718,7 @@ Error DisplayServerX11::embed_process(WindowID p_window, OS::ProcessID p_pid, co
 			}
 		}
 
-		if (desired_rect.size.x < 100 || desired_rect.size.y < 100) {
+		if (desired_rect.size.x <= 100 || desired_rect.size.y <= 100) {
 			p_visible = false;
 		}
 
diff --git a/platform/windows/display_server_windows.cpp b/platform/windows/display_server_windows.cpp
index 3c1a3915217a..68a4b998c226 100644
--- a/platform/windows/display_server_windows.cpp
+++ b/platform/windows/display_server_windows.cpp
@@ -2876,10 +2876,7 @@ static BOOL CALLBACK _enum_proc_find_window_from_process_id_callback(HWND hWnd,
 	GetWindowThreadProcessId(hWnd, &process_id);
 	if (ed.process_id == process_id) {
 		if (GetParent(hWnd) != ed.parent_hWnd) {
-			const DWORD style = GetWindowLongPtr(hWnd, GWL_STYLE);
-			if ((style & WS_VISIBLE) != WS_VISIBLE) {
-				return TRUE;
-			}
+			return TRUE;
 		}
 
 		// Found it.
@@ -2935,24 +2932,9 @@ Error DisplayServerWindows::embed_process(WindowID p_window, OS::ProcessID p_pid
 		ep->is_visible = (style & WS_VISIBLE) == WS_VISIBLE;
 
 		embedded_processes.insert(p_pid, ep);
-
-		HWND old_parent = GetParent(ep->window_handle);
-		if (old_parent != wd.hWnd) {
-			// It's important that the window does not have the WS_CHILD flag
-			// to prevent the current process from interfering with the embedded process.
-			// I observed lags and issues with mouse capture when WS_CHILD is set.
-			// Additionally, WS_POPUP must be set to ensure that the coordinates of the embedded
-			// window remain screen coordinates and not local coordinates of the parent window.
-			if ((style & WS_CHILD) == WS_CHILD || (style & WS_POPUP) != WS_POPUP) {
-				const DWORD new_style = (style & ~WS_CHILD) | WS_POPUP;
-				SetWindowLong(ep->window_handle, GWL_STYLE, new_style);
-			}
-			// Set the parent to current window.
-			SetParent(ep->window_handle, wd.hWnd);
-		}
 	}
 
-	if (p_rect.size.x < 100 || p_rect.size.y < 100) {
+	if (p_rect.size.x <= 100 || p_rect.size.y <= 100) {
 		p_visible = false;
 	}
 
@@ -2961,7 +2943,7 @@ Error DisplayServerWindows::embed_process(WindowID p_window, OS::ProcessID p_pid
 	// (e.g., a screen to the left of the main screen).
 	const Rect2i adjusted_rect = Rect2i(p_rect.position + _get_screens_origin(), p_rect.size);
 
-	SetWindowPos(ep->window_handle, HWND_BOTTOM, adjusted_rect.position.x, adjusted_rect.position.y, adjusted_rect.size.x, adjusted_rect.size.y, SWP_NOZORDER | SWP_NOACTIVATE);
+	SetWindowPos(ep->window_handle, nullptr, adjusted_rect.position.x, adjusted_rect.position.y, adjusted_rect.size.x, adjusted_rect.size.y, SWP_NOZORDER | SWP_NOACTIVATE | SWP_ASYNCWINDOWPOS);
 
 	if (ep->is_visible != p_visible) {
 		if (p_visible) {


Test patch: