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

Trouble with multiple using of shape created with beginShape() #5811

Open AlehAbakshonak opened 2 years ago

AlehAbakshonak commented 2 years ago

Most appropriate sub-area of p5.js?

p5.js version

1.4.2

Web browser and version

105.0.5195.127

Operating System

Windows 10 19043.1826

Steps to reproduce this

I came across a non-trivial situation.

A bit of context: in my last project I was using beginShape() and I had the need to draw each line to the main layer and to the SVG layer. I tried it like this:

createCanvas(100, 100);
let svg = createGraphics(100, 100);

beginShape();
svg.beginShape();
...
vertex(x1, y1);
curveVertex(x2, y2);
vertex(x3, y3);
svg.vertex(x4, y4);
svg.curveVertex(x5, y5);
svg.vertex(x6, y6);
...
endShape();
svg.endShape();

But it turned out that the shape system ignored launching from under the element. All my vertexes were dumped into one shape, i.e. every vertex was duplicated. Which broke all my curves.

However, I discovered another funny thing:

createCanvas(100, 100);
let svg = createGraphics(100, 100);

beginShape();
...
vertex(x1, y1);
curveVertex(x2, y2);
vertex(x3, y3);
...
endShape();
svg.endShape();

This construction works and the shape, once created, can be written into any number of canvases by calling endShape from under an element.

BUT. It turns out that calling endShape will reset parameters of the shape. I.e. it loses information about whether it is a curve or a set of straight lines. This makes no sense at all, because all these parameters are re-filled when the next beginShape is called.

Here are two sketches to demonstrate. Standard p5 library: link A library that has the zeroing to default removed: link

My suggestion: allow the created shape to be used multiple times without such complications OR allow to store created shape in a variable for later use.

welcome[bot] commented 2 years ago

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!

limzykenneth commented 2 years ago

But it turned out that the shape system ignored launching from under the element. All my vertexes were dumped into one shape, i.e. every vertex was duplicated. Which broke all my curves.

The reason this is happening is because calls to beginShape()/endShape()/vertex() etc, monitor the state of the drawing on an instance basis, ie per sketch and not per canvas/graphic. It is somewhat unusual to call beginShape() on one canvas and beginShape() on another canvas before ending the first one, would it not be possible to separate them out like the following?

beginShape();
vertex(x1, y1);
...
endShape();

svg.beginShape();
svg.vertex(x1, y1);
...
svg.endShape();

We can possibly look into changing the behaviour described above by making state tracking per canvas/graphic but it would potentially be helpful to know why separating out the two sets of beginShape()/endShape() calls may not work?

For the second instance described in your issue (including the one share in the link), that is not a documented or supported use case which means that it is essentially undefined behaviour, in other words anything can happen there and I don't think we'll be addressing that directly. (Although it may be solved with the same fix as above, it is still not a suggested usage)

AlehAbakshonak commented 2 years ago

It is somewhat unusual to call beginShape() on one canvas and beginShape() on another canvas before ending the first one, would it not be possible to separate them out like the following?

Well, the reason is, currently i'm calculating a bunch of vertexes one by one in for cycle and pushing each vertex directly into shape without storing it in array. I can possibly store all vertexes and then pass them into a function, which also will include another parameter with canvas/graphic linked. This function will draw vertexes from this array onto provided canvas.

I know that this is a legit solution, it's just much longer and took more memory, operation calls and unnecessary data storing.

We can possibly look into changing the behaviour described above by making state tracking per canvas/graphic

I think it is great solution, because for me instance basis of beginShape was totally unpredicted behaviour, because it's throws no warnings when called from particular canvas, but still, just ignores this canvas. More of it, it appears that endShape works on canvas basis, and call from particular canvas draws the stored shape on this canvas, so those two paired methods have different working approaches.

Although it may be solved with the same fix as above, it is still not a suggested usage

Sure, totally agreed. It's just... allowed :D And i catched it by chance, randomly. I just had two beginShapes and two endShapes and i was thinking that i'm making two shapes in parallel, but in fact it was a shape with each point duplicated and both endShapes was quietly drawing this corrupted shape onto canvases they was called from.

Idk if i need to close the issue or not, so tell me plz if needed.

limzykenneth commented 2 years ago

We can look into changing vertex calls to per instance, could take awhile unless someone would like to take a jab at this?

I know that this is a legit solution, it's just much longer and took more memory, operation calls and unnecessary data storing.

It is a much better solution in my opinion as it clearly separate out different logical concerns in your code (the vertex generation code don't have to worry about the drawing code, vice versa), and in the long term can help with maintaining an increasingly complex codebase. The overhead in terms of memory and function calls are negligible if it was there at all as the JS engine can possibly optimize it for you, and minor performance concerns usually shouldn't take higher precedent over maintainability of your code. Also for a bit of future proofing, who knows how many more drawing canvases one may use in the future, or how may p5 instances, or even to generate vertices for use outside of p5, having reasonable encapsulation with additional functions are almost always worth it.