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

chore: Update android #261

Merged

Conversation

robertjcolley
Copy link
Collaborator

@robertjcolley robertjcolley commented Feb 10, 2024

Android build was wildly out of date. This is the culmination of a few days of battling gradle version issues, kotlin version issues, and java version issues. In this commit, viro react compiles with each of the dependencies (renderer, etc.) and creates the bridge properly both in a React Native project and an Expo project.

There's a "hack" in the Expo withViroAndroid.ts file. Expo generates a MainApplication.kt file that doesn't actually add the ViroReact packages to the application.

Todo:

  • Continue fixing arrow functions in some class components
  • Rebuild virocore and debug crash in VROMathInvertMatrix
  • Add code to show a view instead of initializing AR if the device doesn't support AR Core
  • Should I use a library like react-native-builder-bob to re-scaffold the library? Deferring to later date
  • Deploy release
  • Update starter kits
  • Update installation instructions for Android here

Android build was wildly out of date. This is the culmination of a few
days of battling gradle version issues, kotlin version issues, and java
version issues. In this commit, viro react compiles with each of the
dependencies (renderer, etc.) and creates the bridge properly both in a
React Native project and an Expo project.

There's a "hack" in the Expo withViroAndroid.ts file. Expo generates a
MainApplication.kt file that doesn't actually add the ViroReact packages
to the application.

Todo:
- Continue fixing arrow functions in some class components
- Rebuild virocore and debug crash in VROMathInvertMatrix
- Deploy release
- Add code to show a view instead of initializing AR if the device
  doesn't support AR Core
@robertjcolley
Copy link
Collaborator Author

robertjcolley commented Feb 10, 2024

Currently, this branch is crashing for me on the viro-test-bed 3D demo at:

image

@Avtrkrb
Copy link

Avtrkrb commented Feb 13, 2024

Currently, this branch is crashing for me on the viro-test-bed 3D demo at:

image

@robertjcolley Kotlin plugins need to be v1.8.x or higher v1.6.x is leading to issues. I'm getting similar issues while building my app. Here are the changes I did:

android/build.gradle

dependencies {
        classpath 'com.android.tools.build:gradle:8.2.0'
        classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:1.8.21"
        // NOTE: Do not place your application dependencies here; they belong
        // in the individual module build.gradle files
    }

android/viro_bridge/build.gradle

dependencies {
    androidTestImplementation('androidx.test.espresso:espresso-core:3.1.0', {
        exclude group: 'com.android.support', module: 'support-annotations'
    })
    implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.8.21'
    implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.21"
    implementation 'org.jetbrains.kotlin:kotlin-stdlib:1.8.21'
    implementation 'org.jetbrains.kotlin:kotlin-stdlib-common:1.8.21'

    implementation 'androidx.appcompat:appcompat:1.0.0'
    testImplementation 'junit:junit:4.12'
    implementation 'com.facebook.react:react-native:+'
    implementation project(':arcore_client')
    implementation project(':gvr_common')
    implementation project(':viro_renderer')
    implementation 'com.google.android.exoplayer:exoplayer:2.19.1'
    implementation 'com.google.protobuf.nano:protobuf-javanano:3.0.0-alpha-7'

    implementation 'androidx.localbroadcastmanager:localbroadcastmanager:1.0.0'
    implementation "com.facebook.fresco:fresco:3.1.3"
    implementation "com.facebook.fresco:imagepipeline:3.1.3"
    implementation "com.facebook.fresco:imagepipeline-okhttp3:3.1.3"
    implementation 'com.facebook.fresco:imagepipeline-native:3.1.3'
    implementation 'com.facebook.fresco:memory-type-ashmem:3.1.3'
    implementation "com.facebook.fresco:ui-common:3.1.3"
    implementation "com.facebook.fresco:middleware:3.1.3"
    implementation "com.facebook.fresco:animated-gif:3.1.3"
    implementation 'com.facebook.soloader:nativeloader:0.10.5'
    implementation 'com.facebook.fresco:fbcore:3.1.3'
    implementation 'com.facebook.fresco:drawee:3.1.3'
    implementation 'com.facebook.fresco:memory-type-native:3.1.3'
    implementation 'com.facebook.fresco:memory-type-java:3.1.3'
    implementation 'com.facebook.fresco:nativeimagefilters:3.1.3'
    implementation 'com.facebook.fresco:nativeimagetranscoder:3.1.3'


    implementation fileTree(include: ['*.jar'], dir: 'libs')
}

FYI: I've got these changes in my branch, will open a PR once I'm done testing it out.

@Avtrkrb
Copy link

Avtrkrb commented Feb 14, 2024

@robertjcolley
I've raised a few draft PRs to vircore & viro that should fix a lot of issues for android. It's still WIP though, will tag you once it's ready.

Currently, this branch is crashing for me on the viro-test-bed 3D demo at:

image

My changes should also fix some of these errors.

@robertjcolley robertjcolley changed the title chore: Update android (WIP) chore: Update android Feb 23, 2024
require('expo') in a react native project caused this issue, but for
some reason the error message was about VIRO_VERSION, which was working
correctly.
@robertjcolley robertjcolley merged commit ae348c2 into main Feb 24, 2024
1 check passed
@robertjcolley robertjcolley deleted the 260-fix-android-building-with-java-17-kotlin-and-expo branch February 24, 2024 00:11
Copy link

@illlama illlama left a comment

Choose a reason for hiding this comment

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

Thanks for updates!
But I have a question about console.log.
It takes huge number of lines in my terminal and it occurs overflow too.
Is these things are necessary?


console.log("[ViroScene].render");
console.log("[ViroScene].render", this);
console.log("[ViroScene].render", this.props);
Copy link

Choose a reason for hiding this comment

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

@robertjcolley is these console.log are necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope! I'll remove them in the next release I'm doing this week. Sorry about that! There are a few other places I think I put them.

@robertjcolley robertjcolley mentioned this pull request Mar 5, 2024
3 tasks
robertjcolley added a commit that referenced this pull request Mar 6, 2024
removes console.log statements debugging a previous issue that were left in during #261.

resolves #275
robertjcolley added a commit that referenced this pull request Mar 6, 2024
removes console.log statements debugging a previous issue that were left in during #261.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cannot read property 'VIRO_VERSION' of undefined Fix android building with Java 17, Kotlin, and Expo
3 participants