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

p5.Graphics.remove() doesn't remove some resources #7049

Closed
1 of 17 tasks
nickmcintyre opened this issue May 16, 2024 · 6 comments · Fixed by #7060 · May be fixed by yamcodes/agario-clone#15 or yamcodes/agario-clone#16
Closed
1 of 17 tasks

p5.Graphics.remove() doesn't remove some resources #7049

nickmcintyre opened this issue May 16, 2024 · 6 comments · Fixed by #7060 · May be fixed by yamcodes/agario-clone#15 or yamcodes/agario-clone#16

Comments

@nickmcintyre
Copy link
Member

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Build process
  • Unit testing
  • Internationalization
  • Friendly errors
  • Other (specify if possible)

p5.js version

1.9.3

Web browser and version

Chrome 124.0.6367.208

Operating system

macOS 14.4.1

Steps to reproduce this

Steps:

  1. Call pg.remove().
  2. Call print(pg) or use pg elsewhere.

Snippet:

function doubleClicked() {
  pg.remove();

  // Should this be empty-ish?
  print(pg);
}

Here's the sketch for reference.

This is similar to #7048 but the entire <canvas> still seems to be available after calling pg.remove(). Does pg have to be nullified in order for its resources to be garbage collected? It seems like the only other references are cleared here.

Maybe too in-the-weeds, but if there's another reference somewhere in the sketch, then the resources are never fully freed.

@davepagurek
Copy link
Contributor

Similar to #7048, it will get garbage collected if you remove all lingering references to it. .remove() removes all internal references (it will no longer appear in the page's DOM), and then the user needs to remove their own reference to pg, so I think this is currently working as designed.

That said, we could remove the internal references to the HTML element and its context if we want to ensure the heavier bits can get GCed even if the user never removes their reference.

@nickmcintyre
Copy link
Member Author

Tidying up the heavier bits automatically would be nice! It'd also mirror p5.Framebuffer.remove() a little better. Just updated the reference to reflect best practices.

@davepagurek
Copy link
Contributor

Sounds good! I think that would amount to setting elt, canvas, and _renderer to undefined in the remove method. If anyone's interested in adding this, I think it would be a pretty straightforward change!

@JordanSucher
Copy link
Contributor

JordanSucher commented May 18, 2024

I took a stab at this - however, when I set Graphics.canvas to undefined the image() call in the draw function throws an error.

As far as I understand, that image() call references a global renderer on the p5 instance (created by the createCanvas() call in setup I believe) and makes a renderer.image() call, which then errors out because of the undefined canvas

Thoughts?

the additions to the remove fn:

remove() {
   ...
    this._renderer = null;
    this.canvas = null;
    this.elt = null;
  }

@Akhilbisht798
Copy link
Contributor

I have tried this

remove() {
   ...
    this._renderer = null;
    this.canvas = null;
    this.elt = null;
  }

works fine circle got removed but i am getting a typeError on doubleClick
Uncaught TypeError: u is null

So i tried this with undefined also but still getting the same thing.
Uncaught TypeError: u is undefined

@iambiancafonseca
Copy link
Contributor

Hello, I'm keenly interested in this issue and would like to offer my assistance. I'm considering giving it a try with a boolean approach.

iambiancafonseca added a commit to iambiancafonseca/p5.js that referenced this issue May 19, 2024
…ted resources

The proposed fix involves modifying the p5.Graphics.remove() method by setting this._renderer, this.canvas, and this.elt to undefined. Additionally, a boolean flag doubleClickedBool is introduced to prevent the execution of the draw() function after a double-click event, ensuring that the graphics object is not displayed again after removal. This provides a smoother user experience by preventing the graphics object from reappearing unexpectedly.
iambiancafonseca added a commit to iambiancafonseca/p5.js that referenced this issue May 19, 2024
iambiancafonseca added a commit to iambiancafonseca/p5.js that referenced this issue May 19, 2024
…ted resources

The proposed fix involves modifying the p5.Graphics.remove() method by
setting this._renderer, this.canvas, and this.elt to undefined.
Additionally, a boolean flag doubleClickedBool is introduced to prevent
the execution of the draw() function after a double-click event,
ensuring that the graphics object is not displayed again after removal.
This provides a smoother user experience by preventing the graphics
object from reappearing unexpectedly.
iambiancafonseca added a commit to iambiancafonseca/p5.js that referenced this issue May 19, 2024
resources
The proposed fix involves modifying the p5.Graphics.remove() method by
setting this._renderer, this.canvas, and this.elt to undefined.
Additionally, a boolean flag doubleClickedBool is introduced to prevent
the execution of the draw() function after a double-click event
ensuring that the graphics object is not displayed again after removal
This provides a smoother user experience by preventing the graphics
object from reappearing unexpectedly.
iambiancafonseca added a commit to iambiancafonseca/p5.js that referenced this issue May 19, 2024
resources
The proposed fix involves modifying the p5.Graphics.remove() method by
setting this._renderer, this.canvas, and this.elt to undefined.
Additionally, a boolean flag doubleClickedBool is introduced to prevent
the execution of the draw() function after a double-click event
ensuring that the graphics object is not displayed again after removal
This provides a smoother user experience by preventing the graphics
object from reappearing unexpectedly.
iambiancafonseca added a commit to iambiancafonseca/p5.js that referenced this issue May 19, 2024
… resources

The proposed fix involves modifying the p5.Graphics.remove() method by
setting this._renderer, this.canvas, and this.elt to undefined. Additionally, a boolean flag doubleClickedBool is introduced to prevent
the execution of the draw() function after a double-click event
ensuring that the graphics object is not displayed again after removal
This provides a smoother user experience by preventing the graphics
object from reappearing unexpectedly.
iambiancafonseca added a commit to iambiancafonseca/p5.js that referenced this issue May 19, 2024
… resources

The proposed fix involves modifying the p5.Graphics.remove() method by
setting this._renderer, this.canvas, and this.elt to undefined.
Additionally, a boolean flag doubleClickedBool is introduced to prevent
the execution of the draw() function after a double-click event
ensuring that the graphics object is not displayed again after removal
This provides a smoother user experience by preventing the graphics
object from reappearing unexpectedly.
davepagurek added a commit that referenced this issue May 22, 2024
Fix #7049: P5.Graphics.remove() doesn't release all related resources
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment