Re: d3d10core: Standardize COM aggregation for d3d10_device.

[Henri Verbeet]

On 22 March 2012 00:50, Michael Stefaniuc  wrote:
> -static HRESULT STDMETHODCALLTYPE d3d10_device_inner_QueryInterface(IUnknown *iface, REFIID riid, void **object)
> +static HRESULT STDMETHODCALLTYPE d3d10_device_inner_QueryInterface(IUnknown *iface, REFIID riid,
> - void **ppv)
>
> {
> - struct d3d10_device *This = d3d10_device_from_inner_unknown(iface);
> - struct d3d10_device *This = impl_from_IUnknown(iface);
> - TRACE("iface %p, riid %s, object %p\n", iface, debugstr_guid(riid), object);
> - TRACE("iface %p, riid %s, object %p\n", iface, debugstr_guid(riid), ppv);
> - if (IsEqualGUID(riid, &IID_IUnknown)
> - || IsEqualGUID(riid, &IID_ID3D10Device))
> - if (IsEqualGUID(riid, &IID_IUnknown) || IsEqualGUID(riid, &IID_ID3D10Device))
> - *ppv = This;
> - else if (IsEqualGUID(riid, &IID_IWineDXGIDeviceParent))
> - *ppv = &This->IWineDXGIDeviceParent_iface;
> - else
>
> {
> - ID3D10Device_AddRef(&This->ID3D10Device_iface);
> - *object = This;
> - return S_OK;
> - WARN("%s not implemented, returning E_NOINTERFACE\n", debugstr_guid(riid));
> - *ppv = NULL;
> - return E_NOINTERFACE;
>
> }
> - if (IsEqualGUID(riid, &IID_IWineDXGIDeviceParent))
> - {
> - IWineDXGIDeviceParent_AddRef(&This->IWineDXGIDeviceParent_iface);
> - *object = &This->IWineDXGIDeviceParent_iface;
> - return S_OK;
> - }
> -
> - WARN("%s not implemented, returning E_NOINTERFACE\n", debugstr_guid(riid));
> -
> - *object = NULL;
> - return E_NOINTERFACE;
> - IUnknown_AddRef((IUnknown*)*ppv);
> - return S_OK;
>
> }

I'm not sure this really makes it much better, but I guess it's mostly
up to how Alexandre wants these.

d3d10core: Standardize COM aggregation for d3d10_device.

[Michael Stefaniuc]

Now that Alexandre is back and Jacek didn't want to beat me to it it is
time to finish my reply...

On 03/22/2012 01:06 AM, Henri Verbeet wrote:
> On 22 March 2012 00:50, Michael Stefaniuc  wrote:
> > -static HRESULT STDMETHODCALLTYPE d3d10_device_inner_QueryInterface(IUnknown *iface, REFIID riid, void **object)
> > +static HRESULT STDMETHODCALLTYPE d3d10_device_inner_QueryInterface(IUnknown *iface, REFIID riid,
> > -
> >    void **ppv)
> >
> > {
> > - struct d3d10_device *This = d3d10_device_from_inner_unknown(iface);
> > - struct d3d10_device *This = impl_from_IUnknown(iface);
> > - TRACE("iface %p, riid %s, object %p\n", iface, debugstr_guid(riid), object);
> > - TRACE("iface %p, riid %s, object %p\n", iface, debugstr_guid(riid), ppv);
> > - if (IsEqualGUID(riid, &IID_IUnknown)
> > -
> >        || IsEqualGUID(riid, &IID_ID3D10Device))
> > - if (IsEqualGUID(riid, &IID_IUnknown) || IsEqualGUID(riid, &IID_ID3D10Device))
> > -
> >    *ppv = This;
> > - else if (IsEqualGUID(riid, &IID_IWineDXGIDeviceParent))
> > -
> >    *ppv = &This->IWineDXGIDeviceParent_iface;
> > - else
> > {
> > -
> >    ID3D10Device_AddRef(&This->ID3D10Device_iface);
> > -
> >    *object = This;
> > -
> >    return S_OK;
> > -
> >    WARN("%s not implemented, returning E_NOINTERFACE\n", debugstr_guid(riid));
> > -
> >    *ppv = NULL;
> > -
> >    return E_NOINTERFACE;
> > }
> > - if (IsEqualGUID(riid, &IID_IWineDXGIDeviceParent))
> > - {
> > -
> >    IWineDXGIDeviceParent_AddRef(&This->IWineDXGIDeviceParent_iface);
> > -
> >    *object = &This->IWineDXGIDeviceParent_iface;
> > -
> >    return S_OK;
> > - }
> > -
> > - WARN("%s not implemented, returning E_NOINTERFACE\n", debugstr_guid(riid));
> > -
> > - *object = NULL;
> > - return E_NOINTERFACE;
> > - IUnknown_AddRef((IUnknown*)*ppv);
> > - return S_OK;
> >
> > }
>
> I'm not sure this really makes it much better, but I guess it's mostly
> up to how Alexandre wants these.

Honestly, when Jacek documented it in the beginning I didn't like it
either. But having to deal now with the bad, the ugly and the <censored>
COM implementations in Wine I have overcome my hate for casts and
actually like this way more and more.

Of course if people know COM then it doesn't matter much. But looking at
COM in Wine, feeling the pain of some native COM implementations and how
the applications abuse them it is save to assume that most
developers that write COM stuff don't bother to learn COM.

COM is programmed by cut&paste and beating the code into submission. And
for that use case the way that Jacek documented is way better:

- Generic solution which works in all cases:
  * Single interface implementation in the object (not the most elegant
solution though).
  * Multiple interface implementations with one refcount.
  * Multiple interface implementations with multiple refcounts.

- AddRef in one and only one place which is very explicit on what needs
to be addref'ed (the returned interface). And most importantly the
AddRef is part of the boiler plate which doesn't needs to change during
cut&paste programming.

- And last but not least the 3rd parameter of QueryInterface doesn't
have object in its name. It is *not* an object even though MS loves to
call it ppvObject. "ppv" is just "peepeevee"; any resemblance of
Hungarian notation is pure coincidence ;). Though I don't mind changing
it as long as it doesn't contains obj/object.

bye
    michael