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

[Matteo Bruni]

2016-04-01 13:21 GMT+02:00 Paul Gofman :

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".