Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OpenXR SDK does not compile on Windows without XR_USE_GRAPHICS_API_D3D11/similar #237

Closed
philpax opened this issue Jan 25, 2021 · 7 comments · Fixed by #250
Closed

OpenXR SDK does not compile on Windows without XR_USE_GRAPHICS_API_D3D11/similar #237

philpax opened this issue Jan 25, 2021 · 7 comments · Fixed by #250
Labels
synced to gitlab Synchronized to OpenXR internal GitLab

Comments

@philpax
Copy link

philpax commented Jan 25, 2021

Hi there! This is a fairly minor issue, but it caused me a little grief recently. I've been integrating the SDK into a non-DX codebase using a different build system and used a minimal set of defines to add Windows support:

'API_NAME="OpenXR"',
"XR_USE_TIMESPEC",
"XR_CURRENT_API_MAJOR_VERSION=1",
"XR_CURRENT_API_MINOR_VERSION=0",
"XR_CURRENT_API_PATCH_VERSION=13",
"XR_USE_GRAPHICS_API_OPENGL",
"XR_USE_PLATFORM_WIN32", 
"XR_OS_WINDOWS", 
"WIN32", 
"_WINDOWS",

However, this will not compile. openxr/openxr_platform.h refers to IUnknown, which isn't pulled in by xr_dependencies.h with the above set of defines. Defining the D3D defines pulls in the DX headers, which in turn pulls in the headers for IUnknown.

Again, really minor, but might save someone else in the same predicament some trouble 😅
@brycehutchings
Copy link
Contributor

I think in this case IUnknown is required because of XR_MSFT_perception_anchor_interop and XR_MSFT_holographic_window_attachment. These extensions actually use IUnknown to avoid putting a dependency on a more specific type. I think the minimum include would be <unknwn.h>, if that is available in your build system. I think we consider IUnknown a core type to the Win32 build, and since D3D uses IUnknown, the type is brought in by defining XR_USE_GRAPHICS_API_D3D11/XR_USE_GRAPHICS_API_D3D12.

@philpax
Copy link
Author

philpax commented Jan 26, 2021

Yep, that's right; my suggestion is to pull unknwn.h into xr_dependencies.h if WIN32 is defined. I didn't want to change the XR files so I left it as-is, but that should work fine (assuming that there are no other implicit dependencies).

@rpavlik
Copy link
Contributor

rpavlik commented Jan 29, 2021

Yeah, for some reason our headers aren't being generated with the external #include lines despite being marked up with them in XML. Not sure why you've found something different with the d3d stuff - presumably because you're using the (mess of an internal) header xr_dependencies. Seems like a reasonable thing to add to there.

@rpavlik-bot
Copy link
Collaborator

An issue (number 1520) has been filed to correspond to this issue in the internal Khronos GitLab (Khronos members only: KHR:openxr/openxr#1520 ), to facilitate working group processes.

This GitHub issue will continue to be the main site of discussion.

@rpavlik-bot rpavlik-bot added the synced to gitlab Synchronized to OpenXR internal GitLab label Feb 4, 2021
@rpavlik rpavlik mentioned this issue Apr 2, 2021
Merged
@rpavlik
Copy link
Contributor

rpavlik commented Apr 2, 2021

OK, I've put up a fix to review.

rpavlik added a commit that referenced this issue Apr 14, 2021
@rpavlik
OpenXR SDK 1.0.15 (2021-04-13)
57cfe17 
The main SDK change in this release is that the OpenXR headers NO LONGER
EXPOSE EXTENSION FUNCTION PROTOTYPES because extension functions are not
exported by the loader. This should prevent some confusion during
development without affecting code that correctly compiles and links
with older SDKs. Code that was compiled but not linked (for instance,
the automated tests of example source in the specification) and that
would not have successfully linked may have their defects highlighted by
this change, however. If you need those prototypes still available,
there is a preprocessor define that can re-enable them. The function
pointer definitions are always available.

In addition to that header change, this release contains three new
vendor extensions plus an assortment of SDK fixes.

-   Registry
    -   Add XR_VARJO_foveated_rendering vendor extension. (internal MR
        1981)
    -   Add XR_VARJO_composition_layer_depth_test vendor extension.
        (internal MR 1998)
    -   Add XR_VARJO_environment_depth_estimation vendor extension.
        (internal MR 1998)
    -   Add uint16_t to openxr_platform_defines (and associated scripts)
        so it may be used easily by extensions. (internal MR 2017)
    -   Reserve extension 149 for working group use. (internal MR 1999)
    -   Reserve extension numbers 150 to 155 for ULTRALEAP extensions
        (internal MR 2006)
    -   Reserve extension numbers 156-165 for Facebook. (internal MR
        2018)
-   SDK
    -   Hide prototypes for extension functions unless explicitly
        requested by defining XR_EXTENSION_PROTOTYPES. These functions
        are not exported from the loader, so having their prototypes
        available is confusing and leads to link errors, etc.
        (OpenXR-SDK-Source/#251, OpenXR-SDK-Source/#174, internal issue
        1554, internal issue 1338)
    -   Also list API layers in list tool. (internal MR 1991)
    -   Ensure we expose the OpenXR headers in the build-time interface
        of the loader, as well as the install-time interface, for use
        with FetchContent.cmake. (OpenXR-SDK-Source/#242,
        OpenXR-SDK-Source/#195, internal issue 1409)
    -   Improve BUILDING.md, including adding details on how to specify
        architecture for VS2019. (OpenXR-SDK-Source/#245,
        OpenXR-SDK-Source/#253)
    -   Loader: Fix loader failing to load on Windows 7 due to pathcch
        dependency. (OpenXR-SDK-Source/#239, OpenXR-SDK-Source/#214,
        internal issue 1471, OpenXR-SDK-Source/#236, internal issue
        1519)
    -   Loader: Fix conflicting filename in openxr_loader.def causing a
        linker warning when building debug for Windows.
        (OpenXR-SDK-Source/#246)
    -   Update cgenerator.py to generate header comments in openxr.h to
        show when a struct extends another struct (internal MR 2005)
    -   hello_xr: Check for shaderStorageImageMultisample feature in
        Vulkan plugin before using it. (OpenXR-SDK-Source/#234,
        OpenXR-SDK-Source/#233, internal issue 1518)
    -   hello_xr: Make sure common.h includes the reflection header that
        it uses. (OpenXR-SDK-Source/#247)
    -   layers: Revise documentation, re-formatting and updating to
        refer to real functions and URLs. (internal MR 2012)
    -   loader: Check the instance handle passed to
        xrGetInstanceProcAddr. (internal MR 1980)
    -   loader: Fix building OpenXR-SDK with CMake’s multi-config Ninja
        generator. (OpenXR-SDK-Source/#249, OpenXR-SDK-Source/#231)
    -   openxr_reflection.h: Make reproducible/deterministic by sorting
        protection defines in the script. (internal MR 1993, internal
        issue 1424)
    -   xr_dependencies (shared utility): Include unknwn.h on Windows,
        even without D3D enabled. (OpenXR-SDK-Source/#250,
        OpenXR-SDK-Source/#237)
@bjornbytes bjornbytes mentioned this issue Dec 24, 2021
Closed
@amalon
Copy link

amalon commented Aug 25, 2022

This issue also appears to affect users of the OpenXR headers, not just the loader itself. Somebody on the flightgear mailing list reported it with MSVC 2019 (16.11.18), with OpenXR 1.0.24:
https://sourceforge.net/p/flightgear/mailman/message/37697116/
Visual studio community 2022 didn't seem affected. Including unknwn.h before openxr_platform.h worked around it, but it seems flawed to expect openxr apps to anticipate includes that might be required on windows for extensions they don't use or future extensions they may not know about.

Should the openxr_platform.h header not include unknwn.h itself?

@rpavlik
Copy link
Contributor

rpavlik commented Aug 25, 2022

so that's a good question. The required headers/includes are marked up in the XML but aren't being written to the header files for some reason. Can you open an issue for adding those includes to openxr_platform.h? It is annoying to work around for sure.

@amalon amalon mentioned this issue Aug 25, 2022
Open
rhabacker pushed a commit to rhabacker/OpenXR-SDK-Source that referenced this issue Nov 16, 2022
@rpavlik
OpenXR SDK 1.0.15 (2021-04-13)
2072fe9 
The main SDK change in this release is that the OpenXR headers NO LONGER
EXPOSE EXTENSION FUNCTION PROTOTYPES because extension functions are not
exported by the loader. This should prevent some confusion during
development without affecting code that correctly compiles and links
with older SDKs. Code that was compiled but not linked (for instance,
the automated tests of example source in the specification) and that
would not have successfully linked may have their defects highlighted by
this change, however. If you need those prototypes still available,
there is a preprocessor define that can re-enable them. The function
pointer definitions are always available.

In addition to that header change, this release contains three new
vendor extensions plus an assortment of SDK fixes.

-   Registry
    -   Add XR_VARJO_foveated_rendering vendor extension. (internal MR
        1981)
    -   Add XR_VARJO_composition_layer_depth_test vendor extension.
        (internal MR 1998)
    -   Add XR_VARJO_environment_depth_estimation vendor extension.
        (internal MR 1998)
    -   Add uint16_t to openxr_platform_defines (and associated scripts)
        so it may be used easily by extensions. (internal MR 2017)
    -   Reserve extension 149 for working group use. (internal MR 1999)
    -   Reserve extension numbers 150 to 155 for ULTRALEAP extensions
        (internal MR 2006)
    -   Reserve extension numbers 156-165 for Facebook. (internal MR
        2018)
-   SDK
    -   Hide prototypes for extension functions unless explicitly
        requested by defining XR_EXTENSION_PROTOTYPES. These functions
        are not exported from the loader, so having their prototypes
        available is confusing and leads to link errors, etc.
        (OpenXR-SDK-Source/KhronosGroup#251, OpenXR-SDK-Source/KhronosGroup#174, internal issue
        1554, internal issue 1338)
    -   Also list API layers in list tool. (internal MR 1991)
    -   Ensure we expose the OpenXR headers in the build-time interface
        of the loader, as well as the install-time interface, for use
        with FetchContent.cmake. (OpenXR-SDK-Source/KhronosGroup#242,
        OpenXR-SDK-Source/KhronosGroup#195, internal issue 1409)
    -   Improve BUILDING.md, including adding details on how to specify
        architecture for VS2019. (OpenXR-SDK-Source/KhronosGroup#245,
        OpenXR-SDK-Source/KhronosGroup#253)
    -   Loader: Fix loader failing to load on Windows 7 due to pathcch
        dependency. (OpenXR-SDK-Source/KhronosGroup#239, OpenXR-SDK-Source/KhronosGroup#214,
        internal issue 1471, OpenXR-SDK-Source/KhronosGroup#236, internal issue
        1519)
    -   Loader: Fix conflicting filename in openxr_loader.def causing a
        linker warning when building debug for Windows.
        (OpenXR-SDK-Source/KhronosGroup#246)
    -   Update cgenerator.py to generate header comments in openxr.h to
        show when a struct extends another struct (internal MR 2005)
    -   hello_xr: Check for shaderStorageImageMultisample feature in
        Vulkan plugin before using it. (OpenXR-SDK-Source/KhronosGroup#234,
        OpenXR-SDK-Source/KhronosGroup#233, internal issue 1518)
    -   hello_xr: Make sure common.h includes the reflection header that
        it uses. (OpenXR-SDK-Source/KhronosGroup#247)
    -   layers: Revise documentation, re-formatting and updating to
        refer to real functions and URLs. (internal MR 2012)
    -   loader: Check the instance handle passed to
        xrGetInstanceProcAddr. (internal MR 1980)
    -   loader: Fix building OpenXR-SDK with CMake’s multi-config Ninja
        generator. (OpenXR-SDK-Source/KhronosGroup#249, OpenXR-SDK-Source/KhronosGroup#231)
    -   openxr_reflection.h: Make reproducible/deterministic by sorting
        protection defines in the script. (internal MR 1993, internal
        issue 1424)
    -   xr_dependencies (shared utility): Include unknwn.h on Windows,
        even without D3D enabled. (OpenXR-SDK-Source/KhronosGroup#250,
        OpenXR-SDK-Source/KhronosGroup#237)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
synced to gitlab Synchronized to OpenXR internal GitLab
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

xr_dependencies: Include unknwn.h on Windows even without d3d. rpavlik/OpenXR-SDK-Source
5 participants
@rpavlik @amalon @philpax @brycehutchings @rpavlik-bot