Re: [PATCH 3/3] d3d9/tests: Add test for SW shader in MVP mode.

[Henri Verbeet]

On 23 August 2017 at 16:17, Paul Gofman  wrote:
> -static IDirect3DDevice9 *create_device(IDirect3D9 *d3d, HWND device_window, HWND focus_window, BOOL windowed)
> +static IDirect3DDevice9 *create_device_behavior(IDirect3D9 *d3d, HWND device_window, HWND focus_window,
> -
>    BOOL windowed, DWORD behavior_flags)

It's perhaps a bit of an uncomfortable change to make due to the
number of tests in visual.c, but ideally this would match the version
of create_device() in device.c. Lacking that, there are a few tests in
visual.c that just call IDirect3D9_CreateDevice() directly. In either
case, we'd like device.c and visual.c to converge instead of diverge.
> - hr = IDirect3DDevice9_SetVertexShader(device, NULL);
> - ok(SUCCEEDED(hr), "Got unexpected hr %#x.\n", hr);
> - hr = IDirect3DDevice9_SetVertexShader(device, pure_sw_shader);
> - ok(SUCCEEDED(hr), "Got unexpected hr %#x.\n", hr);

Is resetting the shader to NULL really needed here?
> - hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2, quad, sizeof(*quad));
> - todo_wine
> - ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr);
> - hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2, quad, sizeof(*quad));
> - ok(SUCCEEDED(hr), "Got unexpected hr %#x.\n", hr);

Odd.
> - /* wined3d does not guarantee 0 for out of bounds constant access by default. */

IIRC neither does Windows, in general. Matteo may have a better
recollection of the details, but IIRC while it mostly works out in
practice on current NVIDIA hardware, it doesn't necessarily everywhere
else, and is essentially hardware/driver specific behaviour.

[PATCH 3/3] d3d9/tests: Add test for SW shader in MVP mode.

[Paul Gofman]

On 08/24/2017 04:39 PM, Henri Verbeet wrote:
> On 23 August 2017 at 16:17, Paul Gofman  wrote:
> > -static IDirect3DDevice9 *create_device(IDirect3D9 *d3d, HWND device_window, HWND focus_window, BOOL windowed)
> > +static IDirect3DDevice9 *create_device_behavior(IDirect3D9 *d3d, HWND device_window, HWND focus_window,
> > -
> >    BOOL windowed, DWORD behavior_flags)
>
> It's perhaps a bit of an uncomfortable change to make due to the
> number of tests in visual.c, but ideally this would match the version
> of create_device() in device.c. Lacking that, there are a few tests in
> visual.c that just call IDirect3D9_CreateDevice() directly. In either
> case, we'd like device.c and visual.c to converge instead of diverge.

So I will create the device without this helper function then.
> > - hr = IDirect3DDevice9_SetVertexShader(device, NULL);
> > - ok(SUCCEEDED(hr), "Got unexpected hr %#x.\n", hr);
> > - hr = IDirect3DDevice9_SetVertexShader(device, pure_sw_shader);
> > - ok(SUCCEEDED(hr), "Got unexpected hr %#x.\n", hr);
>
> Is resetting the shader to NULL really needed here?

I will check once again, but I think I saw the second call to 
DrawPrimitiveUP succeeds for the next scene also if already failed once 
for previous scene. So it can be removed along with the first call 
returning invalid value below.
> > - hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2, quad, sizeof(*quad));
> > - todo_wine
> > - ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr);
> > - hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2, quad, sizeof(*quad));
> > - ok(SUCCEEDED(hr), "Got unexpected hr %#x.\n", hr);
>
> Odd.

It looks like pretty consistent behaviour between drivers, at least it 
worked the same way for me on Intel & Nvidia (WIndows 7) and test bot. I 
suppose directx "frontend" part resets the shader internally after 
returning an error on first call, and then falls back to fixed function 
vertex processing on the next call. Even if the shader is reset though 
calling GetVertexShader still returns that invalid one previously set.
> > - /* wined3d does not guarantee 0 for out of bounds constant access by default. */
>
> IIRC neither does Windows, in general. Matteo may have a better
> recollection of the details, but IIRC while it mostly works out in
> practice on current NVIDIA hardware, it doesn't necessarily everywhere
> else, and is essentially hardware/driver specific behaviour.

So should I maybe change the test not to check for exact value but 
rather for not matching constant c256?

[PATCH 3/3] d3d9/tests: Add test for SW shader in MVP mode.

[Henri Verbeet]

On 24 August 2017 at 17:27, Paul Gofman  wrote:
> On 08/24/2017 04:39 PM, Henri Verbeet wrote:
> > On 23 August 2017 at 16:17, Paul Gofman  wrote:
> > > - /* wined3d does not guarantee 0 for out of bounds constant access by
> > >
> > > default. */
> >
> > IIRC neither does Windows, in general. Matteo may have a better
> > recollection of the details, but IIRC while it mostly works out in
> > practice on current NVIDIA hardware, it doesn't necessarily everywhere
> > else, and is essentially hardware/driver specific behaviour.
>
> So should I maybe change the test not to check for exact value but rather
> for not matching constant c256?

It might still match by chance. Perhaps it's best to just not test the
result of the draw and instead document why.