Rewrite p5 preload logic to support instance mode

[lindapaiste]

Fixes #29

• Delete all preload logic from individual models. We are going back to how it was in the old repo where preload handling is applied as a wrapper around each model function, for functions specified in a withPreload dictionary in the index.js file.
• Refactor P5Util class to introduce a distinction between the p5 environment/namespace/extensions (this.p5Extensions) and the p5 object/constructor (this.p5Constructor). This distinction was super confusing to me and the current code comingles the two concepts in a way that is incorrect. In considers both the object with the .loadImage method and the object with the .Image constructor to be "p5", but those are very different objects which have different methods and properties. They are not interchangeable.
  • Add lots of comments to explain which is which.
  • Use utility functions isP5Constructor and isP5Extensions to check for p5 in a more specific way.
  • Return this.p5Extensions from the this.p5Instance getter to match the current usage this.p5Instance.loadImage.
• Introduce a way to manually set the p5 instance if it cannot be found on the window by calling ml5.setP5(p5).
  • Allows p5 and ml5 to be used together when they are loaded as npm modules (in React apps, etc.)
  • We can use this in our unit tests.
  • We will want to document this to our users.
• Introduce new preload logic using p5.prototype.registerMethod, which allows us to access the this of the specific p5 instance where the method was called.
  • Supports instance method, even when there are multiple sketches on the page. (testing playground)
  • Replaces the preloaded methods on the ml5 object with a wrapped version that calls the _incrementPreload() and _decrementPreload() functions for this specific p5 instance.
  • Expects these preloaded methods to return an object with a ready property which is a Promise. This is the pattern which we are already using in all of our models (despite confusing documentation). Using the .ready property is a lot cleaner and easier than manipulating the arguments of the method. All we have to do is result.ready.then(() => { decrement(); }); to call the decrement function when the loading is done.

[shiffman]

Thank you @lindapaiste for this work, it's really great to see this effort into supporting preload properly! @ziyuan-linn this affects a lot of the codebase you worked on, could you review?

[lindapaiste]

FYI I didn't put in any unit tests because there is no jest or other testing framework in the project. I'm looking now and I see two stale branches regarding test setup. @ziyuan-linn is this something that you need help with?

[shiffman]

I'm happy to merge this @lindapaiste could you take a look at the conflicts! Apologies, this may have occured due to some of the other branches I've been merging and it might even just be formatting related to #80

[gohai]

Apologies for not jumping earlier on this - things are a bit busy here because of midterms 🙂 I'd love to review, or rather try to wrap my head around these changes to understand @lindapaiste - could do this mid-next week, but I don't want to hold this up, so also happy to do so after this has been already merged. Thank you for working on this so essential component!

[lindapaiste]

@shiffman I fixed the conflicts so this is good to go.

[shiffman]

@shiffman I fixed the conflicts so this is good to go.

Thank you! It looks great! I'm going to test it manually locally later today and then will merge!

[lindapaiste]

Just realized that I should rename this to ml5Init or something along those lines. I was calling this myLibInit when I was working on the setup in a very generic context for a myLib library.

[shiffman]

@lindapaiste I've tested this and am ready to merge it! Do you want to make this change before I do?

[lindapaiste]

Just fixed it!

[shiffman]

I'll note I'm getting an error with the body segmentation examples, but I think that is not related to this branch, will investigate separately.

[gohai]

@shiffman Those errors disappeared when I changed bodyPix to bodySegmentation in index.js. (Sent PR #92 against main)

[shiffman]

Here we go, merging! Thank you again @lindapaiste!

[jackbdu]

@lindapaiste Thank you for adding support for instance mode. I was able to preload most models (ml5.faceMesh, ml5.handPose, ml5.imageClassifier, ml5.neuralNetwork, and ml5.sentiment) but I'm getting an error for ml5.bodyPose. See this minimal sketch demonstrating the issue. I also included my temporary fix in the comments, which creates blank window._incrementPreload() and window._decrementPreload() to avoid this error.

I'm also getting an error for preloading ml5.bodySegmentation in instance mode, and that could be fixed by declaring a global variable let video;.