D3DRM Implementation patches upto CreateDeviceFromSurface

[Aaryaman Vasishta]

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

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

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



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