Re: d3d9: update locked_rect only if wined3d_surface_map succeeded (try 3)

[Nikolay Sivov]

On 10/15/2013 18:57, Lasse Rasinen wrote:
> According to debugging output, Artemis Spaceship Bridge Simulator 2.0
> calls LockRect twice on the same texture (for whatever reason) and crashes.
>
> http://bugs.winehq.org/show_bug.cgi?id=34271
>
> The return values are cleared explicitly --- game is playable.
> Now with appropriate assignments and test output.
>
> dlls/d3d9/surface.c      | 9 +++++++--
>   dlls/d3d9/tests/device.c | 4 ++++
>   2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/d3d9/surface.c b/dlls/d3d9/surface.c
> index 99f0b0f..20ccc68 100644
> --- a/dlls/d3d9/surface.c
> +++ b/dlls/d3d9/surface.c
> @@ -293,8 +293,13 @@ static HRESULT WINAPI d3d9_surface_LockRect(IDirect3DSurface9 *iface,
>       hr = wined3d_surface_map(surface->wined3d_surface, &map_desc, rect, flags);
>       wined3d_mutex_unlock();
> - locked_rect->Pitch = map_desc.row_pitch;
> - locked_rect->pBits = map_desc.data;
> -
>
> if (SUCCEEDED(hr)) {
> -
>    locked_rect->Pitch = map_desc.row_pitch;
> -
>    locked_rect->pBits = map_desc.data;
> -
>
> } else {
> -
>    locked_rect->Pitch = 0;
> -
>    locked_rect->pBits = NULL;
> -
>
> }
>
> return hr;
> }
>
> diff --git a/dlls/d3d9/tests/device.c b/dlls/d3d9/tests/device.c
> index 02b4174..90d5c38 100644
> --- a/dlls/d3d9/tests/device.c
> +++ b/dlls/d3d9/tests/device.c
> @@ -5404,6 +5404,10 @@ static void test_lockrect_invalid(void)
>       ok(SUCCEEDED(hr), "Failed to lock surface with rect NULL, hr %#x.\n", hr);
>       hr = IDirect3DSurface9_LockRect(surface, &locked_rect, NULL, 0);
>       ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr);
> - locked_rect.pBits = (BYTE *)0xdeadbeef;
> - hr = IDirect3DSurface9_LockRect(surface, &locked_rect, NULL, 0);
> - ok(!SUCCEEDED(hr) && locked_rect.pBits == NULL, "Failed to clear pBits (%p)\n",
> -
>        locked_rect.pBits);
> hr = IDirect3DSurface9_UnlockRect(surface);
> ok(SUCCEEDED(hr), "Failed to unlock surface, hr %#x.\n", hr);

It looks like you could use existing LockRect() call that's already 
tested as failing. Also I think FAILED() is a preferred way to
test HRESULT value for failure, but as existing code does - you could 
also test for exact failure code.

d3d9: update locked_rect only if wined3d_surface_map succeeded (try 3)

[Henri Verbeet]

On 15 October 2013 18:10, Nikolay Sivov  wrote:
> > diff --git a/dlls/d3d9/tests/device.c b/dlls/d3d9/tests/device.c
> > index 02b4174..90d5c38 100644
> > --- a/dlls/d3d9/tests/device.c
> > +++ b/dlls/d3d9/tests/device.c
> > @@ -5404,6 +5404,10 @@ static void test_lockrect_invalid(void)
> >       ok(SUCCEEDED(hr), "Failed to lock surface with rect NULL, hr
> > %#x.\n", hr);
> >       hr = IDirect3DSurface9_LockRect(surface, &locked_rect, NULL, 0);
> >       ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#x.\n", hr);
> > - locked_rect.pBits = (BYTE *)0xdeadbeef;
> > - hr = IDirect3DSurface9_LockRect(surface, &locked_rect, NULL, 0);
> > - ok(!SUCCEEDED(hr) && locked_rect.pBits == NULL, "Failed to clear
> >
> > pBits (%p)\n",
> > -
> >        locked_rect.pBits);
> > hr = IDirect3DSurface9_UnlockRect(surface);
> > ok(SUCCEEDED(hr), "Failed to unlock surface, hr %#x.\n", hr);
>
> It looks like you could use existing LockRect() call that's already tested
> as failing. Also I think FAILED() is a preferred way to
> test HRESULT value for failure, but as existing code does - you could also
> test for exact failure code.

Yeah, I think it really just needs the intialization of pBits before
the existing LockRect() call, and an extra ok() call after the
existing one.