Skip to content

Handle matrix + silhouette updates better #555

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
Show file tree
Hide file tree
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
18 changes: 18 additions & 0 deletions src/Drawable.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const RenderConstants = require('./RenderConstants');
const ShaderManager = require('./ShaderManager');
const Skin = require('./Skin');
const EffectTransform = require('./EffectTransform');
const log = require('./util/log');

/**
* An internal workspace for calculating texture locations from world vectors
Expand Down Expand Up @@ -452,6 +453,8 @@ class Drawable {

/**
* Check if the world position touches the skin.
* The caller is responsible for ensuring this drawable's inverse matrix & its skin's silhouette are up-to-date.
* @see updateCPURenderAttributes
* @param {twgl.v3} vec World coordinate vector.
* @return {boolean} True if the world position touches the skin.
*/
Expand Down Expand Up @@ -632,6 +635,19 @@ class Drawable {
}
}

/**
* Update everything necessary to render this drawable on the CPU.
*/
updateCPURenderAttributes () {
this.updateMatrix();
// CPU rendering always occurs at the "native" size, so no need to scale up this._scale
if (this.skin) {
this.skin.updateSilhouette(this._scale);
} else {
log.warn(`Could not find skin for drawable with id: ${this._id}`);
}
}

/**
* Respond to an internal change in the current Skin.
* @private
Expand Down Expand Up @@ -676,6 +692,8 @@ class Drawable {

/**
* Sample a color from a drawable's texture.
* The caller is responsible for ensuring this drawable's inverse matrix & its skin's silhouette are up-to-date.
* @see updateCPURenderAttributes
* @param {twgl.v3} vec The scratch space [x,y] vector
* @param {Drawable} drawable The drawable to sample the texture from
* @param {Uint8ClampedArray} dst The "color4b" representation of the texture at point.
Expand Down
24 changes: 6 additions & 18 deletions src/RenderWebGL.js
Original file line number Diff line number Diff line change
Expand Up @@ -985,12 +985,7 @@ class RenderWebGL extends EventEmitter {
const bounds = this.clientSpaceToScratchBounds(centerX, centerY, touchWidth, touchHeight);
const worldPos = twgl.v3.create();

drawable.updateMatrix();
if (drawable.skin) {
drawable.skin.updateSilhouette(this._getDrawableScreenSpaceScale(drawable));
} else {
log.warn(`Could not find skin for drawable with id: ${drawableID}`);
}
drawable.updateCPURenderAttributes();

for (worldPos[1] = bounds.bottom; worldPos[1] <= bounds.top; worldPos[1]++) {
for (worldPos[0] = bounds.left; worldPos[0] <= bounds.right; worldPos[0]++) {
Expand Down Expand Up @@ -1021,12 +1016,7 @@ class RenderWebGL extends EventEmitter {
const drawable = this._allDrawables[id];
// default pick list ignores visible and ghosted sprites.
if (drawable.getVisible() && drawable.getUniforms().u_ghost !== 0) {
drawable.updateMatrix();
if (drawable.skin) {
drawable.skin.updateSilhouette(this._getDrawableScreenSpaceScale(drawable));
} else {
log.warn(`Could not find skin for drawable with id: ${id}`);
}
drawable.updateCPURenderAttributes();
return true;
}
return false;
Expand Down Expand Up @@ -1258,8 +1248,8 @@ class RenderWebGL extends EventEmitter {
/** @todo remove this once URL-based skin setting is removed. */
if (!drawable.skin || !drawable.skin.getTexture([100, 100])) return null;

drawable.updateMatrix();
drawable.skin.updateSilhouette(this._getDrawableScreenSpaceScale(drawable));

drawable.updateCPURenderAttributes();
const bounds = drawable.getFastBounds();

// Limit queries to the stage size.
Expand Down Expand Up @@ -1296,8 +1286,7 @@ class RenderWebGL extends EventEmitter {
const drawable = this._allDrawables[id];
if (drawable.skin && drawable._visible) {
// Update the CPU position data
drawable.updateMatrix();
drawable.skin.updateSilhouette(this._getDrawableScreenSpaceScale(drawable));
drawable.updateCPURenderAttributes();
const candidateBounds = drawable.getFastBounds();
if (bounds.intersects(candidateBounds)) {
result.push({
Expand Down Expand Up @@ -1775,8 +1764,7 @@ class RenderWebGL extends EventEmitter {
_getConvexHullPointsForDrawable (drawableID) {
const drawable = this._allDrawables[drawableID];

drawable.updateMatrix();
drawable.skin.updateSilhouette(this._getDrawableScreenSpaceScale(drawable));
drawable.updateCPURenderAttributes();

const [width, height] = drawable.skin.size;
// No points in the hull if invisible or size is 0.
Expand Down
2 changes: 1 addition & 1 deletion src/SVGSkin.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class SVGSkin extends Skin {
return mip;
}

updateSilhouette (scale = 1) {
updateSilhouette (scale = [100, 100]) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

😳

(fortunately, this didn't break anything--all call sites specified a scale explicitly)

// Ensure a silhouette exists.
this.getTexture(scale);
}
Expand Down
6 changes: 6 additions & 0 deletions src/Skin.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,9 @@ class Skin extends EventEmitter {
/**
* Does this point touch an opaque or translucent point on this skin?
* Nearest Neighbor version
* The caller is responsible for ensuring this skin's silhouette is up-to-date.
* @see updateSilhouette
* @see Drawable.updateCPURenderAttributes
* @param {twgl.v3} vec A texture coordinate.
* @return {boolean} Did it touch?
*/
Expand All @@ -232,6 +235,9 @@ class Skin extends EventEmitter {
/**
* Does this point touch an opaque or translucent point on this skin?
* Linear Interpolation version
* The caller is responsible for ensuring this skin's silhouette is up-to-date.
* @see updateSilhouette
* @see Drawable.updateCPURenderAttributes
* @param {twgl.v3} vec A texture coordinate.
* @return {boolean} Did it touch?
*/
Expand Down
3 changes: 1 addition & 2 deletions test/integration/cpu-render.html
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@
if (!(drawable._visible && drawable.skin)) {
return;
}
drawable.updateMatrix();
drawable.skin.updateSilhouette();
drawable.updateCPURenderAttributes();
return { id, drawable };
}).reverse().filter(Boolean);
const color = new Uint8ClampedArray(3);
Expand Down