On Thu, Oct 24, 2019 at 9:50 PM Connor McAdams conmanx360@gmail.com wrote:
This patch moves the implementation of d3d10 shader reflection into the existing d3dcompiler reflection code, and uses ifdefs to make the interface methods conform to the size of d3d10 structures. In the case of the methods GetResourceBindingDesc and GetConstantBuffer, no changes are needed, as the data structures and interfaces are the same between d3d10 and d3d11.
Signed-off-by: Connor McAdams conmanx360@gmail.com
dlls/d3d10/d3d10_main.c | 19 ++---- dlls/d3d10/d3d10_private.h | 8 --- dlls/d3d10/shader.c | 112 ------------------------------- dlls/d3dcompiler_43/reflection.c | 102 ++++++++++++++++++++++------ include/d3dcompiler.h | 4 ++ 5 files changed, 89 insertions(+), 156 deletions(-)
diff --git a/dlls/d3d10/d3d10_main.c b/dlls/d3d10/d3d10_main.c index e3d1c57e44..7c7500e045 100644 --- a/dlls/d3d10/d3d10_main.c +++ b/dlls/d3d10/d3d10_main.c @@ -300,22 +300,11 @@ const char * WINAPI D3D10GetPixelShaderProfile(ID3D10Device *device)
HRESULT WINAPI D3D10ReflectShader(const void *data, SIZE_T data_size, ID3D10ShaderReflection **reflector) {
- struct d3d10_shader_reflection *object;
- FIXME("data %p, data_size %lu, reflector %p stub!\n", data, data_size, reflector);
- if (!(object = heap_alloc_zero(sizeof(*object))))
- {
ERR("Failed to allocate D3D10 shader reflection object memory\n");return E_OUTOFMEMORY;- }
- object->ID3D10ShaderReflection_iface.lpVtbl = &d3d10_shader_reflection_vtbl;
- object->refcount = 1;
- HRESULT hr;
- *reflector = &object->ID3D10ShaderReflection_iface;
- TRACE("data %p, data_size %lu, reflector %p stub!\n", data, data_size, reflector);
- TRACE("Created ID3D10ShaderReflection %p\n", object);
- hr = d3dcompiler_d3d10_reflection_init(data, data_size, (void **)reflector);
Hiding away the interface difference here is NOT okay. Also there is no need to, you should have a d3dcompiler_shader_reflection_init() function that takes a struct d3dcompiler_shader_reflection * and is called by both D3D10ReflectShader and D3DReflect(). More on this below. Also it seems nicer to just move the function to d3dcompiler_43/reflection.c.
diff --git a/dlls/d3dcompiler_43/reflection.c b/dlls/d3dcompiler_43/reflection.c index 5faea4c8e6..696b842dfc 100644 --- a/dlls/d3dcompiler_43/reflection.c +++ b/dlls/d3dcompiler_43/reflection.c @@ -307,7 +307,7 @@ static void reflection_cleanup(struct d3dcompiler_shader_reflection *ref)
/* IUnknown methods */
-static inline struct d3dcompiler_shader_reflection *impl_from_ID3D11ShaderReflection(ID3D11ShaderReflection *iface) +static inline struct d3dcompiler_shader_reflection *impl_from_ID3DShaderReflection(ID3D11ShaderReflection *iface) { return CONTAINING_RECORD(iface, struct d3dcompiler_shader_reflection, ID3DShaderReflection_iface); }
I don't like this. There is no reason to change the function name and I'd still use the full name for the interface field in the struct definition (as mentioned in the previous patch).
@@ -315,9 +315,13 @@ static inline struct d3dcompiler_shader_reflection *impl_from_ID3D11ShaderReflec static HRESULT STDMETHODCALLTYPE d3dcompiler_shader_reflection_QueryInterface(ID3D11ShaderReflection *iface, REFIID riid, void **object) { TRACE("iface %p, riid %s, object %p\n", iface, debugstr_guid(riid), object);
+#ifndef D3D10REFLECT if (IsEqualGUID(riid, &IID_ID3D11ShaderReflection) || IsEqualGUID(riid, &IID_IUnknown)) +#else
- if (IsEqualGUID(riid, &IID_ID3D10ShaderReflection)
|| IsEqualGUID(riid, &IID_IUnknown))+#endif { IUnknown_AddRef(iface); *object = iface;
I had mentioned it on IRC but it was probably missed in the midst of the discussion: in general I'd prefer full "duplicate" functions rather than this kind of #ifdefing inside functions. This also doesn't work when exposing proper vtables.
@@ -398,6 +402,7 @@ static HRESULT STDMETHODCALLTYPE d3dcompiler_shader_reflection_GetDesc(ID3D11Sha desc->EmitInstructionCount = This->emit_instruction_count; desc->GSOutputTopology = This->gs_output_topology; desc->GSMaxOutputVertexCount = This->gs_max_output_vertex_count; +#ifndef D3D10REFLECT desc->InputPrimitive = This->input_primitive; desc->PatchConstantParameters = This->pcsg ? This->pcsg->element_count : 0; desc->cGSInstanceCount = 0; @@ -408,14 +413,14 @@ static HRESULT STDMETHODCALLTYPE d3dcompiler_shader_reflection_GetDesc(ID3D11Sha desc->cBarrierInstructions = 0; desc->cInterlockedInstructions = 0; desc->cTextureStoreInstructions = 0;
+#endif return S_OK; }
Taking this function as an example, what I had in mind was to effectively change this function to d3dcompiler_shader_reflection_get_desc(struct d3dcompiler_shader_reflection *, ...) and have separate ID3D10ShaderReflection / ID3D11ShaderReflection methods that simply recover the object via impl_from_*() and call into the new helper. Most of the methods are trivial and it should be fine to simply duplicate them (i.e. without need of a common helper).
static struct ID3D11ShaderReflectionConstantBuffer * STDMETHODCALLTYPE d3dcompiler_shader_reflection_G @@ -489,7 +494,11 @@ static HRESULT STDMETHODCALLTYPE d3dcompiler_shader_reflection_GetInputParameter return E_INVALIDARG; }
+#ifndef D3D10REFLECT *desc = This->isgn->elements[index]; +#else
- memcpy(desc, &This->isgn->elements[index], sizeof(D3D10_SIGNATURE_PARAMETER_DESC));
+#endif
With separate implementations you can write this as "...sizeof(*desc));" (and you won't need an #ifdef).
- object->ID3DShaderReflection_iface.lpVtbl = (const struct ID3D10ShaderReflectionVtbl *)&d3dcompiler_shader_reflection_vtbl;
This is not okay. You have to have a proper vtbl for ID3D10ShaderReflection, with correct method signatures.
diff --git a/include/d3dcompiler.h b/include/d3dcompiler.h index 5151f94510..c602d05e76 100644 --- a/include/d3dcompiler.h +++ b/include/d3dcompiler.h @@ -148,6 +148,10 @@ typedef HRESULT (WINAPI *pD3DPreprocess)(const void *data, SIZE_T size, const ch
HRESULT WINAPI D3DLoadModule(const void *data, SIZE_T size, ID3D11Module **module);
+#ifdef D3D10REFLECT +HRESULT d3dcompiler_d3d10_reflection_init(const void *data, SIZE_T data_size, void **reflector); +#endif
You can't add Wine-only functions to public headers. This won't be necessary once you move D3D10ReflectShader() to reflection.c.
In general, I was expecting this to be structured differently, like I tried to explain here. Let me know if I haven't been clear enough.