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.

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

OpenXR loader: partial fix for XRLOADER_DISABLE_EXCEPTION_HANDLING #405

Conversation

bialpio
Copy link

@bialpio bialpio commented May 10, 2023

Work left: adjust JNIPP to support no-exceptions mode.

Tested by building the following two configurations (based on the parameters that maintainer-scripts/build-aar.sh script is passing to cmake):
cmake -S . -B out/exceptions -G Ninja -DCMAKE_TOOLCHAIN_FILE=/usr/local/google/code/clankium/src/third_party/android_ndk/build/cmake/android.toolchain.cmake -DCMAKE_ANDROID_NDK=/usr/local/google/code/clankium/src/third_party/android_ndk/ -DANDROID_ABI=arm64-v8a -DANDROID_PLATFORM=24 -DANDROID_STL=c++_static -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=OFF -DBUILD_CONFORMANCE_TESTS=OFF -DBUILD_LOADER=ON -DBUILD_API_LAYERS=OFF

cmake -S . -B out/no-exceptions -G Ninja -DCMAKE_TOOLCHAIN_FILE=/usr/local/google/code/clankium/src/third_party/android_ndk/build/cmake/android.toolchain.cmake -DCMAKE_ANDROID_NDK=/usr/local/google/code/clankium/src/third_party/android_ndk/ -DANDROID_ABI=arm64-v8a -DANDROID_PLATFORM=24 -DANDROID_STL=c++_static -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=OFF -DBUILD_CONFORMANCE_TESTS=OFF -DBUILD_LOADER=ON -DBUILD_API_LAYERS=OFF -DBUILD_LOADER_WITH_EXCEPTION_HANDLING=OFF

@CLAassistant
Copy link

CLAassistant commented May 10, 2023

CLA assistant check
All committers have signed the CLA.

@rpavlik-bot
Copy link
Collaborator

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

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

@rpavlik-bot rpavlik-bot added the synced to gitlab Synchronized to OpenXR internal GitLab label May 17, 2023
Copy link
Contributor

@rpavlik rpavlik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me. Does it make sense to merge this without the rest of the changes?

Can you add a changelog fragment in changes/sdk/pr.405.gh.OpenXR-SDK-Source.md per the readme in that directory?

@bialpio
Copy link
Author

bialpio commented Jun 8, 2023

Looks reasonable to me. Does it make sense to merge this without the rest of the changes?

IMO yes - depending on how downstream consumers compile the loader, it'd allow them to immediately start compiling itw/o exceptions, but still compile the JNIPP with exceptions.

Can you add a changelog fragment in changes/sdk/pr.405.gh.OpenXR-SDK-Source.md per the readme in that directory?

Done, please take a look.

@bialpio
Copy link
Author

bialpio commented Jun 20, 2023

Gentle ping - @rpavlik, can you take a look?

@rpavlik
Copy link
Contributor

rpavlik commented Nov 29, 2023

Updated version of this patch merged in internal tree, released with 1.0.32

@rpavlik rpavlik closed this Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
synced to gitlab Synchronized to OpenXR internal GitLab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants