On Mon Nov 28 23:15:57 2022 +0000, Francisco Casas wrote:
HLSL_TYPE_MODIFIERS_MASK isn't actually stored here; it only goes on
the type [v. apply_type_modifiers()]; Ah, I see now.
it's valid to put nointerpolation on other shader types.
Yep, I failed to mention that pixel shader inputs can also be vertex shader outputs.
I also don't know if it's *correct* that we return an error for all
other storage modifiers. But conceptually I wouldn't bother mentioning exactly which HLSL_STORAGE_* flags are valid because I don't think it's important in order to understand the code. I.e. any other storage flags are either going to be filtered out by hlsl_error() or they're just going to be ignored. We currently trigger an error for all modifiers that are not consumed by `apply_type_modifiers()` except for `HLSL_STORAGE_NOINTERPOLATION` in the `field:` rule. I will just mention that interpolation modifiers are a use case for the field.
[I do think it makes sense to mark that HLSL_MODIFIER_* is stored
on the type but HLSL_STORAGE_* is on the hlsl_field/hlsl_var, lest someone ask "why do we have two different modifiers fields that store the same information?") Got it, I will see how to make this more clear.
Honestly, "rename the field to 'storage_flags'" wouldn't be a bad idea.
Clearly, if nothing else, these documentation patches are pointing out a lot of things that we could do better in terms of code naming and organization. Which is probably not unexpected :-)