Note that use of immediate constants is *not* the way that wined3d currently handles DEF instructions, but rather those are uploaded via uniforms. Is there a reason for this?
We do use immediates when we can, see the "if (!shader->load_local_constsF)" block at the end of shader_generate_glsl_declarations().
Ah, I was looking for DEF handling and didn't see that.
There are a couple of cases where we can't use immediates though; relative addressing of constants is one case, and having non-finite values without GL_ARB_shader_bit_encoding is another.
The non-finite values part makes sense to me, but I don't see how relative addressing would require uniforms: is it not possible in GLSL to declare a const array of values and dynamically index that? Or are there actually applications that require dynamically indexing such that we need to return both internal and external constants?
It's perhaps also worth pointing out that Direct3D 8 can load shader constants from the vertex declaration using "D3DVSD_CONST"; we have a couple of examples of that in the Wine d3d8 tests.
Is there any reason we can't just implement that on top of wined3d_stateblock_set_vs_consts_f() these days?
"wined3d: Compile sm1 bytecode to spirv." isn't quite right; determining the source type is slightly more complicated because of things like Aon9. See also shader_init(); my "glsl-vkd3d" series just stores "source_type" from that function in struct wined3d_shader.
I wasn't too enamoured of that patch either; I'd rather see v-s do the job for us.
+ /** + * Register index of the Direct3D resource. + * + * When mapping legacy Direct3D constants to constant buffers in the target + * environment, this parameter instead names the register set to map, and + * must be a member of enum vkd3d_shader_d3dbc_constant_register. + */
The clarification here seems helpful, but I'm not sure I agree with "instead". From the point of view of struct vkd3d_shader_resource_binding this is consistent with the existing usage of the structure; we map the different d3dbc constant files to CBVs, and then simply specify the register indices of those CBVs here.
It's a CBV slot, yeah, and it makes a lot of sense that way. But the name of the parameter is register_index, and in sm4 it always is a register index (one of two in the case of constant buffers, perhaps), whereas in sm1 the distinction between "b*" / "c*" / "i*" is not one of register index; it is rather a distinction of register set.
Saying that this describes a "register index", for someone familiar with sm1, would be confusing; it'd imply the distinction between "c0" and "c1", which it doesn't. Changing the documentation to eschew the term "register index" in favor of something more neutral like "CBV slot" might help, but the parameter name is still register_index. I want to be clear about what this actually is, so that someone setting up their bindings knows exactly which binding they're setting up.
+ * When scanning a legacy Direct3D shader, vkd3d-shader enumerates each + * constant register set used by the shader as a single constant buffer + * descriptor, as follows: + * - The \ref vkd3d_shader_descriptor_info.type field is set to + * VKD3D_SHADER_DESCRIPTOR_TYPE_CBV. + * - The \ref vkd3d_shader_descriptor_info.register_space field is set to zero. + * - The \ref vkd3d_shader_descriptor_info.register_index field is set to a + * member of enum vkd3d_shader_d3dbc_constant_register denoting which set + * is used. + * - The \ref vkd3d_shader_descriptor_info.resource_type field is set to + * VKD3D_SHADER_RESOURCE_BUFFER. + * - The \ref vkd3d_shader_descriptor_info.resource_data_type field is set to + * VKD3D_SHADER_RESOURCE_DATA_FLOAT. + * - The \ref vkd3d_shader_descriptor_info.flags field is set to zero. + * - The \ref vkd3d_shader_descriptor_info.count field is set to one. + * + * In summary, there may be up to three such descriptors, one for each register + * set used by the shader: float, integer, and boolean.
Is there a particular reason for going for VKD3D_SHADER_RESOURCE_DATA_FLOAT here? In principle "resource_data_type" is meaningless for CBVs, but we currently use VKD3D_SHADER_RESOURCE_DATA_UINT in vkd3d_shader_scan_constant_buffer_declaration(). One consequence is that we'd get different results if we ran scan_with_parser() after running instruction_array_normalise_flat_constants().
No, this was a holdover from an early version that I meant to correct but never did.