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

Add dispose methods #123

Closed

Conversation

CodyJasonBennett
Copy link
Contributor

@CodyJasonBennett CodyJasonBennett commented Jan 5, 2022

Adds disposal methods, as described in #73. I've renamed geometry and program's remove method to dispose. Classes will only dispose of their internals, and user-defined parameters are left unmanaged for user-land control.

@CodyJasonBennett CodyJasonBennett changed the title BREAKING: add dispose method BREAKING: add dispose methods Jan 5, 2022
Comment on lines +114 to +116
this.mesh.program.dispose();
this.mesh.geometry.dispose();
delete this.mesh;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should Mesh have its own disposal method where it will dispose of its program and geometry? My concern would be with shared programs/geometry and whether it's a good idea to let the user choose -- and thus open up to renderer or transform-level disposal.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks again for your work!
This is a really good question - I share your concern for shared elements, however I suppose that's just something the developer has to control. Perhaps if there's a way to flag the geometries/programs to ignore disposal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I'll only be eager to dispose of things internal to a class. Leaving the rest to the user can be annoying, but it only enables advanced usage.

@CodyJasonBennett CodyJasonBennett changed the title BREAKING: add dispose methods feat!: add dispose methods Jul 16, 2022
@CodyJasonBennett CodyJasonBennett changed the title feat!: add dispose methods Add dispose methods Jul 16, 2022
@CodyJasonBennett
Copy link
Contributor Author

Closing as stale. Noting #123 (comment), I'll try handling this in user-land with something like:

function dispose(gl: WebGL2RenderingContext, object: unknown): void {
    if (object === null || (typeof object !== 'object' && typeof object !== 'function')) return;

    switch (object.constructor) {
        case WebGLBuffer:
            gl.deleteBuffer(object);
            break;
        case WebGLFramebuffer:
            gl.deleteFramebuffer(object);
            break;
        case WebGLProgram:
            gl.deleteProgram(object);
            break;
        case WebGLQuery:
            gl.deleteQuery(object);
            break;
        case WebGLRenderbuffer:
            gl.deleteRenderbuffer(object);
            break;
        case WebGLSampler:
            gl.deleteSampler(object);
            break;
        case WebGLShader:
            gl.deleteShader(object);
            break;
        case WebGLSync:
            gl.deleteSync(object);
            break;
        case WebGLTexture:
            gl.deleteTexture(object);
            break;
        case WebGLTransformFeedback:
            gl.deleteTransformFeedback(object);
            break;
        case WebGLVertexArrayObject:
            gl.deleteVertexArray(object);
            break;
        default: {
            // Shallow dispose of known properties
            for (const key in object) dispose(gl, object[key]);
        }
    }
}

@CodyJasonBennett CodyJasonBennett deleted the feat/add-dispose branch May 3, 2024 19:37
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.

2 participants