Android support for BigWheels #46

shanminchao · 2023-03-07T22:22:14Z

This is a rough "as-little-change-as-possible" change to enable BigWheels to run on Android. It has been tested with the gbuffer and fishtornado samples successfully (Note: Fishtornado works on Quest2 but fails on Pixel 6. We will have to re-visit fishtornado as a separate task). Among some of the things changed to support Android:

Disable imgui
Disable cpu_features
Abstract file loading so it goes through Android's asset manager (to load assets from APK)
Use memory-loading versions of 3rd party libraries (tinyobj, stbi, etc) to avoid using freads
Use Android surface and #ifdef out all the glfw code (including input handling)
Added all the necessary files to open and build BigWheels from AndroidStudio via Gradle

Todo:

Avoid populating root assets folder with build artifacts (i.e. SPV files)
Enable cpu_features
Enable imgui for Android (not sure if it's practical to do this)
Handle inputs
Revisit landscape vs portrait orientation and render resolution
Find/design some way to run different samples without editing CMakelists
Fix fishtornado so that it can run on a Pixel 6

chaoticbob

Is it necessary to check in all the XML files? Does grade/cmake not generate them during project generation or build time?

chaoticbob · 2023-03-08T19:37:48Z

CMakeLists.txt

+# Add game-activity for ANDROID
+# ------------------------------------------------------------------------------
+if (PPX_ANDROID)
+    find_package(game-activity REQUIRED CONFIG)


Does this come from the NDK?

shanminchao · 2023-03-10T17:38:47Z

Is it necessary to check in all the XML files? Does grade/cmake not generate them during project generation or build time?

So the NDK template's .gitignore specifically does not include the specific xml files in the .idea folder that I pushed.
However, in both linux and windows, if I delete the entire .idea folder and re-run Android Studio, those files get
re-generated. So it doesn't seem necessary to push them. I've omitted them in a new commit.

android/app/src/main/java/com/example/bigwheels/MainActivity.java

docs/building_and_running.md

Keenuts · 2023-03-21T09:35:48Z

android/app/src/androidTest/java/com/example/bigwheels/ExampleInstrumentedTest.java

+    public void useAppContext() {
+        // Context of the app under test.
+        Context appContext = InstrumentationRegistry.getInstrumentation().getTargetContext();
+        assertEquals("com.example.bigwheels", appContext.getPackageName());


maybe com.google.bigwheels?

Keenuts · 2023-03-21T09:37:03Z

android/app/src/main/cpp/CMakeLists.txt

+
+project("bigwheels")
+
+add_subdirectory("../../../../.." "../../../../../app/build")


Isn't there a variable instead of this .../.../.../.../.../../../../../../ ? like $PPX_DIR or something similar?

So no, none of those variables exist. For the Android builds, this CMakeLists.txt file is the first one that cmake encounters, so none of those $PPX_* variables have been setup yet.

I cleaned it up a bit to make it a little more explicit, but it's still ugly relative paths. Not sure what we can do about that. I'm open to suggestions!

Do any/all of these relative path resolve to similar or the same locations? It might be easier to have a variable that starts a root and then appends the relative paths on. While add_subdirectory implies the current path - it's a little difficult to understand past like 3 relative paths.

Yeah that's what my cleanup does.

However, Keenuts is working on a subsequent PR that will completely clean up the directory structure and make all this relative path nonsense a thing of the past.

Keenuts · 2023-03-21T09:37:39Z

android/app/src/main/cpp/CMakeLists.txt

+# You can define multiple libraries, and CMake builds them for you.
+# Gradle automatically packages shared libraries with your APK.
+
+add_library( # Sets the name of the library.


file format seems odd? clang-format created those newlines?

Looks fine to me. What's odd about the file format?

The new lines between each part of the

add_library\n#comment\n\nbigwheels\n\n#comment\n\nSHARED\n\n#comment\nmaain.cpp

Just unique compared to the other CMakelist.txt

I see. The cmake was generated by android studio, so I kept it as unmodified as possible. They added newlines so they can add comments, I guess. Do you feel strongly about cleaning it up?

I was following a "as little change as possible to the template" policy in general. But it's not a strict requirement.

android/app/src/main/cpp/main.cpp

Keenuts · 2023-03-21T09:49:18Z

android/app/src/main/res/mipmap-anydpi-v26/ic_launcher_round.xml

@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="utf-8"?>


All those webp & SVG icons could be replaced by a single xml with like 1 square/shape?

like this one (res/drawable/icon.xml)

<?xml version="1.0" encoding="utf-8"?> <shape xmlns:android="http://schemas.android.com/apk/res/android" android:shape="rectangle"> <solid android:color="#FF0000"/> </shape>

Which ends up looking like so:

As discussed offline, Keenuts is working on a subsequent PR that will clean all this nonsense up.

I removed most of the files in the res/ folder, which should reduce the overall size of this PR by about 40KB.

cmake/ShaderCompile.cmake

Keenuts · 2023-03-21T10:07:36Z

include/ppx/application.h

@@ -30,6 +31,36 @@
 #   define GLFW_INCLUDE_NONE
 #endif
 #include <GLFW/glfw3.h>
+
+#if defined(PPX_ANDROID)
+    #define SETUP_APPLICATION(AppType)                  \


Really don't like this part.

What about doing the following:

we provide the main function.

we have a desktop_main.cc, or android_main.cc, which is compiled depending on the target.

this function calls an application_main with argv and the app object.

our samples/benchmarks only implements the application_main?

Yes, that would be a little more involved but definitely cleaner.
In the interest of expediting this PR and getting more people working on Android sooner, I'm going to treat this as a cleanup to be done in a later PR. Ok with you?

Keenuts · 2023-03-21T10:11:51Z

projects/fishtornado/Flocking.cpp

-        pFlockingData->resY               = static_cast<int>(mResY);
+
+        // For some reason, reading the velocity texture in the vertex shader at the full 128x128 causes a GPU crash on Pixel 6 Pro.
+        // Also note that when the Pixel Shader is modified to be a solid-color shader, the crash goes away. This even


Could it be caused by a cross-stage optimization?
if you replace the FS by a solid color, it means you don't read normal/tangent, etc.
So those are dead inputs, meaning VS has dead outputs, so all the instructions generating those can be removed across the whole pipeline, meaning no more texture load?

That doesn't explain why reducing the size makes it automagically work...
In any case, I'm going to leave this as an exercise for someone else to solve. It's enough for the purposes of this PR that fishtornado runs on Android.

Funnily enough, now fishtornado is broken again even with this fix. I have to disable the flocking shadow pass before it works again. There's definitely something wrong somewhere.

It runs on Quest2 though, although there's minor graphical artifacts.

Keenuts · 2023-03-21T10:16:23Z

src/ppx/application.cpp

        int  value     = glfwWindowShouldClose(static_cast<GLFWwindow*>(mWindow));
        bool isRunning = (value == 0);
        return isRunning;
+#else
+        return true;


AFAIK this is determined by android_app->destroyRequested.
struct android_app* is provided at the android_main entry.

src/ppx/fs.cpp

wangra-google · 2023-03-23T12:06:39Z

projects/fishtornado/Flocking.cpp

+        // happens when the FlockingVelocity compute job is disabled and the velocity values are hardcoded to a
+        // unit vector. Can't make sense of it right now, and it's possible it has to do with SPIR-V code generation,
+        // since DXC isn't guaranteed to work with mobile.
+        pFlockingData->resY               = 100; //static_cast<int>(mResY);


in FlockingRender.hlsl, there is a line in vsmain:

uint2 xy = uint2((instanceId % Flocking.resX), (instanceId / Flocking.resY));

instanceId is in the range of [0, 128x128-1]

when you make Flocking.resY smaller than 128, the y would be larger than 128 which might cause problems?

Ok you're right. Checking in a fix that magically works is probably not a good idea.
I have reverted this hack and made it a TODO to fix fishtornado.

And I also made the gbuffer sample the default one

Keenuts

OK for me, except need to propagate the sample fixes to the 3 new samples.

Keenuts · 2023-04-05T07:16:38Z

projects/27_mipmap_demo/main.cpp

-
-    return res;
-}
+SETUP_APPLICATION(ProjApp)


3 new samples have been added:

28_gltf

oit

benchmark fork of 28_gltf

Added SETUP_APPLICATION to all 3 of these new samples!

Also added to fluid simulation sample

The folder is auto-generated by Android Studio, so it doesn't seem necessary to check-in

Keenuts · 2023-04-07T09:29:25Z

Added a few issues to keep track of the missing work. Assigned some to you, some left for grabs. Feel free to update assignees.

shanminchao force-pushed the android_cleanup branch 6 times, most recently from 5178bdc to 76165d1 Compare March 8, 2023 16:59

chaoticbob reviewed Mar 8, 2023

View reviewed changes

shanminchao force-pushed the android_cleanup branch from 76165d1 to 5c0408b Compare March 10, 2023 17:36

shanminchao requested a review from vettoreldaniele March 10, 2023 17:49

wangra-google reviewed Mar 17, 2023

View reviewed changes

android/app/src/main/java/com/example/bigwheels/MainActivity.java Show resolved Hide resolved

docs/building_and_running.md Show resolved Hide resolved

Keenuts reviewed Mar 21, 2023

View reviewed changes

shanminchao force-pushed the android_cleanup branch from febd635 to d979ab5 Compare March 22, 2023 23:35

wangra-google reviewed Mar 23, 2023

View reviewed changes

shanminchao force-pushed the android_cleanup branch 2 times, most recently from eba66ea to 5303032 Compare April 4, 2023 21:56

Keenuts reviewed Apr 5, 2023

View reviewed changes

shanminchao force-pushed the android_cleanup branch from 5303032 to 34f58f8 Compare April 6, 2023 17:28

Shan-Min Chao and others added 5 commits April 6, 2023 10:38

Android support for BigWheels

bc2f75d

Remove unnecessary .idea folder

e0f6fcc

The folder is auto-generated by Android Studio, so it doesn't seem necessary to check-in

Lower SDK requirement to allow running on Quest2

d4f1d86

Change from com.example.bigwheels to com.google.bigwheels

b9dc852

Make the relative cmake directories a little more explicit

18b980f

shanminchao force-pushed the android_cleanup branch from 34f58f8 to 18b980f Compare April 6, 2023 17:38

Keenuts approved these changes Apr 7, 2023

View reviewed changes

This was referenced Apr 7, 2023

Cleanup root folder from optional Android files #75

Closed

Enable cpu_feature for Android #76

Open

android: fix fishtornado on Android (pixel 6) #81

Open

Keenuts merged commit 9d7294b into google:main Apr 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Android support for BigWheels #46

Android support for BigWheels #46

shanminchao commented Mar 7, 2023 •

edited

Loading

chaoticbob left a comment

chaoticbob Mar 8, 2023

shanminchao Mar 10, 2023

shanminchao commented Mar 10, 2023

Keenuts Mar 21, 2023

shanminchao Mar 22, 2023

Keenuts Mar 21, 2023

shanminchao Mar 23, 2023 •

edited

Loading

chaoticbob Mar 24, 2023

shanminchao Mar 24, 2023 •

edited

Loading

Keenuts Mar 21, 2023

shanminchao Mar 22, 2023

Keenuts Mar 22, 2023

shanminchao Mar 22, 2023

Keenuts Mar 21, 2023

shanminchao Mar 24, 2023

shanminchao Apr 4, 2023

Keenuts Mar 21, 2023

shanminchao Mar 24, 2023

Keenuts Mar 21, 2023

shanminchao Mar 22, 2023

shanminchao Mar 22, 2023 •

edited

Loading

Keenuts Mar 21, 2023

shanminchao Mar 22, 2023

wangra-google Mar 23, 2023

shanminchao Apr 4, 2023

shanminchao Apr 4, 2023

Keenuts left a comment

Keenuts Apr 5, 2023

shanminchao Apr 6, 2023

shanminchao Apr 6, 2023

Keenuts commented Apr 7, 2023


		project("bigwheels")

		add_subdirectory("../../../../.." "../../../../../app/build")

Android support for BigWheels #46

Android support for BigWheels #46

Conversation

shanminchao commented Mar 7, 2023 • edited Loading

chaoticbob left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shanminchao commented Mar 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shanminchao Mar 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shanminchao Mar 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shanminchao Mar 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Keenuts left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Keenuts commented Apr 7, 2023

shanminchao commented Mar 7, 2023 •

edited

Loading

shanminchao Mar 23, 2023 •

edited

Loading

shanminchao Mar 24, 2023 •

edited

Loading

shanminchao Mar 22, 2023 •

edited

Loading