-----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);elsehr = 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.