[PATCH 2/3] d3drm: Implement IDirect3DRMFrame*::GetScene. (v2)
Aaryaman Vasishta
jem456.vasishta at gmail.com
Tue Aug 9 12:04:05 CDT 2016
On Tue, Aug 9, 2016 at 9:23 PM, Henri Verbeet <hverbeet at gmail.com> wrote:
> On 9 August 2016 at 03:16, Aaryaman Vasishta <jem456.vasishta at gmail.com>
> wrote:
> > static HRESULT WINAPI d3drm_frame3_GetScene(IDirect3DRMFrame3 *iface,
> IDirect3DRMFrame3 **scene)
> > {
> > - FIXME("iface %p, scene %p stub!\n", iface, scene);
> > + HRESULT hr;
> > + IDirect3DRMFrame3 *frame = iface, *parent_frame = NULL;
> >
> > - return E_NOTIMPL;
> > + TRACE("iface %p, scene %p.\n", iface, scene);
> > +
> > + if (!scene)
> > + return D3DRMERR_BADVALUE;
> > +
> > + for (;;)
> > + {
> > + if (FAILED(hr = IDirect3DRMFrame3_GetParent(frame,
> &parent_frame)))
> > + return hr;
> > + if (parent_frame)
> > + {
> > + frame = parent_frame;
> > + IDirect3DRMFrame3_Release(parent_frame);
> > + }
> > + else
> > + break;
> > + }
> You could just do
>
> if (!parent_frame)
> break;
>
> More importantly though, as Stefan already mentioned, the reference
> counting is weird. I think the code would be much nicer if you just
> used impl_from_IDirect3DRMFrame3() and then followed the frame->parent
> pointers.
>
Right, since we have internal access to the struct, why not take advantage
of it? Using the approach you described is just like traversing through a
linked list, simple and easier to read.
Cheers,
Aaryaman
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20160809/0ce3b197/attachment.html>
More information about the wine-devel
mailing list