[PATCH 2/4] d3d10: Apply shader resources for shaders used in pass.

[Matteo Bruni]

On Sat, Mar 21, 2020 at 7:30 PM Connor McAdams <conmanx360 at gmail.com> wrote:
>
> Signed-off-by: Connor McAdams <conmanx360 at gmail.com>
> ---
>  dlls/d3d10/effect.c | 105 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 105 insertions(+)
>
> diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c
> index 580b6079cf..f86ec5ce9f 100644
> --- a/dlls/d3d10/effect.c
> +++ b/dlls/d3d10/effect.c
> @@ -3617,9 +3617,107 @@ static struct ID3D10EffectVariable * STDMETHODCALLTYPE d3d10_effect_pass_GetAnno
>      return &null_variable.ID3D10EffectVariable_iface;
>  }
>
> +static void update_buffer(ID3D10Device *device, struct d3d10_effect_variable *v)
> +{
> +    struct d3d10_effect_buffer_variable *b = &v->u.buffer;
> +
> +    if (!b->changed)
> +        return;
> +
> +    ID3D10Device_UpdateSubresource(device, (ID3D10Resource *)b->buffer, 0, NULL,
> +            (const void *)b->local_buffer, v->data_size, 0);

The (const void *) cast is unnecessary.

> +
> +    b->changed = FALSE;
> +}
> +
> +static void apply_shader_resources(ID3D10Device *device, struct ID3D10EffectShaderVariable *variable)
> +{
> +    struct d3d10_effect_variable *v = impl_from_ID3D10EffectVariable((ID3D10EffectVariable *)variable);

I think we should introduce a impl_from_ID3D10EffectShaderVariable(),
like we recently did for ID3D10EffectScalarVariable and such. Probably
as a separate patch, replacing the existing cases at the same time.

> +    struct d3d10_effect_shader_variable *sv = &v->u.shader;
> +    struct d3d10_effect_shader_resource *sr;
> +    ID3D10ShaderResourceView *const *srv;

I see no reason for the const there, it's not really protecting much.

> +    struct d3d10_effect_variable *rsrc_v;
> +
> +    unsigned int i;

Blank line?

> +
> +    for (i = 0; i < sv->resource_count; ++i)
> +    {
> +        sr = &sv->shader_resource[i];
> +        rsrc_v = sr->resource_variable;
> +
> +        switch (sr->in_type)
> +        {
> +            case D3D10_SIT_CBUFFER:
> +                update_buffer(device, rsrc_v);
> +                switch (v->type->basetype)
> +                {
> +                    case D3D10_SVT_VERTEXSHADER:
> +                        ID3D10Device_VSSetConstantBuffers(device, sr->bind_point, sr->bind_count,
> +                                (ID3D10Buffer *const *)&rsrc_v->u.buffer.buffer);

Those casts are unnecessary AFAICS.

> +                        break;
> +                    case D3D10_SVT_PIXELSHADER:
> +                        ID3D10Device_PSSetConstantBuffers(device, sr->bind_point, sr->bind_count,
> +                                (ID3D10Buffer *const *)&rsrc_v->u.buffer.buffer);
> +                        break;
> +                    case D3D10_SVT_GEOMETRYSHADER:
> +                        ID3D10Device_GSSetConstantBuffers(device, sr->bind_point, sr->bind_count,
> +                                (ID3D10Buffer *const *)&rsrc_v->u.buffer.buffer);
> +                        break;
> +                    default:
> +                        break;

It might be worthwhile to add a WARN() or something for the default case.

> @@ -3627,6 +3725,13 @@ static HRESULT STDMETHODCALLTYPE d3d10_effect_pass_Apply(ID3D10EffectPass *iface
>
>      if (flags) FIXME("Ignoring flags (%#x)\n", flags);
>
> +    if (This->vs.pShaderVariable != (ID3D10EffectShaderVariable *)&null_shader_variable.ID3D10EffectVariable_iface)
> +        apply_shader_resources(device, This->vs.pShaderVariable);

These casts also look ugly but, as I understand it, they're caused by
the idiotic d3d10 effect variable object model and are basically
unavoidable :/

While touching the function, though, you could rename "This" to "pass".