[PATCH 1/2] d3drm: Store animated frame pointer in animation object

[Nikolay Sivov]

On 27.06.2017 15:43, Henri Verbeet wrote:
> On 27 June 2017 at 10:17, Nikolay Sivov <nsivov at 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.