From 6076fcafa19552c55fbed331adc5418723892827 Mon Sep 17 00:00:00 2001
From: Niklas Haas <git@haasn.xyz>
Date: Thu, 21 May 2020 11:01:22 +0200
Subject: [PATCH] vulkan: add pNext chain boilerplate for physical device
features
We now "officially" support enabling arbitrary extra device features,
including both features request by the user and features needed by us.
It's about time for me to write the shitty boilerplate to link and
memdup chains and stuff. Using generated code to get the sizeof()
unknown structs, because how the heck else would you copy over a pNext
chain while only modifying the values you care about?
Still using shitty hacks for the features whitelisting. I hope they
never change the structure of these arrays as just being a list of
VkBool32 values. (But in theory we could just generate code for this, in
the worst case....)
---
src/include/libplacebo/vulkan.h | 13 +++----
src/vulkan/context.c | 65 +++++++++++++++++++++++----------
src/vulkan/utils.c | 51 ++++++++++++++++++++++++++
src/vulkan/utils.h | 15 ++++++++
src/vulkan/utils_gen.py | 28 ++++++++++++++
5 files changed, 145 insertions(+), 27 deletions(-)
diff --git a/src/include/libplacebo/vulkan.h b/src/include/libplacebo/vulkan.h
index 62255c5d9..c5ef9b154 100644
--- a/src/include/libplacebo/vulkan.h
+++ b/src/include/libplacebo/vulkan.h
@@ -125,9 +125,6 @@ struct pl_vulkan {
int num_extensions;
// The device features that were enabled at device creation time.
- //
- // Note: The pNext chain of this only includes features known to
- // libplacebo, which as the time of writing only includes the base set.
const VkPhysicalDeviceFeatures2KHR *features;
// The explicit queue families we are using to provide a given capability,
@@ -235,7 +232,9 @@ struct pl_vulkan_params {
// Optional extra features to enable at device creation time. These are
// opportunistically enabled if supported by the physical device, but
// otherwise kept disabled. Users may include extra extension-specific
- // features in the pNext chain.
+ // features in the pNext chain, however these *must* all be
+ // extension-specific structs, i.e. the use of "meta-structs" like
+ // VkPhysicalDeviceVulkan11Features is not allowed.
const VkPhysicalDeviceFeatures2KHR *features;
// --- Misc/debugging options
@@ -456,9 +455,9 @@ const struct pl_tex *pl_vulkan_wrap(const struct pl_gpu *gpu,
// optional, but provide a hint to the API user as to what might be worth
// enabling at device creation time.
//
-// Note: Currently, libplacebo does not include any extra extension-specific
-// device features in the `pNext` chain of pl_vulkan_recommended_features, but
-// this may change in the future.
+// Note: This also includes physical device features provided by extensions.
+// They are all provided using extension-specific features structs, rather
+// than the more general purpose VkPhysicalDeviceVulkan11Features etc.
extern const char * const pl_vulkan_recommended_extensions[];
extern const int pl_vulkan_num_recommended_extensions;
extern const VkPhysicalDeviceFeatures2KHR pl_vulkan_recommended_features;
diff --git a/src/vulkan/context.c b/src/vulkan/context.c
index 0445b07f0..89bef5211 100644
--- a/src/vulkan/context.c
+++ b/src/vulkan/context.c
@@ -1087,21 +1087,50 @@ static bool device_init(struct vk_ctx *vk, const struct pl_vulkan_params *params
}
}
- // Enable all features that we might need, by iterating through the entire
- // VkPhysicalDeviceFeatures struct as a VkBool32 array and checking each
- // supported feature against the whitelist of features we want
- vk->GetPhysicalDeviceFeatures2KHR(vk->physd, &vk->features);
- for (int i = 0; i < sizeof(VkPhysicalDeviceFeatures) / sizeof(VkBool32); i++) {
- VkBool32 wanted = ((VkBool32 *) &pl_vulkan_recommended_features.features)[i];
- if (params->features)
- wanted |= ((VkBool32 *) ¶ms->features->features)[i];
+ // Query all supported device features by constructing a pNext chain
+ // starting with the features we care about and ending with whatever
+ // features were requested by the user
+ vk->features.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2_KHR;
+ for (const VkBaseInStructure *in = pl_vulkan_recommended_features.pNext;
+ in; in = in->pNext)
+ vk_link_struct(&vk->features, vk_struct_memdup(vk->ta, in));
+
+ for (const VkBaseInStructure *in = (const VkBaseInStructure *) params->features;
+ in; in = in->pNext)
+ {
+ if (vk_find_struct(&vk->features, in->sType))
+ continue; // skip structs already present
+
+ void *copy = vk_struct_memdup(vk->ta, in);
+ if (!copy) {
+ PL_ERR(vk, "Unknown struct type %"PRIu64"?", (uint64_t) in->sType);
+ continue;
+ }
- ((VkBool32 *) &vk->features.features)[i] &= wanted;
+ vk_link_struct(&vk->features, copy);
}
- // Temporarily link the pNext chain of the extra user features into this
- if (params->features)
- vk->features.pNext = params->features->pNext;
+ vk->GetPhysicalDeviceFeatures2KHR(vk->physd, &vk->features);
+
+ // Go through the features chain a second time and mask every option
+ // that wasn't whitelisted by either libplacebo or the user
+ for (VkBaseOutStructure *chain = (VkBaseOutStructure *) &vk->features;
+ chain; chain = chain->pNext)
+ {
+ const VkBaseInStructure *in_a, *in_b;
+ in_a = vk_find_struct(&pl_vulkan_recommended_features, chain->sType);
+ in_b = vk_find_struct(params->features, chain->sType);
+ in_a = PL_DEF(in_a, in_b);
+ in_b = PL_DEF(in_b, in_a);
+ pl_assert(in_a && in_b);
+
+ VkBool32 *req = (VkBool32 *) &chain[1];
+ const VkBool32 *wl_a = (const VkBool32 *) &in_a[1];
+ const VkBool32 *wl_b = (const VkBool32 *) &in_b[1];
+ size_t size = vk_struct_size(chain->sType) - sizeof(chain[0]);
+ for (int i = 0; i < size / sizeof(VkBool32); i++)
+ req[i] &= wl_a[i] || wl_b[i];
+ }
VkDeviceCreateInfo dinfo = {
.sType = VK_STRUCTURE_TYPE_DEVICE_CREATE_INFO,
@@ -1117,7 +1146,6 @@ static bool device_init(struct vk_ctx *vk, const struct pl_vulkan_params *params
PL_INFO(vk, " %s", (*exts)[i]);
VK(vk->CreateDevice(vk->physd, &dinfo, VK_ALLOC, &vk->dev));
- vk->features.pNext = NULL;
// Load all mandatory device-level functions
for (int i = 0; i < PL_ARRAY_SIZE(vk_dev_funs); i++) {
@@ -1186,7 +1214,6 @@ const struct pl_vulkan *pl_vulkan_create(struct pl_context *ctx,
.ctx = ctx,
.inst = params->instance,
.GetInstanceProcAddr = get_proc_addr_fallback(ctx, params->get_proc_addr),
- .features.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2_KHR,
};
if (!vk->GetInstanceProcAddr)
@@ -1337,7 +1364,6 @@ const struct pl_vulkan *pl_vulkan_import(struct pl_context *ctx,
.physd = params->phys_device,
.dev = params->device,
.GetInstanceProcAddr = get_proc_addr_fallback(ctx, params->get_proc_addr),
- .features.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2_KHR,
};
if (!vk->GetInstanceProcAddr)
@@ -1377,11 +1403,10 @@ const struct pl_vulkan *pl_vulkan_import(struct pl_context *ctx,
PRINTF_VER(params->max_api_version), PRINTF_VER(vk->api_ver));
}
- if (params->features) {
- // Explicitly drop the pNext chain of this, since we don't currently
- // "officially" know about any extra features provided by extensions
- vk->features.features = params->features->features;
- }
+ VkPhysicalDeviceFeatures2KHR *features;
+ features = vk_chain_memdup(vk->ta, params->features);
+ if (features)
+ vk->features = *features;
// Load all mandatory device-level functions
for (int i = 0; i < PL_ARRAY_SIZE(vk_dev_funs); i++) {
diff --git a/src/vulkan/utils.c b/src/vulkan/utils.c
index 0d1ae2c26..2a84d906d 100644
--- a/src/vulkan/utils.c
+++ b/src/vulkan/utils.c
@@ -103,3 +103,54 @@ const enum pl_handle_type vk_sync_handle_list[] = {
#endif
0
};
+
+const void *vk_find_struct(const void *chain, VkStructureType stype)
+{
+ const VkBaseInStructure *in = chain;
+ while (in) {
+ if (in->sType == stype)
+ return in;
+
+ in = in->pNext;
+ }
+
+ return NULL;
+}
+
+void vk_link_struct(void *chain, void *in)
+{
+ if (!in)
+ return;
+
+ VkBaseOutStructure *out = chain;
+ while (out->pNext)
+ out = out->pNext;
+
+ out->pNext = in;
+}
+
+void *vk_struct_memdup(void *tactx, const void *pin)
+{
+ if (!pin)
+ return NULL;
+
+ const VkBaseInStructure *in = pin;
+ size_t size = vk_struct_size(in->sType);
+ if (!size)
+ return NULL;
+
+ VkBaseOutStructure *out = talloc_memdup(tactx, in, size);
+ out->pNext = NULL;
+ return out;
+}
+
+void *vk_chain_memdup(void *tactx, const void *pin)
+{
+ const VkBaseInStructure *in = pin;
+ VkBaseOutStructure *out = vk_struct_memdup(tactx, in);
+ if (!out)
+ return NULL;
+
+ out->pNext = vk_chain_memdup(tactx, in->pNext);
+ return out;
+}
diff --git a/src/vulkan/utils.h b/src/vulkan/utils.h
index a017cd434..56261f995 100644
--- a/src/vulkan/utils.h
+++ b/src/vulkan/utils.h
@@ -23,6 +23,9 @@
const char *vk_res_str(VkResult res);
const char *vk_obj_str(VkObjectType obj);
+// Return the size of an arbitrary vulkan struct. Returns 0 for unknown structs
+size_t vk_struct_size(VkStructureType stype);
+
// Enum translation boilerplate
VkExternalMemoryHandleTypeFlagBitsKHR vk_mem_handle_type(enum pl_handle_type);
VkExternalSemaphoreHandleTypeFlagBitsKHR vk_sync_handle_type(enum pl_handle_type);
@@ -36,6 +39,18 @@ bool vk_external_mem_check(const VkExternalMemoryPropertiesKHR *props,
extern const enum pl_handle_type vk_mem_handle_list[];
extern const enum pl_handle_type vk_sync_handle_list[];
+// Find a structure in a pNext chain, or NULL
+const void *vk_find_struct(const void *chain, VkStructureType stype);
+
+// Link a structure into a pNext chain
+void vk_link_struct(void *chain, void *in);
+
+// Make a copy of a structure, not including the pNext chain
+void *vk_struct_memdup(void *tactx, const void *in);
+
+// Make a deep copy of an entire pNext chain
+void *vk_chain_memdup(void *tactx, const void *in);
+
// Convenience macros to simplify a lot of common boilerplate
#define VK_ASSERT(res, str) \
do { \
diff --git a/src/vulkan/utils_gen.py b/src/vulkan/utils_gen.py
index cf3b3fb8d..07d3a5f8e 100644
--- a/src/vulkan/utils_gen.py
+++ b/src/vulkan/utils_gen.py
@@ -43,6 +43,17 @@ const char *vk_obj_str(VkObjectType obj)
default: return "unknown object";
}
}
+
+size_t vk_struct_size(VkStructureType stype)
+{
+ switch (stype) {
+%for struct in vkstructs:
+ case ${struct.stype}: return sizeof(${struct.name});
+%endfor
+
+ default: return 0;
+ }
+}
""")
class Obj(object):
@@ -59,6 +70,22 @@ def get_vkobjects(registry):
yield Obj(enum = e.attrib['name'],
name = e.attrib['comment'])
+def get_vkstructs(registry):
+ for e in registry.findall('types/type[@category="struct"]'):
+ # Strings for platform-specific crap we want to blacklist as they will
+ # most likely cause build failures
+ blacklist_strs = [
+ 'ANDROID', 'Surface', 'Win32', 'D3D12', 'GGP'
+ ]
+
+ if any([ str in e.attrib['name'] for str in blacklist_strs ]):
+ continue
+
+ stype = e.find('member/name[.="sType"]/..')
+ if stype and 'values' in stype.attrib:
+ yield Obj(stype = stype.attrib['values'],
+ name = e.attrib['name'])
+
if __name__ == '__main__':
assert len(sys.argv) == 3
xmlfile = sys.argv[1]
@@ -69,4 +96,5 @@ if __name__ == '__main__':
f.write(TEMPLATE.render(
vkresults = get_vkresults(registry),
vkobjects = get_vkobjects(registry),
+ vkstructs = get_vkstructs(registry),
))
--
GitLab