Re: [PATCH 0/3] MR4875: wined3d: Hook up sm1 support in the Vulkan backend.

[Henri Verbeet (＠hverbeet)]

> +static enum vkd3d_shader_source_type shader_spirv_get_source_type(uint32_t magic)
> +{
> +    if (magic == TAG_DXBC)
> +        return VKD3D_SHADER_SOURCE_DXBC_TPF;
> +    else
> +        return VKD3D_SHADER_SOURCE_D3D_BYTECODE;
> +}

That's not quite right, and somewhat unnecessary. I think we should store "source_type" from shader_init() in struct wined3d_shader, and use that instead.

Do we need to bump the required vkd3d version here? Perhaps it's somewhat moot because we're going to need vkd3d_shader_build_varying_map() and others in patch 3/3, but technically 1.10 isn't a compile-time dependency, and for bisects it tends to help to not bump requirements unnecessarily.
> +            /* The maximum inter-stage register index for an sm1 shader is 11. */
> +            struct vkd3d_shader_varying_map varying_map[12];

Constants are nice, of course.
> +        case WINED3D_SHADER_TYPE_VERTEX:
> +        {
> +            struct wined3d_shader *ps = state->shader[WINED3D_SHADER_TYPE_PIXEL];
> +            struct shader_spirv_graphics_program_vk *ps_program = ps->backend_data;
> +            struct shader_spirv_graphics_program_vk *vs_program = shader->backend_data;
> +            unsigned int count = ARRAY_SIZE(args->u.vs.varying_map);
> +
> +            if (shader->reg_maps.shader_version.major < 4)
> +            {
> +                vkd3d_shader_build_varying_map(&vs_program->signature_info.output,
> +                        &ps_program->signature_info.input, &count, args->u.vs.varying_map);
> +                args->u.vs.varying_count = count;
> +            }
> +            break;
> +        }

"ps" may be NULL. For example when using fixed-function fragment processing with sm1 shaders, but also when stream output is used.

We don't need "count", right?
> +    else if (shader_type == WINED3D_SHADER_TYPE_VERTEX)
> +    {
> +        args->spirv_target.next = &args->varying_map;
> +
> +        args->varying_map.type = VKD3D_SHADER_STRUCTURE_TYPE_VARYING_MAP_INFO;
> +        args->varying_map.next = vkd3d_interface;
> +
> +        if (source_type == VKD3D_SHADER_SOURCE_D3D_BYTECODE)
> +        {
> +            args->varying_map.varying_map = compile_args->u.vs.varying_map;
> +            args->varying_map.varying_count = compile_args->u.vs.varying_count;
> +        }
> +    }

It's not wrong, but it seems somewhat superfluous to add an empty varying map to the chain for non-d3dbc sources.