D3DRM Implementation patches upto CreateDeviceFromSurface

Aaryaman Vasishta jem456.vasishta at gmail.com
Sun Jul 26 07:04:27 CDT 2015

On Sun, Jul 26, 2015 at 4:33 PM, Stefan Dösinger <stefandoesinger at gmail.com>

> 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.
I'll post another incarnation with all the leaks I can possibly find fixed

> > +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".
It took me a while to figure out a name, since a name like
d3drm_device_create_surfaces_from_clipper felt too long for me initially.
But if long function names are accepted then I don't mind putting that name

> > +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.
Right, I'll remove this for now and re-add it in the
CreateDeviceFromSurface patches. Note that z_surface is still needed in
version 1 and 2 of CreateDeviceFromSurface in the cases where the surface
already has ds attached to it by the application (in which case native
wouldn't create its own ds internally, as seen by the tests).

> You are leaking ds here in the success case. You can release it after
> calling AddAttachedSurface.
I'm not sure why I allowed that leak to happen, but I believe it was
causing some refcounting/segfault issues if I released the ds there. I'll
get back with more details about this.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20150726/de1e1404/attachment.html>

More information about the wine-devel mailing list