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 XR mode selection to the Android export process. #29824

Merged
merged 1 commit into from
Jul 2, 2019

Conversation

m4gr3d
Copy link
Contributor

@m4gr3d m4gr3d commented Jun 16, 2019

Follow up to PR #29325.

This allows the user to select the XR mode in the Android export screen. Doing so properly configures GodotView when the Android app starts running.

The changes in this PR are as follow:

  • Rename XRMode.PANCAKE to XRMode.REGULAR, and update its command line parameter and related classes accordingly.
  • Set the default XR mode to REGULAR (regular/flatscreen).
  • Update the export.cpp file to properly configure the manifest meta-data entry based on the selected XR mode.

@akien-mga akien-mga added this to the 3.2 milestone Jun 16, 2019
@m4gr3d m4gr3d changed the title WIP: Add XR mode selection to the Android export process. Add XR mode selection to the Android export process. Jun 19, 2019
@m4gr3d
Copy link
Contributor Author

m4gr3d commented Jun 19, 2019

@BastiaanOlij The PR is ready for review!

@BastiaanOlij
Copy link
Contributor

Cool, looks good to me, I'll give it a try over the weekend and see if it works for me:)

@BastiaanOlij
Copy link
Contributor

Ok, this looks pretty good to me now that I've had time to take a proper look. The only issue is the "vr-only" mode added to the manifest which we need to find a way to make optional.

The problem is that we distribute runtime APKs that have manifests parsed to binary and aren't simple to edit. There is code for this in the exporter that parses the manifest and interjects stuff but that is some serious reverse engineered voodoo code. Can't remember who was responsible for that code (someone did mention to me once who it was and my hats off to that dude(tte) who managed to figure that one out) and it would be great if that person is willing to help out in enhancing the parser. I actually need similar logic to finish the ARCore PR which also introduces new values into the manifest.

@akien-mga
Copy link
Member

@hpvb did the manifest injection code, but I'm not sure it would be needed now that @reduz redesigned the Android export system. It should be designed around making such edits easier now. I'm not familiar with it yet though, but see #27781.

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Jun 30, 2019

@akien-mga @BastiaanOlij I responded to the comment above, but in summary the added meta-data tag is for vr mode detection on Oculus devices and for apps that uses the Oculus mobile SDK. If those conditions are not fulfilled, the meta-data is a no-op and the game behaves and runs as expected.

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Jun 30, 2019

@akien-mga @BastiaanOlij I responded to the comment above, but in summary the added meta-data tag is for vr mode detection on Oculus devices and for apps that uses the Oculus mobile SDK. If those conditions are not fulfilled, the meta-data is a no-op and the game behaves and runs as expected.

@akien-mga @BastiaanOlij I've updated my comment based on further testing.
In summary, the meta-data tag is a no-op on non oculus devices, but on oculus devices, it prompts the user to insert the device into a Gear VR headset.

I'll look for an alternative solution.

@BastiaanOlij
Copy link
Contributor

BastiaanOlij commented Jul 1, 2019 via email

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Jul 1, 2019

@akien-mga @BastiaanOlij I've updated the PR with an alternative solution which consists in leveraging gradle product flavors mechanism to generate separate binary templates for the regular|Regular and oculus mobile vr|OVR xr modes.
The export.cpp logic is updated as well to correctly pick the correct binary template based on the selected xr mode.

@BastiaanOlij your proposed solution of injecting the metadata directly into the manifest binary is another option, however I'm reticent to go down the path of expanding the manifest binary update logic.
Given Android doesn't publicly document that format, the current logic feels very brittle and easily subject to breakages if the Android manifest binary format were to be updated.
Also since @reduz is working to move away from that path, I think it would be counter-productive to have new features depend on that logic.

In addition, the use of gradle product flavors can easily facilitate the integration for ARCore as well.

I've updated the PR description accordingly.

@akien-mga
Copy link
Member

I don't want to ship extra export templates for all Android architectures just for the sake of changing one line in the manifest. That would add 110 MB to the templates package, while we already have logic to edit the manifest and it works well for parameters which are used in all games (namely whether you need gles2 or gles3 libs).

See the android/export/export.cpp changes in b0f782a for an example of how it's done.

Given Android doesn't publicly document that format, the current logic feels very brittle and easily subject to breakages if the Android manifest binary format were to be updated.

If that happens, we'll simply drop the option to use a precompiled APK and force users to use the Android SDK and gradle to build their own APKs (which is the other, more flexible but less direct alternative that @reduz implemented for 3.2).

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Jul 1, 2019

I don't want to ship extra export templates for all Android architectures just for the sake of changing one line in the manifest. That would add 110 MB to the templates package, while we already have logic to edit the manifest and it works well for parameters which are used in all games (namely whether you need gles2 or gles3 libs).

See the android/export/export.cpp changes in b0f782a for an example of how it's done.

Given Android doesn't publicly document that format, the current logic feels very brittle and easily subject to breakages if the Android manifest binary format were to be updated.

If that happens, we'll simply drop the option to use a precompiled APK and force users to use the Android SDK and gradle to build their own APKs (which is the other, more flexible but less direct alternative that @reduz implemented for 3.2).

@akien-mga Sounds good, and thanks for the context. I'll proceed with updating the PR based on the reference commit you provided.

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Jul 2, 2019

@akien-mga @BastiaanOlij I've updated the PR as suggested. It's ready for review!

@akien-mga akien-mga merged commit d2c416e into godotengine:master Jul 2, 2019
@akien-mga
Copy link
Member

Thanks!

@m4gr3d m4gr3d deleted the add_ovr_export branch July 2, 2019 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants