Merge pull request #60894 from derammo/derammo_opengl3_windows

drivers/gles3/rasterizer_gles3.cpp

@@ -210,6 +210,9 @@ RasterizerGLES3::RasterizerGLES3() {
 #ifdef GLAD_ENABLED
 	if (!gladLoadGL()) {
 		ERR_PRINT("Error initializing GLAD");
+		// FIXME this is an early return from a constructor.  Any other code using this instance will crash or the finalizer will crash, because none of
+		// the members of this instance are initialized, so this just makes debugging harder.  It should either crash here intentionally,
+		// or we need to actually test for this situation before constructing this.
 		return;
 	}
 #endif
@@ -285,13 +288,17 @@ void RasterizerGLES3::_blit_render_target_to_screen(RID p_render_target, Display
 
 	// TODO: do we need a keep 3d linear option?
 
+	// Make sure we are drawing to the right context.
+	DisplayServer::get_singleton()->gl_window_make_current(p_screen);
+
 	if (rt->external.fbo != 0) {
 		glBindFramebuffer(GL_READ_FRAMEBUFFER, rt->external.fbo);
 	} else {
 		glBindFramebuffer(GL_READ_FRAMEBUFFER, rt->fbo);
 	}
 	glReadBuffer(GL_COLOR_ATTACHMENT0);
 	glBindFramebuffer(GL_DRAW_FRAMEBUFFER, GLES3::TextureStorage::system_fbo);
+	// Flip content upside down to correct for coordinates.
 	glBlitFramebuffer(0, 0, rt->size.x, rt->size.y, 0, p_screen_rect.size.y, p_screen_rect.size.x, 0, GL_COLOR_BUFFER_BIT, GL_NEAREST);
 }
 

drivers/gles3/storage/config.cpp

@@ -44,7 +44,7 @@ Config::Config() {
 	singleton = this;
 
 	{
-		int max_extensions = 0;
+		GLint max_extensions = 0;
 		glGetIntegerv(GL_NUM_EXTENSIONS, &max_extensions);
 		for (int i = 0; i < max_extensions; i++) {
 			const GLubyte *s = glGetStringi(GL_EXTENSIONS, i);

drivers/gles3/storage/material_storage.cpp

@@ -2542,7 +2542,7 @@ RS::ShaderNativeSourceCode MaterialStorage::shader_get_native_source_code(RID p_
 
 /* MATERIAL API */
 
-void MaterialStorage::_material_queue_update(Material *material, bool p_uniform, bool p_texture) {
+void MaterialStorage::_material_queue_update(GLES3::Material *material, bool p_uniform, bool p_texture) {
 	material->uniform_dirty = material->uniform_dirty || p_uniform;
 	material->texture_dirty = material->texture_dirty || p_texture;
 

main/main.cpp

@@ -641,6 +641,9 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph
 	bool found_project = false;
 #endif
 
+	String default_renderer = "";
+	String renderer_hints = "";
+
 	packed_data = PackedData::get_singleton();
 	if (!packed_data) {
 		packed_data = memnew(PackedData);
@@ -1306,14 +1309,33 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph
 
 	// possibly be worth changing the default from vulkan to something lower spec,
 	// for the project manager, depending on how smooth the fallback is.
-	GLOBAL_DEF_RST("rendering/driver/driver_name", "vulkan");
 
 	// this list is hard coded, which makes it more difficult to add new backends.
 	// can potentially be changed to more of a plugin system at a later date.
+
+	// Start with Vulkan, which will be the default if enabled.
+#ifdef VULKAN_ENABLED
+	renderer_hints = "vulkan";
+#endif
+
+	// And OpenGL3 next, or first if Vulkan is disabled.
+#ifdef GLES3_ENABLED
+	if (!renderer_hints.is_empty()) {
+		renderer_hints += ",";
+	}
+	renderer_hints += "opengl3";
+#endif
+	if (renderer_hints.is_empty()) {
+		ERR_PRINT("No rendering driver available.");
+	}
+
+	default_renderer = renderer_hints.get_slice(",", 0);
+	GLOBAL_DEF_RST("rendering/driver/driver_name", default_renderer);
+
 	ProjectSettings::get_singleton()->set_custom_property_info("rendering/driver/driver_name",
 			PropertyInfo(Variant::STRING,
 					"rendering/driver/driver_name",
-					PROPERTY_HINT_ENUM, "vulkan,opengl3"));
+					PROPERTY_HINT_ENUM, renderer_hints));
 
 	// if not set on the command line
 	if (rendering_driver.is_empty()) {

platform/windows/detect.py

@@ -269,12 +269,14 @@ def configure_msvc(env, manual_msvc_config):
         "dwmapi",
     ]
 
-    env.AppendUnique(CPPDEFINES=["VULKAN_ENABLED"])
-    if not env["use_volk"]:
-        LIBS += ["vulkan"]
-
-    env.AppendUnique(CPPDEFINES=["GLES3_ENABLED"])
-    LIBS += ["opengl32"]
+    if env["vulkan"]:
+        env.AppendUnique(CPPDEFINES=["VULKAN_ENABLED"])
+        if not env["use_volk"]:
+            LIBS += ["vulkan"]
+
+    if env["opengl3"]:
+        env.AppendUnique(CPPDEFINES=["GLES3_ENABLED"])
+        LIBS += ["opengl32"]
 
     env.Append(LINKFLAGS=[p + env["LIBSUFFIX"] for p in LIBS])
 

platform/windows/display_server_windows.cpp

@@ -607,8 +607,11 @@ void DisplayServerWindows::show_window(WindowID p_id) {
 		_update_window_style(p_id);
 	}
 
-	ShowWindow(wd.hWnd, (wd.no_focus || wd.is_popup) ? SW_SHOWNOACTIVATE : SW_SHOW); // Show the window.
-	if (!wd.no_focus && !wd.is_popup) {
+	if (wd.no_focus || wd.is_popup) {
+		// https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-showwindow
+		ShowWindow(wd.hWnd, SW_SHOWNA);
+	} else {
+		ShowWindow(wd.hWnd, SW_SHOW);
 		SetForegroundWindow(wd.hWnd); // Slightly higher priority.
 		SetFocus(wd.hWnd); // Set keyboard focus.
 	}
@@ -1798,7 +1801,9 @@ void DisplayServerWindows::make_rendering_thread() {
 
 void DisplayServerWindows::swap_buffers() {
 #if defined(GLES3_ENABLED)
-	gl_manager->swap_buffers();
+	if (gl_manager) {
+		gl_manager->swap_buffers();
+	}
 #endif
 }
 
@@ -1952,14 +1957,18 @@ void DisplayServerWindows::set_icon(const Ref<Image> &p_icon) {
 void DisplayServerWindows::window_set_vsync_mode(DisplayServer::VSyncMode p_vsync_mode, WindowID p_window) {
 	_THREAD_SAFE_METHOD_
 #if defined(VULKAN_ENABLED)
-	context_vulkan->set_vsync_mode(p_window, p_vsync_mode);
+	if (context_vulkan) {
+		context_vulkan->set_vsync_mode(p_window, p_vsync_mode);
+	}
 #endif
 }
 
 DisplayServer::VSyncMode DisplayServerWindows::window_get_vsync_mode(WindowID p_window) const {
 	_THREAD_SAFE_METHOD_
 #if defined(VULKAN_ENABLED)
-	return context_vulkan->get_vsync_mode(p_window);
+	if (context_vulkan) {
+		return context_vulkan->get_vsync_mode(p_window);
+	}
 #endif
 	return DisplayServer::VSYNC_ENABLED;
 }
@@ -2193,8 +2202,39 @@ LRESULT DisplayServerWindows::MouseProc(int code, WPARAM wParam, LPARAM lParam)
 	return ::CallNextHookEx(mouse_monitor, code, wParam, lParam);
 }
 
-// Our default window procedure to handle processing of window-related system messages/events.
-// Also known as DefProc or DefWindowProc.
+// Handle a single window message received while CreateWindowEx is still on the stack and our data
+// structures are not fully initialized.
+LRESULT DisplayServerWindows::_handle_early_window_message(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam) {
+	switch (uMsg) {
+		case WM_GETMINMAXINFO: {
+			// We receive this during CreateWindowEx and we haven't initialized the window
+			// struct, so let Windows figure out the maximized size.
+			// Silently forward to user/default.
+		} break;
+		case WM_NCCREATE: {
+			// We tunnel an unowned pointer to our window context (WindowData) through the
+			// first possible message (WM_NCCREATE) to fix up our window context collection.
+			CREATESTRUCTW *pCreate = (CREATESTRUCTW *)lParam;
+			WindowData *pWindowData = reinterpret_cast<WindowData *>(pCreate->lpCreateParams);
+
+			// Fix this up so we can recognize the remaining messages.
+			pWindowData->hWnd = hWnd;
+		} break;
+		default: {
+			// Additional messages during window creation should happen after we fixed
+			// up the data structures on WM_NCCREATE, but this might change in the future,
+			// so report an error here and then we can implement them.
+			ERR_PRINT_ONCE(vformat("Unexpected window message 0x%x received for window we cannot recognize in our collection; sequence error.", uMsg));
+		} break;
+	}
+
+	if (user_proc) {
+		return CallWindowProcW(user_proc, hWnd, uMsg, wParam, lParam);
+	}
+	return DefWindowProcW(hWnd, uMsg, wParam, lParam);
+}
+
+// The window procedure for our window class "Engine", used to handle processing of window-related system messages/events.
 // See: https://docs.microsoft.com/en-us/windows/win32/winmsg/window-procedures
 LRESULT DisplayServerWindows::WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam) {
 	if (drop_events) {
@@ -2208,7 +2248,9 @@ LRESULT DisplayServerWindows::WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARA
 	WindowID window_id = INVALID_WINDOW_ID;
 	bool window_created = false;
 
-	// Check whether window exists.
+	// Check whether window exists
+	// FIXME this is O(n), where n is the set of currently open windows and subwindows
+	// we should have a secondary map from HWND to WindowID or even WindowData* alias, if we want to eliminate all the map lookups below
 	for (const KeyValue<WindowID, WindowData> &E : windows) {
 		if (E.value.hWnd == hWnd) {
 			window_id = E.key;
@@ -2217,10 +2259,12 @@ LRESULT DisplayServerWindows::WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARA
 		}
 	}
 
-	// Window doesn't exist or creation in progress, don't handle messages yet.
+	// WARNING: we get called with events before the window is registered in our collection
+	// specifically, even the call to CreateWindowEx already calls here while still on the stack,
+	// so there is no way to store the window handle in our collection before we get here
 	if (!window_created) {
-		window_id = window_id_counter;
-		ERR_FAIL_COND_V(!windows.has(window_id), 0);
+		// don't let code below operate on incompletely initialized window objects or missing window_id
+		return _handle_early_window_message(hWnd, uMsg, wParam, lParam);
 	}
 
 	// Process window messages.
@@ -3388,11 +3432,17 @@ DisplayServer::WindowID DisplayServerWindows::_create_window(WindowMode p_mode,
 				WindowRect.top,
 				WindowRect.right - WindowRect.left,
 				WindowRect.bottom - WindowRect.top,
-				nullptr, nullptr, hInstance, nullptr);
+				nullptr,
+				nullptr,
+				hInstance,
+				// tunnel the WindowData we need to handle creation message
+				// lifetime is ensured because we are still on the stack when this is
+				// processed in the window proc
+				reinterpret_cast<void *>(&wd));
 		if (!wd.hWnd) {
 			MessageBoxW(nullptr, L"Window Creation Error.", L"ERROR", MB_OK | MB_ICONEXCLAMATION);
 			windows.erase(id);
-			return INVALID_WINDOW_ID;
+			ERR_FAIL_V_MSG(INVALID_WINDOW_ID, "Failed to create Windows OS window.");
 		}
 		if (p_mode != WINDOW_MODE_FULLSCREEN && p_mode != WINDOW_MODE_EXCLUSIVE_FULLSCREEN) {
 			wd.pre_fs_valid = true;
@@ -3412,7 +3462,14 @@ DisplayServer::WindowID DisplayServerWindows::_create_window(WindowMode p_mode,
 #ifdef GLES3_ENABLED
 		if (gl_manager) {
 			Error err = gl_manager->window_create(id, wd.hWnd, hInstance, WindowRect.right - WindowRect.left, WindowRect.bottom - WindowRect.top);
-			ERR_FAIL_COND_V_MSG(err != OK, INVALID_WINDOW_ID, "Failed to create an OpenGL window.");
+
+			// shut down OpenGL, to mirror behavior of Vulkan code
+			if (err != OK) {
+				memdelete(gl_manager);
+				gl_manager = nullptr;
+				windows.erase(id);
+				ERR_FAIL_V_MSG(INVALID_WINDOW_ID, "Failed to create an OpenGL window.");
+			}
 		}
 #endif
 
@@ -3457,6 +3514,8 @@ DisplayServer::WindowID DisplayServerWindows::_create_window(WindowMode p_mode,
 		ImmReleaseContext(wd.hWnd, wd.im_himc);
 
 		wd.im_position = Vector2();
+
+		// FIXME this is wrong in cases where the window coordinates were changed due to full screen mode; use WindowRect
 		wd.last_pos = p_rect.position;
 		wd.width = p_rect.size.width;
 		wd.height = p_rect.size.height;
@@ -3747,6 +3806,7 @@ DisplayServerWindows::~DisplayServerWindows() {
 
 #ifdef GLES3_ENABLED
 	// destroy windows .. NYI?
+	// FIXME wglDeleteContext is never called
 #endif
 
 	if (windows.has(MAIN_WINDOW_ID)) {

platform/windows/display_server_windows.h

@@ -448,6 +448,8 @@ class DisplayServerWindows : public DisplayServer {
 	static void _dispatch_input_events(const Ref<InputEvent> &p_event);
 	void _dispatch_input_event(const Ref<InputEvent> &p_event);
 
+	LRESULT _handle_early_window_message(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam);
+
 public:
 	LRESULT WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam);
 	LRESULT MouseProc(int code, WPARAM wParam, LPARAM lParam);