google-ar / arcore-android-sdk

ARCore SDK for Android Studio
https://developers.google.com/ar
Other
4.98k stars 1.22k forks source link

ArSession_getAllTrackables does not clear input list #625

Closed juj closed 5 years ago

juj commented 6 years ago

To continuously print out the number of currently tracked planes in an application, one can write

    ArTrackableList *l = 0;
    ArTrackableList_create(arSession, &l);
    ArSession_getAllTrackables(arSession, AR_TRACKABLE_PLANE, l);
    int n = 0;
    ArTrackableList_getSize(arSession, l, &n);
    LOGI("n: %d", n);
    ArTrackableList_destroy(l);

in a frame update loop, which works well. This starts off printing n:0, then n:1 as a plane is picked up, and then n:2 after a second plane is found and so on, like expected. This code pattern is what the hello_ar_c sample does.

However, reading documentation for ArSession_getAllTrackables() at https://developers.google.com/ar/reference/c/group/session#group__session_1ga40d6d589ca361ceba8a1c39e1404b8ad, it says the following about out_trackable_list:

out_trackable_list | The list to fill. This list must have already been allocated with ArTrackableList_create(). If previously used, the list will first be cleared.
-- | --

which suggests that the above kind of code pattern that hello_ar_c sample uses is a bad practice, and one should essentially create ArTrackableLists and ArAnchorLists only once and reuse them per frame, instead of creating and destroying them each frame. The above code thus transforms to

    static ArTrackableList *l = 0;
    if (!l) ArTrackableList_create(arSession, &l);
    ArSession_getAllTrackables(arSession, AR_TRACKABLE_PLANE, l);
    int n = 0;
    ArTrackableList_getSize(arSession, l, &n);
    LOGI("n: %d", n);
    // ArTrackableList_destroy(l);

which should behave the same, and be more performant and a better practice? However, after doing this transformation, the print n: <number> will go wild, and after a few seconds of running, the list will grow to thousands of elements. It looks like the implementation of ArSession_getAllTrackables() does not clear the existing list of planes, contrary to the documentation suggesting it should?

I have not been able to test, but I wonder if the function ArSession_getAllAnchors() has the same bug, it at least has the same documentation "If previously used, the list will first be cleared." (https://developers.google.com/ar/reference/c/group/session#arsession_getallanchors)

juj commented 6 years ago

Tested on Huawei Nexus 6P (Android 8.1.0, API 27) with

I native  : version_check.cc:23 ARCore Version: APK version:1.5.180910096
I native  : version_check.cc:24 ARCore Version: emulated SDK version:1.5.180815000
I third_party/arcore/ar/core/android/sdk/session_create.cc: ARCore Version: SDK build name: 1.5
inio commented 6 years ago

Thank you for this report, we're attempting to reproduce this behavior. Given the thoroughness of your report though I suspect this is a real bug.

inio commented 6 years ago

We've been able to reproduce this issue and should have it fixed in the next major release (1.6). Thanks again for pointing this out.

inio commented 5 years ago

This is fixed in ARCore 1.6 which was released yesterday.