vkd3d-shader: Signature reflection API.
There are two parts to this:
-
First, a way to retrieve any signature from a DXBC shader. This is, I gather, generally useful for reflection, and it can be used as one source with which to implement d3d10 and d3d11 shader reflection APIs.
The rest of those APIs will need much more data to be exposed from d3d shaders, and while I was originally planning to expose that all in a single "vkd3d_shader_d3d_shader_info" structure, I think that signatures at least are a reasonable enough subset to have a dedicated structure. Moreover, I did not want to block sm1 support on too much API design.
-
Second, signatures synthesized from sm1 byte code. This is conceptually a bit weird, because sm1 does not have signatures, but in terms of how these APIs are used by Wine (or other translation layers, as evidenced not least by the Vulkan test shader runner, which I have locally adapted for sm1 but not submitted yet) it fits rather nicely.
Because this is new API, it of course deserves plenty of discussion, especially the sm1 part. Several open questions which occurred to me while writing are:
-
Should we fix the mask (and used mask) for sm1 signatures as 0xf rather than 0? SPIR-V cares about this in order to declare I/O variables, which makes some amount of sense. In fact I have a patch in my local tree to change this, specifically for that purpose. However, we could also normalize it later.
-
If we do fix the mask as nonzero, should single-component semantics (fog, depth, etc...) be declared as 0x1 instead of 0xf?
-
Should BLENDINDICES get a UINT type? It's float in shader instructions (well, kind of, although in practice it's used as an array index of course), but the vertex attribute type is in practice "supposed" to be integer.
Part 1 of a series to implement sm1 -> spirv translation in vkd3d-shader, brought to you by late nights spent coding and rereading The Waste Land.
Ganga was sunken, and the limp leaves
Waited for rain, while the black clouds
Gathered far distant, over Himavant.
Merge request reports
Activity
Second, signatures synthesized from sm1 byte code. This is conceptually a bit weird, because sm1 does not have signatures, but in terms of how these APIs are used by Wine (or other translation layers, as evidenced not least by the Vulkan test shader runner, which I have locally adapted for sm1 but not submitted yet) it fits rather nicely.
It's not that weird. Shaders have interfaces with each other and the rest of the API. For d3dbc shaders those are largely implicit interfaces, while tpf made them much more explicit. We could call this something else if you prefer, but conceptually it's very similar to the existing vkd3d_shader_scan_descriptor_info structure. In principle there's no reason we couldn't return this kind of information for SPIR-V/HLSL/GLSL/MSL shaders either.
Also, it's perhaps worth pointing out that a (fairly rudimentary) version of this exist in d3dx9 as D3DXGetShaderInputSemantics() and D3DXGetShaderOutputSemantics(); it might be worth making sure that we're able to implement those on top of this, although it doesn't look too hard.
Because this is new API, it of course deserves plenty of discussion, especially the sm1 part. Several open questions which occurred to me while writing are:
- Should we fix the mask (and used mask) for sm1 signatures as 0xf rather than 0? SPIR-V cares about this in order to declare I/O variables, which makes some amount of sense. In fact I have a patch in my local tree to change this, specifically for that purpose. However, we could also normalize it later.
- If we do fix the mask as nonzero, should single-component semantics (fog, depth, etc...) be declared as 0x1 instead of 0xf?
- Should BLENDINDICES get a UINT type? It's float in shader instructions (well, kind of, although in practice it's used as an array index of course), but the vertex attribute type is in practice "supposed" to be integer.
Could we just provide accurate information from the point of view of the user of the API? I.e., provide the actual masks and data types the shader expects and uses?
I'm not quite sure what that means in practice for BLENDINDICES; in wined3d we seem to map everything to WINED3D_TYPE_FLOAT, but it's not clear if that's correct because we have very little existing coverage for BLENDINDICES. But if e.g. Vulkan should pass the data for this as VK_FORMAT_R32_UINT, I think it should be UINT, yes.
+static bool vkd3d_shader_signature_from_shader_signature(struct vkd3d_shader_signature *signature, + const struct shader_signature *src);
I imagine we'll just want to move the function in the final version of this series and avoid the forward declaration.
+static bool add_signature_element(struct shader_signature *signature, const char *name, + unsigned int index, enum vkd3d_shader_sysval_semantic sysval, unsigned int register_index) +{ [...] + element->semantic_name = name; + element->semantic_index = index; + element->stream_index = 0; + element->sysval_semantic = sysval; + element->component_type = VKD3D_SHADER_COMPONENT_FLOAT; + element->register_index = register_index; + element->mask = 0; + element->used_mask = 0; + element->min_precision = VKD3D_SHADER_MINIMUM_PRECISION_NONE; + + return true; +}
I think we should set the "register_count" field above. It will be largely unused for the moment, but vkd3d_shader_signature_from_shader_signature() will print a FIXME if it ends up being larger than 1.
Most of the FIXMEs should use vkd3d_parser_error()/vkd3d_parser_warning().
Could we just provide accurate information from the point of view of the user of the API? I.e., provide the actual masks and data types the shader expects and uses?
I think so.
I'm not quite sure what that means in practice for BLENDINDICES; in wined3d we seem to map everything to WINED3D_TYPE_FLOAT, but it's not clear if that's correct because we have very little existing coverage for BLENDINDICES. But if e.g. Vulkan should pass the data for this as VK_FORMAT_R32_UINT, I think it should be UINT, yes.
I'm going off memory, but I think it's legal in d3d9 to use any data type in the vertex declaration. I think it's also legal for d3d11, even; i.e. there's no need to actually match the shader data type. In practice this means we hit Vulkan validation errors, which we'd need yet more interface data to avoid, and I believe these validation errors also matter (i.e. the driver will just bit-cast the data if the type doesn't match.)
So if my memory is correct in this respect, the vertex attribute type doesn't exactly matter. On the other hand, we can (as always) provide reasonable guesses, and UINT is the best reasonable guess for BLENDINDICES, so it does kind of seem like the most sensible thing to me.
Wrt wined3d, I haven't checked, but GL may implicitly do a proper cast?
I imagine we'll just want to move the function in the final version of this series and avoid the forward declaration.
Sure, I'll add an extra patch for that.
I think we should set the "register_count" field above. It will be largely unused for the moment, but vkd3d_shader_signature_from_shader_signature() will print a FIXME if it ends up being larger than 1.
Indeed; that was oversight while rebasing onto Conor's work.
Most of the FIXMEs should use vkd3d_parser_error()/vkd3d_parser_warning().
Sounds right.
I'm going off memory, but I think it's legal in d3d9 to use any data type in the vertex declaration. I think it's also legal for d3d11, even; i.e. there's no need to actually match the shader data type. In practice this means we hit Vulkan validation errors, which we'd need yet more interface data to avoid, and I believe these validation errors also matter (i.e. the driver will just bit-cast the data if the type doesn't match.)
So if my memory is correct in this respect, the vertex attribute type doesn't exactly matter. On the other hand, we can (as always) provide reasonable guesses, and UINT is the best reasonable guess for BLENDINDICES, so it does kind of seem like the most sensible thing to me.
Wrt wined3d, I haven't checked, but GL may implicitly do a proper cast?
Direct3D 9 and the OpenGL of that time didn't have proper integer attributes; you could send integer data to the driver/GPU, but in the shader it would be visible as floating point data. The GPUs of that time generally just didn't have the capability of doing e.g. integer multiplication. Note also that most of the d3d9 vertex declaration data types are either floating-point formats or normalised formats, although e.g. D3DDECLTYPE_UBYTE4 does exist.
The OpenGL interface is perhaps illustrative here; compare the behaviour of e.g. glVertexAttrib1i() with the behaviour of glVertexAttribI1i(). The GL spec calls these "pure integers".
I think that what you're saying implies that FLOAT would be the correct type for BLENDINDICES.
I think that what you're saying implies that FLOAT would be the correct type for BLENDINDICES.
I think so, but that does then force us to add that extra interface, whereas we could otherwise (always? usually?) get away with guessing UINT for blend indices and FLOAT otherwise. Which is annoying :-/
(I did say that I ran into Vulkan validation errors, but on second reflection I'm not sure if I have, except for BLENDINDICES.)
I think that what you're saying implies that FLOAT would be the correct type for BLENDINDICES.
I think so, but that does then force us to add that extra interface, whereas we could otherwise (always? usually?) get away with guessing UINT for blend indices and FLOAT otherwise. Which is annoying :-/
We could add a compilation option for "pure integer" BLENDINDICES which would perhaps make the issue go away for most cases in practice, but in principle I don't think it would prevent an application from e.g. using D3DDECLTYPE_UBYTE4 texture coordinates.
Okay, I went and actually double checked, and though I didn't test exhaustively, it looks like d3d9 will always cast to float, but d3d11 will simply bit-cast. At least, R32G32B32A32_UINT seems to behave the same as R32G32B32A32_FLOAT.
I feel like you can make a reasonable argument for always defaulting d3d9 attributes to float, for defaulting BLENDINDICES to UINT and the rest to float, and for not having a default at all and making them some VKD3D_SHADER_COMPONENT_UNKNOWN or VOID type.
I feel like it has to be unlikely that any d3d8/9 application is ever going to deviate from the "blendindices is uint, everything else is float" pattern, but if you think it's more sensible to default everything to float and just add that interface anyway, I can take that approach...
Okay, I went and actually double checked, and though I didn't test exhaustively, it looks like d3d9 will always cast to float, but d3d11 will simply bit-cast. At least, R32G32B32A32_UINT seems to behave the same as R32G32B32A32_FLOAT.
Sound like what we'd expect.
I feel like you can make a reasonable argument for always defaulting d3d9 attributes to float, for defaulting BLENDINDICES to UINT and the rest to float, and for not having a default at all and making them some VKD3D_SHADER_COMPONENT_UNKNOWN or VOID type.
I feel like it has to be unlikely that any d3d8/9 application is ever going to deviate from the "blendindices is uint, everything else is float" pattern, but if you think it's more sensible to default everything to float and just add that interface anyway, I can take that approach...
Actually, I don't think we need to handle anything in the shader for this. Vulkan has the _USCALED/_SSCALED formats (and VK_FORMAT_R8G8B8A8_USCALED in particular) which look like what we want here. That would probably imply introducing WINED3DFMT_R8G8B8A8_USCALED and a few other formats and then distinguishing them from their _UINT/_SINT variant, but that would be a wined3d issue, and probably not a bad thing in any case...
Actually, I don't think we need to handle anything in the shader for this. Vulkan has the _USCALED/_SSCALED formats (and VK_FORMAT_R8G8B8A8_USCALED in particular) which look like what we want here. That would probably imply introducing WINED3DFMT_R8G8B8A8_USCALED and a few other formats and then distinguishing them from their _UINT/_SINT variant, but that would be a wined3d issue, and probably not a bad thing in any case...
Oh, that's very convenient. Float it is, then :-)
1565 * 1566 * Members of this structure are allocated by vkd3d-shader and should be freed 1567 * with vkd3d_shader_free_scan_signature_info() when no longer needed. 1568 * 1569 * \since 1.x 1570 */ 1571 struct vkd3d_shader_scan_signature_info 1572 { 1573 /** Must be set to VKD3D_SHADER_STRUCTURE_TYPE_SCAN_SIGNATURE_INFO. */ 1574 enum vkd3d_shader_structure_type type; 1575 /** Optional pointer to a structure containing further parameters. */ 1576 const void *next; 1577 1578 /** 1579 * The parsed input signature embedded in a DXBC shader. 1580 * The structure is zeroed for any other type of shader. changed this line in version 5 of the diff
1550 1555 | ((w & VKD3D_SHADER_SWIZZLE_MASK) << VKD3D_SHADER_SWIZZLE_SHIFT(3)); 1551 1556 } 1552 1557 1558 /** 1559 * A chained structure containing signatures scanned from a DXBC shader. 1560 * 1561 * All members (except for \ref type and \ref next) are output-only. 1562 * 1563 * This structure is passed to vkd3d_shader_scan() and extends 1564 * vkd3d_shader_compile_info. 1565 * 1566 * Members of this structure are allocated by vkd3d-shader and should be freed 1567 * with vkd3d_shader_free_scan_signature_info() when no longer needed. - Comment on lines +1563 to +1567
Does this new interface supersede
vkd3d_shader_parse_input_signature()
, or are there some use cases of the latter which are not covered? If not, I wonder whether it would make sense to add a notice onvkd3d_shader_parse_input_signature()
saying that it's a legacy call andstruct vkd3d_shader_scan_signature_info
now allows queried for the same information and more. That's mostly because it can be confusing for a reader to have two ways to do (mostly) the same thing. Changed in v3. For convenience, here's the wording I used:
+ * This function only parses DXBC shaders, and only retrieves the input + * signature. To retrieve signatures from other shader types, or other signature + * types, use vkd3d_shader_scan() and struct vkd3d_shader_scan_signature_info. + * This function returns the same input signature that is returned in + * struct vkd3d_shader_scan_signature_info.
1581 * - For shader model 3 pixel position inputs (i.e. VPOS, but not POSITION), 1582 * it is set to VKD3D_SHADER_SV_POSITION. 1583 * - For shader model 3 face inputs (i.e. VFACE), it is set to 1584 * VKD3D_SHADER_SV_IS_FRONT_FACE. 1585 * - For all other elements, it is set to VKD3D_SHADER_SV_NONE. 1586 * - The \ref vkd3d_shader_signature_element.component_type field is always set 1587 * to VKD3D_SHADER_COMPONENT_FLOAT. 1588 * - The \ref vkd3d_shader_signature_element.register_index field is set to the 1589 * bytecode register index. 1590 * Note that for shader model 1 and 2 shaders (excepting vertex shader 1591 * inputs), the register index of colour and texture coordinate registers will 1592 * be equal to the usage index, and hence may not be unique. 1593 * - The \ref vkd3d_shader_signature_element.mask field is set to the mask given 1594 * in the DCL instruction, if one is present. If there is no DCL instruction 1595 * for this semantic, it is set to the same value as the \ref used_mask field. 1596 * For the scalar registers vFace, oDepth, oFog, and oPts, it is always 1. - Comment on lines +1593 to +1596
It seems that the program does something slightly different: it ORs together all the masks, including the one in the DCL. Which probably usually coincides with what you say, given that hopefully the DCL mask is a subset of any other, but I guess there might be exceptions.
I don't know if it makes sense to be precise about this kind of things, so feel free to ignore this. I just noticed this fact, so I wanted to mention it.
changed this line in version 5 of the diff
+ /** + * The structure is a vkd3d_shader_scan_signature_info structure. + * \since 1.x + */
That should be "\since 1.8". There are a few other instances of this as well.
+/** + * A chained structure containing signatures scanned from a DXBC shader. + * + * All members (except for \ref type and \ref next) are output-only. + * + * This structure is passed to vkd3d_shader_scan() and extends + * vkd3d_shader_compile_info. + * + * Members of this structure are allocated by vkd3d-shader and should be freed + * with vkd3d_shader_free_scan_signature_info() when no longer needed. + * + * \since 1.x + */ +struct vkd3d_shader_scan_signature_info +{ + /** Must be set to VKD3D_SHADER_STRUCTURE_TYPE_SCAN_SIGNATURE_INFO. */ + enum vkd3d_shader_structure_type type; + /** Optional pointer to a structure containing further parameters. */ + const void *next; + + /** + * The parsed input signature embedded in a DXBC shader. + * The structure is zeroed for any other type of shader. + * + * The signature may contain pointers into the input shader, and should + * only be accessed while the input shader remains valid. + */ + struct vkd3d_shader_signature input; + + /** + * The parsed output signature embedded in a DXBC shader. + * The structure is zeroed for any other type of shader. + * + * The signature may contain pointers into the input shader, and should + * only be accessed while the input shader remains valid. + */ + struct vkd3d_shader_signature output; + + /** + * The parsed patch constant signature embedded in a DXBC hull shader. + * The structure is zeroed for any other type of shader. + * + * The signature may contain pointers into the input shader, and should + * only be accessed while the input shader remains valid. + */ + struct vkd3d_shader_signature patch_constant; +};
The text here ties this fairly strongly to the input format, as well as the specifics of DXBC containers. I'm not going to object too much, but I'm not sure that's entirely warranted; I could imagine the the target format/API making a difference for what we return here, as well as compilation options. I could also imagine e.g. compiling HLSL to GLSL, and still wanting information about the input/output interface. Perhaps notably, the API tests here use HLSL as actual source.
The text here ties this fairly strongly to the input format, as well as the specifics of DXBC containers. I'm not going to object too much, but I'm not sure that's entirely warranted; I could imagine the the target format/API making a difference for what we return here, as well as compilation options. I could also imagine e.g. compiling HLSL to GLSL, and still wanting information about the input/output interface. Perhaps notably, the API tests here use HLSL as actual source.
Is there a notion of shader signature which is at the same time sufficiently general so that it applies to all (or at least most) shader formats and sufficiently specific so that it's actually usable for something?
The text here ties this fairly strongly to the input format, as well as the specifics of DXBC containers. I'm not going to object too much, but I'm not sure that's entirely warranted; I could imagine the the target format/API making a difference for what we return here, as well as compilation options. I could also imagine e.g. compiling HLSL to GLSL, and still wanting information about the input/output interface. Perhaps notably, the API tests here use HLSL as actual source.
Is there a notion of shader signature which is at the same time sufficiently general so that it applies to all (or at least most) shader formats and sufficiently specific so that it's actually usable for something?
I think the current vkd3d_shader_signature structure should work for most purposes, although an argument could be made for extending it with some of the additional DXIL and SPIR-V features.
Taking a step back though, signatures provide information about the interface between shaders and their environment:
- For a vertex shader input signature, that's mainly the mapping of shader input registers to vertex attributes in the target API (i.e., OpenGL/Vulkan/Direct3D/Metal).
- For a vertex shader output signature, that's the mapping of shader output registers to the interstage interface with the next shader.
- For a root signature, that's the mapping of shader resources (CBV/SRV/UAV) and samplers to the corresponding bind points in the target API.
That means there are broadly two kinds of usage for this information:
- Setting up bindings (attributes, resources, samplers, etc.) in the target environment.
- Matching up the interstage interface during shader translation.
Due to differences in target APIs, it can sometimes be useful to specify the interface from the point of view of the target environment as well. For example, this is the case when translating Direct3D 11 shaders to SPIR-V/Vulkan; Direct3D 11 has fixed API bind points for resources, while Vulkan has the more flexible descriptor sets/layouts.
The approach taken for descriptor bindings is that the vkd3d_shader_scan_descriptor_info structure returns information about the descriptors used by the shader, while the vkd3d_shader_interface_info structure provides information about how to map these to the target environment. (And by contrast, vkd3d_shader_parse_root_signature() and vkd3d_shader_parse_input_signature() just parse the raw data.)
There's probably an argument to be made for not lumping input attribute binding information and interstage interface information together as well...
That should be "\since 1.8". There are a few other instances of this as well.
Of course this was intentional, but yes, I'll change it now.
The text here ties this fairly strongly to the input format, as well as the specifics of DXBC containers. I'm not going to object too much, but I'm not sure that's entirely warranted; I could imagine the the target format/API making a difference for what we return here, as well as compilation options. I could also imagine e.g. compiling HLSL to GLSL, and still wanting information about the input/output interface. Perhaps notably, the API tests here use HLSL as actual source.
I'm not really sure I understand this comment.
In the first place, yes, this is tied to DXBC, because vkd3d_shader_signature is very much based on DXBC. You can come up with something similar for other formats—as done in 2/2—but it requires inventing some degree of mapping (even for d3dbc it's not clear how semantic names or masks get mapped). As an API consumer, I'd want it to be very clear what i/o variables are included in the shader or not (consider DXBC isn't exactly consistent about this: from an HLSL perspective, it seems arbitrary that pixel shader SV_Position is included in the signature but vertex shader SV_VertexID isn't) and how that mapping is done.
In the second place, I don't understand how the target format affects anything. This is a scanning option, and it's currently implemented for vkd3d_shader_compile() only inasmuch as vkd3d_shader_compile() implicitly scans the source. If we want it to do something different, I think we need to work that out right now and document it.
I think what you're trying to say is that we should put information about the GLSL (or Vulkan) binding here, but that seems to me like fundamentally a contradictory model to what we already have. Descriptor info doesn't work this way. If we want to provide feedback about the default mapping of varyings to the target format, and we can't rely on storing reflection data in that format (like with hlsl -> dxbc translation) I think we need to do that in some separate structure.
The text here ties this fairly strongly to the input format, as well as the specifics of DXBC containers. I'm not going to object too much, but I'm not sure that's entirely warranted; I could imagine the the target format/API making a difference for what we return here, as well as compilation options. I could also imagine e.g. compiling HLSL to GLSL, and still wanting information about the input/output interface. Perhaps notably, the API tests here use HLSL as actual source.
I'm not really sure I understand this comment.
In the first place, yes, this is tied to DXBC, because vkd3d_shader_signature is very much based on DXBC. You can come up with something similar for other formats—as done in 2/2—but it requires inventing some degree of mapping (even for d3dbc it's not clear how semantic names or masks get mapped).
Sure, but I'm not sure that's necessarily an argument for tying this to DXBC. After this series, the only unsupported source format is HLSL, and I think it would be straightforward enough to support there as well. In terms of potential future formats, we'd definitely want this to support DXIL, and likewise for something like d3d-asm or dxil-asm.
I don't expect we'll ever support e.g. GLSL as source format, but note that the mapping there would be straightforward as well.
On the other hand, if we really wanted to do something specific to DXBC, we'd only need to expose some variant of shader_parse_signature(); we already have the API for the caller to extract the ISGN/OSGN/PCSG DXBC sections and their variants, and there would be no need to parse the shader's source in that case.
As an API consumer, I'd want it to be very clear what i/o variables are included in the shader or not (consider DXBC isn't exactly consistent about this: from an HLSL perspective, it seems arbitrary that pixel shader SV_Position is included in the signature but vertex shader SV_VertexID isn't) and how that mapping is done.
Sure, although I'd argue that what you actually want to know as a user of this API is either what setup you'll need to do in the target API to use this shader, or what interface the shader for the next stage needs to have. Note that the former may somewhat depend on the target format/API. For example, for SPIR-V/Vulkan the "rasterizer" (VKD3DSPR_RASTERIZER) register needs to be supplied explicitly, while that's implicit for TPF/d3d11.
In the second place, I don't understand how the target format affects anything. This is a scanning option, and it's currently implemented for vkd3d_shader_compile() only inasmuch as vkd3d_shader_compile() implicitly scans the source. If we want it to do something different, I think we need to work that out right now and document it.
Consider e.g. the earlier BLENDINDICES discussion; when targetting an API without _USCALED/_SSCALED formats, we may want to consider BLENDINDICES as UINT. I don't necessarily have a lot of good examples here, but I wouldn't want to rule the scenario out up-front either.
I think what you're trying to say is that we should put information about the GLSL (or Vulkan) binding here, but that seems to me like fundamentally a contradictory model to what we already have. Descriptor info doesn't work this way. If we want to provide feedback about the default mapping of varyings to the target format, and we can't rely on storing reflection data in that format (like with hlsl -> dxbc translation) I think we need to do that in some separate structure.
No, I think that's a misunderstanding. I'm mostly just suggesting that I think it would be fine for the documentation to be phrased in terms of something along the lines of "a description of the shader's input/output parameters", instead of something along the lines of "this thing we extracted from a DXBC blob, except when we didn't".
I don't think there's an issue with the vkd3d_shader_scan_signature_info structure itself, and I could live with the current text if needed.
The text here ties this fairly strongly to the input format, as well as the specifics of DXBC containers. I'm not going to object too much, but I'm not sure that's entirely warranted; I could imagine the the target format/API making a difference for what we return here, as well as compilation options. I could also imagine e.g. compiling HLSL to GLSL, and still wanting information about the input/output interface. Perhaps notably, the API tests here use HLSL as actual source.
I'm not really sure I understand this comment.
In the first place, yes, this is tied to DXBC, because vkd3d_shader_signature is very much based on DXBC. You can come up with something similar for other formats—as done in 2/2—but it requires inventing some degree of mapping (even for d3dbc it's not clear how semantic names or masks get mapped).
Sure, but I'm not sure that's necessarily an argument for tying this to DXBC. After this series, the only unsupported source format is HLSL, and I think it would be straightforward enough to support there as well. In terms of potential future formats, we'd definitely want this to support DXIL, and likewise for something like d3d-asm or dxil-asm.
It's not impossible, but I'm not sure it's trivial and obvious either. What goes in the register_index field? (Zero?) Should SV_ThreadID be mapped?
On the other hand, if we really wanted to do something specific to DXBC, we'd only need to expose some variant of shader_parse_signature(); we already have the API for the caller to extract the ISGN/OSGN/PCSG DXBC sections and their variants, and there would be no need to parse the shader's source in that case.
It's not that I think this API needs to be specific to DXBC—far from it, the whole impetus for that patch is 2/2 which extends it to sm1—but I don't think we can just support every format right off the bat without nontrivial API design. I don't exactly want that to hold up sm1 support if there's not a good reason.
As an API consumer, I'd want it to be very clear what i/o variables are included in the shader or not (consider DXBC isn't exactly consistent about this: from an HLSL perspective, it seems arbitrary that pixel shader SV_Position is included in the signature but vertex shader SV_VertexID isn't) and how that mapping is done.
Sure, although I'd argue that what you actually want to know as a user of this API is either what setup you'll need to do in the target API to use this shader, or what interface the shader for the next stage needs to have. Note that the former may somewhat depend on the target format/API. For example, for SPIR-V/Vulkan the "rasterizer" (VKD3DSPR_RASTERIZER) register needs to be supplied explicitly, while that's implicit for TPF/d3d11.
In theory, sure. In practice, the information DXBC exposes doesn't really align with that. For one thing, I misremembered: SV_VertexID actually does go in the input signature, despite not actually hooking up to the target API in any way. On the other hand, SV_DispatchThreadID doesn't.
In any case, this is something I really want to be spelled out as an API consumer. There is no greater crime than vagueness.
In the second place, I don't understand how the target format affects anything. This is a scanning option, and it's currently implemented for vkd3d_shader_compile() only inasmuch as vkd3d_shader_compile() implicitly scans the source. If we want it to do something different, I think we need to work that out right now and document it.
Consider e.g. the earlier BLENDINDICES discussion; when targetting an API without _USCALED/_SSCALED formats, we may want to consider BLENDINDICES as UINT. I don't necessarily have a lot of good examples here, but I wouldn't want to rule the scenario out up-front either.
I'm not sure this kind of thing really belongs in the scan information. Honestly it just feels ugly to me to imply that scanning can depend on anything other than the shader itself.
I think what you're trying to say is that we should put information about the GLSL (or Vulkan) binding here, but that seems to me like fundamentally a contradictory model to what we already have. Descriptor info doesn't work this way. If we want to provide feedback about the default mapping of varyings to the target format, and we can't rely on storing reflection data in that format (like with hlsl -> dxbc translation) I think we need to do that in some separate structure.
No, I think that's a misunderstanding. I'm mostly just suggesting that I think it would be fine for the documentation to be phrased in terms of something along the lines of "a description of the shader's input/output parameters", instead of something along the lines of "this thing we extracted from a DXBC blob, except when we didn't".
I don't think there's an issue with the vkd3d_shader_scan_signature_info structure itself, and I could live with the current text if needed.
Sure, I think I understand not wanting to marry the text to a specific output format. The problem is that anything less than that just isn't specific enough. I could probably reword it so that the lede isn't married to the output format, but at some point I think we really do need to specify how the signature was parsed or generated.
Sure, but I'm not sure that's necessarily an argument for tying this to DXBC. After this series, the only unsupported source format is HLSL, and I think it would be straightforward enough to support there as well. In terms of potential future formats, we'd definitely want this to support DXIL, and likewise for something like d3d-asm or dxil-asm.
It's not impossible, but I'm not sure it's trivial and obvious either. What goes in the register_index field? (Zero?) Should SV_ThreadID be mapped?
Well, we'd want it to return the same information as we'd get when first compiling to TPF or d3dbc and then scanning the resulting shader. (And the straightforward though perhaps not entirely optimal way to implement HLSL scans would of course be to do just that.) That does imply that e.g. the HLSL target profile may make a difference.
Sure, although I'd argue that what you actually want to know as a user of this API is either what setup you'll need to do in the target API to use this shader, or what interface the shader for the next stage needs to have. Note that the former may somewhat depend on the target format/API. For example, for SPIR-V/Vulkan the "rasterizer" (VKD3DSPR_RASTERIZER) register needs to be supplied explicitly, while that's implicit for TPF/d3d11.
In theory, sure. In practice, the information DXBC exposes doesn't really align with that. For one thing, I misremembered: SV_VertexID actually does go in the input signature, despite not actually hooking up to the target API in any way. On the other hand, SV_DispatchThreadID doesn't.
Right, but if anything, that seems to argue against just returning the parsed input/output signatures contained in the DXBC.
In any case, this is something I really want to be spelled out as an API consumer. There is no greater crime than vagueness.
I don't know; I think that over the years I've come to appreciate the fact that ambiguity is the glue that holds many things together. :)
Consider e.g. the earlier BLENDINDICES discussion; when targetting an API without _USCALED/_SSCALED formats, we may want to consider BLENDINDICES as UINT. I don't necessarily have a lot of good examples here, but I wouldn't want to rule the scenario out up-front either.
I'm not sure this kind of thing really belongs in the scan information. Honestly it just feels ugly to me to imply that scanning can depend on anything other than the shader itself.
I tend to think of vkd3d_shader_scan() as providing the information needed to use the shader in the target environment. (And note that the function fairly deliberately takes a vkd3d_shader_compile_info structure as its input.) For modern formats like TPF and DXIL that can largely be derived purely from the original shader source; for something like d3dbc there is a lot more implicit state (e.g., fixed-function fog state) provided by the environment, as I'm sure you've already encountered.
There may be value in providing information purely about the original shader in its original intended environment, and we could potentially support that in vkd3d_shader_scan() by supporting VKD3D_SHADER_TARGET_NONE as target, but it's not quite how vkd3d_shader_scan() is currently intended.
No, I think that's a misunderstanding. I'm mostly just suggesting that I think it would be fine for the documentation to be phrased in terms of something along the lines of "a description of the shader's input/output parameters", instead of something along the lines of "this thing we extracted from a DXBC blob, except when we didn't".
I don't think there's an issue with the vkd3d_shader_scan_signature_info structure itself, and I could live with the current text if needed.
Sure, I think I understand not wanting to marry the text to a specific output format. The problem is that anything less than that just isn't specific enough. I could probably reword it so that the lede isn't married to the output format, but at some point I think we really do need to specify how the signature was parsed or generated.
I don't necessarily have an issue with documenting that, although it does feel like documenting implementation details instead of documenting what's provided by the API.