KhronosGroup / OpenCL-Headers

Khronos OpenCL-Headers
Apache License 2.0
656 stars 236 forks source link

inconsistent handling of interop API headers #146

Open bashbaug opened 3 years ago

bashbaug commented 3 years ago

I discovered this issue while working generated OpenCL headers for extensions (see #113).

We currently handle interop API headers (e.g. DX headers for DX interop) inconsistently:

The interop API header for DX9 media sharing is most inconsistent. Should we continue to guard the include for d3d9.h, or should we include it unconditionally, similar to DX10, DX11, and VAAPI sharing?

In practice, I think another way to phrase this question is: Should an application be responsible for including the header for DX9 sharing only platforms where the DX9 headers are available, or should the interop API headers attempt to be include-able for all platforms?

bashbaug commented 2 years ago

I stumbled across this issue again just now, the part about the DX9 media sharing extension in particular.

So I'm less likely to forget next time, the DX10 and DX11 sharing extensions both essentially have:

#if defined(_MSC_VER)
#if _MSC_VER >=1500
#pragma warning( push )
#pragma warning( disable : 4201 )
#pragma warning( disable : 5105 )
#endif
#endif
#include <d3d10.h>
#if defined(_MSC_VER)
#if _MSC_VER >=1500
#pragma warning( pop )
#endif
#endif

But, the DX9 media sharing header has:

#if defined(_WIN32)
#if defined(_MSC_VER)
#if _MSC_VER >=1500
#pragma warning( push )
#pragma warning( disable : 4201 )
#pragma warning( disable : 5105 )
#endif
#endif
#include <d3d9.h>
#if defined(_MSC_VER)
#if _MSC_VER >=1500
#pragma warning( pop )
#endif
#endif
#endif

Note the extra outermost check for #if defined(_WIN32).

Is this difference intentional? Or can we drop the guard for _WIN32? I can't imagine how the header would work without _WIN32, and indeed our testing checks for _WIN32 before including the DX9 media sharing header:

https://github.com/KhronosGroup/OpenCL-Headers/blob/main/tests/test_cl_dx9_media_sharing.h.c#L19