Confusion between .elt and .canvas

[Zalastax]

@Spongman highlights in #3155 that there is a confusion between the properties .elt and .canvas causing us to write defensive code like so:

p5.js/src/core/p5.Renderer2D.js

Line 112 in 5a46133

cnv = img.canvas || img.elt;

What is the purpose of having both .elt and .canvas? How can it be that the canvas should sometimes come from .elt and sometimes from .canvas?

Searching for .canvas = I get these results:

p5.js/lib/addons/p5.dom.js

Line 2637 in 5a46133

this.canvas = document.createElement('canvas');

p5.js/src/core/p5.Graphics.js

Line 29 in 5a46133

this.canvas = document.createElement('canvas');

p5.js/src/core/p5.Renderer.js

Line 26 in 5a46133

this.canvas = elt;

p5.js/src/image/p5.Image.js

Line 144 in 5a46133

this.canvas = document.createElement('canvas');

p5.js/src/webgl/p5.RendererGL.js

Line 176 in 5a46133

this._pInst.canvas = c;

Searching for .elt = I get these results:

p5.js/src/core/p5.Element.js

Line 44 in 5a46133

this.elt = elt;

p5.js/lib/addons/p5.dom.js

Line 585 in 5a46133

elt = this.elt = arg.elt;

p5.js/lib/addons/p5.dom.js

Line 723 in 5a46133

elt = this.elt = existing_radios.elt;

and a comment about checking if the canvas is associated with the p5 instance

p5.js/lib/addons/p5.dom.js

Lines 1938 to 1939 in 5a46133

	// main canvas associated with p5 instance
	if (this._pInst._curElement.elt === this.elt) {

Some of the places that assign canvas also assign elt afterwards, some do not.
In #3155 @Spongman solves the problem via if (!this.canvas) this.loadPixels();, is that a general approach we should apply elsewhere?
Do we have a place where we can add (or already have?) YUIDocs for .elt and .canvas?

[Spongman]

the solution to the #3155 issue is unrelated to this. p5.MediaElement does actually require both .elt and .canvas properties - the .elt is the video element, and the .canvas is the bitmap cache that supports get & set.

the issue here is that p5.Element defines a .elt property, p5.Graphics, p5.Renderer derive from p5.Element (so inherit the .elt property), but they also define their own .canvas property and much of the code assumes the existence of .canvas, however sometimes those methods are used to act on p5.Image objects that (being p5.Elements) only define .elt.

IMO, (except for p5.MediaElement) those canvas properties should be removed and code referencing them should instead refer to .elt.

ok, it's a little more complicated than that. MediaElement is an Element, but MediaElement pretends to be a Renderer when doing its pixels stuff which means there's a conflict between canvas and elt in MediaElement - they're not the same thing. so doing the above would break MediaElement, again.

[Zalastax]

Thanks for the clarification!

Is this correct?
We have the class p5.Element which is extended and an interface Renderable

interface Renderable {
  canvas: HTMLCanvasElement
}

Some functions accept parameters of type p5.Element | Renderable.
A parameter that is both p5.Element and Renderable will be seen only as a Renderable (cnv = img.canvas || img.elt;).

What is the type for .elt? Any HMLElement but sometimes assumed to be HTMLCanvasElement?
Are there any actions we can and should take?

[Spongman]

it does look a bit like that, yes. although instead of those functions accepting parameters of different types, they're invoked with this being objects of different types. just a syntactic difference.

i'm not sure there's anything short of major architectural reworking that can be done to fix this right now.

but... my pie-in-the-sky idea: instead of p5.Graphics 'borrowing' methods from p5, and p5.Image 'borrowing' methods from p5.Renderer, maybe things could be turned around a little to end up with an actual inheritance chain that looks something like this:

• p5 -> p5.Graphics -> p5.RenderableElement -> p5.Element
• p5.Image -> p5.Graphics
• p5.MediaElement -> p5.RenderableElement
• perhaps p5.Renderer -> p5.RenderableElement ? not sure about this one

the API would mostly stay the same except for the following:

• most 'graphics' methods (line(), stroke(), etc...) would move into p5.Graphics (and be inherited by p5) the reference docs generator would need to updated to support this.
• p5.Graphics would 'lose' methods like min(), millis(), trim() which it really has no business having today.
• p5.Image would 'gain' all the p5.Graphics functionality. createImage() would just essentially be an alias for createGraphics(P2D) (but with its own p5.Image prototype to maintain backwards compat)
• p5.RenderableElement would implement the 2d 'pixels' functions that are the source of this issue, and abstract away fetching the drawingContext (if necessary).
• the this._XXX = (function(){ ... }).bind(this); trickery in p5 would no longer be necessary for hiding stuff from Graphics.