[dawn] Add custom deleters for surface descriptor glfw_utils. - Because wgpu::ChainedStruct doesn't have a virtual dtor, naively using std::unique_ptr doesn't clean up the allocated descriptors properly. I tried adding virtual dtor for wgpu::ChainedStruct, but as a result of adding that, some of the static_asserts we use to verify struct layouts started failing. Since this is just a helper util, I just added custom deleters where applicable which just casts the wgpu::ChainedStruct into the backing implementation before deleting. Change-Id: Ib37d4430b394f8e6da754033533ca36b30f14754 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/204415 Commit-Queue: Corentin Wallez <cwallez@chromium.org> Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/include/webgpu/webgpu_glfw.h b/include/webgpu/webgpu_glfw.h index c35252c..a319ffc 100644 --- a/include/webgpu/webgpu_glfw.h +++ b/include/webgpu/webgpu_glfw.h
@@ -63,8 +63,8 @@ // Use for testing only. Does everything that CreateSurfaceForWindow does except the call to // CreateSurface. Useful to be able to modify the descriptor for testing, or when trying to // avoid using the global proc table. -WGPU_GLFW_EXPORT std::unique_ptr<wgpu::ChainedStruct> SetupWindowAndGetSurfaceDescriptor( - GLFWwindow* window); +WGPU_GLFW_EXPORT std::unique_ptr<wgpu::ChainedStruct, void (*)(wgpu::ChainedStruct*)> +SetupWindowAndGetSurfaceDescriptor(GLFWwindow* window); } // namespace wgpu::glfw
diff --git a/src/dawn/glfw/utils.cpp b/src/dawn/glfw/utils.cpp index 2723374..4003931 100644 --- a/src/dawn/glfw/utils.cpp +++ b/src/dawn/glfw/utils.cpp
@@ -48,8 +48,7 @@ namespace wgpu::glfw { wgpu::Surface CreateSurfaceForWindow(const wgpu::Instance& instance, GLFWwindow* window) { - std::unique_ptr<wgpu::ChainedStruct> chainedDescriptor = - SetupWindowAndGetSurfaceDescriptor(window); + auto chainedDescriptor = SetupWindowAndGetSurfaceDescriptor(window); wgpu::SurfaceDescriptor descriptor; descriptor.nextInChain = chainedDescriptor.get(); @@ -59,47 +58,53 @@ } // SetupWindowAndGetSurfaceDescriptorCocoa defined in GLFWUtils_metal.mm -std::unique_ptr<wgpu::ChainedStruct> SetupWindowAndGetSurfaceDescriptorCocoa(GLFWwindow* window); +std::unique_ptr<wgpu::ChainedStruct, void (*)(wgpu::ChainedStruct*)> +SetupWindowAndGetSurfaceDescriptorCocoa(GLFWwindow* window); -std::unique_ptr<wgpu::ChainedStruct> SetupWindowAndGetSurfaceDescriptor(GLFWwindow* window) { +std::unique_ptr<wgpu::ChainedStruct, void (*)(wgpu::ChainedStruct*)> +SetupWindowAndGetSurfaceDescriptor(GLFWwindow* window) { if (glfwGetWindowAttrib(window, GLFW_CLIENT_API) != GLFW_NO_API) { dawn::ErrorLog() << "GL context was created on the window. Disable context creation by " "setting the GLFW_CLIENT_API hint to GLFW_NO_API."; - return nullptr; + return {nullptr, [](wgpu::ChainedStruct*) {}}; } #if DAWN_PLATFORM_IS(WINDOWS) - std::unique_ptr<wgpu::SurfaceDescriptorFromWindowsHWND> desc = - std::make_unique<wgpu::SurfaceDescriptorFromWindowsHWND>(); + wgpu::SurfaceDescriptorFromWindowsHWND* desc = new wgpu::SurfaceDescriptorFromWindowsHWND(); desc->hwnd = glfwGetWin32Window(window); desc->hinstance = GetModuleHandle(nullptr); - return std::move(desc); + return {desc, [](wgpu::ChainedStruct* desc) { + delete reinterpret_cast<wgpu::SurfaceDescriptorFromWindowsHWND*>(desc); + }}; #elif defined(DAWN_ENABLE_BACKEND_METAL) return SetupWindowAndGetSurfaceDescriptorCocoa(window); #elif defined(DAWN_USE_WAYLAND) || defined(DAWN_USE_X11) #if defined(GLFW_PLATFORM_WAYLAND) && defined(DAWN_USE_WAYLAND) if (glfwGetPlatform() == GLFW_PLATFORM_WAYLAND) { - std::unique_ptr<wgpu::SurfaceDescriptorFromWaylandSurface> desc = - std::make_unique<wgpu::SurfaceDescriptorFromWaylandSurface>(); + wgpu::SurfaceDescriptorFromWaylandSurface* desc = + new wgpu::SurfaceDescriptorFromWaylandSurface(); desc->display = glfwGetWaylandDisplay(); desc->surface = glfwGetWaylandWindow(window); - return std::move(desc); + return {desc, [](wgpu::ChainedStruct* desc) { + delete reinterpret_cast<wgpu::SurfaceDescriptorFromWaylandSurface*>(desc); + }}; } else // NOLINT(readability/braces) #endif #if defined(DAWN_USE_X11) { - std::unique_ptr<wgpu::SurfaceDescriptorFromXlibWindow> desc = - std::make_unique<wgpu::SurfaceDescriptorFromXlibWindow>(); + wgpu::SurfaceDescriptorFromXlibWindow* desc = new wgpu::SurfaceDescriptorFromXlibWindow(); desc->display = glfwGetX11Display(); desc->window = glfwGetX11Window(window); - return std::move(desc); + return {desc, [](wgpu::ChainedStruct* desc) { + delete reinterpret_cast<wgpu::SurfaceDescriptorFromXlibWindow*>(desc); + }}; } #else { - return nullptr; + return {nullptr, [](wgpu::ChainedStruct*) {}}; } #endif #else - return nullptr; + return {nullptr, [](wgpu::ChainedStruct*) {}}; #endif }
diff --git a/src/dawn/glfw/utils_metal.mm b/src/dawn/glfw/utils_metal.mm index 6131a46..be71ac7 100644 --- a/src/dawn/glfw/utils_metal.mm +++ b/src/dawn/glfw/utils_metal.mm
@@ -41,7 +41,8 @@ namespace wgpu::glfw { -std::unique_ptr<wgpu::ChainedStruct> SetupWindowAndGetSurfaceDescriptorCocoa(GLFWwindow* window) { +std::unique_ptr<wgpu::ChainedStruct, void (*)(wgpu::ChainedStruct*)> +SetupWindowAndGetSurfaceDescriptorCocoa(GLFWwindow* window) { @autoreleasepool { NSWindow* nsWindow = glfwGetCocoaWindow(window); NSView* view = [nsWindow contentView]; @@ -54,10 +55,11 @@ // Use retina if the window was created with retina support. [[view layer] setContentsScale:[nsWindow backingScaleFactor]]; - std::unique_ptr<wgpu::SurfaceDescriptorFromMetalLayer> desc = - std::make_unique<wgpu::SurfaceDescriptorFromMetalLayer>(); + wgpu::SurfaceDescriptorFromMetalLayer* desc = new wgpu::SurfaceDescriptorFromMetalLayer(); desc->layer = [view layer]; - return std::move(desc); + return {desc, [](wgpu::ChainedStruct* desc) { + delete reinterpret_cast<wgpu::SurfaceDescriptorFromMetalLayer*>(desc); + }}; } }
diff --git a/src/dawn/tests/end2end/WindowSurfaceTests.cpp b/src/dawn/tests/end2end/WindowSurfaceTests.cpp index 5c03f04..c1dd093 100644 --- a/src/dawn/tests/end2end/WindowSurfaceTests.cpp +++ b/src/dawn/tests/end2end/WindowSurfaceTests.cpp
@@ -99,8 +99,7 @@ // descriptor). TEST_F(WindowSurfaceInstanceTests, ControlCase) { GLFWwindow* window = CreateWindow(); - std::unique_ptr<wgpu::ChainedStruct> chainedDescriptor = - wgpu::glfw::SetupWindowAndGetSurfaceDescriptor(window); + auto chainedDescriptor = wgpu::glfw::SetupWindowAndGetSurfaceDescriptor(window); wgpu::SurfaceDescriptor descriptor; descriptor.nextInChain = chainedDescriptor.get(); @@ -141,10 +140,8 @@ // Test that it is invalid to give two valid chained descriptors TEST_F(WindowSurfaceInstanceTests, TwoChainedDescriptors) { GLFWwindow* window = CreateWindow(); - std::unique_ptr<wgpu::ChainedStruct> chainedDescriptor1 = - wgpu::glfw::SetupWindowAndGetSurfaceDescriptor(window); - std::unique_ptr<wgpu::ChainedStruct> chainedDescriptor2 = - wgpu::glfw::SetupWindowAndGetSurfaceDescriptor(window); + auto chainedDescriptor1 = wgpu::glfw::SetupWindowAndGetSurfaceDescriptor(window); + auto chainedDescriptor2 = wgpu::glfw::SetupWindowAndGetSurfaceDescriptor(window); wgpu::SurfaceDescriptor descriptor; descriptor.nextInChain = chainedDescriptor1.get(); @@ -158,8 +155,7 @@ // Tests that GLFWUtils returns a descriptor of HWND type TEST_F(WindowSurfaceInstanceTests, CorrectSTypeHWND) { GLFWwindow* window = CreateWindow(); - std::unique_ptr<wgpu::ChainedStruct> chainedDescriptor = - wgpu::glfw::SetupWindowAndGetSurfaceDescriptor(window); + auto chainedDescriptor = wgpu::glfw::SetupWindowAndGetSurfaceDescriptor(window); ASSERT_EQ(chainedDescriptor->sType, wgpu::SType::SurfaceDescriptorFromWindowsHWND); } @@ -194,8 +190,7 @@ // Tests that GLFWUtils returns a descriptor of Xlib type TEST_F(WindowSurfaceInstanceTests, CorrectSTypeXlib) { GLFWwindow* window = CreateWindow(); - std::unique_ptr<wgpu::ChainedStruct> chainedDescriptor = - wgpu::glfw::SetupWindowAndGetSurfaceDescriptor(window); + auto chainedDescriptor = wgpu::glfw::SetupWindowAndGetSurfaceDescriptor(window); ASSERT_EQ(chainedDescriptor->sType, wgpu::SType::SurfaceDescriptorFromXlibWindow); } @@ -234,8 +229,7 @@ // Tests that GLFWUtils returns a descriptor of Metal type TEST_F(WindowSurfaceInstanceTests, CorrectSTypeMetal) { GLFWwindow* window = CreateWindow(); - std::unique_ptr<wgpu::ChainedStruct> chainedDescriptor = - wgpu::glfw::SetupWindowAndGetSurfaceDescriptor(window); + auto chainedDescriptor = wgpu::glfw::SetupWindowAndGetSurfaceDescriptor(window); ASSERT_EQ(chainedDescriptor->sType, wgpu::SType::SurfaceDescriptorFromMetalLayer); }