Re: [PATCH v2 2/3] d3dcompiler: unify d3d11 + d3d10 reflection ifaces

[Connor McAdams]

I had to do an #ifndef inside of D3DReflect because when it was
defined as an ID3D10 interface, it would throw a warning to an
incompatible vtbl. But, reconfiguring it to be separate methods will
fix this, so I'll do that instead.

Going to take your suggestions and rework things. Thanks for the review.

On Fri, Oct 25, 2019 at 4:43 AM Matteo Bruni  wrote:
> On Thu, Oct 24, 2019 at 9:57 PM Connor McAdams  wrote:
> > Use ifdefs to change the type of the reflection iface, and change the
> > name of the variable throughout.
> > Signed-off-by: Connor McAdams
> >
> > dlls/d3dcompiler_43/reflection.c | 18 ++++++++++++------
> >  1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/dlls/d3dcompiler_43/reflection.c b/dlls/d3dcompiler_43/reflection.c
> > index ce634ac95a..5faea4c8e6 100644
> > --- a/dlls/d3dcompiler_43/reflection.c
> > +++ b/dlls/d3dcompiler_43/reflection.c
> > @@ -22,6 +22,7 @@
> >  #endif
> >  #include "d3dcompiler_private.h"
> >  #include "winternl.h"
> > +#include "d3d10.h"
> >
> > WINE_DEFAULT_DEBUG_CHANNEL(d3dcompiler);
> >
> > @@ -94,7 +95,11 @@ struct d3dcompiler_shader_reflection_constant_buffer
> >  /* ID3D11ShaderReflection */
> >  struct d3dcompiler_shader_reflection
> >  {
> > - ID3D11ShaderReflection ID3D11ShaderReflection_iface;
> >
> > +#ifdef D3D10REFLECT
> > - ID3D10ShaderReflection ID3DShaderReflection_iface;
> >
> > +#else
> > - ID3D11ShaderReflection ID3DShaderReflection_iface;
> >
> > +#endif
>
> I'd keep the standard names for the interface fields. They will be
> different between the two versions but I think that's actually a
> positive.
> > static HRESULT STDMETHODCALLTYPE d3dcompiler_shader_reflection_QueryInterface(ID3D11ShaderReflection *iface, REFIID riid, void **object)
> > @@ -702,7 +707,6 @@ static const struct ID3D11ShaderReflectionVtbl d3dcompiler_shader_reflection_vtb
> >      d3dcompiler_shader_reflection_GetThreadGroupSize,
> >      d3dcompiler_shader_reflection_GetRequiresFlags,
> >  };
> > -
> >
> > /* ID3D11ShaderReflectionConstantBuffer methods */
>
> Stray whitespace change.
> > static inline struct d3dcompiler_shader_reflection_constant_buffer *impl_from_ID3D11ShaderReflectionConstantBuffer(ID3D11ShaderReflectionConstantBuffer *iface)
> > @@ -1693,9 +1697,6 @@ static HRESULT d3dcompiler_shader_reflection_init(struct d3dcompiler_shader_refl
> >      HRESULT hr;
> >      unsigned int i;
> > -
> >
> > reflection->ID3D11ShaderReflection_iface.lpVtbl = &d3dcompiler_shader_reflection_vtbl;
> > -
> >
> > reflection->refcount = 1;
> > -
> >
> > wine_rb_init(&reflection->types, d3dcompiler_shader_reflection_type_compare);
> >
> > hr = dxbc_parse(data, data_size, &src_dxbc);
> >
> > @@ -1838,6 +1839,11 @@ HRESULT WINAPI D3DReflect(const void *data, SIZE_T data_size, REFIID riid, void
> >      if (!object)
> >          return E_OUTOFMEMORY;
> >
> > +#ifndef D3D10REFLECT
> > - object->ID3DShaderReflection_iface.lpVtbl = &d3dcompiler_shader_reflection_vtbl;
> > - object->refcount = 1;
> >
> > +#endif
> > -
>
> You're not calling D3DReflect() from D3D10ReflectShader() anymore, right?