Re: [v9 6/6] d3dx9: Implement setting named shader constants in effect.

[Matteo Bruni]

2016-04-01 13:21 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>:

A few nitpicks:
> +HRESULT d3dx_set_shader_const_fxlc(struct ID3DXEffectImpl *effect, struct d3dx_pass *pass,
> +        struct d3dx_parameter *param, BOOL vs)
> +{
> +    IDirect3DDevice9 *device = effect->device;
> +    HRESULT hr, ret;
> +    struct d3dx_parameter **params;
> +    D3DXCONSTANT_DESC *cdesc;
> +    unsigned int parameters_count;
> +    unsigned int i, j;
> +
> +    if (!param->param_eval)
> +    {
> +        FIXME("Preshader structure is null.\n");
> +        return D3DERR_INVALIDCALL;
> +    }

The name of the function and the FIXME above seem somewhat mixed up
WRT the naming change we had earlier.
> +    if (FAILED(hr = d3dx_param_eval_set_shader_constants(device, param->param_eval)))
> +        return hr;
> +    params = param->param_eval->shader_inputs.inputs_param;
> +    cdesc = param->param_eval->shader_inputs.inputs;
> +    parameters_count = param->param_eval->shader_inputs.input_count;
> +    ret = D3D_OK;
> +    for (i = 0; i < parameters_count; i++)
> +    {
> +        if (params[i] && params[i]->class == D3DXPC_OBJECT && (params[i]->type == D3DXPT_SAMPLER
> +            || params[i]->type == D3DXPT_SAMPLER1D || params[i]->type == D3DXPT_SAMPLER2D
> +            || params[i]->type == D3DXPT_SAMPLER3D || params[i]->type == D3DXPT_SAMPLERCUBE))

Line continuations should have 8 spaces indentation. You can also
replace i++ with ++i like in the other patches.
> +        TRACE("setting %u consts at %u.\n", n, start);

Capitalize "setting", spell out "constants".