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

Fix "touching color" for background color #489

Merged
merged 7 commits into from
Jul 14, 2020
Merged
Show file tree
Hide file tree
Changes from 3 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
89 changes: 73 additions & 16 deletions src/RenderWebGL.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,23 @@ class RenderWebGL extends EventEmitter {
/** @type {function} */
this._exitRegion = null;

/** @type {object} */
this._backgroundDrawRegionId = {
enter: () => this._enterDrawBackground(),
exit: () => this._exitDrawBackground()
};

/** @type {Array.<snapshotCallback>} */
this._snapshotCallbacks = [];

/** @type {Array<number>}
* @readonly */
this._backgroundColor4f = [0, 0, 0, 0];

/** @type {Uint8ClampedArray}
* @readonly */
this._backgroundColor3b = new Uint8ClampedArray(3);

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 didn't remove this._backgroundColor because it never existed here-- it's first defined when setBackgroundColor is called

this._createGeometry();

this.on(RenderConstants.Events.NativeSizeChanged, this.onNativeSizeChanged);
Expand Down Expand Up @@ -244,7 +258,12 @@ class RenderWebGL extends EventEmitter {
* @param {number} blue The blue component for the background.
*/
setBackgroundColor (red, green, blue) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's consider removing this function entirely in a future PR

this._backgroundColor = [red, green, blue, 1];
this._backgroundColor4f = [red, green, blue, 1];
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might generate less GC work:

Suggested change
this._backgroundColor4f = [red, green, blue, 1];
this._backgroundColor4f[0] = red;
this._backgroundColor4f[1] = green;
this._backgroundColor4f[2] = blue;
this._backgroundColor4f[3] = 1;


this._backgroundColor3b[0] = red * 255;
this._backgroundColor3b[1] = green * 255;
this._backgroundColor3b[2] = blue * 255;

}

/**
Expand Down Expand Up @@ -623,7 +642,7 @@ class RenderWebGL extends EventEmitter {

twgl.bindFramebufferInfo(gl, null);
gl.viewport(0, 0, gl.canvas.width, gl.canvas.height);
gl.clearColor.apply(gl, this._backgroundColor);
gl.clearColor.apply(gl, this._backgroundColor4f);
gl.clear(gl.COLOR_BUFFER_BIT);

this._drawThese(this._drawList, ShaderManager.DRAW_MODE.default, this._projection);
Expand Down Expand Up @@ -739,12 +758,20 @@ class RenderWebGL extends EventEmitter {
*/
isTouchingColor (drawableID, color3b, mask3b) {
const candidates = this._candidatesTouching(drawableID, this._visibleDrawList);
if (candidates.length === 0) {

let bounds;
if (colorMatches(color3b, this._backgroundColor3b, 0)) {
// If the color we're checking for is the background color, don't confine the check to
// candidate drawables' bounds--since the background spans the entire stage, we must check
// everything that lies inside the drawable.
bounds = this._touchingBounds(drawableID);
} else if (candidates.length === 0) {
// If not checking for the background color, we can return early if there are no candidate drawables.
return false;
} else {
bounds = this._candidatesBounds(candidates);
}

const bounds = this._candidatesBounds(candidates);

const maxPixelsForCPU = this._getMaxPixelsForCPU();

const debugCanvasContext = this._debugCanvas && this._debugCanvas.getContext('2d');
Expand Down Expand Up @@ -805,6 +832,19 @@ class RenderWebGL extends EventEmitter {
}
}

_enterDrawBackground () {
const gl = this.gl;
const currentShader = this._shaderManager.getShader(ShaderManager.DRAW_MODE.background, 0);
gl.disable(gl.BLEND);
gl.useProgram(currentShader.program);
twgl.setBuffersAndAttributes(gl, currentShader, this._bufferInfo);
}

_exitDrawBackground () {
const gl = this.gl;
gl.enable(gl.BLEND);
}

_isTouchingColorGpuStart (drawableID, candidateIDs, bounds, color3b, mask3b) {
this._doExitDrawRegion();

Expand All @@ -816,15 +856,8 @@ class RenderWebGL extends EventEmitter {
gl.viewport(0, 0, bounds.width, bounds.height);
const projection = twgl.m4.ortho(bounds.left, bounds.right, bounds.top, bounds.bottom, -1, 1);

let fillBackgroundColor = this._backgroundColor;

// When using masking such that the background fill color will showing through, ensure we don't
// fill using the same color that we are trying to detect!
if (color3b[0] > 196 && color3b[1] > 196 && color3b[2] > 196) {
fillBackgroundColor = [0, 0, 0, 255];
}

gl.clearColor.apply(gl, fillBackgroundColor);
// 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);
gl.clear(gl.COLOR_BUFFER_BIT | gl.STENCIL_BUFFER_BIT);

let extraUniforms;
Expand All @@ -836,6 +869,9 @@ class RenderWebGL extends EventEmitter {
}

try {
// Using the stencil buffer, mask out the drawing to either the drawable's bounds
// or pixels of the drawable which match the mask color, depending on whether a mask color is given.
// Masked-out pixels will not be checked.
gl.enable(gl.STENCIL_TEST);
gl.stencilFunc(gl.ALWAYS, 1, 1);
gl.stencilOp(gl.KEEP, gl.KEEP, gl.REPLACE);
Expand All @@ -856,12 +892,25 @@ class RenderWebGL extends EventEmitter {
gl.stencilOp(gl.KEEP, gl.KEEP, gl.KEEP);
gl.colorMask(true, true, true, true);

// Draw the background as a quad. Drawing a background with gl.clear will not mask to the stenciled area.
this.enterDrawRegion(this._backgroundDrawRegionId);

const uniforms = {
u_backgroundColor: this._backgroundColor4f
};

const currentShader = this._shaderManager.getShader(ShaderManager.DRAW_MODE.background, 0);
twgl.setUniforms(currentShader, uniforms);
twgl.drawBufferInfo(gl, this._bufferInfo, gl.TRIANGLES);

// Draw the candidate drawables on top of the background.
this._drawThese(candidateIDs, ShaderManager.DRAW_MODE.default, projection,
{idFilterFunc: testID => testID !== drawableID}
);
} finally {
gl.colorMask(true, true, true, true);
gl.disable(gl.STENCIL_TEST);
this._doExitDrawRegion();
}
}

Expand All @@ -880,7 +929,8 @@ class RenderWebGL extends EventEmitter {
}

for (let pixelBase = 0; pixelBase < pixels.length; pixelBase += 4) {
if (colorMatches(color3b, pixels, pixelBase)) {
// Transparent pixels are masked (either by the drawable's bounds or color mask).
if (pixels[pixelBase + 3] !== 0 && colorMatches(color3b, pixels, pixelBase)) {
return true;
}
}
Expand Down Expand Up @@ -1204,7 +1254,7 @@ class RenderWebGL extends EventEmitter {
gl.viewport(0, 0, bounds.width, bounds.height);
const projection = twgl.m4.ortho(bounds.left, bounds.right, bounds.top, bounds.bottom, -1, 1);

gl.clearColor.apply(gl, this._backgroundColor);
gl.clearColor.apply(gl, this._backgroundColor4f);
gl.clear(gl.COLOR_BUFFER_BIT);
this._drawThese(this._drawList, ShaderManager.DRAW_MODE.default, projection);

Expand Down Expand Up @@ -1292,6 +1342,13 @@ class RenderWebGL extends EventEmitter {
// Update the CPU position data
drawable.updateCPURenderAttributes();
const candidateBounds = drawable.getFastBounds();

// Push bounds out to integers. If a drawable extends out into half a pixel, that half-pixel stil
// needs to be tested. Plus, in some areas we construct another rectangle from the union of these,
// and iterate over its pixels (width * height). Turns out that doesn't work so well when the
// width/height aren't integers.
candidateBounds.snapToInt();

if (bounds.intersects(candidateBounds)) {
result.push({
id,
Expand Down
7 changes: 6 additions & 1 deletion src/ShaderManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,12 @@ ShaderManager.DRAW_MODE = {
/**
* Draw a line with caps.
*/
line: 'line'
line: 'line',

/**
* Draw the background in a certain color. Must sometimes be used instead of gl.clear.
*/
background: 'background'
};

module.exports = ShaderManager;
14 changes: 12 additions & 2 deletions src/shaders/sprite.frag
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ uniform float u_lineLength;
uniform vec4 u_penPoints;
#endif // DRAW_MODE_line

#ifdef DRAW_MODE_background
uniform vec4 u_backgroundColor;
#endif // DRAW_MODE_background

uniform sampler2D u_skin;

varying vec2 v_texCoord;
Expand Down Expand Up @@ -110,7 +114,7 @@ const vec2 kCenter = vec2(0.5, 0.5);

void main()
{
#ifndef DRAW_MODE_line
#if !(defined(DRAW_MODE_line) || defined(DRAW_MODE_background))
vec2 texcoord0 = v_texCoord;

#ifdef ENABLE_mosaic
Expand Down Expand Up @@ -216,7 +220,9 @@ void main()
gl_FragColor.rgb /= gl_FragColor.a + epsilon;
#endif

#else // DRAW_MODE_line
#endif // !(defined(DRAW_MODE_line) || defined(DRAW_MODE_background))

#ifdef DRAW_MODE_line
// Maaaaagic antialiased-line-with-round-caps shader.

// "along-the-lineness". This increases parallel to the line.
Expand All @@ -235,4 +241,8 @@ void main()
// the closer we are to the line, invert it.
gl_FragColor = u_lineColor * clamp(1.0 - line, 0.0, 1.0);
#endif // DRAW_MODE_line

#ifdef DRAW_MODE_background
gl_FragColor = u_backgroundColor;
#endif
}
3 changes: 3 additions & 0 deletions src/shaders/sprite.vert
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ void main() {
// Apply view transform
position *= 2.0 / u_stageSize;
gl_Position = vec4(position, 0, 1);
#elif defined(DRAW_MODE_background)
gl_Position = vec4(a_position * 2.0, 0, 1);
v_texCoord = a_texCoord;
#else
gl_Position = u_projectionMatrix * u_modelMatrix * vec4(a_position, 0, 1);
v_texCoord = a_texCoord;
Expand Down
Binary file added test/integration/scratch-tests/clear-color.sb3
Binary file not shown.