Make docs clearer, add more warnings and edge case handling

Author: davepagurek

src/image/pixels.js

@@ -510,8 +510,8 @@ p5.prototype.filter = function(operation, value) {
  * @method get
  * @param  {Number}         x x-coordinate of the pixel
  * @param  {Number}         y y-coordinate of the pixel
- * @param  {Number}         w width
- * @param  {Number}         h height
+ * @param  {Number}         w width of the section to be returned
+ * @param  {Number}         h height of the section to be returned
  * @return {p5.Image}       the rectangle <a href="#/p5.Image">p5.Image</a>
  * @example
  * <div>

src/webgl/p5.Framebuffer.js

@@ -951,6 +951,12 @@ class Framebuffer {
     this.end();
   }
 
+  /**
+   * Call this befpre updating <a href="#/p5.Framebuffer/pixels">pixels</a>
+   * and calling <a href="#/p5.Framebuffer/updatePixels">updatePixels</a>
+   * to replace the content of the framebuffer with the data in the pixels
+   * array.
+   */
   loadPixels() {
     const gl = this.gl;
     const prevFramebuffer = gl.getParameter(gl.FRAMEBUFFER_BINDING);
@@ -971,7 +977,9 @@ class Framebuffer {
   }
 
   /**
-   * Get a region of pixels, or a single pixel, from the canvas.
+   * Get a region of pixels from the canvas in the form of a
+   * <a href="#/p5.Image">p5.Image</a>, or a single pixel as an array of
+   * numbers.
    *
    * Returns an array of [R,G,B,A] values for any pixel or grabs a section of
    * an image. If the Framebuffer has been set up to not store alpha values, then
@@ -985,8 +993,8 @@ class Framebuffer {
    * @method get
    * @param  {Number}         x x-coordinate of the pixel
    * @param  {Number}         y y-coordinate of the pixel
-   * @param  {Number}         w width
-   * @param  {Number}         h height
+   * @param  {Number}         w width of the section to be returned
+   * @param  {Number}         h height of the section to be returned
    * @return {p5.Image}       the rectangle <a href="#/p5.Image">p5.Image</a>
    */
   /**
@@ -1000,15 +1008,20 @@ class Framebuffer {
    * @return {Number[]}      color of pixel at x,y in array format [R, G, B, A]
    */
   get(x, y, w, h) {
+    p5._validateParameters('p5.Framebuffer.get', arguments);
     const colorFormat = this._glColorFormat();
     if (x === undefined && y === undefined) {
       x = 0;
       y = 0;
       w = this.width;
       h = this.height;
     } else if (w === undefined && h === undefined) {
-      if (x < 0 || y < 0 || w >= this.width || h >= this.height) {
-        return [0, 0, 0, 0];
+      if (x < 0 || y < 0 || x >= this.width || y >= this.height) {
+        console.warn(
+          'The x and y values passed to p5.Framebuffer.get are outside of its range and will be clamped.'
+        );
+        x = this.target.constrain(x, 0, this.width - 1);
+        y = this.target.constrain(y, 0, this.height - 1);
       }
 
       return readPixelWebGL(
@@ -1021,6 +1034,11 @@ class Framebuffer {
       );
     }
 
+    x = this.target.constrain(x, 0, this.width - 1);
+    y = this.target.constrain(y, 0, this.height - 1);
+    w = this.target.constrain(w, 1, this.width - x);
+    h = this.target.constrain(h, 1, this.height - y);
+
     const rawData = readPixelsWebGL(
       undefined,
       this.gl,
@@ -1039,18 +1057,23 @@ class Framebuffer {
     const fullData = new Uint8ClampedArray(
       w * h * this.density * this.density * 4
     );
+
+    // Default channels that aren't in the framebuffer (e.g. alpha, if the
+    // framebuffer is in RGB mode instead of RGBA) to 255
+    fullData.fill(255);
+
     const channels = colorFormat.type === this.gl.RGB ? 3 : 4;
     for (let y = 0; y < h * this.density; y++) {
       for (let x = 0; x < w * this.density; x++) {
         for (let channel = 0; channel < 4; channel++) {
           const idx = (y * w * this.density + x) * 4 + channel;
-          if (channel >= channels) {
-            fullData[idx] = 255;
-          } else {
-            const prevIdx = channels === 4
+          if (channel < channels) {
+            // Find the index of this pixel in `rawData`, which might have a
+            // different number of channels
+            const rawDataIdx = channels === 4
               ? idx
               : (y * w * this.density + x) * channels + channel;
-            fullData[idx] = rawData[prevIdx];
+            fullData[idx] = rawData[rawDataIdx];
           }
         }
       }
@@ -1138,6 +1161,21 @@ class Framebuffer {
     const gl = this.gl;
     this.colorP5Texture.bindTexture();
     const colorFormat = this._glColorFormat();
+
+    const channels = colorFormat.format === gl.RGBA ? 4 : 3;
+    const len =
+      this.width * this.height * this.density * this.density * channels;
+    const TypedArrayClass = colorFormat.type === gl.UNSIGNED_BYTE
+      ? Uint8Array
+      : Float32Array;
+    if (
+      !(this.pixels instanceof TypedArrayClass) || this.pixels.length !== len
+    ) {
+      throw new Error(
+        'The pixels array has not been set correctly. Please call loadPixels() before updatePixels().'
+      );
+    }
+
     gl.texImage2D(
       gl.TEXTURE_2D,
       0,

src/webgl/p5.RendererGL.js

@@ -944,8 +944,8 @@ p5.RendererGL.prototype._getTempFramebuffer = function() {
  * @param {Uint8Array|Float32Array|undefined} pixels An existing pixels array to reuse if the size is the same
  * @param {WebGLRenderingContext} gl The WebGL context
  * @param {WebGLFramebuffer|null} framebuffer The Framebuffer to read
- * @param {Number} x The x coordiante to read (factoring in pixel density)
- * @param {Number} y The y coordiante to read (factoring in pixel density)
+ * @param {Number} x The x coordiante to read, premultiplied by pixel density
+ * @param {Number} y The y coordiante to read, premultiplied by pixel density
  * @param {Number} width The width in pixels to be read (factoring in pixel density)
  * @param {Number} height The height in pixels to be read (factoring in pixel density)
  * @param {GLEnum} format Either RGB or RGBA depending on how many channels to read
@@ -1014,8 +1014,8 @@ export function readPixelsWebGL(
  * @private
  * @param {WebGLRenderingContext} gl The WebGL context
  * @param {WebGLFramebuffer|null} framebuffer The Framebuffer to read
- * @param {Number} x The x coordinate to read (factoring in pixel density)
- * @param {Number} y The y coordinate to read (factoring in pixel density)
+ * @param {Number} x The x coordinate to read, premultiplied by pixel density
+ * @param {Number} y The y coordinate to read, premultiplied by pixel density
  * @param {GLEnum} format Either RGB or RGBA depending on how many channels to read
  * @param {GLEnum} type The datatype of each channel, e.g. UNSIGNED_BYTE or FLOAT
  * @param {Number|undefined} flipY If provided, the total height with which to flip the y axis about

test/unit/webgl/p5.Framebuffer.js

@@ -414,4 +414,42 @@ suite('p5.Framebuffer', function() {
       }
     });
   });
+
+  test(
+    'loadPixels works in arbitrary order for multiple framebuffers',
+    function() {
+      myp5.createCanvas(20, 20, myp5.WEBGL);
+      const fbo1 = myp5.createFramebuffer();
+      const fbo2 = myp5.createFramebuffer();
+
+      fbo1.loadPixels();
+      fbo2.loadPixels();
+      for (let i = 0; i < fbo1.pixels.length; i += 4) {
+        // Set everything red
+        fbo1.pixels[i] = 255;
+        fbo1.pixels[i + 1] = 0;
+        fbo1.pixels[i + 2] = 0;
+        fbo1.pixels[i + 3] = 255;
+      }
+      for (let i = 0; i < fbo2.pixels.length; i += 4) {
+        // Set everything blue
+        fbo2.pixels[i] = 0;
+        fbo2.pixels[i + 1] = 0;
+        fbo2.pixels[i + 2] = 255;
+        fbo2.pixels[i + 3] = 255;
+      }
+      fbo2.updatePixels();
+      fbo1.updatePixels();
+
+      myp5.imageMode(myp5.CENTER);
+
+      myp5.clear();
+      myp5.image(fbo1, 0, 0);
+      assert.deepEqual(myp5.get(0, 0), [255, 0, 0, 255]);
+
+      myp5.clear();
+      myp5.image(fbo2, 0, 0);
+      assert.deepEqual(myp5.get(0, 0), [0, 0, 255, 255]);
+    }
+  );
 });