diff --git a/src/Drawable.js b/src/Drawable.js index 7a2813d88..b7fc46e0c 100644 --- a/src/Drawable.js +++ b/src/Drawable.js @@ -13,7 +13,6 @@ const log = require('./util/log'); * @type {twgl.v3} */ const __isTouchingPosition = twgl.v3.create(); -const FLOATING_POINT_ERROR_ALLOWANCE = 1e-6; /** * Convert a scratch space location into a texture space float. Uses the @@ -26,23 +25,23 @@ const FLOATING_POINT_ERROR_ALLOWANCE = 1e-6; * @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]; + // World coordinates/screen-space coordinates refer to pixels by integer coordinates. + // The GL rasterizer considers a pixel to be an area sample. + // Without multisampling, it samples once from the pixel center, + // which is offset by (0.5, 0.5) from the pixel's integer coordinate. + // If you think of it as a pixel grid, the coordinates we're given are grid lines, but we want grid boxes. + // That's why we offset by 0.5 (-0.5 in the Y direction because it's flipped). + const v0 = vec[0] + 0.5; + const v1 = vec[1] - 0.5; 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 // localPosition matches that transformation. localPosition[0] = 0.5 - (((v0 * m[0]) + (v1 * m[4]) + m[12]) / d); localPosition[1] = (((v0 * m[1]) + (v1 * m[5]) + m[13]) / d) + 0.5; - // Fix floating point issues near 0. Filed https://github.com/LLK/scratch-render/issues/688 that - // they're happening in the first place. - // TODO: Check if this can be removed after render pull 479 is merged - if (Math.abs(localPosition[0]) < FLOATING_POINT_ERROR_ALLOWANCE) localPosition[0] = 0; - if (Math.abs(localPosition[1]) < FLOATING_POINT_ERROR_ALLOWANCE) localPosition[1] = 0; // Apply texture effect transform if the localPosition is within the drawable's space, // and any effects are currently active. if (drawable.enabledEffects !== 0 && @@ -404,7 +403,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]; diff --git a/src/RenderWebGL.js b/src/RenderWebGL.js index c65e67470..f15ca8784 100644 --- a/src/RenderWebGL.js +++ b/src/RenderWebGL.js @@ -309,8 +309,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)); } @@ -334,6 +333,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. @@ -578,7 +591,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); @@ -592,7 +605,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. @@ -766,7 +779,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} color3b Test if the Drawable is touching this color. * @param {Array} [mask3b] Optionally mask the check to this part of Drawable. @@ -800,7 +813,7 @@ class RenderWebGL extends EventEmitter { // if there are just too many pixels to CPU render efficiently, we need to let readPixels happen if (bounds.width * bounds.height * (candidates.length + 1) >= maxPixelsForCPU) { - this._isTouchingColorGpuStart(drawableID, candidates.map(({id}) => id).reverse(), bounds, color3b, mask3b); + this._isTouchingColorGpuStart(drawableID, candidates.map(({id}) => id), bounds, color3b, mask3b); } const drawable = this._allDrawables[drawableID]; @@ -814,13 +827,13 @@ class RenderWebGL extends EventEmitter { const effectMask = ~ShaderManager.EFFECT_INFO.ghost.mask; // Scratch Space - +y is top - for (let y = bounds.bottom; y <= bounds.top; y++) { - if (bounds.width * (y - bounds.bottom) * (candidates.length + 1) >= maxPixelsForCPU) { - return this._isTouchingColorGpuFin(bounds, color3b, y - bounds.bottom); + for (let y = 0; y < bounds.height; ++y) { + if (bounds.width * y * (candidates.length + 1) >= maxPixelsForCPU) { + return this._isTouchingColorGpuFin(bounds, color3b, y); } - for (let x = bounds.left; x <= bounds.right; x++) { - point[1] = y; - point[0] = x; + 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, effectMask), mask3b) : @@ -828,10 +841,10 @@ class RenderWebGL extends EventEmitter { RenderWebGL.sampleColor3b(point, candidates, color); if (debugCanvasContext) { debugCanvasContext.fillStyle = `rgb(${color[0]},${color[1]},${color[2]})`; - debugCanvasContext.fillRect(x - bounds.left, bounds.bottom - y, 1, 1); + debugCanvasContext.fillRect(x, y, 1, 1); } // ...and the target color is drawn at this pixel - if (colorMatches(color, color3b, 0)) { + if (colorMatches(color3b, color, 0)) { return true; } } @@ -874,7 +887,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); // Clear the query buffer to fully transparent. This will be the color of pixels that fail the stencil test. gl.clearColor(0, 0, 0, 0); @@ -968,7 +981,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; } @@ -1003,7 +1016,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. @@ -1114,7 +1127,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]; @@ -1207,12 +1220,11 @@ 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, clampedWidth, clampedHeight); - const projection = twgl.m4.ortho( + const projection = this._makeOrthoProjection( scratchBounds.left, scratchBounds.right, scratchBounds.top, - scratchBounds.bottom, - -1, 1 + scratchBounds.bottom ); gl.clearColor(0, 0, 0, 0); @@ -1283,7 +1295,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(...this._backgroundColor4f); gl.clear(gl.COLOR_BUFFER_BIT); @@ -1350,8 +1362,7 @@ class RenderWebGL extends EventEmitter { } /** - * Filter a list of candidates for a touching query into only those that - * could possibly intersect the given bounds. + * Filter a list of candidates for a touching query into only those that could possibly intersect the given bounds. * @param {int} drawableID - ID for drawable of query. * @param {Array} candidateIDs - Candidates for touching query. * @return {?Array< {id, drawable, intersection} >} Filtered candidates with useful data. @@ -1362,8 +1373,7 @@ class RenderWebGL extends EventEmitter { if (bounds === null) { return result; } - // iterate through the drawables list BACKWARDS - we want the top most item to be the first we check - for (let index = candidateIDs.length - 1; index >= 0; index--) { + for (let index = 0; index < candidateIDs.length; ++index) { const id = candidateIDs[index]; if (id !== drawableID) { const drawable = this._allDrawables[id]; @@ -1619,7 +1629,7 @@ class RenderWebGL extends EventEmitter { 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); // Draw the stamped sprite onto the PenSkin's framebuffer. this._drawThese([stampID], ShaderManager.DRAW_MODE.default, projection, {ignoreVisibility: true}); @@ -1700,7 +1710,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 @@ -1878,12 +1888,16 @@ class RenderWebGL extends EventEmitter { // *Not* Scratch Space-- +y is bottom // Loop over all rows of pixels, starting at the top for (let y = 0; y < height; y++) { - _pixelPos[1] = y / height; + // See comment in Drawable.getLocalPosition for why we're adding 0.5 here. + // Essentially, _pixelPos is supposed to be in "texture space", and "texture space" positions are offset + // by 0.5. Notice that we're calling drawable.skin.isTouchingLinear (operates in texture space) + // and not drawable.isTouching (operates in Scratch space). + _pixelPos[1] = (y + 0.5) / height; // We start at the leftmost point, then go rightwards until we hit an opaque pixel let x = 0; for (; x < width; x++) { - _pixelPos[0] = x / width; + _pixelPos[0] = (x + 0.5) / width; EffectTransform.transformPoint(drawable, _pixelPos, _effectPos); if (drawable.skin.isTouchingLinear(_effectPos)) { currentPoint = [x, y]; @@ -1920,7 +1934,7 @@ class RenderWebGL extends EventEmitter { // Now we repeat the process for the right side, looking leftwards for a pixel. for (x = width - 1; x >= 0; x--) { - _pixelPos[0] = x / width; + _pixelPos[0] = (x + 0.5) / width; EffectTransform.transformPoint(drawable, _pixelPos, _effectPos); if (drawable.skin.isTouchingLinear(_effectPos)) { currentPoint = [x, y]; @@ -1958,8 +1972,7 @@ class RenderWebGL extends EventEmitter { * Sample a "final" color from an array of drawables at a given scratch space. * Will blend any alpha values with the drawables "below" it. * @param {twgl.v3} vec Scratch Vector Space to sample - * @param {Array} drawables A list of drawables with the "top most" - * drawable at index 0 + * @param {Array} drawables A list of drawables with the "bottom most" drawable at index 0 * @param {Uint8ClampedArray} dst The color3b space to store the answer in. * @return {Uint8ClampedArray} The dst vector with everything blended down. */ @@ -1967,7 +1980,7 @@ class RenderWebGL extends EventEmitter { dst = dst || new Uint8ClampedArray(3); dst.fill(0); let blendAlpha = 1; - for (let index = 0; blendAlpha !== 0 && index < drawables.length; index++) { + for (let index = drawables.length - 1; blendAlpha !== 0 && index >= 0; --index) { /* if (left > vec[0] || right < vec[0] || bottom > vec[1] || top < vec[0]) { diff --git a/src/Silhouette.js b/src/Silhouette.js index b96348a81..17705241c 100644 --- a/src/Silhouette.js +++ b/src/Silhouette.js @@ -16,19 +16,19 @@ const intMin = (i, j) => j ^ ((i ^ j) & ((i - j) >> 31)); const intMax = (i, j) => i ^ ((i ^ j) & ((i - j) >> 31)); /** - * 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 a pixel's alpha + * from silhouette data, matching texture sampling rules. * @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, _colorData: data}, x, y) => { - // 0 if outside bounds, otherwise read from data. - if (x >= width || y >= height || x < 0 || y < 0) { - return 0; - } + // Clamp coords to edge, matching GL_CLAMP_TO_EDGE. + x = intMax(0, intMin(x, width - 1)); + y = intMax(0, intMin(y, height - 1)); + return data[(((y * width) + x) * 4) + 3]; }; @@ -57,10 +57,6 @@ const getColor4b = ({_width: width, _height: height, _colorData: data}, x, y, ds x = intMax(0, intMin(x, width - 1)); y = intMax(0, intMin(y, height - 1)); - // 0 if outside bounds, otherwise read from data. - if (x >= width || y >= height || x < 0 || y < 0) { - return dst.fill(0); - } const offset = ((y * width) + x) * 4; // premultiply alpha const alpha = data[offset + 3] / 255; @@ -157,7 +153,7 @@ class Silhouette { } 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; @@ -173,8 +169,8 @@ class Silhouette { colorAtNearest (vec, dst) { return this._getColor( this, - Math.floor(vec[0] * (this._width - 1)), - Math.floor(vec[1] * (this._height - 1)), + Math.floor(vec[0] * this._width), + Math.floor(vec[1] * this._height), dst ); } @@ -187,8 +183,13 @@ class Silhouette { * @returns {Uint8ClampedArray} dst */ colorAtLinear (vec, dst) { - const x = vec[0] * (this._width - 1); - const y = vec[1] * (this._height - 1); + // In texture space, pixel centers are at integer coords. Here, the *corners* are at integers. + // We cannot skip the "add 0.5 in Drawable.getLocalPosition -> subtract 0.5 here" roundtrip + // because the two spaces are different--we add 0.5 in Drawable.getLocalPosition in "Scratch space" + // (-240,240 & -180,180), but subtract 0.5 in silhouette space (0, width or height). + // See https://web.archive.org/web/20190125211252/http://hacksoflife.blogspot.com/2009/12/texture-coordinate-system-for-opengl.html + const x = (vec[0] * (this._width)) - 0.5; + const y = (vec[1] * (this._height)) - 0.5; const x1D = x % 1; const y1D = y % 1; @@ -218,10 +219,17 @@ class Silhouette { */ isTouchingNearest (vec) { if (!this._colorData) return; + + // Never touching if the coord falls outside the texture space. + if (vec[0] < 0 || vec[0] > 1 || + vec[1] < 0 || vec[1] > 1) { + return false; + } + return getPoint( this, - Math.floor(vec[0] * (this._width - 1)), - Math.floor(vec[1] * (this._height - 1)) + Math.floor(vec[0] * this._width), + Math.floor(vec[1] * this._height) ) > 0; } @@ -233,8 +241,15 @@ class Silhouette { */ isTouchingLinear (vec) { if (!this._colorData) return; - const x = Math.floor(vec[0] * (this._width - 1)); - const y = Math.floor(vec[1] * (this._height - 1)); + + // Never touching if the coord falls outside the texture space. + if (vec[0] < 0 || vec[0] > 1 || + vec[1] < 0 || vec[1] > 1) { + return false; + } + + const x = Math.floor((vec[0] * this._width) - 0.5); + const y = Math.floor((vec[1] * this._height) - 0.5); return getPoint(this, x, y) > 0 || getPoint(this, x + 1, y) > 0 || getPoint(this, x, y + 1) > 0 || diff --git a/test/integration/cpu-render.html b/test/integration/cpu-render.html index 7eec0a612..00af88be8 100644 --- a/test/integration/cpu-render.html +++ b/test/integration/cpu-render.html @@ -1,3 +1,9 @@ + + + + @@ -44,12 +50,17 @@ } drawable.updateCPURenderAttributes(); return { id, drawable }; - }).reverse().filter(Boolean); + }).filter(Boolean); const color = new Uint8ClampedArray(3); - for (let x = -239; x <= 240; x++) { - for (let y = -180; y< 180; y++) { - render.constructor.sampleColor3b([x, y], drawBits, color); - const offset = (((179-y) * 480) + 239 + x) * 4 + const vec = [0, 0]; + for (let x = 0; x < 480; x++) { + // leftmost pixel is -240, rightmost is 239 + vec[0] = x - 240; + for (let y = 0; y < 360; y++) { + // bottommost pixel is -179, topmost is 180 + vec[1] = 180 - y; + render.constructor.sampleColor3b(vec, drawBits, color); + const offset = ((y * 480) + x) * 4; cpuImageData.data.set(color, offset); } } diff --git a/test/integration/scratch-tests/collision-bounds.sb3 b/test/integration/scratch-tests/collision-bounds.sb3 new file mode 100644 index 000000000..6b707cb9e Binary files /dev/null and b/test/integration/scratch-tests/collision-bounds.sb3 differ diff --git a/test/integration/scratch-tests/stencil-touching-circle.sb2 b/test/integration/scratch-tests/stencil-touching-circle.sb2 new file mode 100644 index 000000000..b6e75c1aa Binary files /dev/null and b/test/integration/scratch-tests/stencil-touching-circle.sb2 differ