Re: [PATCH] d3d11.idl: Add ID3D11VideoContext interface (try 2)
On Wed, Jan 13, 2016 at 8:08 AM, Andrey Turkin <andrey.turkin(a)gmail.com> wrote:
UINT Usage:1;
It would preferably be written as "UINT Usage : 1;". This applies to other bit fields.
+ void VideoProcessorSetOutputConstriction( + [in] ID3D11VideoProcessor *pVideoProcessor, + BOOL Enable, + SIZE Size); + void VideoProcessorSetOutputStereoMode( + [in] ID3D11VideoProcessor *pVideoProcessor, + BOOL Enable);
The Enable and Size parameters should be annotated as input parameters. Ideally, the method parameters would be renamed and written in lower_case_with_underscores.
2016-01-15 3:35 GMT+03:00 Józef Kucia <joseph.kucia(a)gmail.com>:
It would preferably be written as "UINT Usage : 1;". This applies to other bit fields.
Make sense, will do The Enable and Size parameters should be annotated as input parameters.
Will do
Ideally, the method parameters would be renamed and written in lower_case_with_underscores.
Existing interfaces in that file don't use this naming scheme.
On 15 January 2016 at 09:30, Andrey Turkin <andrey.turkin(a)gmail.com> wrote:
2016-01-15 3:35 GMT+03:00 Józef Kucia <joseph.kucia(a)gmail.com>:
It would preferably be written as "UINT Usage : 1;". This applies to other bit fields.
Make sense, will do
The main thing I'd personally be worried about would be the portability of bit fields. I think that in case of D3D11_VIDEO_PROCESSOR_COLOR_SPACE the layout ends up being the same between MSVC and gcc on x86, but I'm not sure that's something we can depend on, or if that also holds on different compilers or e.g. 64-bit.
I thought about that too. In the end, I don't think I can really do anything to make them more portable while also keeping compatibility with DX SDK. We are a compilers' mercy here; at least (recent) GCC targeting Windows will use MSVC-compatible bitfield layout. Anyway, there's already some bitfield use in WinSDK headers in Wine - same concern applies there. 2016-01-15 12:44 GMT+03:00 Henri Verbeet <hverbeet(a)gmail.com>:
On 15 January 2016 at 09:30, Andrey Turkin <andrey.turkin(a)gmail.com> wrote:
2016-01-15 3:35 GMT+03:00 Józef Kucia <joseph.kucia(a)gmail.com>:
It would preferably be written as "UINT Usage : 1;". This applies to other bit fields.
Make sense, will do
The main thing I'd personally be worried about would be the portability of bit fields. I think that in case of D3D11_VIDEO_PROCESSOR_COLOR_SPACE the layout ends up being the same between MSVC and gcc on x86, but I'm not sure that's something we can depend on, or if that also holds on different compilers or e.g. 64-bit.
On 01/15/16 10:44, Henri Verbeet wrote:
On 15 January 2016 at 09:30, Andrey Turkin <andrey.turkin(a)gmail.com> wrote:
2016-01-15 3:35 GMT+03:00 Józef Kucia <joseph.kucia(a)gmail.com>:
It would preferably be written as "UINT Usage : 1;". This applies to other bit fields.
Make sense, will do
The main thing I'd personally be worried about would be the portability of bit fields. I think that in case of D3D11_VIDEO_PROCESSOR_COLOR_SPACE the layout ends up being the same between MSVC and gcc on x86, but I'm not sure that's something we can depend on, or if that also holds on different compilers or e.g. 64-bit.
I don't think this should block the patch. We already use bit fields in Wine. Even if we ever find a problem with it (which is currently not the case AFAIK) and we'd need something special for Wine, we should still have a variant of declaration that is source compatible with PSDK (except probably ifdefed). Jacek
On 15 January 2016 at 14:19, Jacek Caban <jacek(a)codeweavers.com> wrote:
I don't think this should block the patch. We already use bit fields in Wine. Even if we ever find a problem with it (which is currently not the case AFAIK) and we'd need something special for Wine, we should still have a variant of declaration that is source compatible with PSDK (except probably ifdefed).
Probably. At first sight I see some minor style issues like the brace placement after "union" in D3D11_VIDEO_COLOR, D3D11_OMAC_SIZE should probably be a constant, and I'm really not a fan of the parameter naming convention in ID3D11VideoContext either, but ultimately nothing that would necessarily block the patch. I didn't review the patch closely though, and it's not assigned to me.
participants (4)
-
Andrey Turkin -
Henri Verbeet -
Jacek Caban -
Józef Kucia