Skip to content
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

Remove dead code from DisplayServerWindows::window_set_size #86029

Merged
merged 1 commit into from
May 30, 2024
Merged
Changes from all commits
Commits
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
26 changes: 8 additions & 18 deletions platform/windows/display_server_windows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1832,24 +1832,6 @@ void DisplayServerWindows::window_set_size(const Size2i p_size, WindowID p_windo

int w = p_size.width;
int h = p_size.height;

wd.width = w;
wd.height = h;

#if defined(RD_ENABLED)
if (rendering_context) {
rendering_context->window_set_size(p_window, w, h);
}
#endif
#if defined(GLES3_ENABLED)
if (gl_manager_native) {
gl_manager_native->window_resize(p_window, w, h);
}
if (gl_manager_angle) {
gl_manager_angle->window_resize(p_window, w, h);
}
#endif
Comment on lines -1844 to -1851
Copy link
Member

@bruvzg bruvzg May 28, 2024

Choose a reason for hiding this comment

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

GL resize part (even if it's not really doing anything right now) probably should be moved to WM_WINDOWPOSCHANGED instead of removing it, currently it's only calling RD window_set_sized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, to be fair I didn't retest this, but when I initially pushed this PR it worked just fine without. And it still should, the window can be resized externally without ever calling window_set_size so the code to handle resizing needs to be already present somewhere else.

Copy link
Member

Choose a reason for hiding this comment

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

In the Windows GL manager implementation these functions do nothing, so it should work fine without it. But I think it should call it from WM_WINDOWPOSCHANGED anyway, just in case GL manager ever change, and for consistency with other platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see what you mean. Will add it


RECT rect;
GetWindowRect(wd.hWnd, &rect);

Expand Down Expand Up @@ -4640,6 +4622,14 @@ LRESULT DisplayServerWindows::WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARA
// Note: Trigger resize event to update swapchains when window is minimized/restored, even if size is not changed.
rendering_context->window_set_size(window_id, window.width, window.height);
}
#endif
#if defined(GLES3_ENABLED)
if (gl_manager_native) {
gl_manager_native->window_resize(window_id, window.width, window.height);
}
if (gl_manager_angle) {
gl_manager_angle->window_resize(window_id, window.width, window.height);
}
#endif
}

Expand Down
Loading