-----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-----