Skip to content

Commit fe2dce5

Browse files
committed
Move effect transform + bounds check out of getLocalPosition
This results in a tad bit of duplicated code, but considering that we have 4 different code paths (colorAtNearest, colorAtLinear, isTouchingNearest, and isTouchingLinear) for sampling a silhouette, that's to be expected. This more closely matches the GPU pipeline, in which color and position calculations are more intertwined. This replaces the hacky fix in #424 with a solution that matches the GPU: instead of not transforming points outside the skin bounds, just return transparency/false early.
1 parent 78ae491 commit fe2dce5

File tree

4 files changed

+29
-19
lines changed

4 files changed

+29
-19
lines changed

src/Drawable.js

+12-12
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,6 @@ const getLocalPosition = (drawable, vec) => {
4343
// TODO: Check if this can be removed after render pull 479 is merged
4444
if (Math.abs(localPosition[0]) < FLOATING_POINT_ERROR_ALLOWANCE) localPosition[0] = 0;
4545
if (Math.abs(localPosition[1]) < FLOATING_POINT_ERROR_ALLOWANCE) localPosition[1] = 0;
46-
// Apply texture effect transform if the localPosition is within the drawable's space,
47-
// and any effects are currently active.
48-
if (drawable.enabledEffects !== 0 &&
49-
(localPosition[0] >= 0 && localPosition[0] < 1) &&
50-
(localPosition[1] >= 0 && localPosition[1] < 1)) {
51-
52-
EffectTransform.transformPoint(drawable, localPosition, localPosition);
53-
}
5446
return localPosition;
5547
};
5648

@@ -501,11 +493,17 @@ class Drawable {
501493
}
502494

503495
_isTouchingNearest (vec) {
504-
return this.skin.isTouchingNearest(getLocalPosition(this, vec));
496+
const localPosition = getLocalPosition(this, vec);
497+
if (!this.skin.pointInsideLogicalBounds(localPosition)) return false;
498+
if (this.enabledEffects !== 0) EffectTransform.transformPoint(this, localPosition, localPosition);
499+
return this.skin._silhouette.isTouchingNearest(localPosition);
505500
}
506501

507502
_isTouchingLinear (vec) {
508-
return this.skin.isTouchingLinear(getLocalPosition(this, vec));
503+
const localPosition = getLocalPosition(this, vec);
504+
if (!this.skin.pointInsideLogicalBounds(localPosition)) return false;
505+
if (this.enabledEffects !== 0) EffectTransform.transformPoint(this, localPosition, localPosition);
506+
return this.skin._silhouette.isTouchingLinear(localPosition);
509507
}
510508

511509
/**
@@ -769,15 +767,17 @@ class Drawable {
769767
*/
770768
static sampleColor4b (vec, drawable, dst, effectMask) {
771769
const localPosition = getLocalPosition(drawable, vec);
772-
if (localPosition[0] < 0 || localPosition[1] < 0 ||
773-
localPosition[0] > 1 || localPosition[1] > 1) {
770+
if (!this.skin.pointInsideLogicalBounds(localPosition)) {
774771
dst[0] = 0;
775772
dst[1] = 0;
776773
dst[2] = 0;
777774
dst[3] = 0;
778775
return dst;
779776
}
780777

778+
// Apply texture effect transform if any effects are currently active.
779+
if (drawable.enabledEffects !== 0) EffectTransform.transformPoint(drawable, localPosition, localPosition);
780+
781781
const textColor =
782782
// commenting out to only use nearest for now
783783
// drawable.skin.useNearest(drawable._scale, drawable) ?

src/EffectTransform.js

+2-5
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,12 @@ class EffectTransform {
138138
dst[1] = (dst[1] - skinUniforms.u_logicalBounds[1]) /
139139
(skinUniforms.u_logicalBounds[3] - skinUniforms.u_logicalBounds[1]);
140140

141-
const pointInsideLogicalBounds = dst[0] >= 0 && dst[0] <= 1 && dst[1] >= 0 && dst[1] <= 1;
142-
143-
// Only apply mosaic and pixelate effects to points inside the "logical bounds".
144-
if ((effects & ShaderManager.EFFECT_INFO.mosaic.mask) !== 0 && pointInsideLogicalBounds) {
141+
if ((effects & ShaderManager.EFFECT_INFO.mosaic.mask) !== 0) {
145142
// texcoord0 = fract(u_mosaic * texcoord0);
146143
dst[0] = uniforms.u_mosaic * dst[0] % 1;
147144
dst[1] = uniforms.u_mosaic * dst[1] % 1;
148145
}
149-
if ((effects & ShaderManager.EFFECT_INFO.pixelate.mask) !== 0 && pointInsideLogicalBounds) {
146+
if ((effects & ShaderManager.EFFECT_INFO.pixelate.mask) !== 0) {
150147
// vec2 pixelTexelSize = u_skinSize / u_pixelate;
151148
const texelX = skinUniforms.u_skinSize[0] / uniforms.u_pixelate;
152149
const texelY = skinUniforms.u_skinSize[1] / uniforms.u_pixelate;

src/RenderWebGL.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -1979,7 +1979,7 @@ class RenderWebGL extends EventEmitter {
19791979
for (; x < width; x++) {
19801980
_pixelPos[0] = x / width;
19811981
EffectTransform.transformPoint(drawable, _pixelPos, _effectPos);
1982-
if (drawable.skin.isTouchingLinear(_effectPos)) {
1982+
if (drawable.skin.pointInsideLogicalBounds(_pixelPos) && drawable.skin.isTouchingLinear(_effectPos)) {
19831983
currentPoint = [x, y];
19841984
break;
19851985
}
@@ -2016,7 +2016,7 @@ class RenderWebGL extends EventEmitter {
20162016
for (x = width - 1; x >= 0; x--) {
20172017
_pixelPos[0] = x / width;
20182018
EffectTransform.transformPoint(drawable, _pixelPos, _effectPos);
2019-
if (drawable.skin.isTouchingLinear(_effectPos)) {
2019+
if (drawable.skin.pointInsideLogicalBounds(_pixelPos) && drawable.skin.isTouchingLinear(_effectPos)) {
20202020
currentPoint = [x, y];
20212021
break;
20222022
}

src/Skin.js

+13
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,19 @@ class Skin extends EventEmitter {
219219
this.emit(Skin.Events.WasAltered);
220220
}
221221

222+
/**
223+
* Is this (texture-space, in the range [0, 1]) point inside the skin's "logical bounds"?
224+
* @param {twgl.v3} vec A texture coordinate.
225+
* @return {boolean} True if the point is inside the skin's "logical bounds"
226+
*/
227+
pointInsideLogicalBounds (vec) {
228+
const logicalBounds = this._uniforms.u_logicalBounds;
229+
return vec[0] >= logicalBounds[0] &&
230+
vec[0] <= logicalBounds[2] &&
231+
vec[1] >= logicalBounds[1] &&
232+
vec[1] <= logicalBounds[3];
233+
}
234+
222235
/**
223236
* Does this point touch an opaque or translucent point on this skin?
224237
* Nearest Neighbor version

0 commit comments

Comments
 (0)