Re: [PATCH 2/3] d3drm: Implement IDirect3DRMFrame*::GetScene. (v5)

[Józef Kucia]

On Fri, Aug 12, 2016 at 12:48 AM, Aaryaman Vasishta
<jem456.vasishta(a)gmail.com> wrote:
> v4: Shorter implementation.
> v3: Directly traverse through parent frames instead of using GetParent.
>
> Signed-off-by: Aaryaman Vasishta <jem456.vasishta(a)gmail.com>
> ---
>  dlls/d3drm/frame.c       | 43 +++++++++++++++++++++++++++++++++++++------
>  dlls/d3drm/tests/d3drm.c | 24 ++++++++++++++++++++++++
>  2 files changed, 61 insertions(+), 6 deletions(-)
>
> diff --git a/dlls/d3drm/frame.c b/dlls/d3drm/frame.c
> index 2ff0241..4d7ab42 100644
> --- a/dlls/d3drm/frame.c
> +++ b/dlls/d3drm/frame.c
> @@ -1339,23 +1339,54 @@ static HRESULT WINAPI d3drm_frame1_GetRotation(IDirect3DRMFrame *iface,
>
> static HRESULT WINAPI d3drm_frame3_GetScene(IDirect3DRMFrame3 *iface, IDirect3DRMFrame3 **scene)
>  {
> -    FIXME("iface %p, scene %p stub!\n", iface, scene);
> +    struct d3drm_frame *frame = impl_from_IDirect3DRMFrame3(iface);
>
> -    return E_NOTIMPL;
> +    TRACE("iface %p, scene %p.\n", iface, scene);
> +
> +    if (!scene)
> +        return D3DRMERR_BADVALUE;
> +
> +    *scene = &frame->IDirect3DRMFrame3_iface;

Do you really need this assignment?
> +
> +    while (frame->parent)
> +        frame = frame->parent;
> +
> +    IDirect3DRMFrame3_AddRef(*scene = &frame->IDirect3DRMFrame3_iface);
> +
> +    return D3DRM_OK;
>  }

[PATCH 2/3] d3drm: Implement IDirect3DRMFrame*::GetScene. (v5)

[Aaryaman Vasishta]

On Fri, Aug 12, 2016 at 4:29 AM, Józef Kucia <joseph.kucia(a)gmail.com> wrote:
> On Fri, Aug 12, 2016 at 12:48 AM, Aaryaman Vasishta
> <jem456.vasishta(a)gmail.com> wrote:
> > v4: Shorter implementation.
> > v3: Directly traverse through parent frames instead of using GetParent.
> >
> > Signed-off-by: Aaryaman Vasishta <jem456.vasishta(a)gmail.com>
> > ---
> >  dlls/d3drm/frame.c       | 43 ++++++++++++++++++++++++++++++
> > +++++++------
> >  dlls/d3drm/tests/d3drm.c | 24 ++++++++++++++++++++++++
> >  2 files changed, 61 insertions(+), 6 deletions(-)
> >
> > diff --git a/dlls/d3drm/frame.c b/dlls/d3drm/frame.c
> > index 2ff0241..4d7ab42 100644
> > --- a/dlls/d3drm/frame.c
> > +++ b/dlls/d3drm/frame.c
> > @@ -1339,23 +1339,54 @@ static HRESULT WINAPI d3drm_frame1_GetRotation(IDirect3DRMFrame
> > *iface,
> >
> > static HRESULT WINAPI d3drm_frame3_GetScene(IDirect3DRMFrame3 *iface,
> > IDirect3DRMFrame3 **scene)
> >  {
> > -    FIXME("iface %p, scene %p stub!\n", iface, scene);
> > +    struct d3drm_frame *frame = impl_from_IDirect3DRMFrame3(iface);
> >
> > -    return E_NOTIMPL;
> > +    TRACE("iface %p, scene %p.\n", iface, scene);
> > +
> > +    if (!scene)
> > +        return D3DRMERR_BADVALUE;
> > +
> > +    *scene = &frame->IDirect3DRMFrame3_iface;
>
> Do you really need this assignment?
> > +
> > +    while (frame->parent)
> > +        frame = frame->parent;
> > +
> > +    IDirect3DRMFrame3_AddRef(*scene = &frame->IDirect3DRMFrame3_iface);
> > +
> > +    return D3DRM_OK;
> >  }
>
> You're right, it's redundant. Will resend this. Thanks!

Cheers,
Aaryaman