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

Add meta scene capture api support #53

Merged
merged 3 commits into from
Nov 21, 2023

Conversation

m4gr3d
Copy link
Collaborator

@m4gr3d m4gr3d commented Oct 21, 2023

Follow-up to the discussion in #40 (comment) about migrating Migeran's implementation of the Meta Scene capture API to the Godot OpenXR Loaders (subject to rename) plugin.

This PR also updates the project to include a godot-cpp and openxr dependencies for use when implementing the OpenXR gdextension.

@m4gr3d m4gr3d added the enhancement New feature or request label Oct 21, 2023
@m4gr3d m4gr3d added this to the 2.0.0 milestone Oct 21, 2023
@m4gr3d m4gr3d requested review from kisg and BastiaanOlij October 21, 2023 00:20
@m4gr3d
Copy link
Collaborator Author

m4gr3d commented Oct 21, 2023

@kisg @konczg Feedback welcomed!

If this looks good to you, I'll move the PR out of draft state.

@m4gr3d m4gr3d force-pushed the add_meta_scene_capture_api_support branch 3 times, most recently from 44829ce to 3648605 Compare October 21, 2023 00:50
@m4gr3d m4gr3d force-pushed the add_meta_scene_capture_api_support branch 2 times, most recently from 26a5a29 to a67ec6e Compare October 21, 2023 01:42
@kisg
Copy link
Collaborator

kisg commented Oct 21, 2023

@m4gr3d We will check it out, but this weekend is a long weekend in Hungary, we will be back at work on Tuesday.

@m4gr3d m4gr3d force-pushed the add_meta_scene_capture_api_support branch 2 times, most recently from 51b69f2 to 7c4abae Compare October 25, 2023 15:14
@m4gr3d m4gr3d marked this pull request as ready for review October 25, 2023 15:15
@BastiaanOlij
Copy link
Member

I ran into a problem compiling this:

> Task :godotopenxrmeta:buildCMakeDebug[arm64-v8a] FAILED
C/C++: ninja: error: '../../../../../thirdparty/godot-cpp/bin/libgodot-cpp.android.template_debug.arm64.a', needed by '../../../../build/intermediates/cxx/Debug/4n23k6j1/obj/arm64-v8a/libgodotopenxrmeta.so', missing and no known rule to make it

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':godotopenxrmeta:buildCMakeDebug[arm64-v8a]'.
> com.android.ide.common.process.ProcessException: ninja: Entering directory `C:\Projects\godot-assets\godot_openxr_vendors\godotopenxrmeta\.cxx\Debug\4n23k6j1\arm64-v8a'

  C++ build system [build] failed while executing:
      @echo off
      "C:\\Users\\mux21\\AppData\\Local\\Android\\Sdk\\cmake\\3.22.1\\bin\\ninja.exe" ^
        -C ^
        "C:\\Projects\\godot-assets\\godot_openxr_vendors\\godotopenxrmeta\\.cxx\\Debug\\4n23k6j1\\arm64-v8a" ^
        godotopenxrmeta
    from C:\Projects\godot-assets\godot_openxr_vendors\godotopenxrmeta
  ninja: error: '../../../../../thirdparty/godot-cpp/bin/libgodot-cpp.android.template_debug.arm64.a', needed by '../../../../build/intermediates/cxx/Debug/4n23k6j1/obj/arm64-v8a/libgodotopenxrmeta.so', missing and no known rule to make it

@BastiaanOlij
Copy link
Member

BastiaanOlij commented Oct 31, 2023

oh my bad, forgot to initialise submodules, not used to those not being there yet :)

okay, spoke to soon, having issues compiling godot-cpp for Android:

PS C:\Projects\godot-assets\godot_openxr_vendors\thirdparty\godot-cpp> scons platform=android target=template_debug
scons: Reading SConscript files ...
Auto-detected 16 CPU cores available for build parallelism. Using 15 cores by default. You can override it with the -j argument.
Building for architecture arm64 on platform android
ValueError: Required toolchain not found for platform android:
  File "C:\Projects\godot-assets\godot_openxr_vendors\thirdparty\godot-cpp\SConstruct", line 53:
    cpp_tool.generate(env)
  File "C:\Projects\godot-assets\godot_openxr_vendors\thirdparty\godot-cpp\tools\godotcpp.py", line 248:
    raise ValueError("Required toolchain not found for platform " + env["platform"])

Not sure whats going on here as I have no problems compiling Godot for Android, is godot-cpp looking for a specific environment variable or could this be an issue that I upgrade to java 17?

@m4gr3d
Copy link
Collaborator Author

m4gr3d commented Nov 1, 2023

oh my bad, forgot to initialise submodules, not used to those not being there yet :)

okay, spoke to soon, having issues compiling godot-cpp for Android:

PS C:\Projects\godot-assets\godot_openxr_vendors\thirdparty\godot-cpp> scons platform=android target=template_debug
scons: Reading SConscript files ...
Auto-detected 16 CPU cores available for build parallelism. Using 15 cores by default. You can override it with the -j argument.
Building for architecture arm64 on platform android
ValueError: Required toolchain not found for platform android:
  File "C:\Projects\godot-assets\godot_openxr_vendors\thirdparty\godot-cpp\SConstruct", line 53:
    cpp_tool.generate(env)
  File "C:\Projects\godot-assets\godot_openxr_vendors\thirdparty\godot-cpp\tools\godotcpp.py", line 248:
    raise ValueError("Required toolchain not found for platform " + env["platform"])

Not sure whats going on here as I have no problems compiling Godot for Android, is godot-cpp looking for a specific environment variable or could this be an issue that I upgrade to java 17?

@BastiaanOlij This happens when the ANDROID_NDK_ROOT env variable is not defined.

We've moved away from this variable in Godot core, I'll send a PR to godot-cpp to update the logic.

@BastiaanOlij
Copy link
Member

Ok, got it all to compile and works, will be playing with it some more

Copy link
Member

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

I think we should just merge this, if anything comes up that needs to be perfected that will come out of people playing around with it.

@m4gr3d
Copy link
Collaborator Author

m4gr3d commented Nov 10, 2023

I think we should just merge this, if anything comes up that needs to be perfected that will come out of people playing around with it.

Sounds good to me!
I need to update the demo so it doesn't kick-in automatically every time the app launches, and then start a draft for the documentation.

@kisg Any additional feedback?

@kisg
Copy link
Collaborator

kisg commented Nov 11, 2023

@m4gr3d

Just would like to add a copyright notice for the source files that we contributed will send a PR on top of this one in a few minutes.

@kisg
Copy link
Collaborator

kisg commented Nov 11, 2023

@m4gr3d Opened PR with copyright headers: m4gr3d#1

@BastiaanOlij
Copy link
Member

Just noticed that we need to improve the .gitignore file, it's including compiled binaries now that we are compiling sourcecode :)
On windows it's just excluding .obj but I assume we need more for the other platforms.

@m4gr3d m4gr3d force-pushed the add_meta_scene_capture_api_support branch from 7c4abae to 8b356af Compare November 21, 2023 19:09
@m4gr3d m4gr3d force-pushed the add_meta_scene_capture_api_support branch from d64af98 to e67afda Compare November 21, 2023 19:57
Install Android NDK r23c
@m4gr3d m4gr3d force-pushed the add_meta_scene_capture_api_support branch from a60c0fd to b57b16b Compare November 21, 2023 20:09
@m4gr3d m4gr3d merged commit 67777fb into GodotVR:master Nov 21, 2023
1 check passed
@m4gr3d m4gr3d deleted the add_meta_scene_capture_api_support branch November 21, 2023 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants