[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: [d3dcompiler] - add missing fields to d3dcompiler_shader_reflection
From:       Matteo Bruni <matteo.mystral () gmail ! com>
Date:       2016-06-28 17:09:17
Message-ID: CABvNrtPH8Afg9YWCHV43kS0huWHRAw8NasCPhsWAN4KomgP+kQ () mail ! gmail ! com
[Download RAW message or body]

2016-06-23 1:45 GMT+02:00 Vijay Kiran Kamuju <infyquest@gmail.com>:
> [d3dcompiler] - add missing fields to d3dcompiler_shader_reflection
>
> This add missing fields to the d3dcompiler_shader_reflection based on
> http://timjones.tw/blog/archive/2015/09/02/parsing-direct3d-shader-bytecode
> https://msdn.microsoft.com/en-us/library/windows/desktop/ff476590(v=vs.85).aspx
> https://msdn.microsoft.com/en-us/library/windows/desktop/bb694550(v=vs.85).aspx
> https://msdn.microsoft.com/en-us/library/windows/desktop/bb172432(v=vs.85).aspx
> https://msdn.microsoft.com/en-us/library/windows/desktop/ff476209(v=vs.85).aspx

Thank you for the links, that saved me some time to have to look them up.

> +    UINT sample_frequency_shader;

As far as I understand, that would be better served by a BOOL.

>      UINT dcl_count;
> +    UINT def_count;

Nitpick but def_count is right before dcl_count in both
D3D11_SHADER_DESC and in the shader bytecode, it would be nice to have
the same order everywhere.

We have tests for this in d3dcompiler_43/tests/reflection.c. Can you
please add some tests for those new fields too? Notice that you don't
necessarily need n separate new tests for each of the n new features.

The patch subject should be something like "d3dcompiler: Add d3d11
fields to struct d3dcompiler_shader_reflection." i.e. no square
brackets for the dll prefix and put a colon after it. Also, while not
strictly required, it's encouraged to use git send-email, it will
inline the patch without wrapping and generally take care of most of
the formatting details, assuming git is configured correctly.



[prev in list] [next in list] [prev in thread] [next in thread]