[prev in list] [next in list] [prev in thread] [next in thread]
List: wine-devel
Subject: Re: [PATCH 1/2] d3drm: Store animated frame pointer in animation object
From: Nikolay Sivov <bunglehead () gmail ! com>
Date: 2017-06-27 14:35:41
Message-ID: f5d9746a-4bc5-0baf-2625-d84cec177474 () gmail ! com
[Download RAW message or body]
On 27.06.2017 15:43, Henri Verbeet wrote:
> On 27 June 2017 at 10:17, Nikolay Sivov <nsivov@codeweavers.com> wrote:
> > static HRESULT WINAPI d3drm_animation1_SetFrame(IDirect3DRMAnimation *iface, \
> > IDirect3DRMFrame *frame) {
> > - FIXME("iface %p, frame %p.\n", iface, frame);
> > + struct d3drm_animation *animation = impl_from_IDirect3DRMAnimation(iface);
> > + HRESULT hr = D3DRM_OK;
> >
> > - return E_NOTIMPL;
> > + TRACE("iface %p, frame %p.\n", iface, frame);
> > +
> > + if (frame)
> > + {
> > + hr = IDirect3DRMFrame_QueryInterface(frame, &IID_IDirect3DRMFrame3, \
> > (void **)&animation->frame); + if (SUCCEEDED(hr))
> > + IDirect3DRMFrame3_Release(animation->frame);
> > + }
> > + else
> > + animation->frame = NULL;
> > +
> > + return hr;
> > }
> There's nothing necessarily wrong with this, but I feel the following
> alternative is worth pointing out:
>
> struct d3drm_frame *f = unsafe_impl_from_IDirect3DRMFrame(frame);
> ...
> animation->frame = f ? &f->IDirect3DRMFrame3_iface : NULL;
>
> return D3DRM_OK;
>
> or even
>
> ...
> return d3drm_animation2_SetFrame(&animation->IDirect3DRMAnimation2_iface,
> f ? &f->IDirect3DRMFrame3_iface : NULL);
>
> which is a pattern that's used in ddraw for this kind of thing.
>
>
That makes sense, I usually avoid unsafe_* stuff when public methods are
enough. I'll resend as is, to send updated patch 2/2, but will probably
patch it later in a way you suggested.
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic