[PATCH 02/10] d3d10: Implement scalar effect variable set methods.

[Matteo Bruni]

On Sat, Dec 7, 2019 at 7:23 PM Connor McAdams <conmanx360 at gmail.com> wrote:
>
> Implement SetFloat/SetFloatArray, SetInt/SetIntArray, and
> SetBool/SetBoolArray methods for the scalar effect variable interface.
>
> Signed-off-by: Connor McAdams <conmanx360 at gmail.com>
> ---
>  dlls/d3d10/effect.c | 83 ++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 71 insertions(+), 12 deletions(-)
>
> diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c
> index f0932409b1..4ecc0753de 100644
> --- a/dlls/d3d10/effect.c
> +++ b/dlls/d3d10/effect.c
> @@ -4212,6 +4212,43 @@ static const struct ID3D10EffectConstantBufferVtbl d3d10_effect_constant_buffer_
>      d3d10_effect_constant_buffer_GetTextureBuffer,
>  };
>
> +static inline void write_variable_to_cbuffer(struct d3d10_effect_variable *variable, void *data)

I'm not too happy about the name of this function but I'm not sure
about a better alternative. Maybe just _buffer (no c)?

Also I'd drop the inline.

> +{
> +    memcpy(variable->buffer->u.buffer.local_buffer + variable->buffer_offset, data, variable->type->size_packed);
> +
> +    variable->buffer->u.buffer.changed = 1;
> +}
> +
> +static void write_variable_array_to_cbuffer(struct d3d10_effect_variable *variable, void *data, UINT count)
> +{
> +    char *cbuf = variable->buffer->u.buffer.local_buffer + variable->buffer_offset;
> +    char *cur_element = data;
> +    DWORD element_size;
> +    UINT i;

No reason to use UINT or DWORD here and in the function arguments.

> +
> +    /*
> +     * If for some reason we try to use an array write on a variable that
> +     * isn't an array, just default back to the normal variable write.
> +     */
> +    if (!variable->type->element_count)
> +    {
> +        write_variable_to_cbuffer(variable, data);
> +        return;
> +    }

I don't think this comment should be there, there isn't anything
special about using array methods to write to non-array variables.

> +    element_size = variable->type->elementtype->size_packed;
> +
> +    for (i = 0; i < count; i++)
> +    {
> +        memcpy(cbuf, cur_element, element_size);
> +
> +        cur_element += element_size;
> +        cbuf += variable->type->stride;
> +    }
> +
> +    variable->buffer->u.buffer.changed = 1;
> +}
> +
>  /* ID3D10EffectVariable methods */
>
>  static BOOL STDMETHODCALLTYPE d3d10_effect_scalar_variable_IsValid(ID3D10EffectScalarVariable *iface)
> @@ -4370,9 +4407,12 @@ static HRESULT STDMETHODCALLTYPE d3d10_effect_scalar_variable_GetRawValue(ID3D10
>  static HRESULT STDMETHODCALLTYPE d3d10_effect_scalar_variable_SetFloat(ID3D10EffectScalarVariable *iface,
>          float value)
>  {
> -    FIXME("iface %p, value %.8e stub!\n", iface, value);
> +    struct d3d10_effect_variable *effect_var = impl_from_ID3D10EffectVariable((ID3D10EffectVariable *)iface);

There are already a few instances of this (i.e. upcasting to
ID3D10EffectVariable) and I'm not sure that the alternative is
necessarily better, but another option would be to introduce separate
impl_from_ID3D10Effect*Variable() for each variable type.

> -    return E_NOTIMPL;
> +    TRACE("iface %p, value %.8e.\n", iface, value);
> +    write_variable_to_cbuffer(effect_var, &value);
> +
> +    return S_OK;
>  }
>
>  static HRESULT STDMETHODCALLTYPE d3d10_effect_scalar_variable_GetFloat(ID3D10EffectScalarVariable *iface,
> @@ -4383,12 +4423,19 @@ static HRESULT STDMETHODCALLTYPE d3d10_effect_scalar_variable_GetFloat(ID3D10Eff
>      return E_NOTIMPL;
>  }
>
> +/*
> + * According to MSDN, array writing functions for Scalar/Vector effect
> + * variables have offset go unused.
> + */

We want a test for that specifically (and for all of those additions
in general).