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

Window minimize crash fix: #37327 #37382

Conversation

ThakeeNathees
Copy link
Contributor

Fix: #37327

@@ -1093,6 +1093,7 @@ bool DisplayServerWindows::can_any_window_draw() const {
_THREAD_SAFE_METHOD_

for (Map<WindowID, WindowData>::Element *E = windows.front(); E; E = E->next()) {
if (E->key() == -1) continue;
Copy link
Member

@bruvzg bruvzg Mar 28, 2020

Choose a reason for hiding this comment

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

Should be INVALID_WINDOW_ID (which is -1) instead of -1.

Probably there's missing check for INVALID_WINDOW_ID somewhere in the window creation, AFAIK it should not be in the map.

Copy link
Member

Choose a reason for hiding this comment

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

Some missing checks I found:

WindowID window_id = _create_window(p_mode, p_flags, p_rect);
WindowData &wd = windows[window_id];

_create_window can return -1

WindowID window_id = INVALID_WINDOW_ID;
for (Map<WindowID, WindowData>::Element *E = windows.front(); E; E = E->next()) {
if (E->get().hWnd == hWnd) {
window_id = E->key();
break;
}
}

Can potentially use -1 if window is not in the map (window is created but not added to the map, or removed from the map but still closing, not sure if any of this is possible).

@ThakeeNathees ThakeeNathees force-pushed the window-minimize-crash-fix branch from 61f811a to 4ca4f39 Compare March 28, 2020 13:46
@akien-mga akien-mga changed the title Window minimize carsh fix: #37327 Window minimize crash fix: #37327 Mar 28, 2020
@akien-mga akien-mga requested a review from reduz March 28, 2020 16:43
@reduz
Copy link
Member

reduz commented Mar 28, 2020

Fix does not look correct, I have the feeling the problem is the creation flags for the minimized window..

@ThakeeNathees
Copy link
Contributor Author

ThakeeNathees commented Mar 28, 2020

after the @bruvzg 's review I think that this is not a proper fix. I've just implemented some error fails for the crash and it worked. this might be a temporary fix.
@reduz

@HaSa1002
Copy link
Contributor

You should catch it in the WndProc function with

if (window_id == INVALID_WINDOW_ID)
    return;

@marstaik
Copy link
Contributor

@reduz @HaSa1002 I actually think this PR is correct. There are no windows creation flags for Vulkan, seen here: https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkWin32SurfaceCreateFlagsKHR.html

Also, according to here: https://www.khronos.org/registry/vulkan/specs/1.2-extensions/html/vkspec.html#VkWin32SurfaceCreateFlagsKHR

Note

Due to above restrictions, it is only possible to create a new swapchain on this platform with imageExtent being equal to the current size of the window.

The window size may become (0, 0) on this platform (e.g. when the window is minimized), and so a swapchain cannot be created until the size changes.

which the code does, it sees windows size being 0,0 and returns. But there is no code for handling a nullptr swapchain further up.

@KoBeWi
Copy link
Member

KoBeWi commented May 15, 2021

The original issue is already fixed. Thanks for the contribution nonetheless.

@KoBeWi KoBeWi closed this May 15, 2021
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.

Editor crash when minimizing on Windows (crash in VulkanContext::prepare_buffers)
7 participants