Skip to content

Add support for Android platform as it builds and runs successfully. #319

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

Closed
wants to merge 4 commits into from

Conversation

Jayanth-L
Copy link
Contributor

Just updating the SConstruct file to inlclude code to build for android using NDK
and updating README.md with small tutorial to build for armeabi-v7a and arm64-v8a.

@piiertho
Copy link
Contributor

piiertho commented Aug 6, 2019

Hi !
It seems to miss a lot of environments variables, like LD, RANLIB.
By the way, #298 deal with Android. You may have some ideas to simplify it ?

@Jayanth-L
Copy link
Contributor Author

Hi !
It seems to miss a lot of environments variables, like LD, RANLIB.
By the way, #298 deal with Android. You may have some ideas to simplify it ?

I don't have much knowledge of linkers, but for this those tools aren't necessary i think,
linking is taken care by this command:
aarch64-linux-android29-clang++ -o demo/bin/android/libgdexample_64.so -shared src/gdexample.os src/gdexample2.os src/gdlibrary.os src/sphere_controller.os -Lgodot-cpp/bin -lgodot-cpp.android.debug.64

@TGRCdev
Copy link
Contributor

TGRCdev commented Aug 13, 2019

I have some notes about this PR

  • Leaving bits undefined makes the compiler stay as the default for the system instead of using the NDK compilers
  • Compilation on Windows uses the wrong flags (using / flags instead of -). Suggestion:
if env['platform'] == 'android':
    if host_platform == 'windows':
        env = Environment(TOOLS=['mingw'])
        opts.Update(env)

This will make Scons use gcc-style flags when compiling/linking.

  • Compilation on Windows also fails due to the final ar line being too long, but this is more a godot-cpp problem than this PR's fault.
  • The resulting output is in the format libgodot-cpp.android.{target}.{bits}.a, but I think a better output would be libgodot-cpp.android.{target}.{android arch}.a since GDNative library resources have four members for Android libraries armeabi-v7a, arm64-v8a, x86, x86_64, and each arch implies the bits.
  • Compiling based on bits only lets the user compile for armv7 and arm64v8, not x86 or x86_64. Android compilation should use an additional option for android_arch and ignore bits, like Godot.
  • Is there a reason why android_api_level can only be 26 through 29? Godot sets ndk_platform to 18 by default (21 if compiling for a 64-bit arch)
  • These lines:
    https://github.com/GodotNativeTools/godot-cpp/blob/c56c58984f591b16776f0a681f058b803521e439/SConstruct#L134-L137
    assume that the user has their NDK toolchains bin folder added to their PATH. I think Scons should search for toolchains at $ANDROID_NDK_ROOT instead, since that environment variable would be set if a user has compiled Godot for Android before. Assuming <ndk_num> is the Android API number and <host_platform> is one of four host platforms windows, windows-x86_64, linux-x86_64, darwin-x86_64, the correct compilers can be found at these paths (separated by Android arch):
    • armv7 : $ANDROID_NDK_ROOT/toolchains/llvm/prebuilt/<host_platform>/bin/armv7a-linux-androideabi<ndk_num>-clang++
    • arm64v8 : $ANDROID_NDK_ROOT/toolchains/llvm/prebuilt/<host_platform>/bin/aarch64-linux-android<ndk_num>-clang++
    • x86 : $ANDROID_NDK_ROOT/toolchains/llvm/prebuilt/<host_platform>/bin/i686-linux-android<ndk_num>-clang++
    • x86_64 : $ANDROID_NDK_ROOT/toolchains/llvm/prebuilt/<host_platform>/bin/x86_64-linux-android<ndk_num>-clang++

If you want, I can consolidate my fixes into a PR to your branch before it merges to godot-cpp.

@Jayanth-L
Copy link
Contributor Author

I have some notes about this PR

  • Leaving bits undefined makes the compiler stay as the default for the system instead of using the NDK compilers
  • Compilation on Windows uses the wrong flags (using / flags instead of -). Suggestion:
if env['platform'] == 'android':
    if host_platform == 'windows':
        env = Environment(TOOLS=['mingw'])
        opts.Update(env)

This will make Scons use gcc-style flags when compiling/linking.

  • Compilation on Windows also fails due to the final ar line being too long, but this is more a godot-cpp problem than this PR's fault.

  • The resulting output is in the format libgodot-cpp.android.{target}.{bits}.a, but I think a better output would be libgodot-cpp.android.{target}.{android arch}.a since GDNative library resources have four members for Android libraries armeabi-v7a, arm64-v8a, x86, x86_64, and each arch implies the bits.

  • Compiling based on bits only lets the user compile for armv7 and arm64v8, not x86 or x86_64. Android compilation should use an additional option for android_arch and ignore bits, like Godot.

  • Is there a reason why android_api_level can only be 26 through 29? Godot sets ndk_platform to 18 by default (21 if compiling for a 64-bit arch)

  • These lines:
    https://github.com/GodotNativeTools/godot-cpp/blob/c56c58984f591b16776f0a681f058b803521e439/SConstruct#L134-L137

    assume that the user has their NDK toolchains bin folder added to their PATH. I think Scons should search for toolchains at $ANDROID_NDK_ROOT instead, since that environment variable would be set if a user has compiled Godot for Android before. Assuming <ndk_num> is the Android API number and <host_platform> is one of four host platforms windows, windows-x86_64, linux-x86_64, darwin-x86_64, the correct compilers can be found at these paths (separated by Android arch):

    • armv7 : $ANDROID_NDK_ROOT/toolchains/llvm/prebuilt/<host_platform>/bin/armv7a-linux-androideabi<ndk_num>-clang++
    • arm64v8 : $ANDROID_NDK_ROOT/toolchains/llvm/prebuilt/<host_platform>/bin/aarch64-linux-android<ndk_num>-clang++
    • x86 : $ANDROID_NDK_ROOT/toolchains/llvm/prebuilt/<host_platform>/bin/i686-linux-android<ndk_num>-clang++
    • x86_64 : $ANDROID_NDK_ROOT/toolchains/llvm/prebuilt/<host_platform>/bin/x86_64-linux-android<ndk_num>-clang++

If you want, I can consolidate my fixes into a PR to your branch before it merges to godot-cpp.

Thank you very much for finding out problems with this PR , I am quite busy these days with my college and other work, I have only Linux system so didn't give a thought about windows and Mac os Android NDK toolchain binaries.

I would be very happy and appreciate if you added your modifications to this PR 😀

@TGRCdev
Copy link
Contributor

TGRCdev commented Aug 13, 2019

Will do. I'll work on making compilation work and getting a functioning GDNative plugin on Android. Once it works properly, I'll submit a PR to your branch. 👍

@CedNaru
Copy link

CedNaru commented Aug 14, 2019

The missing environnement variables for the Android compiling give me a linking error when I want to use the generated binding with my GdNative plugin. (https://github.com/utopia-rise/fmod-gdnative) This plugin uses an additionnal dynamic library (Fmod) so the linking step is a bit more complex.

When compiling I've already got this warning:
ranlib bin/libgodot-cpp.android.release.64.a
warning: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: warning for library: bin/libgodot-cpp.android.release.64.a the table of contents is empty (no object file members in the library define global symbols)

Then when I wanted to use it when compiling my Gdnative:

/Users/cednaru/testProjects/gdnative-android/fmod-gdnative/../godot-cpp/include/core/Godot.hpp:124: undefined reference to `nativescript_1_1_api'
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [obj/local/arm64-v8a/libandroid_fmod_gdnative.so] Error 1

@TGRCdev
Copy link
Contributor

TGRCdev commented Aug 14, 2019

@CedNaru Try generating bindings using this godot-cpp and report back. I'm currently fixing bugs for this PR per-platform for Android compilation, so your input would be really helpful.

@Jayanth-L
Copy link
Contributor Author

@TGRCdev TGRCdev Sorry for the late reply

As this is your work and you have done the most of the work, I would suggest you to create a pull request on the main repository, and I will withdraw mine ;)

It's your decision, if you tell me to merge then I'll merge.

Very much thank you for your work ☺️

For now I have merged ;)

@Jayanth-L
Copy link
Contributor Author

Jayanth-L commented Sep 18, 2019

Hello @BastiaanOlij @TGRCdev has done a great work in making the GDNative work for Android, I would request you to review this pull request and make appropriate action.

@TGRCdev
Copy link
Contributor

TGRCdev commented Sep 18, 2019

@Jayanth-L I have created a final pull request for the branch, including both of our changes, an up-to-date summary, and some rebasing (I merged two of your commits, but maintained your authorship). This wouldn't have been possible without your commit, as it lead me in the right direction when I had no knowledge whatsoever of the NDK.

@BastiaanOlij
Copy link
Collaborator

Ok so it's the other pull request that needs to be merged?
If you guys are happy with how it works I'm happy to merge it in. We'll see what feedback we get.

@Jayanth-L
Copy link
Contributor Author

Yes @BastiaanOlij, you can merge other PR I'll be happy :)

@BastiaanOlij
Copy link
Collaborator

Done! I'll close this one then :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants