Skip to content

Fix bad memory leak in the GL backend.#3894

Merged
prideout merged 1 commit into
mainfrom
pr/gltfio_memleak
May 5, 2021
Merged

Fix bad memory leak in the GL backend.#3894
prideout merged 1 commit into
mainfrom
pr/gltfio_memleak

Conversation

@prideout
Copy link
Copy Markdown
Contributor

@prideout prideout commented May 4, 2021

This leak was introduced in the following PR on April 7 and was therefore released to the wild in v1.9.20.
#3775

We were creating and allocating low-level VBO handles, but then they got clobbered later when the new BufferObject VBO handles were pushed to the VertexBuffer.

Fixes #3888

This leak was introduced in the following PR on April 7.
#3775

The guilty party has been contacted and properly admonished for his
transgression.

This was tested by adding the following code after applyAnimation in
gltf_viewer.cpp

        static int nframes = 0;
        if (!gpath.empty() && nframes++ > 100) {
            static int count = 0;
            printf("reloading %d\n", count++);
            nframes = 0;
            app.resourceLoader->asyncCancelLoad();
            app.resourceLoader->evictResourceData();
            app.viewer->removeAsset();
            app.assetLoader->destroyAsset(app.asset);
            loadAsset(gpath, app);
            loadResources(gpath, app);
        }
@prideout prideout changed the title Fix horrible memory leak in the GL driver. Fix bad memory leak in the GL backend. May 4, 2021
@prideout prideout merged commit 7a8dd52 into main May 5, 2021
@prideout prideout deleted the pr/gltfio_memleak branch May 5, 2021 00:50
prideout added a commit that referenced this pull request May 5, 2021
This leak was introduced in the following PR on April 7.
#3775

The guilty party has been contacted and properly admonished for his
transgression.

This was tested by adding the following code after applyAnimation in
gltf_viewer.cpp

        static int nframes = 0;
        if (!gpath.empty() && nframes++ > 100) {
            static int count = 0;
            printf("reloading %d\n", count++);
            nframes = 0;
            app.resourceLoader->asyncCancelLoad();
            app.resourceLoader->evictResourceData();
            app.viewer->removeAsset();
            app.assetLoader->destroyAsset(app.asset);
            loadAsset(gpath, app);
            loadResources(gpath, app);
        }
elisemorysc pushed a commit to elisemorysc/filament that referenced this pull request May 5, 2021
This leak was introduced in the following PR on April 7.
google#3775

The guilty party has been contacted and properly admonished for his
transgression.

This was tested by adding the following code after applyAnimation in
gltf_viewer.cpp

        static int nframes = 0;
        if (!gpath.empty() && nframes++ > 100) {
            static int count = 0;
            printf("reloading %d\n", count++);
            nframes = 0;
            app.resourceLoader->asyncCancelLoad();
            app.resourceLoader->evictResourceData();
            app.viewer->removeAsset();
            app.assetLoader->destroyAsset(app.asset);
            loadAsset(gpath, app);
            loadResources(gpath, app);
        }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Release resources allocated with ResourceLoader::loadResources

3 participants