Re: d3dx8 [2/2]: add basic functions for MatrixStack
Am Mittwoch, 5. Dezember 2007 18:13:30 schrieb David.Adam(a)math.cnrs.fr:
This is my first attempt with ICOM objects. So, all the advices are welcome. A few suggestions:
+HRESULT WINAPI ID3DXMatrixStackImpl_QueryInterface(ID3DXMatrixStack *iface, REFIID riid, void **ppvObject) + ... + IClassFactory_AddRef(iface); It is better to use ID3DXMatrixStack_AddRef(iface) here. It is essentially the same, but it looks more consistent The AddRef() and Release() methods should have a TRACE which write either the new or old refcount, otherwise tracking refcounting bugs is hell.
On 05/12/2007, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote:
Am Mittwoch, 5. Dezember 2007 18:13:30 schrieb David.Adam(a)math.cnrs.fr:
This is my first attempt with ICOM objects. So, all the advices are welcome. A few suggestions:
+HRESULT WINAPI ID3DXMatrixStackImpl_QueryInterface(ID3DXMatrixStack *iface, REFIID riid, void **ppvObject) + ... + IClassFactory_AddRef(iface); It is better to use ID3DXMatrixStack_AddRef(iface) here. It is essentially the same, but it looks more consistent
The AddRef() and Release() methods should have a TRACE which write either the new or old refcount, otherwise tracking refcounting bugs is hell.
Having a TRACE in QueryInterface is occasionally useful as well. A few more comments: - QueryInterface should return S_OK rather than D3D_OK, unless you've got tests to prove it doesn't. - AFAIK on failure you should set *ppvObject to NULL. - There's no real reason to stick to MS names for parameters.
Am Donnerstag, 6. Dezember 2007 09:38:51 schrieb H. Verbeet:
On 05/12/2007, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote:
Am Mittwoch, 5. Dezember 2007 18:13:30 schrieb David.Adam(a)math.cnrs.fr:
This is my first attempt with ICOM objects. So, all the advices are welcome.
A few suggestions:
+HRESULT WINAPI ID3DXMatrixStackImpl_QueryInterface(ID3DXMatrixStack *iface, REFIID riid, void **ppvObject) + ... + IClassFactory_AddRef(iface); It is better to use ID3DXMatrixStack_AddRef(iface) here. It is essentially the same, but it looks more consistent
The AddRef() and Release() methods should have a TRACE which write either the new or old refcount, otherwise tracking refcounting bugs is hell.
Having a TRACE in QueryInterface is occasionally useful as well.
A few more comments: - QueryInterface should return S_OK rather than D3D_OK, unless you've got tests to prove it doesn't. Well, that one is unprovable since D3D_OK is just S_OK :-)
- AFAIK on failure you should set *ppvObject to NULL. - There's no real reason to stick to MS names for parameters. Agreed
participants (3)
-
H. Verbeet -
Stefan Dösinger -
Stefan Dösinger