D3DRM Implementation patches upto CreateDeviceFromSurface

Aaryaman Vasishta jem456.vasishta at gmail.com
Thu Jul 9 09:34:50 CDT 2015

On Thu, Jul 9, 2015 at 5:20 PM, Stefan Dösinger <stefandoesinger at gmail.com>

> Is there really a need to store (and AddRef) the clipper? The primary
> surface already keeps a reference to it, so you should be OK. It won't give
> you the increment by 2 that native has though, but I don't think that's
> important.
At the moment there doesn't seem to be a significant need, except for the
refcounting requirements.

> > +        hr = IDirectDraw_CreateSurface(ddraw, &surface_desc, &ds, NULL);
> > +        if (FAILED(hr))
> > +            return hr;
> > +
> > +        hr = IDirectDrawSurface_AddAttachedSurface(surface, ds);
> > +        if (FAILED(hr))
> > +        {
> > +            IDirectDrawSurface_Release(ds);
> > +            return hr;
> > +        }
> Similarly: Is there a need to store the depth stencil? You could just
> release your reference after attaching it to the render target, it will
> stay alive anyway. (If it's just for the DeleteAttachedSurface later:
> Ignore it)
It's just for the DeleteAttachedSurface part. I'll remove it for now so
it's easier to thunk towards version 3.

> You have to keep ddraw alive because nothing (neither your surfaces nor
> your device) hold a reference to it. Same for the primary. I think it's
> also good to store a reference to the render target, I believe you need it
> to show the rendering results on the screen (The immediate mode device
> holds a ref too, but it's awkward to always query it from the device).
> > +    if (device->ref == 1 && device->z_created_internally)
> > +        IDirectDrawSurface_DeleteAttachedSurface(device->render_target,
> 0, device->depth_surface);
> As mentioned on IRC this is not thread safe A lot can happen between your
> ref == 1 comparison and the actual InterlockedDecrement.
Since we're not gonna use DeleteAttachedSurface, we won't need to read the
refcount either. I agree there are some synchronization issues in this
case. A good idea for future reference would be to use flags to determine
which version the device was created on, to make it thread-safe and generic
enough for the release method to be used by all versions.

> Wrt patch 6 (d3drm1_CreateDeviceFromClipper): It's a bit unfortunate that
> this is mostly a copypasted version of version 3, and the difference that
> actually matters is in init_device anyway. Maybe there is a nicer solution,
> e.g. a helper function for v1 and v3 with a version parameter.
Similar to set_primary_surface defined above init_device in the patches
(though we won't need the version parameter there anymore as I'll be using
the struct there.).

I've taken note of all the other suggestions and will incorporate them in
the next patchset. I do agree there's quite a bit of optimization left with
regards to the thunking and version checks.

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

More information about the wine-devel mailing list