Unmatched push() calls cause progressive performance degradation and browser freeze (no warning)

[Homaid]

Most appropriate sub-area of p5.js?

• Accessibility
• Color
• Core/Environment/Rendering
• Data
• DOM
• Events
• Image
• IO
• Math
• Typography
• Utilities
• p5.strands
• WebGL
• DevOps, Build process, Unit testing
• Internationalization (i18n)
• Friendly Errors
• Other (specify if possible)

p5.js version

1.11.10

Web browser and version

all browser

Operating system

windows 11

Steps to reproduce this

Steps:

1.Create a new p5.js sketch with the code below
2.Run the sketch
3.Observe that the animation starts smoothly but becomes progressively laggy after 3-4 seconds
4..After 6-7 seconds, the entire browser tab freezes and becomes unresponsive
5.Notice there are no console warnings or errors about unmatched push/pop calls

Snippet:

function setup() {
  createCanvas(800, 800);
}

function draw() {
  background(220);
  translate(width/2, height/2);
  
  for (let i = 0; i < 100; i++) {
    push();
    rotate(frameCount * 0.01 + i);
    translate(i * 2, 0);
    rect(0, 0, 10, 10);
    // Missing pop()
  
  }
}

[Homaid]

Hii @davepagurek what do you think about this issue and i would love to work on this

[Homaid]

btw It also occurs in 2.0.5.

[davepagurek]

This is definitely an issue I've run into, but I'm also not 100% sure yet what the fix should be. Did you have something in mind?

Part of the complication is that while in general it makes sense that you'd want to have every draw call have a matching number of pushes and pops, it doesn't always make sense. As an example, let's say you've got a recursive fractal structure you want to draw via depth first search, but it's got so many items that you want to split the drawing up over multiple frames:

function* drawFractal(root) {
  const queue = []
  const visit = (node) => {
    push()
    translate(node.x, node.y)
    rotate(node.angle)
    node.draw()
    for (const child of node.children) {
      queue.push(() => visit(child))
    }
    queue.push(() => pop())
  }
  queue.push(() => visit(root))
  while (queue.length > 0) {
    const step = queue.shift()
    step()
    yield { done: false } // This pauses the generator function so we can do other stuff
  }
  yield { done: true }
}

let fractal
let drawTask
let result = { done: false }
function setup() {
  createCanvas(200, 200)
  fractal = createFractal() // assume this builds the tree structure
  drawTask = drawFractal()
}

function draw() {
  let start = millis()
  // Only draw for 1/60 of a second before waiting for the next frame
  while (!result.done && millis() - start < 1000 / 60) {
    result = drawTask.next() // Run the generator function a little longer
  }
}

This is definitely an advanced example, so a possible decision could be that we just don't want to support usage like this.

[Homaid]

I think adding a warning (not an error) in the friendly Error System that triggers when the push/pop stack exceeds a reasonable threshold at the end of draw(). it will catch accidental mistakes for beginners and does not break or throw errors and for advanced usecase they can disable FES if the warning is annoying

[Homaid]

I also think not supporting cross-frame unmatched push/pop is reasonable because most users especially beginners won't encounter it and benefits of splitting rendering could be achieved other ways. let me know your thoughts thank you @davepagurek

[davepagurek]

I think an FES message is a good way to start, since there's not a huge benefit of making that a heavier "real" error, and then we haven't changed the functionality of the library. Thanks @Homaid!

[davepagurek]

oh actually one last point here -- maybe we should start with 2.x here? We can definitely add it there. 1.x updates are only for bugfixes, and this may not fall into that category (@ksen0 do you have thoughts?)

[Homaid]

I think that makes sense it's more of a enhancement rather than a bugfix, so I will create the PR for dev-2.0