[PATCH 3/3] d3dx9/tests: Add tests for array of samplers.

[Paul Gofman]

On 07/05/2017 09:12 PM, Matteo Bruni wrote:
> 2017-07-05 20:00 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
>> On 07/05/2017 08:55 PM, Matteo Bruni wrote:
>>> 2017-07-05 19:45 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
>>>> On 07/05/2017 08:40 PM, Paul Gofman wrote:
>>>>> On 07/05/2017 08:29 PM, Matteo Bruni wrote:
>>>>>> 2017-06-28 13:28 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
>>>>>>
>>>>>> Sorry for the delayed reviews. I did it at last...
>>>>>>
>>>>>>>     #define test_effect_preshader_compare_shader_bytecode(a, b, c, d) \
>>>>>>> @@ -4505,12 +4551,38 @@ static void
>>>>>>> test_effect_preshader(IDirect3DDevice9 *device)
>>>>>>>         hr = IDirect3DDevice9_GetSamplerState(device, 0,
>>>>>>> D3DSAMP_MAGFILTER, &value);
>>>>>>>         ok(hr == D3D_OK, "Got result %#x.\n", hr);
>>>>>>>         todo_wine ok(value == 3, "Unexpected sampler 0 magfilter
>>>>>>> %u.\n",
>>>>>>> value);
>>>>>>> +
>>>>>>> +    hr = IDirect3DDevice9_GetSamplerState(device, 1,
>>>>>>> D3DSAMP_MINFILTER,
>>>>>>> &value);
>>>>>>> +    ok(hr == D3D_OK, "Got result %#x.\n", hr);
>>>>>>> +    ok(value == 1, "Unexpected sampler 1 minfilter %u.\n", value);
>>>>>>> +    hr = IDirect3DDevice9_GetSamplerState(device, 1,
>>>>>>> D3DSAMP_MAGFILTER,
>>>>>>> &value);
>>>>>>> +    ok(hr == D3D_OK, "Got result %#x.\n", hr);
>>>>>>> +    todo_wine
>>>>>>> +    ok(value == 1, "Unexpected sampler 1 magfilter %u.\n", value);
>>>>>> Not new or really related to this patch: any idea about what's going
>>>>>> on with the magfilter state computation? I didn't really look into it,
>>>>>> does
>>>>>> it have to do with default parameter values by any chance?
>>>>>>
>>>>>>
>>>>> It needs type casting, the output table for FXLC is always float, but
>>>>> the
>>>>> input for state is integer, thus an enormous value failing the test. I
>>>>> was
>>>>> thinking of adding type specifier to state table in effect.c and doing
>>>>> the
>>>>> cast, but did not come to that yet.
>>>>>
>>>> More exactly, the state parameter in sampler state is float for FXLC
>>>> constants for some reason (which is not the case for "normal" states,
>>>> which
>>>> are assigned ok). Maybe it should be set to correct type during the parse
>>>> actually.
>>> Maybe FXLC preshaders are always supposed to output float, with the
>>> optional conversion after the fact. Not sure it makes any difference
>>> in practice.
>>>
>> Yes, sure, but the output of FXLC preshader is actually converted to the
>> type which is specified in state parameter which holds FXLC. That works fine
>> for "top level" states, but for some reason the type of FXLC state parameter
>> for sampler is float in the test case.
> Doesn't it come from the effect itself though?
> Converting the state parameter value to the actual state type would
> match a tiny bit better with what the effect specifies, for what it
> might be worth. It probably won't make any practical difference either
> way you go, the additional type field in state_table[] should mostly
> be it in any case.
>
It comes from effect (parameter typedef), but it comes wrong for sampler 
states and FXLC parameter. I would suggest to "fix" that parameter type 
during parse state (based on the type added to state_table[]) with the 
WARN output. Doing it on the fly during state application would add 
unwanted extra checks and array access at runtime.