Hi,
A few more suggestions, once they're implemented go ahead and send those patches to wine-patches :-) .
Patch 1:
- struct d3drm_device *object = NULL;
I don't think you need to initialize to NULL here. Looks good otherwise.
+HRESULT init_device(struct d3drm_device *device, REFIID riid, IUnknown **out) DECLSPEC_HIDDEN;
You're not using this anywhere.
Patch 2 looks good
Patch 3, also applies to the others: I guess you have to set *device = NULL if the call fails. Please extend the tests accordingly. I recommend to also test what happens if you pass device = NULL. If this returns an error, you can do something like
if (!device) return ERROR; *device = NULL;
if (!width || !height ...) return ERROR;
That way you have to bother about setting *device = NULL only once and not in every error case. If device = NULL always results in a crash you don't even need the if (!device) check first.
I also recommend to move the IDirect3D2_Release(d3d2); in patch 3 already, and not patch 5. The place where patch 5 puts it is better than the one where you add it initially.
Patch 4 looks good
Patch 5: Don't forget to set *device = NULL if the tests say you should do that. Also take care of the thunk :-)
Patch 6 looks good
Patch 7:
- if (!(desc.ddsCaps.dwCaps & DDSCAPS_3DDEVICE))
return DDERR_INVALIDCAPS;
Do you need an explicit check for this? I imagine that IDirect3D2::CreateDevice or IDirectDrawSurface::QueryInterface(&IID_IDirect3DDevice) take care of this task for you.
- hr = IDirectDrawSurface_GetAttachedSurface(surface, &caps, &ds);
- if (SUCCEEDED(hr))
- {
create_z_surface = FALSE;IDirectDrawSurface_Release(ds);- }
... if (FAILED(hr)) {
IDirectDrawSurface_DeleteAttachedSurface(surface, 0, ds);
if (ds)IDirectDrawSurface_DeleteAttachedSurface(surface, 0, ds); return hr;
I think this can detach an application-attached depth surface. The GetAttachedSurface call sets ds != NULL, then something fails and you call DeleteAttachedSurface. I think the proper thing to do is to set ds = NULL in the if (SUCCEEDED(hr)) branch after the GetAttachedSurface call.
I also think you don't need the if (ds) check before calling DeleteAttachedSurface. Just let DeleteAttachedSurface(surface, 0, NULL) fail.
Patch 8 looks ok.