Skip to content

ARCore 1.26.0, Filament 1.12.0 Update. Refactoring #143

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

Merged
merged 17 commits into from
Aug 20, 2021

Conversation

RGregat
Copy link
Contributor

@RGregat RGregat commented Aug 18, 2021

Changed

  • Updated build.gradle parameters
    • compileSdkVersion 30 -> 31
    • targetSdkVersion 30 -> 31
    • sourceCompatibility VERSION_1_8 -> VERSION_11
    • targetCompatibility VERSION_1_8 -> VERSION_11
  • Updated dependencies
    • filament 1.10.7 -> 1.12.0
    • arcore 1.25.0 -> 1.26.0
    • appcompat 1.2.0 -> 1.3.1
  • Compiled filament materials for version 1.12.0
  • Refactored a few big functions and splitted them into smaller parts
  • Updated and tested all samples

Fixed

  • Fixed a few deprecation warnings
  • In CameraStream the function initializeTexture was called twice. This is now fixed

Added

  • Added in BaseArFragment a new function to close a session
public void closeSession() {
    if(arSceneView.getSession() != null)
        arSceneView.getSession().close();
        arSceneView.setSession(null);
    }
}

r.gregat added 10 commits August 16, 2021 10:49
* Splitted up the `render` function from the Sceneform Renderer class
into smaller parts.
* The Filament renderer `begingFrame` function call is now before
anything ArCore related is updated.
* Swapped order of the `doUpdate` and `doRender` function call in
SceneView.
* Added `Choreographer.getInstance().removeFrameCallback(this);` to the
destroy function call in SceneView.
* Splitted up the `onBeginFrame`function in ArSceneView a little bit.
* Added to BaseArFragment a new function to close the session and set
the variable to null. `
* compileSdkVersion 30 -> 31
* targetSdkVersion 30 -> 31
* sourceCompatibility VERSION_1_8 -> VERSION_11
* targetCompatibility VERSION_1_8 -> VERSION_11
* filament 1.10.7 -> 1.12.0
* arcore 1.25.0 -> 1.26.0
* appcompat 1.2.0 -> 1.3.1
@ThomasGorisse ThomasGorisse self-requested a review August 19, 2021 12:32
@ThomasGorisse ThomasGorisse self-assigned this Aug 19, 2021
@ThomasGorisse ThomasGorisse added the enhancement New feature or request label Aug 19, 2021
Copy link
Collaborator

@ThomasGorisse ThomasGorisse 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 this PR @RGregat !!!
I'm back from holidays and currently reviewing it.
Please don't commit right now since I'm also bringing some cleanup here.
Can you just answer questions first?

@@ -26,7 +26,8 @@
android:id="@+id/transparentSceneView"
android:layout_width="match_parent"
android:layout_height="0dp"
android:layout_weight="1" />
android:layout_weight="1"
android:background="#FF0000FF"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does our dragon transparency issue comes from the background?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I added the blue color in the last version to see if the issue comes from that. I changed it back to transparent. By the way, everything ist rendering now again, maybe because of the Filament update in this version.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you simply check if the DragonAttenuation.glb (transparency one) is displayed correctly on your devices?
It's not the case on my Pixel 4a.

ThomasGorisse and others added 7 commits August 20, 2021 01:18
…t everything is well called

- Add setters for ArFragment and ArSceneview Session and Session Config
- Improve session.getConfig() calls
- Review the SceneView/ArSceneView resume/pause order
- Remove the ensure updateMode == Config.UpdateMode.LATEST_CAMERA_IMAGE
- Cleanup
- Improvements
* Replaced `requestPermissions` and `onRequestPermissionResult` with the
new way by creating a `ActivityResultLauncher` object. The result of the
request is handled via a callback function.
… transparent again.

* Both dragons and backdrops are rendered.
@RGregat
Copy link
Contributor Author

RGregat commented Aug 20, 2021

I find it a little bit difficult to profile and analyze the memory footprint of Sceneform on my laptop. The Laptop is not strong enough :).
Here is a visualization of a captured java/kotlin object allocation, when the gltf sample app was running.

image

As you can see the allocation rate rises continuously until the GC is cleaning unused objects. From the result I can't tell if the Trackable object allocation every frame has a huge impact. But the PlaneRenderer has still a huge impact.

@RGregat
Copy link
Contributor Author

RGregat commented Aug 20, 2021

Ok found the updateTrackable calls

image

looks not so bad, especially if you compare it with the PlaneRenderer

@ThomasGorisse
Copy link
Collaborator

You are right, the updateTrackable doesn't consume so much but the problem was/is that it generates leaks since the created Trackable array elements are never cleaned and each call generates new ones. That made/makes a good part of the memory consumption increasing from what I think.

For the PlaneRender, we should check why normalToTangent is taking so much.

@RGregat
Copy link
Contributor Author

RGregat commented Aug 20, 2021

You are right, the updateTrackable doesn't consume so much but the problem was/is that it generates leaks since the created Trackable array elements are never cleaned and each call generates new ones. That made/makes a good part of the memory consumption increasing from what I think.

For the PlaneRender, we should check why normalToTangent is taking so much.

Maybe this is something for a separate Branch/PR?

@ThomasGorisse ThomasGorisse merged commit 0c59e5e into master Aug 20, 2021
@ThomasGorisse ThomasGorisse deleted the refactor/minor_cleanup branch August 20, 2021 12:50
@ThomasGorisse ThomasGorisse linked an issue Aug 26, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Screen flickering during render - Augmented Image Demo
2 participants