From d0897a14da085fb3f799b734090d56d888deec24 Mon Sep 17 00:00:00 2001 From: Robin Voetter Date: Sat, 17 Feb 2024 16:13:37 +0100 Subject: [PATCH] fix parsing issues with vk 1.3.278 This change introduced len/optional attributes for arrays. Previously, we assumed that these would only occur on pointers, but now, this information is also available for arrays. This adapts the registery and parsing code to also parse these properly. No modifications are made to the rendering part as of yet, since these partially filled arrays cannot be cleanly represented in Zig. --- generator/vulkan/c_parse.zig | 2 + generator/vulkan/parse.zig | 78 +++++++++++++++++++++-------------- generator/vulkan/registry.zig | 25 ++++++++++- 3 files changed, 73 insertions(+), 32 deletions(-) diff --git a/generator/vulkan/c_parse.zig b/generator/vulkan/c_parse.zig index 5601ee4..6b8ffe0 100644 --- a/generator/vulkan/c_parse.zig +++ b/generator/vulkan/c_parse.zig @@ -386,6 +386,8 @@ fn parseDeclaration(allocator: Allocator, xctok: *XmlCTokenizer, ptrs_optional: inner_type.* = .{ .array = .{ .size = array_size, + .valid_size = .all, // Refined later + .is_optional = true, .child = child, }, }; diff --git a/generator/vulkan/parse.zig b/generator/vulkan/parse.zig index 8491b81..57fae8a 100644 --- a/generator/vulkan/parse.zig +++ b/generator/vulkan/parse.zig @@ -305,18 +305,36 @@ fn parsePointerMeta(fields: Fields, type_info: *registry.TypeInfo, elem: *xml.El if (elem.getAttribute("len")) |lens| { var it = mem.split(u8, lens, ","); var current_type_info = type_info; - while (current_type_info.* == .pointer) { - // TODO: Check altlen - const size = if (it.next()) |len_str| blk: { - const size_optional = lenToPointer(fields, len_str); - current_type_info.pointer.is_optional = size_optional[1]; - break :blk size_optional[0]; - } else .many; - current_type_info.pointer.size = size; - current_type_info = current_type_info.pointer.child; - len_attribute_depth += 1; - } + while (true) switch (current_type_info.*) { + .pointer => |*ptr| { + if (it.next()) |len_str| { + ptr.size, ptr.is_optional = lenToPointer(fields, len_str); + } else { + ptr.size = .many; + } + + current_type_info = ptr.child; + len_attribute_depth += 1; + }, + .array => |*arr| { + if (it.next()) |len_str| { + const size, _ = lenToPointer(fields, len_str); + arr.valid_size = switch (size) { + .one => .all, + .many => .many, + .other_field => |field| .{ .other_field = field }, + .zero_terminated => .zero_terminated, + }; + } else { + arr.valid_size = .all; + } + + current_type_info = arr.child; + len_attribute_depth += 1; + }, + else => break, + }; if (it.next()) |_| { // There are more elements in the `len` attribute than there are pointers @@ -331,28 +349,28 @@ fn parsePointerMeta(fields: Fields, type_info: *registry.TypeInfo, elem: *xml.El if (elem.getAttribute("optional")) |optionals| { var it = mem.split(u8, optionals, ","); var current_type_info = type_info; - while (current_type_info.* == .pointer) { - if (it.next()) |optional_str| { + while (true) switch (current_type_info.*) { + inline .pointer, .array => |*info| { + if (it.next()) |optional_str| { - // The pointer may have already been marked as optional due to its `len` attribute. - var is_already_optional = false; - if (current_depth < len_attribute_depth) - is_already_optional = current_type_info.pointer.is_optional; + // The pointer may have already been marked as optional due to its `len` attribute. + const is_already_optional = current_depth < len_attribute_depth and info.is_optional; + info.is_optional = is_already_optional or mem.eql(u8, optional_str, "true"); + } else { + // There is no information for this pointer, probably incorrect. + // Currently there is one definition where this is the case, VkCudaLaunchInfoNV. - current_type_info.pointer.is_optional = - is_already_optional or mem.eql(u8, optional_str, "true"); - } else { - // There is no information for this pointer, probably incorrect. - // Currently there is one definition where this is the case, VkCudaLaunchInfoNV. - // We work around these by assuming that they are optional, so that in the case - // that they are, we can assign null to them. - // See https://github.com/Snektron/vulkan-zig/issues/109 - current_type_info.pointer.is_optional = true; - } + // We work around these by assuming that they are optional, so that in the case + // that they are, we can assign null to them. + // See https://github.com/Snektron/vulkan-zig/issues/109 + info.is_optional = true; + } - current_type_info = current_type_info.pointer.child; - current_depth += 1; - } + current_type_info = info.child; + current_depth += 1; + }, + else => break, + }; } } diff --git a/generator/vulkan/registry.zig b/generator/vulkan/registry.zig index 22b210f..2f951bc 100644 --- a/generator/vulkan/registry.zig +++ b/generator/vulkan/registry.zig @@ -123,8 +123,10 @@ pub const Command = struct { pub const Pointer = struct { pub const PointerSize = union(enum) { one, - many, // The length is given by some complex expression, possibly involving another field - other_field: []const u8, // The length is given by some other field or parameter + /// The length is given by some complex expression, possibly involving another field + many, + /// The length is given by some other field or parameter + other_field: []const u8, zero_terminated, }; @@ -140,7 +142,26 @@ pub const Array = struct { alias: []const u8, // Field size is given by an api constant }; + pub const ArrayValidSize = union(enum) { + /// All elements are valid. + all, + /// The length is given by some complex expression, possibly involving another field + many, + /// The length is given by some complex expression, possibly involving another field + other_field: []const u8, + /// The valid elements are terminated by a 0, or by the bounds of the array. + zero_terminated, + }; + + /// This is the total size of the array size: ArraySize, + /// The number of items that are actually filled with valid values + valid_size: ArrayValidSize, + /// Some members may indicate than an array is optional. This happens with + /// VkPhysicalDeviceHostImageCopyPropertiesEXT::optimalTilingLayoutUUID for example. + /// The spec is not entirely clear about what this means, but presumably it should + /// be filled with all zeroes. + is_optional: bool, child: *TypeInfo, };