Dav1dde / glad

Multi-Language Vulkan/GL/GLES/EGL/GLX/WGL Loader-Generator based on the official specs.
https://glad.dav1d.de/
Other
3.79k stars 448 forks source link

current vk.xml confuses glad2 #411

Closed tycho closed 1 year ago

tycho commented 1 year ago

Khronos made some changes to the Vulkan specification XML that appear to confuse GLAD:

$ glad --api vulkan --out-path="$PWD" c --loader --alias
[17.02.2023 02:56:32][DEBUG     ][glad.plugin   ]: loaded language c: <class 'glad.generator.c.__init__.CGenerator'>
[17.02.2023 02:56:32][DEBUG     ][glad.plugin   ]: loaded language rust: <class 'glad.generator.rust.__init__.RustGenerator'>
[17.02.2023 02:56:32][DEBUG     ][glad.plugin   ]: loaded specification egl: <class 'glad.specification.EGL'>
[17.02.2023 02:56:32][DEBUG     ][glad.plugin   ]: loaded specification gl: <class 'glad.specification.GL'>
[17.02.2023 02:56:32][DEBUG     ][glad.plugin   ]: loaded specification glx: <class 'glad.specification.GLX'>
[17.02.2023 02:56:32][DEBUG     ][glad.plugin   ]: loaded specification vk: <class 'glad.specification.VK'>
[17.02.2023 02:56:32][DEBUG     ][glad.plugin   ]: loaded specification wgl: <class 'glad.specification.WGL'>
[17.02.2023 02:56:32][INFO      ][glad          ]: getting 'vk' specification from remote location
[17.02.2023 02:56:32][INFO      ][glad.opener   ]: opening: 'https://raw.githubusercontent.com/KhronosGroup/Vulkan-Docs/main/xml/vk.xml'
[17.02.2023 02:56:32][INFO      ][glad          ]: generating vulkan:None/vk=None
Traceback (most recent call last):
  File "/home/steven/Development/glad-venv/bin/glad", line 33, in <module>
    sys.exit(load_entry_point('glad2', 'console_scripts', 'glad')())
  File "/home/steven/Development/glad/glad/__main__.py", line 181, in main
    feature_sets = list(select(specification, api, info) for api, info in apis)
  File "/home/steven/Development/glad/glad/__main__.py", line 181, in <genexpr>
    feature_sets = list(select(specification, api, info) for api, info in apis)
  File "/home/steven/Development/glad/glad/__main__.py", line 178, in select
    return generator.select(specification, api, info.version, info.profile, extensions, config, sink=logging_sink)
  File "/home/steven/Development/glad/glad/generator/c/__init__.py", line 321, in select
    return JinjaGenerator.select(self, spec, api, version, profile, extensions, config, sink=sink)
  File "/home/steven/Development/glad/glad/generator/__init__.py", line 60, in select
    return spec.select(api, version, profile, extensions, sink=sink)
  File "/home/steven/Development/glad/glad/parse.py", line 657, in select
    raise ValueError('Invalid API {!r} for specification {!r}'.format(api, self.name))
ValueError: Invalid API 'vulkan' for specification 'vk'

Seems to be a result of Khronos adding comma-separated API names, e.g.:

<feature api="vulkan,vulkansc" name="VK_VERSION_1_0" number="1.0" comment="Vulkan core API interface definitions">

I tried doing a quick workaround:

diff --git a/glad/parse.py b/glad/parse.py
index a133b40..807f923 100644
--- a/glad/parse.py
+++ b/glad/parse.py
@@ -450,7 +450,8 @@ class Specification(object):
         features = defaultdict(dict)
         for element in self.root.iter('feature'):
             num = Version(*map(int, element.attrib['number'].split('.')))
-            features[element.attrib['api']][num] = Feature.from_element(element)
+            for api in element.attrib['api'].split(','):
+                features[api][num] = Feature.from_element(element)

         for api, api_features in features.items():
             features[api] = OrderedDict(sorted(api_features.items(), key=lambda x: x[0]))

but this leads to other issues I haven't dug into yet:

$ glad --api vulkan --out-path="$PWD" c --loader --alias
[17.02.2023 03:00:54][DEBUG     ][glad.plugin   ]: loaded language c: <class 'glad.generator.c.__init__.CGenerator'>
[17.02.2023 03:00:54][DEBUG     ][glad.plugin   ]: loaded language rust: <class 'glad.generator.rust.__init__.RustGenerator'>
[17.02.2023 03:00:54][DEBUG     ][glad.plugin   ]: loaded specification egl: <class 'glad.specification.EGL'>
[17.02.2023 03:00:54][DEBUG     ][glad.plugin   ]: loaded specification gl: <class 'glad.specification.GL'>
[17.02.2023 03:00:54][DEBUG     ][glad.plugin   ]: loaded specification glx: <class 'glad.specification.GLX'>
[17.02.2023 03:00:54][DEBUG     ][glad.plugin   ]: loaded specification vk: <class 'glad.specification.VK'>
[17.02.2023 03:00:54][DEBUG     ][glad.plugin   ]: loaded specification wgl: <class 'glad.specification.WGL'>
[17.02.2023 03:00:54][INFO      ][glad          ]: getting 'vk' specification from remote location
[17.02.2023 03:00:54][INFO      ][glad.opener   ]: opening: 'https://raw.githubusercontent.com/KhronosGroup/Vulkan-Docs/main/xml/vk.xml'
[17.02.2023 03:00:54][INFO      ][glad          ]: generating vulkan:None/vk=None
[17.02.2023 03:00:54][INFO      ][glad          ]: no explicit version given for api vulkan, using Version(major=1, minor=3)
[17.02.2023 03:00:54][INFO      ][glad          ]: adding all extensions for api vulkan to result
[17.02.2023 03:00:54][DEBUG     ][glad.parse    ]: type VkQueryPoolCreateFlagBits with category enum but without <enums>
[17.02.2023 03:00:55][DEBUG     ][glad.parse    ]: type VkDeviceCreateFlagBits with category enum but without <enums>
Traceback (most recent call last):
  File "/home/steven/Development/glad-venv/bin/glad", line 33, in <module>
    sys.exit(load_entry_point('glad2', 'console_scripts', 'glad')())
  File "/home/steven/Development/glad/glad/__main__.py", line 181, in main
    feature_sets = list(select(specification, api, info) for api, info in apis)
  File "/home/steven/Development/glad/glad/__main__.py", line 181, in <genexpr>
    feature_sets = list(select(specification, api, info) for api, info in apis)
  File "/home/steven/Development/glad/glad/__main__.py", line 178, in select
    return generator.select(specification, api, info.version, info.profile, extensions, config, sink=logging_sink)
  File "/home/steven/Development/glad/glad/generator/c/__init__.py", line 321, in select
    return JinjaGenerator.select(self, spec, api, version, profile, extensions, config, sink=sink)
  File "/home/steven/Development/glad/glad/generator/__init__.py", line 60, in select
    return spec.select(api, version, profile, extensions, sink=sink)
  File "/home/steven/Development/glad/glad/parse.py", line 715, in select
    result = result.union(found)
  File "/home/steven/Development/glad/glad/parse.py", line 523, in find
    self._combined.update(self.types)
  File "/home/steven/Development/glad/glad/util.py", line 165, in memoized
    cache[key] = func(*args, **kwargs)
  File "/home/steven/Development/glad/glad/parse.py", line 350, in types
    raise ValueError('extension enum {} required multiple times '
ValueError: extension enum VK_ERROR_UNKNOWN required multiple times with different values
tycho commented 1 year ago

This workaround seems to generate an almost-correct header + loader, but I don't like commenting out that check:

diff --git a/glad/parse.py b/glad/parse.py
index a133b40..5492a38 100644
--- a/glad/parse.py
+++ b/glad/parse.py
@@ -367,9 +367,10 @@ class Specification(object):
                             else:
                                 # technically not required, but better throw more
                                 # than generate broken code because of a broken specification
-                                if not enum.value == enums[enum.name].value:
-                                    raise ValueError('extension enum {} required multiple times '
-                                                     'with different values'.format(e.name))
+                                #if not enum.value == enums[enum.name].value:
+                                #    raise ValueError('extension enum {} required multiple times '
+                                #                     'with different values'.format(e.name))
+                                pass

                             enums[enum.name].also_extended_by(extension.attrib['name'])
                     t.enums = list(enums.values())
@@ -450,7 +451,8 @@ class Specification(object):
         features = defaultdict(dict)
         for element in self.root.iter('feature'):
             num = Version(*map(int, element.attrib['number'].split('.')))
-            features[element.attrib['api']][num] = Feature.from_element(element)
+            for api in element.attrib['api'].split(','):
+                features[api][num] = Feature.from_element(element)

         for api, api_features in features.items():
             features[api] = OrderedDict(sorted(api_features.items(), key=lambda x: x[0]))
@@ -1321,7 +1323,7 @@ class Extension(IdentifiedByName):
     @classmethod
     def from_element(cls, element):
         name = element.attrib['name']
-        supported = element.attrib['supported'].split('|')
+        supported = re.split(r'\||,', element.attrib['supported'])

         requires = [Require.from_element(require) for require in element.findall('require')]

There's some wonky results though, so there is still something wrong:

typedef struct VkPipelineShaderStageCreateInfo {
    VkStructureType   sType;
    const  void *             pNext;
    VkPipelineShaderStageCreateFlags      flags;
    VkShaderStageFlagBits    stage;
    VkShaderModule   module;
    const  char *  pName;
    const  char *  pName; // <--- DUPE, one is defined as api="vulkan" another is defined as api="vulkansc", so it gets added twice somehow
    const  VkSpecializationInfo *  pSpecializationInfo;
} VkPipelineShaderStageCreateInfo;

It looks like the member element handler needs to understand the api attribute. And looking at other issues, it looks like the param handler needs it as well.

Dav1dde commented 1 year ago

Thanks for making me aware of it and the good summary.

This is quite an annoying change to deal with and makes parsing the XML so much harder then it needs to be with a lot more edge cases than before. Instead of allowing sub attributes (struct members and command parameters) to behave differently between APIs they could have just duplicated the blocks and used the previously already used api attribute on those.

I would argue this doesn't even help for human readability, it certainly makes it a lot harder to parse from code.

Glad now duplicates enums, types and commands for each found sub-api. The selection algorithm could already deal with that (used in GL).

The additional change introduced with depends attributes does not affect glad, because it already has a smarter dependency selection algorithm and does not rely on these attributes at all.

I am currently finishing a fix for this. If everything goes well there will be a fixed version within 30 minutes.

tycho commented 1 year ago

Thanks for the quick fix @Dav1dde!

And I agree, it makes a mess of the XML that didn't need to be there, and is inconsistent with the GL XML as well (the pipe vs comma delimiting for example).