mtrofin / wasm-android-demo Goto Github PK

The word "sample" is confusing/redundant

Do we have duplicate teapot vertex definitions, e.g. apple-to-apple.js and then also within teapots.js? Do we need them all?

Also, please ensure build artifacts are .git ignored.

Remove all commented code lines/regions (e.g. 23, 39->down)

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/webGL/webgl-demo.js#L52

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/webGL/webgl-demo.js#L52

remove commented lines

https://github.com/mtrofin/wasm-android-demo/tree/master/app/src/main/jni

There is support for pinching, dragging, etc. Do we need that?

Remove comments

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/common/vecmath.h#L202

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/common/vecmath.h#L400

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/common/vecmath.h#L805

remove comment

Do we have duplicate teapot vertex definitions, e.g. apple-to-apple.js and then also within teapots.js? Do we need them all?

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/jni/MoreTeapotsNativeActivity.cpp#L64

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/jni/MoreTeapotsNativeActivity.cpp#L69

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/jni/MoreTeapotsNativeActivity.cpp#L165

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/jni/MoreTeapotsNativeActivity.cpp#L177

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/jni/MoreTeapotsNativeActivity.cpp#L198

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/jni/MoreTeapotsNativeActivity.cpp#L198

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/jni/MoreTeapotsNativeActivity.cpp#L352

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/jni/MoreTeapotsNativeActivity.cpp#L421

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/common/MoreTeapotsRenderer.cpp#L33

Instead of the many #if checks, consider an alternative for ensuring exactly only one option is enabled. You could sum up the presence of each option, and ensure it adds up to 1, for instance.

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/common/shader.h#L27

Does having this namespace make sense anymore?

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/common/gl3stub.h#L69

doesn't this belong in an android-specific file?

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/common/MoreTeapotsRenderer.h#L114

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/common/MoreTeapotsRenderer.h#L96

remove comment

instead of having 2 copies of the vertex definitions for teapots (one in the .inl file under common, and one in apple-to-apple.js), could we have a file in common with the list of values? Then, the native side could #include that file, and the javascript side could be either generated using the C-preprocessor, or you could load and parse the list at the web app startup time, and form the vertex arrays that way.

remove comments:
https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/java/com/sample/moreteapots/MoreTeapotsApplication.java#L38

This refers to the folder in the root of the repository. Is this still necessary after the recent refactoring?

Remove comments

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/wasm/emsMain.cpp#L66

remove comment

also, why "emsMain.cpp" - could we rename it to "MoreTeapots-wasm.cpp" or something like that?

Hello, the README says

The project has two goals. The first one is to evaluate the engineering cost of porting an Android application to WASM. The other one is trying to figure out the performance overhead of WASM, compared to native Androis apps.

Curious to know what were the findings.

Is this an external dependency, or is it built? Can we remove it from the repo and obtain it differently?

rename "apple-to-apple.js" to teapot.js or teapot-vertices.js

as the title suggests - that way, all android stuff is under one folder. The fact that it's split in c/c++ and java is relevant to android, not to the overall architecture of the cross-targeting application.

In #28 gradle-wrapper.jar was inexplicably removed from the workspace. This breaks step 2 of the Android build; the "gradlew" script puts it on the CLASSPATH.

Checking out 4efa6c6 allows the build to proceed a bit further, but there are hardcoded paths in some of the build files (local.properties, in particular).

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/common/teapot.inl#L23

https://github.com/mtrofin/wasm-android-demo/blob/master/app/src/main/common/teapot.inl#L1944

remove comments

What's this file? is it autogen-ed? if yes, please remove it.

remove this folder and contents:
https://github.com/mtrofin/wasm-android-demo/tree/master/app/src/main/java/com/sample/helper