Skip to content

Commit 65fc3eb

Browse files
solidpixelsolidpixel
authored
Use VUL safe_struct (#118)
The Vulkan API accepts a lot of const pointers to input structure trees, many of which have unstructured content due to the use of opaque pNext pointers to support extensions. Layers often want to change input data before calling down the stack, and the current code just uses const_cast on the inputs and modifies in place. This is obviously violating the API contract with the application so we need to fix it. This PR will clone structure trees where needed, using the safe_struct library from the Vulkan Utility Libraries project. Fixes #56
1 parent 4e8c730 commit 65fc3eb

File tree

17 files changed

+108
-280
lines changed

17 files changed

+108
-280
lines changed

‎.gitmodules‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
11
[submodule "khronos/vulkan"]
22
path = source_third_party/khronos/vulkan
33
url = https://github.com/KhronosGroup/Vulkan-Headers
4-
[submodule "khronos/opengles"]
5-
path = source_third_party/khronos/opengles
6-
url = https://github.com/KhronosGroup/OpenGL-Registry
7-
[submodule "khronos/egl"]
8-
path = source_third_party/khronos/egl
9-
url = https://github.com/KhronosGroup/EGL-Registry
104
[submodule "source_third_party/gtest"]
115
path = source_third_party/gtest
126
url = https://github.com/google/googletest
137
[submodule "source_third_party/protopuf"]
148
path = source_third_party/protopuf
159
url = https://github.com/PragmaTwice/protopuf.git
10+
[submodule "source_third_party/khronos/vulkan-utilities"]
11+
path = source_third_party/khronos/vulkan-utilities
12+
url = https://github.com/KhronosGroup/Vulkan-Utility-Libraries/

‎generator/vk_codegen/source_CMakeLists.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ target_include_directories(
5353

5454
target_include_directories(
5555
${VK_LAYER} SYSTEM PRIVATE
56-
../../source_third_party/khronos/vulkan/include/)
56+
../../source_third_party/khronos/vulkan/include/
57+
../../source_third_party/khronos/vulkan-utilities/include/)
5758

5859
lgl_set_build_options(${VK_LAYER})
5960

‎layer_example/source/CMakeLists.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ target_include_directories(
5454

5555
target_include_directories(
5656
${VK_LAYER} SYSTEM PRIVATE
57-
../../source_third_party/khronos/vulkan/include/)
57+
../../source_third_party/khronos/vulkan/include/
58+
../../source_third_party/khronos/vulkan-utilities/include/)
5859

5960
lgl_set_build_options(${VK_LAYER})
6061

‎layer_gpu_support/source/CMakeLists.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ target_include_directories(
6262
target_include_directories(
6363
${VK_LAYER} SYSTEM PRIVATE
6464
../../source_third_party/
65-
../../source_third_party/khronos/vulkan/include/)
65+
../../source_third_party/khronos/vulkan/include/
66+
../../source_third_party/khronos/vulkan-utilities/include/)
6667

6768
lgl_set_build_options(${VK_LAYER})
6869

‎layer_gpu_support/source/layer_device_functions_image.cpp‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
* IN THE SOFTWARE.
2323
* ----------------------------------------------------------------------------
2424
*/
25+
#include <vulkan/utility/vk_safe_struct.hpp>
26+
#include <vulkan/utility/vk_struct_helper.hpp>
2527

2628
#include "device.hpp"
2729
#include "framework/device_dispatch_table.hpp"
@@ -119,16 +121,17 @@ VKAPI_ATTR VkResult VKAPI_CALL layer_vkCreateImage<user_tag>(VkDevice device,
119121
}
120122
}
121123

122-
VkImageCreateInfo newCreateInfo = *pCreateInfo;
123-
124-
VkImageCompressionControlEXT newCompressionControl;
125-
// TODO: This currently relies on const_cast to make user struct writable
126-
// We should replace this with a generic clone (issue #56)
127-
auto* userCompressionControl =
128-
searchNextList<VkImageCompressionControlEXT>(VK_STRUCTURE_TYPE_IMAGE_COMPRESSION_CONTROL_EXT,
129-
pCreateInfo->pNext);
124+
// Create modifiable structures we can patch
125+
vku::safe_VkImageCreateInfo newCreateInfoSafe(pCreateInfo);
126+
auto* newCreateInfo = reinterpret_cast<VkImageCreateInfo*>(&newCreateInfoSafe);
127+
// We know we can const-cast here because this is a safe-struct clone
128+
void* pNextBase = const_cast<void*>(newCreateInfoSafe.pNext);
130129

130+
// Create extra structures we can patch in
131+
VkImageCompressionControlEXT newCompressionControl = vku::InitStructHelper();
131132
VkImageCompressionControlEXT* compressionControl = &newCompressionControl;
133+
134+
auto* userCompressionControl = vku::FindStructInPNextChain<VkImageCompressionControlEXT>(pNextBase);
132135
if (userCompressionControl)
133136
{
134137
compressionControl = userCompressionControl;
@@ -138,21 +141,18 @@ VKAPI_ATTR VkResult VKAPI_CALL layer_vkCreateImage<user_tag>(VkDevice device,
138141

139142
if (forceDisable)
140143
{
141-
compressionControl->sType = VK_STRUCTURE_TYPE_IMAGE_COMPRESSION_CONTROL_EXT;
142144
compressionControl->flags = VK_IMAGE_COMPRESSION_DISABLED_EXT;
143145
compressionControl->compressionControlPlaneCount = 0;
144146
compressionControl->pFixedRateFlags = nullptr;
145147
}
146148
else if (forceDefault)
147149
{
148-
compressionControl->sType = VK_STRUCTURE_TYPE_IMAGE_COMPRESSION_CONTROL_EXT;
149150
compressionControl->flags = VK_IMAGE_COMPRESSION_DEFAULT_EXT;
150151
compressionControl->compressionControlPlaneCount = 0;
151152
compressionControl->pFixedRateFlags = nullptr;
152153
}
153154
else if (selectedLevel)
154155
{
155-
compressionControl->sType = VK_STRUCTURE_TYPE_IMAGE_COMPRESSION_CONTROL_EXT;
156156
compressionControl->flags = VK_IMAGE_COMPRESSION_FIXED_RATE_EXPLICIT_EXT;
157157
compressionControl->compressionControlPlaneCount = 1;
158158
compressionControl->pFixedRateFlags = reinterpret_cast<VkImageCompressionFixedRateFlagsEXT*>(&selectedLevel);
@@ -162,12 +162,11 @@ VKAPI_ATTR VkResult VKAPI_CALL layer_vkCreateImage<user_tag>(VkDevice device,
162162
patchNeeded = false;
163163
}
164164

165-
// If this is new, patch it in to the pNext chain
165+
// Add a config if not already configured by the application
166166
if (patchNeeded)
167167
{
168-
compressionControl->pNext = newCreateInfo.pNext;
169-
newCreateInfo.pNext = reinterpret_cast<const void*>(compressionControl);
168+
vku::AddToPnext(newCreateInfoSafe, *compressionControl);
170169
}
171170

172-
return layer->driver.vkCreateImage(device, &newCreateInfo, pAllocator, pImage);
171+
return layer->driver.vkCreateImage(device, newCreateInfo, pAllocator, pImage);
173172
}

‎layer_gpu_timeline/source/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ target_include_directories(
6666
${VK_LAYER} SYSTEM PRIVATE
6767
../../source_third_party/
6868
../../source_third_party/khronos/vulkan/include/
69+
../../source_third_party/khronos/vulkan-utilities/include/
6970
../../source_third_party/protopuf/include/)
7071

7172
lgl_set_build_options(${VK_LAYER})

‎source_common/comms/CMakeLists.txt‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# SPDX-License-Identifier: MIT
22
# -----------------------------------------------------------------------------
3-
# Copyright (c) 2024 Arm Limited
3+
# Copyright (c) 2024-2025 Arm Limited
44
#
55
# Permission is hereby granted, free of charge, to any person obtaining a copy
66
# of this software and associated documentation files (the "Software"), to
@@ -33,12 +33,13 @@ add_library(
3333
target_include_directories(
3434
${LIB_BINARY} PRIVATE
3535
../
36-
../../source_third_party/khronos/vulkan/include/)
36+
../../source_third_party/khronos/vulkan/include/
37+
../../source_third_party/khronos/vulkan-utilities/include/)
3738

3839
lgl_set_build_options(${LIB_BINARY})
3940

4041
if(${LGL_UNITTEST})
4142
add_subdirectory(test)
4243
endif()
4344

44-
add_clang_tools()
45+
add_clang_tools()

‎source_common/comms/test/CMakeLists.txt‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# SPDX-License-Identifier: MIT
22
# -----------------------------------------------------------------------------
3-
# Copyright (c) 2024 Arm Limited
3+
# Copyright (c) 2024-2025 Arm Limited
44
#
55
# Permission is hereby granted, free of charge, to any person obtaining a copy
66
# of this software and associated documentation files (the "Software"), to
@@ -34,6 +34,7 @@ target_include_directories(
3434
../../
3535
../../../source_third_party/
3636
../../../source_third_party/khronos/vulkan/include
37+
../../source_third_party/khronos/vulkan-utilities/include/
3738
${gtest_SOURCE_DIR}/include)
3839

3940
target_link_libraries(
@@ -76,4 +77,4 @@ install(
7677
TARGETS ${TEST_BINARY}
7778
DESTINATION bin)
7879

79-
add_clang_tools()
80+
add_clang_tools()

‎source_common/framework/CMakeLists.txt‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,14 @@ set(LIB_BINARY lib_layer_framework)
2626
add_library(
2727
${LIB_BINARY} STATIC
2828
device_functions.cpp
29-
device_query.cpp
3029
instance_functions.cpp
31-
manual_functions.cpp)
30+
manual_functions.cpp
31+
../../source_third_party/khronos/vulkan-utilities/src/vulkan/vk_safe_struct_core.cpp
32+
../../source_third_party/khronos/vulkan-utilities/src/vulkan/vk_safe_struct_ext.cpp
33+
../../source_third_party/khronos/vulkan-utilities/src/vulkan/vk_safe_struct_khr.cpp
34+
../../source_third_party/khronos/vulkan-utilities/src/vulkan/vk_safe_struct_manual.cpp
35+
../../source_third_party/khronos/vulkan-utilities/src/vulkan/vk_safe_struct_utils.cpp
36+
../../source_third_party/khronos/vulkan-utilities/src/vulkan/vk_safe_struct_vendor.cpp)
3237

3338
target_include_directories(
3439
${LIB_BINARY} PRIVATE
@@ -41,8 +46,9 @@ target_include_directories(
4146
${LIB_BINARY} SYSTEM PRIVATE
4247
../
4348
../../source_third_party/
44-
../../source_third_party/khronos/vulkan/include/)
49+
../../source_third_party/khronos/vulkan/include/
50+
../../source_third_party/khronos/vulkan-utilities/include/)
4551

4652
lgl_set_build_options(${LIB_BINARY})
4753

48-
add_clang_tools()
54+
add_clang_tools()

‎source_common/framework/device_query.cpp‎

Lines changed: 0 additions & 60 deletions
This file was deleted.

0 commit comments

Comments
 (0)