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