-
Notifications
You must be signed in to change notification settings - Fork 12
A Proposal for Shader Components #34
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
base: main
Are you sure you want to change the base?
Conversation
juliusikkala
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I quite like it, but I'm a bit worried how alien this feature may feel to developers. If the feature is not familiar and requires a long and complicated explanation, it may be difficult to get people to start using it.
| ``` | ||
| component BadScene1 | ||
| { | ||
| part LightSampling; // ERROR: `RNG` requirement of `LightSampling` was not satsified |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this happen also for require RNG or require IRNG only?
|
|
||
| ##### Named Requirements | ||
|
|
||
| First, we propose to allow a `require` declaration to include an explicit name that will be used to reference the other component in code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this a lot, and would like a similar named import.
| ### Do Nothing | ||
|
|
||
| The obvious alternative to most feature proposals is to simply not adopt the feature. | ||
| In this case, the alternative leaves developers with the two main alternatives we describe in the background section (field-oriented decompositions, and type-based decomposition using `struct`s), when they are trying to achieve good separation of concerns for the features of a shader codebase. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"field-oriented", is this a typo for "file-oriented"?
| The obvious alternative to most feature proposals is to simply not adopt the feature. | ||
| In this case, the alternative leaves developers with the two main alternatives we describe in the background section (field-oriented decompositions, and type-based decomposition using `struct`s), when they are trying to achieve good separation of concerns for the features of a shader codebase. | ||
|
|
||
| We believe that the status quo is not acceptable, but even if others agree, it may not justify the effort and risk that a feature like this entails. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having too many different language mechanisms for separation of concerns could lead some, especially less experienced developers, into "analysis paralysis" - "which one do I choose"?
|
|
||
| * Try to define a subset of `class` types that will be allowed on current GPU targets | ||
|
|
||
| * Find a way to extend modules or `namespace`s so that they can avoid introducing global-scope shader parameters that might not be wanted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding this point, it looks like component-based architecture would be somewhat similar to a module-based architecture (if one module handles one concern), except with more boilerplate:
// LightEnvSampling.slang
import RNG;
Light pickRandomLight()
{
// ...
let r = rand();
// ...
}vs.
import RNG;
component LightEnvSampling
{
require RNG;
Light pickRandomLight()
{
// ...
let r = rand();
// ...
}
}It is true that these aren't exactly the same thing, especially as interface and part come into play. One concept that comes to mind is a "component-module", where the module name is the component name and import is equivalent to require.
I think that would require an "interface-module" as well to reach feature parity with this proposal. An equivalent of "part" for "component-modules" would also be needed. Although in practice, this is only a surface-level, syntactic difference to what is proposed in this proposal.
| A superficially-simple extension of the ideas in this proposal would be to allow functions (and other callables) to include `require` clauses (perhaps in the same syntactic position used for `where` clauses). | ||
| The body of a function could benefit from implicit scoping to access the requirement, and callers of the function could rely on inference of appropriate arguments at calls sites, instead of explicit arguments. | ||
|
|
||
| Such an idea is closely related to the way that `implicit` parameters (and other uses of the `implicit` keyword) work in Scala, and it might be better to instead focus on such a feature as an adoption of ideas from `implicit`s rather than an expansion of `require`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This I do not like at all. Perhaps I'm traumatized by a singular horrid Scala codebase where previous coworkers loved to abuse implicit parameters, while I had very little Scala experience at that time. I found it really hard to figure out what's going on in code where a bunch of things are implicitly pulled from the call site, it's not always immediately clear. Often it felt that implicit only served to confuse the reader and obfuscate the code.
|
We spoke a lot about this already, but figured I’d chime in here too that I’m in favor of this feature proposal. I tend to agree with @juliusikkala that the feature might go unused if it looks too "alien”, though I think this can be addressed. Like you said, main focus here is on the semantics. This would address many of the somewhat pervasive issues I’ve had with slang projects over the years. These days I rarely have vertex or fragment shaders, but I do have many compute kernels all in the same file. Eg a BVH builder can easily be 10 or so compute entrypoints in a single file. A “component” system like this will allow me to keep that code more “DRY” and maintainable. |
Looking for feedback.