Re: [PATCH v3 4/5] d3dcompiler: Implement d3d12_shader_reflection_GetResourceBindingDesc[ByName]().

[Matteo Bruni]

On Tue, Sep 1, 2020 at 5:11 PM Paul Gofman  wrote:
> On 9/1/20 17:43, Matteo Bruni wrote:
> > On Tue, Aug 11, 2020 at 2:31 PM Paul Gofman  wrote:
> > > Signed-off-by: Paul Gofman
> > >
> > > v2:
> > >      - fix reflection test in d3d10.
> > > v3:
> > >      - test more precisely how interface aliasing works for GetResourceBindingDesc.
> > > -
> > >    if (ref12)
> > > -
> > >    {
> > > -
> > >        memset(&desc12, 0, sizeof(desc12));
> > > -
> > >        desc12.Space = 0xdeadbeef;
> > > -
> > >        desc12.uID = 0xdeadbeef;
> > > -
> > > -
> > >        hr = ref12->lpVtbl->GetResourceBindingDescByName(ref12, pdesc->Name, &desc12);
> > > -
> > >        ok(hr == S_OK, "Got unexpected hr %x, i %u.\n", hr, i);
> > > -
> > > -
> > >        ok(!strcmp(desc12.Name, pdesc->Name), "Got unexpected Name \"%s\", i %u.\n", desc12.Name, i);
> > > -
> > >        ok(!memcmp(&desc12.Type, &desc11.Type, sizeof(desc11) - offsetof(D3D11_SHADER_INPUT_BIND_DESC, Type)),
> > > -
> > >                "D3D11 and D3D12 descs do not match.\n");
> > > -
> > >        ok(desc12.Space == pdesc->Space, "Got unexpected Space %u, i %u.\n", desc12.Space, i);
> > > -
> > >        ok(desc12.uID == pdesc->uID, "Got unexpected uID %#x, i %u.\n", desc12.uID, i);
> > > -
> > > -
> > >        memset(&desc12, 0, sizeof(desc12));
> > > -
> > >        desc12.Space = 0xdeadbeef;
> > > -
> > >        desc12.uID = 0xdeadbeef;
> > > -
> > >        hr = ref12_from_d3d11->lpVtbl->GetResourceBindingDescByName(ref12_from_d3d11, pdesc->Name, &desc12);
> > > -
> > >        ok(hr == S_OK, "Got unexpected hr %x, i %u.\n", hr, i);
> > > -
> > > -
> > >        ok(!strcmp(desc12.Name, pdesc->Name), "Got unexpected Name \"%s\", i %u.\n", desc12.Name, i);
> > > -
> > >        ok(!memcmp(&desc12.Type, &desc11.Type, sizeof(desc11) - offsetof(D3D11_SHADER_INPUT_BIND_DESC, Type)),
> > > -
> > >                "D3D11 and D3D12 descs do not match.\n");
> > > -
> > >        /* Native d3dcompiler_47 returns the same interface pointer when queried for ID3D12ShaderReflection
> > > -
> > >         * from ID3D11ShaderReflection. Space field still gets the correct value on x64 due to
> > > -
> > >         * D3D11_SHADER_INPUT_BIND_DESC padding. */
> > > -
> > >        expected = offsetof(D3D12_SHADER_INPUT_BIND_DESC, Space) < sizeof(D3D11_SHADER_INPUT_BIND_DESC)
> > > -
> > >                ? pdesc->Space : 0xdeadbeef;
> > > -
> > >        todo_wine_if(expected == 0xdeadbeef) ok(desc12.Space == expected, "Got unexpected Space %u, i %u.\n",
> > > -
> > >                desc12.Space, i);
> > > -
> > >        todo_wine ok(desc12.uID == 0xdeadbeef, "Got unexpected uID %#x, i %u.\n", desc12.uID, i);
> >
> > Interesting, and also worrisome... What happens in the opposite case
> > i.e. ref11_from_d3d12? I assume it expects and overwrites the whole
> > D3D12_ struct, given that's the same vtbl.
> >
> > Actually I hacked a quick test and that seems to be the case.
>
> As far as I interpret the tests, the logic behind this behaviour is:
> - the interface implementation is identical, querying d3d12 for d3d12
>
> and vice versa is equivalent a pointer cast;
> - the size of the structure to be copied in _GetResourceBindingDesc()
>
> (as well as the values for d3d12 only desc structure member) is
> determined during the reflection structure creation in D3DReflect.

Yeah, that matches my findings.