[v3 1/6] d3dx9: add test for preshader in effect.

[Matteo Bruni]

2016-03-15 8:18 GMT+01:00 Paul Gofman <gofmanp at gmail.com>:
> Thanks, I will follow up and resend the patch series on Thursday, 17th.
> I have a few questions below, could you please look?
>
> On 03/15/2016 05:50 AM, Matteo Bruni wrote:
>>
>>> +    for (i = 0; i < TEST_EFFECT_PRES_NFLOATP; i++)
>>> +    {
>>> +        hr = IDirect3DDevice9_SetPixelShaderConstantF(device, i,
>>> +                (const float *)&fvect_empty, 1);
>>> +        ok(hr == D3D_OK, "Got result %#x.\n", hr);
>>> +    }
>> I know it was me who suggested to clear all the 256 float shader
>> constants but it looks a bit weird if the same doesn't happen for the
>> other constant types. It's also not very useful if those additional
>> constants aren't checked later on. I would change the initialization
>> of the other constant types in the same way (maybe with a define or
>> something instead of the plain number?) and then check that those are
>> not modified by BeginPass().
> I can't just set 256 pixel shader constants, that fails on Marvin
> testbot (that was my unlucky v2 patch series). So I probably need to get
> the actual number of constants supported through GetDeviceCaps. Do we
> really need this in this test, or maybe I can just check the number of
> constants used + 1 for all types of constants?

Oh, I see, that's because SM3 pixel shaders provide 224 float
constants. But yes, this needs some more care. You should probably
skip the test entirely if vertex or pixel shader 3 aren't supported,
then you can initialize and check all the constants.

>>
>>> +    ok(!!vshader, "Got NULL vshader.\n");
>> This double negation is unnecessary.
>>
> There was a compiler warning if just "ok(vshader)". Or should I get it
> back as ok(vshader == NULL)?

Uh, you're right. Just ignore my comment then.