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.