Merge pull request #407 from cwillisf/coordinates-fixups

Adjust CPU isTouchingColor to match GPU results

Author: cwillisf

src/Drawable.js

@@ -24,12 +24,11 @@ const __isTouchingPosition = twgl.v3.create();
  * @return {twgl.v3} [x,y] texture space float vector - transformed by effects and matrix
  */
 const getLocalPosition = (drawable, vec) => {
-    // Transfrom from world coordinates to Drawable coordinates.
+    // Transform from world coordinates to Drawable coordinates.
     const localPosition = __isTouchingPosition;
     const v0 = vec[0];
     const v1 = vec[1];
     const m = drawable._inverseMatrix;
-    // var v2 = v[2];
     const d = (v0 * m[3]) + (v1 * m[7]) + m[15];
     // The RenderWebGL quad flips the texture's X axis. So rendered bottom
     // left is 1, 0 and the top right is 0, 1. Flip the X axis so
@@ -342,7 +341,7 @@ class Drawable {
         // Drawable configures a 3D matrix for drawing in WebGL, but most values
         // will never be set because the inputs are on the X and Y position axis
         // and the Z rotation axis. Drawable can bring the work inside
-        // _calculateTransform and greatly reduce the ammount of math and array
+        // _calculateTransform and greatly reduce the amount of math and array
         // assignments needed.
 
         const scale0 = this._skinScale[0];
@@ -625,11 +624,6 @@ class Drawable {
      */
     static sampleColor4b (vec, drawable, dst) {
         const localPosition = getLocalPosition(drawable, vec);
-        if (localPosition[0] < 0 || localPosition[1] < 0 ||
-            localPosition[0] > 1 || localPosition[1] > 1) {
-            dst[3] = 0;
-            return dst;
-        }
         const textColor =
         // commenting out to only use nearest for now
         // drawable.useNearest ?

src/RenderWebGL.js

@@ -256,8 +256,7 @@ class RenderWebGL extends EventEmitter {
         this._yBottom = yBottom;
         this._yTop = yTop;
 
-        // swap yBottom & yTop to fit Scratch convention of +y=up
-        this._projection = twgl.m4.ortho(xLeft, xRight, yBottom, yTop, -1, 1);
+        this._projection = this._makeOrthoProjection(xLeft, xRight, yBottom, yTop);
 
         this._setNativeSize(Math.abs(xRight - xLeft), Math.abs(yBottom - yTop));
     }
@@ -281,6 +280,20 @@ class RenderWebGL extends EventEmitter {
         this.emit(RenderConstants.Events.NativeSizeChanged, {newSize: this._nativeSize});
     }
 
+    /**
+     * Build a projection matrix for Scratch coordinates. For example, `_makeOrthoProjection(-240,240,-180,180)` will
+     * mean the lower-left pixel is at (-240,-179) and the upper right pixel is at (239,180), matching Scratch 2.0.
+     * @param {number} xLeft - the left edge of the projection volume (-240)
+     * @param {number} xRight - the right edge of the projection volume (240)
+     * @param {number} yBottom - the bottom edge of the projection volume (-180)
+     * @param {number} yTop - the top edge of the projection volume (180)
+     * @returns {module:twgl/m4.Mat4} - a projection matrix containing [xLeft,xRight) and (yBottom,yTop]
+     */
+    _makeOrthoProjection (xLeft, xRight, yBottom, yTop) {
+        // swap yBottom & yTop to fit Scratch convention of +y=up
+        return twgl.m4.ortho(xLeft, xRight, yBottom, yTop, -1, 1);
+    }
+
     /**
      * Create a new bitmap skin from a snapshot of the provided bitmap data.
      * @param {ImageData|HTMLImageElement|HTMLCanvasElement|HTMLVideoElement} bitmapData - new contents for this skin.
@@ -518,7 +531,7 @@ class RenderWebGL extends EventEmitter {
      * Returns the position of the given drawableID in the draw list. This is
      * the absolute position irrespective of layer group.
      * @param {number} drawableID The drawable ID to find.
-     * @return {number} The postion of the given drawable ID.
+     * @return {number} The position of the given drawable ID.
      */
     getDrawableOrder (drawableID) {
         return this._drawList.indexOf(drawableID);
@@ -532,7 +545,7 @@ class RenderWebGL extends EventEmitter {
      * "go to back": setDrawableOrder(id, 1); (assuming stage at 0).
      * "go to front": setDrawableOrder(id, Infinity);
      * @param {int} drawableID ID of Drawable to reorder.
-     * @param {number} order New absolute order or relative order adjusment.
+     * @param {number} order New absolute order or relative order adjustment.
      * @param {string=} group Name of layer group drawable belongs to.
      * Reordering will not take place if drawable cannot be found within the bounds
      * of the layer group.
@@ -703,7 +716,7 @@ class RenderWebGL extends EventEmitter {
 
     /**
      * Check if a particular Drawable is touching a particular color.
-     * Unlike touching drawable, if the "tester" is invisble, we will still test.
+     * Unlike touching drawable, if the "tester" is invisible, we will still test.
      * @param {int} drawableID The ID of the Drawable to check.
      * @param {Array<int>} color3b Test if the Drawable is touching this color.
      * @param {Array<int>} [mask3b] Optionally mask the check to this part of Drawable.
@@ -728,23 +741,23 @@ class RenderWebGL extends EventEmitter {
         const color = __touchingColor;
         const hasMask = Boolean(mask3b);
 
-        for (let y = bounds.bottom; y <= bounds.top; y++) {
-            if (bounds.width * (y - bounds.bottom) * (candidates.length + 1) >= __cpuTouchingColorPixelCount) {
-                return this._isTouchingColorGpuFin(bounds, color3b, y - bounds.bottom);
+        // Scratch Space - +y is top
+        for (let y = 0; y < bounds.height; ++y) {
+            if (bounds.width * y * (candidates.length + 1) >= __cpuTouchingColorPixelCount) {
+                return this._isTouchingColorGpuFin(bounds, color3b, y);
             }
-            // Scratch Space - +y is top
-            for (let x = bounds.left; x <= bounds.right; x++) {
-                point[1] = y;
-                point[0] = x;
-                if (
-                    // if we use a mask, check our sample color
-                    (hasMask ?
-                        maskMatches(Drawable.sampleColor4b(point, drawable, color), mask3b) :
-                        drawable.isTouching(point)) &&
-                    // and the target color is drawn at this pixel
-                    colorMatches(RenderWebGL.sampleColor3b(point, candidates, color), color3b, 0)
-                ) {
-                    return true;
+            for (let x = 0; x < bounds.width; ++x) {
+                point[0] = bounds.left + x; // bounds.left <= point[0] < bounds.right
+                point[1] = bounds.top - y; // bounds.bottom < point[1] <= bounds.top ("flipped")
+                // if we use a mask, check our sample color...
+                if (hasMask ?
+                    maskMatches(Drawable.sampleColor4b(point, drawable, color), mask3b) :
+                    drawable.isTouching(point)) {
+                    RenderWebGL.sampleColor3b(point, candidates, color);
+                    // ...and the target color is drawn at this pixel
+                    if (colorMatches(color, color3b, 0)) {
+                        return true;
+                    }
                 }
             }
         }
@@ -760,7 +773,7 @@ class RenderWebGL extends EventEmitter {
         // Limit size of viewport to the bounds around the target Drawable,
         // and create the projection matrix for the draw.
         gl.viewport(0, 0, bounds.width, bounds.height);
-        const projection = twgl.m4.ortho(bounds.left, bounds.right, bounds.top, bounds.bottom, -1, 1);
+        const projection = this._makeOrthoProjection(bounds.left, bounds.right, bounds.top, bounds.bottom);
 
         let fillBackgroundColor = this._backgroundColor;
 
@@ -843,7 +856,7 @@ class RenderWebGL extends EventEmitter {
         const candidates = this._candidatesTouching(drawableID,
             // even if passed an invisible drawable, we will NEVER touch it!
             candidateIDs.filter(id => this._allDrawables[id]._visible));
-        // if we are invisble we don't touch anything.
+        // if we are invisible we don't touch anything.
         if (candidates.length === 0 || !this._allDrawables[drawableID]._visible) {
             return false;
         }
@@ -876,7 +889,7 @@ class RenderWebGL extends EventEmitter {
 
     /**
      * Convert a client based x/y position on the canvas to a Scratch 3 world space
-     * Rectangle.  This creates recangles with a radius to cover selecting multiple
+     * Rectangle. This creates rectangles with a radius to cover selecting multiple
      * scratch pixels with touch / small render areas.
      *
      * @param {int} centerX The client x coordinate of the picking location.
@@ -993,7 +1006,7 @@ class RenderWebGL extends EventEmitter {
             for (worldPos[0] = bounds.left; worldPos[0] <= bounds.right; worldPos[0]++) {
 
                 // Check candidates in the reverse order they would have been
-                // drawn. This will determine what candiate's silhouette pixel
+                // drawn. This will determine what candidate's silhouette pixel
                 // would have been drawn at the point.
                 for (let d = candidateIDs.length - 1; d >= 0; d--) {
                     const id = candidateIDs[d];
@@ -1077,7 +1090,7 @@ class RenderWebGL extends EventEmitter {
         // Limit size of viewport to the bounds around the target Drawable,
         // and create the projection matrix for the draw.
         gl.viewport(0, 0, bounds.width, bounds.height);
-        const projection = twgl.m4.ortho(bounds.left, bounds.right, bounds.top, bounds.bottom, -1, 1);
+        const projection = this._makeOrthoProjection(bounds.left, bounds.right, bounds.top, bounds.bottom);
 
         gl.clearColor(0, 0, 0, 0);
         gl.clear(gl.COLOR_BUFFER_BIT);
@@ -1152,7 +1165,7 @@ class RenderWebGL extends EventEmitter {
         const pickY = bounds.top - scratchY;
 
         gl.viewport(0, 0, bounds.width, bounds.height);
-        const projection = twgl.m4.ortho(bounds.left, bounds.right, bounds.top, bounds.bottom, -1, 1);
+        const projection = this._makeOrthoProjection(bounds.left, bounds.right, bounds.top, bounds.bottom);
 
         gl.clearColor.apply(gl, this._backgroundColor);
         gl.clear(gl.COLOR_BUFFER_BIT);
@@ -1395,7 +1408,7 @@ class RenderWebGL extends EventEmitter {
 
         // Limit size of viewport to the bounds around the stamp Drawable and create the projection matrix for the draw.
         gl.viewport(0, 0, bounds.width, bounds.height);
-        const projection = twgl.m4.ortho(bounds.left, bounds.right, bounds.top, bounds.bottom, -1, 1);
+        const projection = this._makeOrthoProjection(bounds.left, bounds.right, bounds.top, bounds.bottom);
 
         gl.clearColor(0, 0, 0, 0);
         gl.clear(gl.COLOR_BUFFER_BIT);
@@ -1484,7 +1497,7 @@ class RenderWebGL extends EventEmitter {
      * can skip superfluous extra state calls when it is already in that
      * region. Since one region may be entered from within another a exit
      * handle can also be registered that is called when a new region is about
-     * to be entered to restore a common inbetween state.
+     * to be entered to restore a common in-between state.
      *
      * @param {any} regionId - id of the region to enter
      * @param {function} enter - handle to call when first entering a region
@@ -1616,7 +1629,7 @@ class RenderWebGL extends EventEmitter {
          *
          * The determinant is useful in this case to know if AC is counter
          * clockwise from AB. A positive value means the AC is counter
-         * clockwise from AC. A negative value menas AC is clockwise from AB.
+         * clockwise from AC. A negative value means AC is clockwise from AB.
          *
          * @param {Float32Array} A A 2d vector in space.
          * @param {Float32Array} B A 2d vector in space.

src/Silhouette.js

@@ -11,16 +11,16 @@
 let __SilhouetteUpdateCanvas;
 
 /**
- * Internal helper function (in hopes that compiler can inline).  Get a pixel
- * from silhouette data, or 0 if outside it's bounds.
+ * Internal helper function (in hopes that compiler can inline). Get the alpha value for a texel in the silhouette
+ * data, or 0 if outside it's bounds.
  * @private
- * @param {Silhouette} silhouette - has data width and height
- * @param {number} x - x
- * @param {number} y - y
+ * @param {Silhouette} $0 - has data, width, and height
+ * @param {number} x - X position in texels (0..width).
+ * @param {number} y - Y position in texels (0..height).
  * @return {number} Alpha value for x/y position
  */
 const getPoint = ({_width: width, _height: height, _data: data}, x, y) => {
-    // 0 if outside bouds, otherwise read from data.
+    // 0 if outside bounds, otherwise read from data.
     if (x >= width || y >= height || x < 0 || y < 0) {
         return 0;
     }
@@ -39,14 +39,14 @@ const __cornerWork = [
 
 /**
  * Get the color from a given silhouette at an x/y local texture position.
- * @param {Silhouette} The silhouette to sample.
- * @param {number} x X position of texture (0-1).
- * @param {number} y Y position of texture (0-1).
- * @param {Uint8ClampedArray} dst A color 4b space.
- * @return {Uint8ClampedArray} The dst vector.
+ * @param {Silhouette} $0 - The silhouette to sample.
+ * @param {number} x - X position in texels (0..width).
+ * @param {number} y - Y position in texels (0..height).
+ * @param {Uint8ClampedArray} dst - A color 4b space.
+ * @return {Uint8ClampedArray} - The dst vector.
  */
 const getColor4b = ({_width: width, _height: height, _colorData: data}, x, y, dst) => {
-    // 0 if outside bouds, otherwise read from data.
+    // 0 if outside bounds, otherwise read from data.
     if (x >= width || y >= height || x < 0 || y < 0) {
         return dst.fill(0);
     }
@@ -102,7 +102,7 @@ class Silhouette {
 
         this._data = new Uint8ClampedArray(imageData.data.length / 4);
         this._colorData = imageData.data;
-        // delete our custom overriden "uninitalized" color functions
+        // delete our custom overridden "uninitialized" color functions
         // let the prototype work for itself
         delete this.colorAtNearest;
         delete this.colorAtLinear;
@@ -120,12 +120,10 @@ class Silhouette {
      * @returns {Uint8ClampedArray} dst
      */
     colorAtNearest (vec, dst) {
-        return getColor4b(
-            this,
-            Math.floor(vec[0] * (this._width - 1)),
-            Math.floor(vec[1] * (this._height - 1)),
-            dst
-        );
+        const x = Math.round(vec[0] * this._width);
+        const y = Math.round(vec[1] * this._height);
+        const color = getColor4b(this, x, y, dst);
+        return color;
     }
 
     /**