Skip to content

Adjust CPU isTouchingColor to match GPU results #407

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

Merged
merged 3 commits into from
Feb 6, 2019
Merged
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
10 changes: 2 additions & 8 deletions src/Drawable.js
Original file line number Diff line number Diff line change
@@ -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 ?
73 changes: 43 additions & 30 deletions src/RenderWebGL.js
Original file line number Diff line number Diff line change
@@ -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.
36 changes: 17 additions & 19 deletions src/Silhouette.js
Original file line number Diff line number Diff line change
@@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

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

I went back and forth a long time on if -1 was needed here or not -- do you have some ninja unit/integration testing that proves it isn't?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was using #406 extensively while working on this, and skipping the -1 here is one of the main changes that makes the results exactly the same as the GPU version. In fact, this work is why I did #406 in the first place :)

I also drew it out on paper to convince myself that it's correct: take a look at which texel you "want" to sample in a 4-texel-wide image when your texture coordinate is 0.01, 0.24, 0.26, 0.49, 0.51, 0.74, 0.76, and 0.99.

const y = Math.round(vec[1] * this._height);
const color = getColor4b(this, x, y, dst);
return color;
}

/**