processing / processing/p5.js

Library functions registered with `registerPreloadMethod` keep calling `_incrementPreload` after preloading is done.

Open
#5,677 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Area:Utilities Bug
Dominant language
JavaScript
Stars
24k
Forks
3.8k
Avg merge
3d 16h
Merged PRs (30d)
25

Description

Most appropriate sub-area of p5.js?
  • Accessibility (Web Accessibility)
  • Build tools and processes
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Friendly error system
  • Image
  • IO (Input/Output)
  • Localization
  • Math
  • Unit Testing
  • Typography
  • Utilities
  • WebGL
  • Other (specify if possible)
p5.js version

1.4.0

Web browser and version

chrome 100.0.4896.127

Operating System

Windows 10

Steps to reproduce this
Steps:
  1. Load up this p5 web editor replicating the issue.
  2. Watch the console to see how it calls setup() repeatedly in an infinite loop.
  3. Notice how window.myLibFunction was initially undefined, but later gets assigned a value.
  4. Keep reading this issue and I'll explain why that happens. 😄
Snippet:

myLib.js -- a package that contains a named object myLib, and registers object methods with p5

const myLib = {
  myLibFunction: (...args) => {
    console.log("called myLibFunction with arguments", ...args);
    return new Promise((resolve) => {
      setTimeout(() => {
        window._decrementPreload();
        console.log("myLibFunction complete");
        resolve();
      }, 2000);
    })
  }
}

window.p5.prototype.registerPreloadMethod('myLibFunction', myLib);

sketch.js -- a p5 sketch that calls the preload-registered function myLib.myLibFunction() in setup() rather than in preload()

function preload() {
  // will be: false, undefined
  console.log("is myLibFunction on window?", !!window.myLibFunction, window.myLibFunction);
}

function setup() {
  noLoop();
  createCanvas(400, 400);
  // call it twice, just for example.
  myLib.myLibFunction().then(myLib.myLibFunction).then(redraw);
  console.log("preload count in setup()", window._preloadCount);
  // will be: true, function
  console.log("is myLibFunction on window?", !!window.myLibFunction, window.myLibFunction);
}

function draw() {
  console.log("preload count in draw()", window._preloadCount);
  background(220);
}

complete example on p5 web editor

Issue:

I've been trying to fix some bugs with registering preloads in ml5.js, which is a p5.js library. It turns out that one of the issues that we are having is actually a bug in p5 itself! I dug pretty deep into the source code to figure things out so I can explain exactly what the problem is.

The issue occurs when calling registerPreloadMethod with a property of a custom object (ie. the library) rather than with p5 as the source object. When a custom object is used, the method gets wrapped in a function that calls _incrementPreload, but it never gets unwrapped. Things will work ok if the method is called in preload(). But if it is called in setup() then it will increment the _preloadCount up to 1 when the function starts and decrement it back to 0 when the function resolves, which then triggers p5 calling setup() again...and again...in an infinite loop.

The looped function chain is myLib.myLibFunction => _decrementPreload => _runIfPreloadsAreDone => _setup() => myLib.myLibFunction.

Here are the relevant lines of code:

https://github.com/processing/p5.js/blob/8d7b608c24cb5911734e8c517a8b958aef619884/src/core/main.js#L267

obj here is the myLib object. So this code is setting myLib.myLibFunction to the version which calls _incrementPreload. That's all good, until:

https://github.com/processing/p5.js/blob/196d3afbf44de116c84936ff20cf4a5b8056ccb7/src/core/main.js#L325-L334

The code to reset the function back to its previous version ignores obj as the source.

  • context is the context of p5 which is the window
  • f is the name of the method 'myLibFunction'
  • this._preloadMethods is a dictionary of method names to source objects, so:
  • this._preloadMethods[f] is myLib
  • this._preloadMethods[f][f] is myLib.myLibFunction

In short, the assignment on line 329 is equivalent to window.myLibFunction = myLib.myLibFunction. Its sets myLibFunction on the window, even though it wasn't a global variable before. It does not set myLib.myLibFunction back to its original version.

Fix?

The "unwrap" code block needs to be modified to support the case where the method is not attached to p5.

There needs to be a reference to the original version of the method. I believe this._preloadMethods[f] would point to the same object instance as myLib so it would contain the overwritten function. It seems like this._registeredPreloadMethods holds the necessary reference to the original, based on:

https://github.com/processing/p5.js/blob/196d3afbf44de116c84936ff20cf4a5b8056ccb7/src/core/main.js#L266-L267

Edit: it looks like this has been a known issue since 2015!

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading the linked sections of src/core/main.js around registerPreloadMethod and the preload unwrapping logic. Reproduce the loop with the linked p5 web editor example, then inspect how custom-object methods are registered and restored. Done means calls from setup() no longer repeatedly trigger setup() and the original custom-object method remains intact.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.