D3DRM Implementation patches upto CreateDeviceFromSurface

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

Looks better! There are a few minor issues (mostly not affecting functionality). As far as I can see the refcounting works right now, except for one leak.

> +HRESULT d3drm_device_init(struct d3drm_device *device, UINT version, IDirect3DRM *d3drm, IDirectDraw *ddraw, IDirectDrawSurface *surface,
> +            int width, int height)
You shouldn't need the width and height parameters here. You can just query it from the render target and thus avoid some code duplication.

You can move the DDSCAPS_3DDEVICE check from d3drm1_CreateDeviceFromSurface into device_init as well, that way you don't need to fetch the surface description twice.

> +    hr = IDirect3DRMDevice3_QueryInterface(device3, &IID_IDirect3DRMDevice2, (void**)device);
> +    IDirect3DRMDevice3_Release(device3);
> +    if (FAILED(hr))
> +        return hr;
>  
>      return D3DRM_OK;
You can replace the last 3 lines with "return hr;"

- From patch 3:
>      if (FAILED(hr))
>      {
> -        IDirectDrawSurface_DeleteAttachedSurface(surface, 0, ds);
> +        IDirect3D2_Release(d3d2);
>          return hr;
>      }
Shouldn't you release d3d2 even in the success case whenever you have QI'd it from ddraw? You're leaking it otherwise. I'd say the best place to release it is right after the CreateDevice call.

"create_z_surface" is a better name than "z_surface" or "use_z_surface" (in patch 4). Technically patch 4 is a better place to add this parameter than patch 3 because in patch 3 it is always true. But I don't care too much about where you add it because it's needed after the patch series.

> -    todo_wine ok(hr == D3DRM_OK, "Cannot get IDirect3DDevice2 interface (hr = %x).\n", hr);
> +    ok(hr == D3DRM_OK, "Cannot get IDirect3DDevice2 interface (hr = %x).\n", hr);
>      if (FAILED(hr))
>          goto cleanup;
You can now remove the if (FAILED(hr)).

> -    todo_wine ok(hr == DD_OK, "Cannot get attached depth surface (hr = %x).\n", hr);
> +    ok(hr == DD_OK, "Cannot get attached depth surface (hr = %x).\n", hr);
>      if (SUCCEEDED(hr))
>      {
Same. There are a few more occurances of that in patch 4.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJVv0TXAAoJEN0/YqbEcdMw1kgP/Axc4DBFymQ/kU5guofGIBQF
Qoj3pRXHPYqCS4IdrghBqkCpK2dh8GUZN5zTw09EV7VWDJokhSJGq1wE34XylZZO
bFcZqdXZSZj1pjNB01OIUpAOqbcyQdDLVSqVq9V1zCGbkXlwjuK8qRmRVSL1/qX5
SwtLAsizkK7Ncz1n1z/Z/7xAokHLHdio36h225ocbOBkQ5CJsnd7h6m3dhGfAplN
N6suX/l1JdnBmyw2EYqlA88yaJA4BTzZPaYPQHBGa8p4Z3YkM7lK/XS7uPRQFzzU
V2ALMQVx/6pNeLeQSuF/nyNgiml1gO6x5Okw0cTMk9W4xdfY4+AL3DD1sGvvjkcS
vgxXVkS3PgKJUmvzLuR1DjJ6jWqXKrnOYIFE2zO8fq/VcURaUkTZhYrmJUlDvVQ0
QUAalmyfcAr6vz9SgqILfl59URyt2CI3QL6SABqhpxIq+zqS0mQJoWf9k5zfJHk9
3fbetDbVcaBS4Iy7MIgy711iTK18TqktF7tWeyJSuU5/bDrLKDsJ/i1AjLtO11sL
wG7bL74b5Txb0ctxYBQIxZWoLa1YvAnicdzQnv7zjaKi8qbtUzFlBDEJGFAZuyJx
N2Pa1OmDBCAgmyztIsxWjNa7BRMiF9p9lOT3rdrEDATbvIUbH68NVUPgaVmtTXln
VEh3hGOyVbVjvCuPvuvo
=7PE5
-----END PGP SIGNATURE-----