[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);
}
