D3DRM Implementation patches upto CreateDeviceFromSurface

[Stefan Dösinger]

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

Hi,

I like the overall structure a lot more than the previous incarnation!

Here are a few more comments:

> +    hr = DirectDrawCreate(NULL, &ddraw, NULL);
> +    if (FAILED(hr))
> +        return hr;
> +
>      hr = Direct3DRMDevice_create(&object);
>      if (FAILED(hr))
>          return hr;
You're leaking "ddraw" here. There are many more incomplete error handling paths in the patches.

> +HRESULT set_clipper_surface(struct d3drm_device *object, IDirectDraw *ddraw, IDirectDrawClipper *clipper, int width, int height,
> +            IDirectDrawSurface **surface) DECLSPEC_HIDDEN;
I'm not entirely happy with this name, because it doesn't merely set any surfaces, it creates them. What do you think about d3drm_device_create_surfaces_from_clipper? Check for consistency wrt the "d3drm_device" part. E.g. the next function you could name "d3drm_device_init" or just "device_init".

> +HRESULT init_device(struct d3drm_device *device, UINT version, IDirect3DRM *d3drm, IDirectDraw *ddraw, IDirectDrawSurface *surface, BOOL z_surface, IDirect3D2 *d3d2,
> +            int width, int height)
"BOOL z_surface" is unused right now. It won't be needed until you add CreateDeviceFromSurface version 3.

Since you're passing in a version parameter as I requested, you can now remove the "d3d2" parameter. The function can QI it from ddraw. That reduces duplicated code in the calling functions.

> +        hr = IDirectDraw_CreateSurface(ddraw, &surface_desc, &ds, NULL);
> +        if (FAILED(hr))
> +            return hr;
> +
> +        hr = IDirectDrawSurface_AddAttachedSurface(surface, ds);
> +        if (FAILED(hr))
> +        {
> +            IDirectDrawSurface_Release(ds);
> +            return hr;
> +        }
> +
> +        if (version == 1)
> +            hr = IDirectDrawSurface_QueryInterface(surface, &IID_IDirect3DRGBDevice, (void **)&device1);
> +        else
> +            hr = IDirect3D2_CreateDevice(d3d2, &IID_IDirect3DRGBDevice, surface, &device2);
> +        if (FAILED(hr))
> +        {
> +            IDirectDrawSurface_DeleteAttachedSurface(surface, 0, ds);
> +            IDirectDrawSurface_Release(ds);
> +            return hr;
> +        }
You are leaking ds here in the success case. You can release it after calling AddAttachedSurface.

>      cref2 = get_refcount((IUnknown *)clipper);
> -    todo_wine ok(cref2 > cref1, "expected cref2 > cref1, got cref1 = %u , cref2 = %u.\n", cref1, cref2);
> +    ok(cref2 > cref1, "expected cref2 > cref1, got cref1 = %u , cref2 = %u.\n", cref1, cref2);
I'd expect cref2 to match the exact value Windows returns here (3). If that is indeed the case you can make this cfref2 == cref1 + 2 or even cref2 == 3.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJVtL6SAAoJEN0/YqbEcdMwGMsP/12R5+3oztVIOpd0MOvKJjUQ
Ys1wMU+1Ntpy3xrVw05s7UuZdLS4rAeaodCELYsV4tMKjWhSpQ0uzH4rbBlafDx1
Mfem1clZ+UEwoH2cvNJ6KwyUwLZTuTln3WuW3xTDyF2BIvu2jyjepWJ+p30nV2MB
C63nZAZUt0VruBQlYMy3xhRIOLJMRiTo+a7Zw6Ge8YXIDWCufYcU8iH7ROl+POSY
cZokIqwUaGOAn/PDXQfo+vhJUcLoHsaQBDBB+svc0CueFqNLlXExpE279jNgZaZz
wvpEcPpmibOSw1JBE6edipRs7t7UScLh3Tt+JUCqJiM+vQoBnZoSYk1wYTo604Oy
l5YI7Eeony6J8HSuQccC+XAog1emONiQlMU7Qb7zScNxXUCpZe7EUq7ExCGSBbdv
H/L+Qc9iWl/lqVabRI1KYixJcQpizlIp4k/Q6EXhwaBN25XrSdGyq+OHiM+E/ut3
ugQGYvWZtCf+xHV3N4zBJM/P5R7kBR5jBFola1TFXy4pxnAFWENn5Waz7XdUdqdq
p72J6VyV1rMeQ6uLMHj/3BCvlMW9Kr+6CP6UC8wltgn89H+xh6dJ7uiTBDuq2Y5b
7RrBCvS1/S9CMByXCEmugq0SPW3tv8criYz+FRFOP5s57tvnRSBHzfrhOOPlN0O/
cPAjhf9s9CuQRZPChhgX
=mIH2
-----END PGP SIGNATURE-----