Re: d3d10/test: Add effect shader test.

[Stefan Dösinger]

Am 03.02.2010 um 18:52 schrieb Rico Schüller:
> Hi,
>
> this patch adds a test case for d3d10 shader parsing and ID3D10EffectPass::Get*ShaderDesc() functions.
> +static DWORD fx_local_shader[] = {
> +0x43425844, 0x95577e13, 0xab5facae, 0xd06d9eab,
> +0x8b127be0, 0x00000001, 0x00001652, 0x00000001,
> ...

the binary shader is pretty long. Wouldn't it be better to load the bytes from an external file, or if that makes the tests too complicated, put the DWORD[] into a separate .c or .h file?
> - hr = D3D10CreateEffectFromMemory(fx_local_shader, fx_local_shader[6], 0, device, NULL, &effect);

Is there a reason for using fx_local_shader[6] instead of sizeof(fx_local_shader)? I see that the other tests do it as well.

d3d10/test: Add effect shader test.

[Henri Verbeet]

On 3 February 2010 19:37, Stefan Dösinger  wrote:
> the binary shader is pretty long. Wouldn't it be better to load the bytes from an external file, or if that makes the tests too complicated, put the DWORD[] into a separate .c or .h file?

In general we want the tests to be standalone as a single .c file.
Using 8 DWORDs per line might help a bit with the amount of vertical
space used though.
> > - hr = D3D10CreateEffectFromMemory(fx_local_shader, fx_local_shader[6], 0, device, NULL, &effect);
>
> Is there a reason for using fx_local_shader[6] instead of sizeof(fx_local_shader)? I see that the other tests do it as well.

They're not the same, the DWORD array contains 2 extra bytes padding
in this case. The first test mentions this in a comment, the others
don't. A simple wrapper function might help.

d3d10/test: Add effect shader test.

[Rico Schüller]

Am 03.02.2010 20:04, schrieb Henri Verbeet:
> On 3 February 2010 19:37, Stefan Dösinger  wrote:
> > the binary shader is pretty long. Wouldn't it be better to load the bytes from an external file, or if that makes the tests too complicated, put the DWORD[] into a separate .c or .h file?
>
> In general we want the tests to be standalone as a single .c file.
> Using 8 DWORDs per line might help a bit with the amount of vertical
> space used though.

Sure that could be done.
> > > - hr = D3D10CreateEffectFromMemory(fx_local_shader, fx_local_shader[6], 0, device, NULL,&effect);
> >
> > Is there a reason for using fx_local_shader[6] instead of sizeof(fx_local_shader)? I see that the other tests do it as well.
>
> They're not the same, the DWORD array contains 2 extra bytes padding
> in this case. The first test mentions this in a comment, the others
> don't. A simple wrapper function might help.

Yes, that's the reason.

I'm fine with all suggestions, we just have to agree on something. I 
think, the shader could also be split up, but that doesn't reduce the 
needed shader blob size.

Cheers
Rico