processing / p5.js

p5.js is a client-side JS platform that empowers artists, designers, students, and anyone to learn to code and express themselves creatively on the web. It is based on the core principles of Processing. http://twitter.com/p5xjs —
http://p5js.org/
GNU Lesser General Public License v2.1
21.08k stars 3.22k forks source link

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

Closed nickmcintyre closed 1 month ago

nickmcintyre commented 1 month ago

Most appropriate sub-area of p5.js?

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 commented 1 month ago

Similar to https://github.com/processing/p5.js/issues/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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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.