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

BufferAttribute: Introduce dispose(). #17063

Closed
wants to merge 3 commits into from
Closed

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Jul 21, 2019

This PR replaces #15308. Fixed #15261.

A first step to make BufferAttribute fixed sized.

The new policy should be: If you need to resize buffer data, you have to create a new instance of BufferAttribute.

@WestLangley
Copy link
Collaborator

/bump overlooked?

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Oct 26, 2019

Resolved the merge conflicts.

@WestLangley WestLangley added this to the r110 milestone Oct 26, 2019
@mrdoob mrdoob modified the milestones: r110, r111 Oct 30, 2019
@0b5vr
Copy link
Collaborator

0b5vr commented Nov 21, 2019

I think I need this right now!

@mrdoob mrdoob modified the milestones: r111, r112 Nov 27, 2019
@Mugen87
Copy link
Collaborator Author

Mugen87 commented Dec 18, 2019

@mrdoob Any chances to merge the PR? If not, do you mind sharing your concerns about this change? Maybe we can find a different solution if this one is not satisfying?

@mrdoob mrdoob modified the milestones: r112, r113 Dec 23, 2019
@mrdoob mrdoob modified the milestones: r113, r114 Jan 29, 2020
@mrdoob mrdoob modified the milestones: r114, r115 Feb 29, 2020
@mrdoob mrdoob modified the milestones: r115, r116 Mar 25, 2020
@mrdoob mrdoob modified the milestones: r116, r117 Apr 30, 2020
@mrdoob mrdoob modified the milestones: r117, r118 May 27, 2020
@mrdoob mrdoob modified the milestones: r118, r119 Jun 24, 2020
@mrdoob mrdoob removed this from the r119 milestone Jul 29, 2020
@mrdoob mrdoob added this to the r120 milestone Jul 29, 2020
@mrdoob mrdoob modified the milestones: r120, r121 Aug 25, 2020
@mrdoob mrdoob modified the milestones: r121, r122 Sep 29, 2020
@mrdoob mrdoob modified the milestones: r122, r123 Oct 28, 2020
@Mugen87 Mugen87 closed this Nov 6, 2020
@Mugen87 Mugen87 removed this from the r123 milestone Nov 6, 2020
@OndrejSpanel
Copy link
Contributor

Note: there is currently no method to properly dispose instanceMatrix and instanceColor in the InstancedMesh.

I think this is really a glaring hole in the resource management. A proper cleanup should always be possible. Someone may have a scene with a lot of dynamic instancing going on (I am now experimenting with exactly such scene) and the missing cleanup causes slowing growing allocation of WebGL resources in such case.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Dec 3, 2020

After a deeper analysis I do not vote to introduce BufferAttribute.dispose() anymore since this easily breaks code in WebGLBindingStates. Geometry data are managed as a single entity. Calling dispose() on attribute level and thus deleting the WebGL buffer would not automatically update the respective VAO.

InstancedMesh.dispose() has to be implemented without BufferAttribute.dispose().

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.

add dispose() to BufferAttribute
5 participants