8000 Basic support for HLSL templating by new-cassellito · Pull Request #434 · laurentlb/shader-minifier · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Basic support for HLSL templating #434

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

Merged
merged 4 commits into from
Aug 9, 2024

Conversation

new-cassellito
Copy link
Contributor

Support parsing & pass-through of basic HLSL templating introduced in HLSL 2021: https://devblogs.microsoft.com/directx/announcing-hlsl-2021/

Let me know if there's a way to do this that makes more sense, not too familiar with the codebase

parse and pass through class inheritance

fix templated struct declaration

add unit test
@new-cassellito
Copy link
Contributor Author

github isn't letting me request a review for some reason - @laurentlb @eldritchconundrum

@eldritchconundrum
Copy link
Collaborator

Support for accepting (and mostly ignoring for now) templates (and inheritance)... Interesting!

  • template<> being parsed as TLVerbatim seems a bit too unstructured, we could add it into the TypeDecl that comes after it.
  • At this point, maybe TypeBlock should probably get its own type, with named members.
  • The inside of <...> is kept as a string for now, not parsed at all. No interactions with renames for now I suppose.
  • The tests only use templates as top-level type declarations, but this also supports declaring local and global variables of templated types. We could have tests for that.

That looks good!

Comment on lines +6 to +7
template<>
struct is_floating_point<float>:true_type{};
Copy link
Contributor Author
@new-cassellito new-cassellito Aug 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eldritchconundrum should the template defn be emitted on the same line? does that matter for minification? same char count. '\n' or ' '

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It matters a little because it affects the compression of the generated shader.
In the printer, we use \n when the newline is mandatory (the shader won't work without it), and we use \0 to mean "unessential" newline that can be removed by the formatter when no indentation is needed (e.g. with --format text).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah got it, i guess that would require not having the template definition in a verbatim block though

Copy link
Collaborator
@eldritchconundrum eldritchconundrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I have improvements ideas, hopefully I'll do them next week.

@new-cassellito
Copy link
Contributor Author

Looks good to me. I have improvements ideas, hopefully I'll do them next week.

sweet tag me in the PR(s) if you're down i'd love to help - i'm tryna get stuff like _Static_assert(is_floating_point<T>::value); working so i can use your awesome tool with code using language features invented after the 80s 🙃

@laurentlb laurentlb merged commit 0879fd7 into laurentlb:master Aug 9, 2024
1 check passed
@laurentlb
Copy link
Owner

Thanks a lot!

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
0