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

[shader-ast] Better design for defn with anonymous qualified arguments #441

Open
sylee957 opened this issue Jan 3, 2024 · 7 comments
Open

Comments

@sylee957
Copy link

sylee957 commented Jan 3, 2024

I just observe that the signature of defn is a bit messy to work in practice, for some reasons:

  • Although named arguments may be useful for some debugging purpose, however, I think that it is least useful in practice. There may be some reason that function names, or global variable names to have fixed name, if I need to link the shader into some other code, however, names of function parameter variables or local variables are completely arbitrary.
    And making them anonymous is often better because I don't want to repeat names, one for javascript variable identifier and the other for shader
    (const myfunc = defn(..., 'myfunc', ...), or defn(..., [['float', 'x'], ['float', 'y']], (x, y) => {})
  • Qualifiers, like out or inout, are much more useful, especially if I have to port some code that someone have made, or if I have to work around to avoid struct.

Although I can around the issue by implementing some 'helper' for defn, however, I wonder the better design of the signature could be:

defn(type: Type, args: Arg<Type>[], body: (...Arg<Type>[]) => Term[], name?: string)

where it has advantages

  • It may be better to pull out _args.map(defArg) expose factory like defArg as user API. I think that it is better for deciphering the types if we expect user to make objects homogeneously.
  • The names should be more optional and put into backwards, so users don't need to fill undefined or null positionally if they want anonymous functions or variables.
@postspectacular
Copy link
Member

Can you please provide a more concrete example of what you're proposing here? I'm not sure I follow... if this is just to avoid having to give a name for args when also wanting to specify its SymOpts, then I'd propse to change the type of Arg instead (because I don't understand your proposal for defn...) and change the tuple type into an arg object:

// current
export type Arg<A extends Type> = A | [A, string?, SymOpts?];

// maybe could be...
export type Arg<A extends Type> = A | Pick<SymOpts, "q" | "num" | "prec"> & { type: A, id?: string };

Then use in situ like so:

defn("float", [{q: "in", type: "float", num: 4}], (x) => [...])

@sylee957
Copy link
Author

sylee957 commented Jan 3, 2024

I expect something like

defn("float", [arg('float'), inarg('float'), outarg('float')], (x, y, z) => [...])

@sylee957
Copy link
Author

sylee957 commented Jan 3, 2024

And also, simple type like export type Arg<A extends Type> = { type: A, id?: string, tag: 'arg' };
could be better if we just expect users to use factories like above instead of plain objects.
(Although we would anyway deal with backward compatibility of keeping A as bare strings)

@postspectacular
Copy link
Member

@sylee957 Thank you for all these thoughts, but may I please ask a question here: What is your general use case for this library? I'm just genuinely super curious, since you've been filing a ton of issues in a very short amount of time and mainly about aspects I'd personally consider pretty specialized (not necessarily edge cases, but definitely aspects I've rarely needed in the 4.5 years since this project started). E.g. case in hand here: The times I've required defining function args with options was only when working with arrays args or a handful of cases with inout qualified args. The other 99% is just passing the arg type names. Please don't take this the wrong way, I'm just trying to understand how all these different issues, requests & preferences all fit together (from your POV)... What are you working on (if I may ask)?

Also, related, there're features (like support for structs and uniform blocks) I definitely would like to support and have attempted several times in the past, but always got stuck by getting into super complex type constructs and generics nightmares before giving up again. In those situations, I often come to the conclusion that my time is better spent on more useful things... at least until there's a more obvious way forward.

Lastly, I also want to stress once more that GLSL (and its semantics/details) is always just gonna be one (if currently still the main) target of multiple others and 100% feature parity with GLSL was never the aim. This coming year I want to focus more on WebGPU/WGSL — so please do keep these aspects in mind here too...

@sylee957
Copy link
Author

sylee957 commented Jan 4, 2024

I often come to the conclusion that my time is better spent on more useful things... at least until there's a more obvious way forward.

Unfortunately, I'm unclear about the interest or roadmap about the project, after hearing this comment though.

Sadly, I don't think that there is much competitor projects in implementing shader languages in Javascript.
I think that I may look for TSL in three.js, if they have more interests and bigger community to deal with more general use cases, thanks.

@postspectacular
Copy link
Member

@sylee957 I think you're very much misreading my previous comment: "My time being better spent" referred to my own previous attempts to add struct & UBO support, absolutely NOT regarding your efforts... I was (still am) genuinely interested in learning more abour your use case(s), since it would help me to better understand how you're intending to use this package (and what for). Some of the changes & syntax features you address are great for general completeness, but I've been trying to explain in your various issues a) why certain things are not there (yet) or are currently implemented differently than what you propose (e.g. template literal types discussion in #438 ) and b) I'm always cautious about spending too much precious time on things with dubious ROI, since each of these additions also means increased long term maintenance effort (usually on my end). I believe these are all fair points, but obviously you beg to differ... it's also why I've been repeatedly asking you to provide a bit more context about your own interest/usage of this project.

I've not said "no" to any of your points, just please understand that this package is just a small fragment of a much larger undertaking and there's a need to first experiment with some of these proposals (to get a better "feel" for them) before comitting to them. Some of the things in question are quite fundational (but also subjective/opinionated) changes (e.g. the above proposed arg handling) and so please understand that I need to ask for more context (or have different views) and/or need more time to think about wider implications of some of these proposals. None of these things should imply disinterest and I apologize if this is what you took away from this....

@sylee957
Copy link
Author

sylee957 commented Jan 4, 2024

I was (still am) genuinely interested in learning more abour your use case(s), since it would help me to better understand how you're intending to use this package (and what for).

I’m doing studies of how to supplement all the knowledges I have about typescript, functional programming, object oriented programming, dependent types,
to implement shaders more formally and completely in GLSL specifications,
and implement some complimentary features in downstream project for the things that you find opinionated, difficult, or hadn’t worked on, important, or less important.

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

No branches or pull requests

2 participants