[p5.js 2.0 Bug Report]: You need to await redraw() now

[davepagurek]

Most appropriate sub-area of p5.js?

• Accessibility
• Color
• Core/Environment/Rendering
• Data
• DOM
• Events
• Image
• IO
• Math
• Typography
• Utilities
• WebGL
• Build process
• Unit testing
• Internationalization
• Friendly errors
• Other (specify if possible)

p5.js version

2.0.0

Web browser and version

Firefox

Operating system

MacOS

Steps to reproduce this

Consider a sketch like this, where I want to export a higher res image on key press. Here's how I'd do this in 1.x:

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

function draw() {
  background(220);
  circle(width/2, height/2, 200)
}

function keyPressed() {
  pixelDensity(3)
  redraw()
  saveCanvas('test.png')
}

However, in 2.0.0, this saves a blank image. Live: https://editor.p5js.org/davepagurek/sketches/GYscX3tl1

I think this is because our lifecycle hooks for pre/post draw are potentially async, and so they are internally awaited.

I can see two ways of resolving this, both valid:

• Monitor the return values of lifecycles and only await them if they're async, so that draw is synchronous if all the lifecycle hooks are synchronous
• Document that redraw must be awaited (and therefore functions calling redraw must be async functions)

I don't have a strong opinion on which approach is the right one here! The first one is probably more complex to maintain, but would be less of a breaking change from 1.x.

[ksen0]

The first method (Monitor the return values of lifecycles and only await them if they're async, so that draw is synchronous if all the lifecycle hooks are synchronous) has least friction to the end-user, but what's the maintenance cost exactly? And is this something that can be covered in test suite?

[davepagurek]

It'd probably have to look something like this (pseudocode) to account for some hooks being synchronous and some maybe async, where we can jump to using promises halfway through:

let donePromise
for (const hook of hooks) {
  if (donePromise) {
    donePromise = donePromise.then(() => hook())
  } else {
    const result = hook()
    if (result instanceof Promise) {
      donePromise = result
    }
  }
}
return donePromise

Not terrible all things considered and unit tests will help!

The other downside is that depending on what addons a user adds, they might have to start awaiting redraw when previously they didn't have to. It may not be obvious why this is the case to a user if the addon doesn't document that it needs this.

[limzykenneth]

Although the implementation of draw/redraw (basically the draw loop including hooks) currently uses async await, I think an argument can be made for them to not be async at all. While it enables the draw loop itself to be async and thus you can use await promises in it, requestAnimationFrame does not wait for the previous frame to complete its async execution before running the next frame, which can make async draw loop chaotic when each draw frame is longer asynchronously than a single frame time.

A downside to this is that setup and remove hooks can be async while draw hook cannot be so this can be an area of confusion. I'm also not 100% sure there are no good usage of async draw loop.

So for example this part from structure.js

p5.js/src/core/structure.js

Lines 378 to 385 in 55ed78e

	await this._runLifecycleHook('predraw');
	this._inUserDraw = true;
	try {
	await context.draw();
	} finally {
	this._inUserDraw = false;
	}
	await this._runLifecycleHook('postdraw');

We can potentially remove the awaits which can make everything upstream non-async. Addon and sketch authors can themselves make the hooks or draw function async, just that it can break the order in which the hooks and draw function execute because we are not awaiting them.

[davepagurek]

I'm also not 100% sure there are no good usage of async draw loop.

The main async draw use case I can think of is something like ml5, where you want to do some processing on e.g. the webcam each frame to detect a pose. The processing is generally async. In 1.x examples, they don't block on the processing and just use whatever the last available pose is each frame, but this leads to the webcam being slightly ahead of the pose detection. Especially if one wants to record an export frame by frame, being able to ensure that each frame is complete when there's an async step would be beneficial.

[davepagurek]

@quinton-ashley right, it's less the sync/async that matters but the return value. I think my implementation above handles that, it just checks if the return value is a promise or not in order to catch both async or synchronous promise-returning functions.

[davepagurek]

Cause the idea is that redraw should only be async if draw or the predraw or postdraw hooks is async?

Yep! So if no hooks return promises, then it never does any promise chaining that would need to be awaited. It still leaves something to be desired because a sketch author might not be aware of whether or not an add-on does something async, but I'm thinking maybe those are edge cases enough that those add-ons would explain their async usage in their own docs?

[quinton-ashley]

@davepagurek Okay sorry I get how the pseudo-code works and what the goal is now.

The point I was trying to get at though, is it seems like it'd really complicate the implementation of redraw, _draw, and _runLifecycleHook.

Documenting redraw as always being an async function in p5 v2 (a breaking change) is more straightforward for users too.

Or specifically saying "postsetup" and "presetup" hooks can be async but "init", "predraw", and "postdraw" hooks can not, seems preferable too.

[davepagurek]

Making redraw always be async is also an option. If we go that route, we probably need to add code to detect when draw is running so that we don't start the next animation frame while a redraw from e.g. an event handler is being run. We would also need to decide to either special case an init hook to only ever be synchronous so that it can be run in the constructor, or adapt when we run global initialization hooks to come after an await.

I think there are valid reasons to support async pre and post draw, e.g. #7770 (comment), so ideally we'd use an API that permits that.

[limzykenneth]

@davepagurek

Making redraw always be async is also an option.

I'm thinking I prefer it to not need to be awaited as much as possible but I guess it might be confusing documentation wise?

If we go that route, we probably need to add code to detect when draw is running so that we don't start the next animation frame while a redraw from e.g. an event handler is being run.

This is likely possible if we want to do that, ie. we defer calling requestAnimationFrame until the draw loop promise has resolved, it possibly will affect the framerate when long running promises are awaited but might be an expected outcome?

[davepagurek]

I think that sounds good to me. If that makes sense for everyone maybe a next step could be to prototype it just to double check that nothing weird happens with the animation frame deferral that we aren't expecting?

[limzykenneth]

Do go ahead, hopefully it won't messed up the next frame timing otherwise we'll have to debug the event loop itself.