Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup of P5Util class #1319

Merged
merged 2 commits into from
Mar 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions src/BodyPix/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,13 @@ class BodyPix {
bodyPartsSpec(colorOptions) {
const result = colorOptions !== undefined || Object.keys(colorOptions).length >= 24 ? colorOptions : this.config.palette;

// Check if we're getting p5 colors, make sure they are rgb
if (p5Utils.checkP5() && result !== undefined && Object.keys(result).length >= 24) {
// Check if we're getting p5 colors, make sure they are rgb
const p5 = p5Utils.p5Instance;
if (p5 && result !== undefined && Object.keys(result).length >= 24) {
// Ensure the p5Color object is an RGB array
Object.keys(result).forEach(part => {
if (result[part].color instanceof window.p5.Color) {
if (result[part].color instanceof p5.Color) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

positive affirmation:

  • nice one! 🥇

result[part].color = this.p5Color2RGB(result[part].color);
} else {
result[part].color = result[part].color;
}
});
}
Expand Down Expand Up @@ -452,4 +451,4 @@ const bodyPix = (videoOrOptionsOrCallback, optionsOrCallback, cb) => {
return callback ? instance : instance.ready;
}

export default bodyPix;
export default bodyPix;
22 changes: 3 additions & 19 deletions src/FaceApi/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

/* eslint prefer-destructuring: ["error", {AssignmentExpression: {array: false}}] */
/* eslint no-await-in-loop: "off" */
/* eslint class-methods-use-this: "off" */

/*
* FaceApi: real-time face recognition, and landmark detection
Expand All @@ -14,6 +15,7 @@
import * as tf from "@tensorflow/tfjs";
import * as faceapi from "face-api.js";
import callCallback from "../utils/callcallback";
import modelLoader from "../utils/modelLoader";

const DEFAULTS = {
withLandmarks: true,
Expand Down Expand Up @@ -93,7 +95,7 @@ class FaceApiBase {

Object.keys(this.config.MODEL_URLS).forEach(item => {
if (modelOptions.includes(item)) {
this.config.MODEL_URLS[item] = this.getModelPath(this.config.MODEL_URLS[item]);
this.config.MODEL_URLS[item] = modelLoader.getModelPath(this.config.MODEL_URLS[item]);
}
});

Expand Down Expand Up @@ -354,18 +356,6 @@ class FaceApiBase {
return _param !== undefined ? _param : _default;
}

/**
* Checks if the given string is an absolute or relative path and returns
* the path to the modelJson
* @param {String} absoluteOrRelativeUrl
*/
getModelPath(absoluteOrRelativeUrl) {
const modelJsonPath = this.isAbsoluteURL(absoluteOrRelativeUrl)
? absoluteOrRelativeUrl
: window.location.pathname + absoluteOrRelativeUrl;
return modelJsonPath;
}

/**
* Sets the return options for .detect() or .detectSingle() in case any are given
* @param {Object} faceApiOptions
Expand Down Expand Up @@ -399,12 +389,6 @@ class FaceApiBase {
});
}

/* eslint class-methods-use-this: "off" */
isAbsoluteURL(str) {
const pattern = new RegExp("^(?:[a-z]+:)?//", "i");
return !!pattern.test(str);
}

/**
* get parts from landmarks
* @param {*} result
Expand Down
22 changes: 18 additions & 4 deletions src/utils/modelLoader.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,28 @@
/**
* Check if the provided URL string starts with a hostname,
* such as http://, https://, etc.
* @param {string} str
* @returns {boolean}
*/
function isAbsoluteURL(str) {
const pattern = new RegExp('^(?:[a-z]+:)?//', 'i');
return !!pattern.test(str);
return pattern.test(str);
}

/**
* Accepts a URL that may be a complete URL, or a relative location.
* Returns an absolute URL based on the current window location.
* @param {string} absoluteOrRelativeUrl
* @returns {string}
*/
function getModelPath(absoluteOrRelativeUrl) {
const modelJsonPath = isAbsoluteURL(absoluteOrRelativeUrl) ? absoluteOrRelativeUrl : window.location.pathname + absoluteOrRelativeUrl
return modelJsonPath;
if (!isAbsoluteURL(absoluteOrRelativeUrl) && typeof window !== 'undefined') {
return window.location.pathname + absoluteOrRelativeUrl;
}
return absoluteOrRelativeUrl;
}

export default {
isAbsoluteURL,
getModelPath
}
}
96 changes: 56 additions & 40 deletions src/utils/p5Utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,31 @@

class P5Util {
constructor() {
if (typeof window !== "undefined") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

positive affirmation:

  • huzzah! 🙌

/**
* Store the window as a private property regardless of whether p5 is present.
* Can also set this property by calling method setP5Instance().
* @property {Window | p5 | {p5: p5} | undefined} m_p5Instance
* @private
*/
this.m_p5Instance = window;
}
}

/**
* Set p5 instance globally.
* @param {Object} p5Instance
* Set p5 instance globally in order to enable p5 features throughout ml5.
* Call this function with the p5 instance when using p5 in instance mode.
* @param {p5 | {p5: p5}} p5Instance
*/
setP5Instance(p5Instance) {
this.m_p5Instance = p5Instance;
this.m_p5Instance = p5Instance;
}

/**
* This getter will return p5, checking first if it is in
* the window and next if it is in the p5 property of this.m_p5Instance
* @returns {boolean} if it is in p5
* Dynamic getter checks if p5 is loaded and will return undefined if p5 cannot be found,
* or will return an object containing all of the global p5 properties.
* It first checks if p5 is in the window, and then if it is in the p5 property of this.m_p5Instance.
* @returns {p5 | undefined}
*/
get p5Instance() {
if (typeof this.m_p5Instance !== "undefined" &&
Expand All @@ -33,75 +43,81 @@ class P5Util {

/**
* This function will check if the p5 is in the environment
* Either it is in the p5Instance mode OR it is in the window
* @returns {boolean} if it is in p5
* Either it is in the p5Instance mode OR it is in the window
* @returns {boolean} if it is in p5
*/
checkP5() {
return !!this.p5Instance;
}

/**
* Convert a canvas to Blob
* @param {HTMLCanvasElement} inputCanvas
* @returns {Blob} blob object
*/
* Convert a canvas to a Blob object.
* @param {HTMLCanvasElement} inputCanvas
* @returns {Promise<Blob>}
*/
/* eslint class-methods-use-this: ["error", { "exceptMethods": ["getBlob"] }] */
getBlob(inputCanvas) {
return new Promise((resolve) => {
return new Promise((resolve, reject) => {
inputCanvas.toBlob((blob) => {
resolve(blob);
if (blob) {
resolve(blob);
} else {
reject(new Error('Canvas could not be converted to Blob.'));
}
});
});
};

/**
* Load image in async way.
* @param {String} url
*/
* Load a p5.Image from a URL in an async way.
* @param {string} url
* @return {Promise<p5.Image>}
*/
loadAsync(url) {
return new Promise((resolve) => {
return new Promise((resolve, reject) => {
this.p5Instance.loadImage(url, (img) => {
resolve(img);
}, () => {
reject(new Error(`Could not load image from url ${url}`));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

positive affirmation:

  • Yesss! Thanks so much for digging into the error handling here <3 🏅

});
});
};

/**
* convert raw bytes to blob object
* @param {Array} raws
* @param {number} x
* @param {number} y
* @returns {Blob}
*/
async rawToBlob(raws, x, y) {
* convert raw bytes to blob object
* @param {number[] | Uint8ClampedArray | ArrayLike<number>} raws
* @param {number} width
* @param {number} height
* @returns {Promise<Blob>}
*/
async rawToBlob(raws, width, height) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

positive affirmation:

  • this is so helpful and makes much more sense to use width & height 🙌

const arr = Array.from(raws)
const canvas = document.createElement('canvas'); // Consider using offScreenCanvas when it is ready?
const ctx = canvas.getContext('2d');

canvas.width = x;
canvas.height = y;
canvas.width = width;
canvas.height = height;

const imgData = ctx.createImageData(x, y);
const { data } = imgData;
const imgData = ctx.createImageData(width, height);
const {data} = imgData;

for (let i = 0; i < x * y * 4; i += 1 ) data[i] = arr[i];
for (let i = 0; i < width * height * 4; i += 1) data[i] = arr[i];
ctx.putImageData(imgData, 0, 0);

const blob = await this.getBlob(canvas);
return blob;
return this.getBlob(canvas);
};

/**
* Conver Blob to P5.Image
* @param {Blob} blob
* @param {Object} p5Img
*/
* Convert Blob to P5.Image
* @param {Blob} blob
* Note: may want to reject instead of returning null.
* @returns {Promise<p5.Image | null>}
*/
async blobToP5Image(blob) {
if (this.checkP5()) {
const p5Img = await this.loadAsync(URL.createObjectURL(blob));
return p5Img;
if (this.checkP5() && typeof URL !== "undefined") {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello! I'm just reviewing code for fun. Curious – can you not just say if (this.checkP5() && URL) {...? Is it necessary to use typeof in order to check that it's not undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question! That particular check is part of a larger effort towards making sure that ml5 can run in a backend Node.js environment without error. Here I am making sure that the global URL constructor exists before I try to use it.

If a variable such as URL is never declared anywhere (with let, const, var, class, etc.) then trying to access if (URL) is a fatal ReferenceError. But checking typeof URL !== "undefined" is ok and lets you know that it's safe to use that variable. We definitely have to do this when accessing window.

It might be that's it's not actually needed for URL because Node does have its own URL class, which I didn't know until right now 😄

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! That makes sense to me. Appreciate the response – I've learned something new!

return this.loadAsync(URL.createObjectURL(blob));
}
return null;
return null;
};

}
Expand Down