Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Vulkan functions, types and variables #9

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

JolifantoBambla
Copy link

Hey, I've added all functions, variables and types from GL_KHR_vulkan_glsl using the [spec for GLSL version 4.60(https://www.khronos.org/registry/OpenGL/specs/gl/GLSLangSpec.4.60.html) as a reference.

I've added the properties :opengl-only and :vulkan-only for all functions and variables which could be useful for validation.

I've also added the new texture combined sampler constructors in extra files (texture-combined-sampler-constructors).
See section 5.4.5 in the spec.

I didn't regenerate the docs though, because I wasn't sure which repository is meant by the comment in dump.lisp:

- clone docs.gl repo

Is it this one? If so, it doesn't contain Vulkan stuff anyways.

@JolifantoBambla
Copy link
Author

@cbaggers Sorry, I missed that version numbers :xxx represent the compatibility profile. I changed all versions for Vulkan specfic stuff to :xxx-CORE now.

@JolifantoBambla
Copy link
Author

@cbaggers just linking the other PR in varjo

@cbaggers
Copy link
Owner

Oh wow, great work. I'll take a look at this sometime this next week

@baggers-br
Copy link

Well that week got away from me. I'm very interested in getting this in. I'll keep you posted

@baggers-br
Copy link

Great. This is looking very promising. opengl-only and vulkan-only are neat but would break things for those using destructuring-bind on the entries. I'm wondering if we could use the :versions list instead.
That would mean :100-vulkan is vulkan support. Instead of vulkan-only the list would be '(:100-vulkan) and gl only would be a list without the :100-vulkan keyword.
This feels like it would be in line with the handling of core that way (e.g. :330-core)

@baggers-br
Copy link

Hmm I guess it's an issue vulkan version is independent of glsl version

This extension is not enabled with a #extension as other extensions are.
It is also not enabled through use of a profile or #version. The intended
level of GLSL/ESSL features, independent from Vulkan-specific usage, comes
from the traditional use of #version, profile, and #extension.

@baggers-br
Copy link

I considered a :extensions list to each entry. It would be a list of extensions required to use the element. This way we could have :100-vulkan in that list when it's vulkan only. This could be handy in general (for supporting extensions) but this would leave us still needing a way to express gl-only.

Maybe then the list is a more general :requirements list. Then:

  • :vulkan-only nil :opengl-only nil would map to an empty requirement list
  • :vulkan-only nil :opengl-only t would map to a requirement list containing :opengl
  • :vulkan-only t :opengl-only nil would map to a requirement list containing :100-vulkan

We can then reuse this list for extensions too.

@baggers-br
Copy link

If we do try a :requirements list then it would be different from :versions as the latter just requires that your version is somewhere in the list whereas :requirements requires everything to be set.

This makes the 100-vulkan requirement a bit tricky as what happens when a new vulkan version comes around? You can't have both 100-vulkan and XXX-vulkan in the hypothetical list.

I guess we just use :vulkan and than then in future we can allow nested lists in :requirements to indicate 'on of these' e.g. ((:100-vulkan :200-vulkan) :some-other-extension)

@baggers-br
Copy link

In the process of thinking about this I made the following branches to see how the code would look:

@baggers-br
Copy link

p.s. sorry about the force-push there, I screwed up which repo I was pushing code to

@JolifantoBambla
Copy link
Author

Awesome! I'm leaning more towards the :requirements list, because it also clears the path for functions and types which require GLSL extensions to be enabled. For example, all functions and types introduced in the NV and KHR ray tracing & ray query extensions require their respective extension to be enabled in the shader directly, so it would be nice to have a way of specifying this here.

Would you be fine with mixing keywords (for :vulkan/:opengl) and strings (for extension names (e.g. "GL_EXT_ray_query") in the requirements list or would you specify extensions as keywords as well? (I'm thinking about adding the ray tracing stuff as soon as I have some hardware to test it.)

Oh and I guess you've seen it already, but if not: the new types (e.g. subpassInput) referenced by the functions in this PR break parsing the spec in Varjo's master, because they have no counterpart in Varjo/Vari. So if you merge this PR before my other one over there you'll have to make some changes there too before the next quicklisp update comes around. Sorry again for the inconvenience :/

@JolifantoBambla
Copy link
Author

And I think this is the way to go:

I guess we just use :vulkan and than then in future we can allow nested lists in :requirements to indicate 'on of these' e.g. ((:100-vulkan :200-vulkan) :some-other-extension)

@JolifantoBambla
Copy link
Author

In the Vulkan spec they differentiate between features (like Vulkan 1.0, 1.1 & 1.2) and extensions. So the :requirements list could also be split into a :features and an :extensions list.
vulkan-only nil opengl-only nil would map to :features '(:opengl :vulkan)
Might be overkill though, especially if new Vulkan specific stuff gets introduced via GLSL version specs directly instead of spec extensions.

@cbaggers
Copy link
Owner

Nice! I really like the sounds of features and requirements so I'll mull that over.
I'm on the road today but hope to have some time to code tomorrow or the next day. Thanks again for all this work 🙂

@JolifantoBambla
Copy link
Author

Cool! Thanks! Glad to help :)

@JolifantoBambla
Copy link
Author

JolifantoBambla commented Sep 8, 2021

Hey, I added a branch to see how a features list replacing opengl-only & vulkan-only would look like:
https://github.com/JolifantoBambla/glsl-spec/tree/features-extensions

In the future, stuff that would require an extension to be enabled would get an additional list.
So a function like void rayQueryGenerateIntersectionEXT(rayQueryEXT q, float tHit); would look like this:

(:LISP-NAME "RAY-QUERY-GENERATE-INTERSECTION-EXT" :NAME "rayQueryGenerateIntersectionEXT" :RETURN "void" :ARGS (("q" "rayQueryEXT") ("tHit" "float")) :VERSIONS (:140-CORE :150-CORE :330-CORE :400-CORE :410-CORE :420-CORE :430-CORE :440-CORE :450-CORE :460-CORE) :PURE NIL :FEATURES (:VULKAN) :EXTENSIONS ("GL_EXT_ray_query"))

@baggers-br
Copy link

I fell of the lisp wagon again. I'm getting caught up over the Christmas break

@baggers-br
Copy link

Wow, you spotted that fast :D I think I'm gonna start experimenting with just :requirements. :features might make a reappearance late, but I just need to get moving on these projects again.

@JolifantoBambla
Copy link
Author

Haha yeah, I was just on my phone when the email notification popped up^^ Sounds good :)

@JolifantoBambla
Copy link
Author

Hey, I think I'll start adding the RT extensions in my forks soonish (within the next three months or so), which would require some way of specifying extensions.
With a :requirements list this would look like this:

(:LISP-NAME "RAY-QUERY-GENERATE-INTERSECTION-EXT" :NAME "rayQueryGenerateIntersectionEXT" :RETURN "void" :ARGS (("q" "rayQueryEXT") ("tHit" "float")) :VERSIONS (:140-CORE :150-CORE :330-CORE :400-CORE :410-CORE :420-CORE :430-CORE :440-CORE :450-CORE :460-CORE) :PURE NIL :REQUIREMENTS ((:VULKAN) <extension>))

where <extension> would be replaced by one of

  • "GL_EXT_ray_query"
  • :gl-ext-ray-query
  • :ray-query (not sure if all extensions have the same prefix. Probably not)

Do you have a preference on the format?
I think I'd go for strings, since I guess keywords would require an extra file mapping them to their names, right?

:requirements could also use keywords in the sublist to be more explicit, but would probably only make sense if it would contain more than just APIs and an extension (I don't think this will be the case):

(:api (:VULKAN) :extension <extension>)

@JolifantoBambla
Copy link
Author

I've now added all functions and variables defined by the GL_EXT_ray_query extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants