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.7k stars 3.33k forks source link

setAttributes() invalidates references to earlier canvases #5902

Open davepagurek opened 1 year ago

davepagurek commented 1 year ago

Most appropriate sub-area of p5.js?

p5.js version

1.5.0

Web browser and version

107.0.1

Operating System

macOS 12.5.1

Steps to reproduce this

Calling setAttributes in WebGL mode might end up recreating the underlying canvas and renderer object.

Say you're trying to position the element, e.g. like this:

let canvas = createCanvas(100, 100, sketch.WEBGL);
// ...
canvas.position(0,0);

Unfortunately, this stops working after a setAttributes call, because after that call, canvas !== _renderer. Currently, to fix it, one would have to do:

createCanvas(100, 100, sketch.WEBGL);
setAttributes({ alpha: true });
let canvas = _renderer;
// ...
canvas.position(0,0);
Ucodia commented 1 year ago

Was able to reproduce the issue. When setAttributes() is called, the canvas context is reset, which recreates the renderer, thus the initial reference returned by createCanvas() points to the old renderer.

See code:

Calling new p5.RendererGL() is the core of the issue here. Now I don't know how this can be fixed in the library as I don't have experience with WebGL. What I know is that attributes have to be passed to canvas.getContext(), meaning that any change to these attributes requires a recreation of the context, this is a limitation of the HTMLCanvasElement API.

In the meantime, the safest method is to always refer to the renderer from its internal pointer this._renderer rather than the instance returned from createCanvas().

davepagurek commented 1 year ago

A fix might require a slight redesign of what gets returned by createCanvas. Maybe it could return a thin wrapper over p5.Renderer, so that when we make a new renderer, we can also update the canvas object to have the new renderer? When createCanvas returns the renderer directly, it's a little hard to replace with a totally new renderer.

Artimus100 commented 10 months ago

hello @davepagurek this seems interesting too ! Could you explain this or maybe elaborate on what needs to be done here lets see if i can be of any help!

-_- Thank You

Artimus100 commented 10 months ago

@davepagurek Should i start working on this?

davepagurek commented 10 months ago

@Artimus100 feel free to start looking into it, but this one probably could use some discussion here in the issue once you have an idea of how to solve it before making a PR.

The problem under the hood is that the canvas created by createCanvas is a p5.RendererGL. Changing the attributes of a canvas requires us to recreate the canvas. That means that the return value of the original createCanvas call will no longer refer to the real canvas after calling setAttributes because we've made a new one and set this._renderer to the new value.

I mentioned in a comment that one way to address the issue is, rather than directly returning the renderer from createCanvas, we instead return an object that keeps a reference to the p5 instance, and when you call e.g. .position(x, y) on it, it internally calls p5Instance._renderer.position(x, y), since the _renderer property will point to the up-to-date canvas. Maybe this could be done via a Proxy object? There are also other ways to implement it too, and potentially other designs that work differently. I think the next step would be to look into what it would take to implement an idea and propose it so we can think about whether the design would fully solve the problem.

Forchapeatl commented 3 months ago

@Artimus100 , @davepagurek can I take this up ? I would love to look into this.

davepagurek commented 3 months ago

Thanks @Forchapeatl! Let me know if you have any ideas on approaches to take here.