-
Notifications
You must be signed in to change notification settings - Fork 350
Put back in "Merge pull request #489 from adroitwhiz/touching-white-fixes" #676
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
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
90040df
Revert "Revert "Revert "Revert "Merge pull request #489 from adroitwh…
fsih 00554df
Handle null bounds
b119d35
Fix floating point issues near 0
749b110
Add todo
5fa9c76
Change local space to clamp to texture space in is touching queries
b0acea9
Pull in parts of pull 479 which move clamping to texture space into s…
4a16869
Merge branch 'develop' into pull676
13d62bd
Add back clamping in sampleColor4b, and handling of values near zero …
0c73c58
Fix tests on node 12
63513a9
Add back todo as well
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -188,9 +188,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>} */ | ||
// Don't set this directly-- use setBackgroundColor so it stays in sync with _backgroundColor3b | ||
this._backgroundColor4f = [0, 0, 0, 1]; | ||
|
||
/** @type {Uint8ClampedArray} */ | ||
// Don't set this directly-- use setBackgroundColor so it stays in sync with _backgroundColor4f | ||
this._backgroundColor3b = new Uint8ClampedArray(3); | ||
|
||
this._createGeometry(); | ||
|
||
this.on(RenderConstants.Events.NativeSizeChanged, this.onNativeSizeChanged); | ||
|
@@ -250,7 +264,14 @@ class RenderWebGL extends EventEmitter { | |
* @param {number} blue The blue component for the background. | ||
*/ | ||
setBackgroundColor (red, green, blue) { | ||
this._backgroundColor = [red, green, blue, 1]; | ||
this._backgroundColor4f[0] = red; | ||
this._backgroundColor4f[1] = green; | ||
this._backgroundColor4f[2] = blue; | ||
|
||
this._backgroundColor3b[0] = red * 255; | ||
this._backgroundColor3b[1] = green * 255; | ||
this._backgroundColor3b[2] = blue * 255; | ||
|
||
} | ||
|
||
/** | ||
|
@@ -629,7 +650,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); | ||
|
@@ -745,12 +766,22 @@ 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue is here, touchingBounds can return null: https://github.com/LLK/scratch-render/blob/954cfff02b08069a082cbedd415c1fecd9b1e4fb/src/RenderWebGL.js#L1385 Then we reference bounds.width on line 792 |
||
// e.g. empty costume, or off the stage | ||
if (bounds === null) return false; | ||
} 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'); | ||
|
@@ -811,6 +842,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(); | ||
|
||
|
@@ -822,15 +866,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; | ||
|
@@ -842,6 +879,9 @@ class RenderWebGL extends EventEmitter { | |
} | ||
|
||
try { | ||
// Using the stencil buffer, mask out the drawing to either the drawable's alpha channel | ||
// 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); | ||
|
@@ -862,12 +902,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(); | ||
} | ||
} | ||
|
||
|
@@ -886,7 +939,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 alpha channel or color mask). | ||
if (pixels[pixelBase + 3] !== 0 && colorMatches(color3b, pixels, pixelBase)) { | ||
return true; | ||
} | ||
} | ||
|
@@ -1321,7 +1375,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); | ||
|
||
|
@@ -1411,6 +1465,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 still | ||
// 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, | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file not shown.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to add "resolves #680" to the PR description... at least I think that's what this is...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I broke this out into #683