Re: [PATCH 8/8] d3d10/effect: Use shader variable directly from pass structure on Apply().

[Matteo Bruni]

On Wed, Oct 6, 2021 at 9:54 AM Nikolay Sivov  wrote:
> Signed-off-by: Nikolay Sivov
>
> dlls/d3d10/d3d10_private.h |  15 ---
>  dlls/d3d10/effect.c        | 185 +++++++++++++------------------------
>  2 files changed, 64 insertions(+), 136 deletions(-)

Love to see that! Very nice patch overall.

There's only a couple of small things...
> diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c
> index 0ba906375d7..d4e44a7f419 100644
> --- a/dlls/d3d10/effect.c
> +++ b/dlls/d3d10/effect.c
> @@ -4109,12 +4029,35 @@ static void apply_shader_resources(ID3D10Device *device, struct d3d10_effect_var
>      }
>  }
>
> +static void d3d10_effect_pass_set_shader(struct d3d10_effect_pass *pass,
> -
>    const struct d3d10_effect_pass_shader_desc *shader_desc)
>
> +{
> - ID3D10Device *device = pass->technique->effect->device;
> - struct d3d10_effect_variable *v = shader_desc->shader;
> -
> - if (v->type->element_count)
> -
>    v = &v->elements[shader_desc->index];
> -
> - switch (v->type->basetype)
> - {
> -
>    case D3D10_SVT_VERTEXSHADER:
> -
>        ID3D10Device_VSSetShader(device, v->u.shader.shader.vs);
> -
>        break;
> -
>    case D3D10_SVT_PIXELSHADER:
> -
>        ID3D10Device_PSSetShader(device, v->u.shader.shader.ps);
> -
>        break;
> -
>    case D3D10_SVT_GEOMETRYSHADER:
> -
>        ID3D10Device_GSSetShader(device, v->u.shader.shader.gs);
> -
>        break;
> -
>    default:
> -
>        WARN("Unexpected shader type %u.\n", v->type->basetype);
> - }
>
> +}
> -

Like in some of the previous patches, this is actually a fix to
shaders arrays / D3D10_EOO_CONST_INDEX, which would be nice to
mention. FWIW this is more D3D10_EOO_VAR_INDEX-ready than the others.
> static HRESULT STDMETHODCALLTYPE d3d10_effect_pass_Apply(ID3D10EffectPass *iface, UINT flags)
>  {
>      struct d3d10_effect_pass *pass = impl_from_ID3D10EffectPass(iface);
>      ID3D10Device *device = pass->technique->effect->device;
> -
>
> HRESULT hr = S_OK;
> -
>
> unsigned int i;
>
> TRACE("iface %p, flags %#x\n", iface, flags);
>
> @@ -4134,14 +4077,14 @@ static HRESULT STDMETHODCALLTYPE d3d10_effect_pass_Apply(ID3D10EffectPass *iface
>      if (pass->blend)
>          ID3D10Device_OMSetBlendState(device, pass->blend->u.state.object.blend,
>                  pass->blend_factor, pass->sample_mask);
> - if (pass->vs.shader != &null_shader_variable)
> -
>    d3d10_effect_pass_set_shader(pass, &pass->vs);
> - if (pass->ps.shader != &null_shader_variable)
> -
>    d3d10_effect_pass_set_shader(pass, &pass->ps);
> - if (pass->gs.shader != &null_shader_variable)
> -
>    d3d10_effect_pass_set_shader(pass, &pass->gs);
>
> - for (i = 0; i < pass->object_count; ++i)
> - {
> -
>    hr = d3d10_effect_object_apply(&pass->objects[i]);
> -
>    if (FAILED(hr)) break;
> - }
> -
> - return hr;
> - return S_OK;
>
> }

I suspect you still want to set the shaders unconditionally (e.g. what
if pass 0 sets a non-NULL geometry shader and pass 1 sets NULL?)