Re: [v3 PATCH] d2d1: Added support for ID2D1GdiInteropRenderTarget interface

[Jacek Caban]

Hi Nikolay,

It looks correct to me now.

On 14.02.2017 16:19, Nikolay Sivov wrote:
> --- a/dlls/d2d1/bitmap_render_target.c
> +++ b/dlls/d2d1/bitmap_render_target.c
> @@ -32,6 +32,8 @@ static inline struct d2d_bitmap_render_target *impl_from_ID2D1BitmapRenderTarget
>  static HRESULT STDMETHODCALLTYPE d2d_bitmap_render_target_QueryInterface(ID2D1BitmapRenderTarget *iface,
>          REFIID iid, void **out)
>  {
> -
>
> struct d2d_bitmap_render_target *render_target = impl_from_ID2D1BitmapRenderTarget(iface);
> -
>
> TRACE("iface %p, iid %s, out %p.\n", iface, debugstr_guid(iid), out);
>
> if (IsEqualGUID(iid, &IID_ID2D1BitmapRenderTarget)
>
> @@ -43,6 +45,8 @@ static HRESULT STDMETHODCALLTYPE d2d_bitmap_render_target_QueryInterface(ID2D1Bi
>          *out = iface;
>          return S_OK;
>      }
> - else if (IsEqualGUID(iid, &IID_ID2D1GdiInteropRenderTarget))
> -
>    return ID2D1RenderTarget_QueryInterface(render_target->dxgi_target, iid, out);

You could just call QI on dxgi_target unconditionally here. It will
handle failure for you as well. Also by having this logic in one place,
if there are any more interfaces that you'd like to expose from
dxgi_target in the future, only dxgi_target QI change would be needed.

That's not very important at the moment and I don't really know relevant
code, so feel free to ignore that comment :)

Thanks,

Jacek

[v3 PATCH] d2d1: Added support for ID2D1GdiInteropRenderTarget interface

[Henri Verbeet]

On 14 February 2017 at 18:54, Jacek Caban  wrote:
> You could just call QI on dxgi_target unconditionally here. It will
> handle failure for you as well. Also by having this logic in one place,
> if there are any more interfaces that you'd like to expose from
> dxgi_target in the future, only dxgi_target QI change would be needed.

But it would also automatically expose interfaces you potentially
don't want to expose.

The more usual way to do this would have been to aggregate the
ID2D1GdiInteropRenderTarget interface directly instead of the entire
dxgi_target, but there are trade-offs there as well.