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.

Sign up for GitHub

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

Remove setRotationCenter API #581

Merged
merged 1 commit into from
Jul 9, 2020
Merged

Remove setRotationCenter API #581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion src/BitmapSkin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ class BitmapSkin extends Skin {
this._textureSize = BitmapSkin._getBitmapSize(bitmapData);

if (typeof rotationCenter === 'undefined') rotationCenter = this.calculateRotationCenter();
this.setRotationCenter.apply(this, rotationCenter);
this._rotationCenter[0] = rotationCenter[0];
this._rotationCenter[1] = rotationCenter[1];

this.emit(Skin.Events.WasAltered);
}
Expand Down
29 changes: 0 additions & 29 deletions src/RenderWebGL.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1337,32 +1337,6 @@ class RenderWebGL extends EventEmitter {
drawable.skin = this._allSkins[skinId];
}

/**
* Update a drawable's skin rotation center.
* @param {number} drawableID The drawable's id.
* @param {Array.<number>} rotationCenter The rotation center for the skin.
*/
updateDrawableRotationCenter (drawableID, rotationCenter) {
const drawable = this._allDrawables[drawableID];
// TODO: https://github.com/LLK/scratch-vm/issues/2288
if (!drawable) return;
drawable.skin.setRotationCenter(rotationCenter[0], rotationCenter[1]);
}

/**
* Update a drawable's skin and rotation center together.
* @param {number} drawableID The drawable's id.
* @param {number} skinId The skin to update to.
* @param {Array.<number>} rotationCenter The rotation center for the skin.
*/
updateDrawableSkinIdRotationCenter (drawableID, skinId, rotationCenter) {
const drawable = this._allDrawables[drawableID];
// TODO: https://github.com/LLK/scratch-vm/issues/2288
if (!drawable) return;
drawable.skin = this._allSkins[skinId];
drawable.skin.setRotationCenter(rotationCenter[0], rotationCenter[1]);
}

/**
* Update a drawable's position.
* @param {number} drawableID The drawable's id.
Expand Down Expand Up @@ -1456,9 +1430,6 @@ class RenderWebGL extends EventEmitter {
if ('skinId' in properties) {
this.updateDrawableSkinId(drawableID, properties.skinId);
}
if ('rotationCenter' in properties) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to document the properties, but not necessarily in this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess if this function is going away it doesn't matter

this.updateDrawableRotationCenter(drawableID, properties.rotationCenter);
}
drawable.updateProperties(properties);
}

Expand Down
15 changes: 4 additions & 11 deletions src/SVGSkin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,6 @@ class SVGSkin extends Skin {
return this._svgRenderer.size;
}

/**
* Set the origin, in object space, about which this Skin should rotate.
* @param {number} x - The x coordinate of the new rotation center.
* @param {number} y - The y coordinate of the new rotation center.
*/
setRotationCenter (x, y) {
const viewOffset = this._svgRenderer.viewOffset;
super.setRotationCenter(x - viewOffset[0], y - viewOffset[1]);
}

/**
* Create a MIP for a given scale.
* @param {number} scale - The relative size of the MIP
Expand Down Expand Up @@ -174,7 +164,10 @@ class SVGSkin extends Skin {
this.resetMIPs();

if (typeof rotationCenter === 'undefined') rotationCenter = this.calculateRotationCenter();
this.setRotationCenter.apply(this, rotationCenter);
const viewOffset = this._svgRenderer.viewOffset;
this._rotationCenter[0] = rotationCenter[0] - viewOffset[0];
this._rotationCenter[1] = rotationCenter[1] - viewOffset[1];

this.emit(Skin.Events.WasAltered);
});
}
Expand Down
33 changes: 0 additions & 33 deletions src/Skin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,6 @@ const twgl = require('twgl.js');
const RenderConstants = require('./RenderConstants');
const Silhouette = require('./Silhouette');

/**
* Truncate a number into what could be stored in a 32 bit floating point value.
* @param {number} num Number to truncate.
* @return {number} Truncated value.
*/
const toFloat32 = (function () {
const memory = new Float32Array(1);
return function (num) {
memory[0] = num;
return memory[0];
};
}());

class Skin extends EventEmitter {
/**
* Create a Skin, which stores and/or generates textures for use in rendering.
Expand Down Expand Up @@ -101,26 +88,6 @@ class Skin extends EventEmitter {
return [0, 0];
}

/**
* Set the origin, in object space, about which this Skin should rotate.
* @param {number} x - The x coordinate of the new rotation center.
* @param {number} y - The y coordinate of the new rotation center.
* @fires Skin.event:WasAltered
*/
setRotationCenter (x, y) {
const emptySkin = this.size[0] === 0 && this.size[1] === 0;
// Compare a 32 bit x and y value against the stored 32 bit center
// values.
const changed = (
toFloat32(x) !== this._rotationCenter[0] ||
toFloat32(y) !== this._rotationCenter[1]);
if (!emptySkin && changed) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: this is replaced by https://github.com/LLK/scratch-render/pull/561/files

this._rotationCenter[0] = x;
this._rotationCenter[1] = y;
this.emit(Skin.Events.WasAltered);
}
}

/**
* Get the center of the current bounding box
* @return {Array<number>} the center of the current bounding box
Expand Down
10 changes: 10 additions & 0 deletions test/fixtures/MockSkin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ class MockSkin extends Skin {
get size () {
return this.dimensions || [0, 0];
}

set rotationCenter (center) {
this._rotationCenter[0] = center[0];
this._rotationCenter[1] = center[1];
this.emit(Skin.Events.WasAltered);
}

get rotationCenter () {
return this._rotationCenter;
}
}

module.exports = MockSkin;
16 changes: 8 additions & 8 deletions test/unit/DrawableTests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ test('translate by costume center', t => {
drawable.skin = new MockSkin();
drawable.skin.size = [200, 50];

drawable.skin.setRotationCenter(1, 0);
drawable.skin.rotationCenter = [1, 0];
expected.initFromBounds(-1, 199, -50, 0);
t.same(snapToNearest(drawable.getAABB()), expected);

drawable.skin.setRotationCenter(0, -2);
drawable.skin.rotationCenter = [0, -2];
expected.initFromBounds(0, 200, -52, -2);
t.same(snapToNearest(drawable.getAABB()), expected);

Expand All @@ -73,7 +73,7 @@ test('translate and rotate', t => {
expected.initFromBounds(-49, 1, -198, 2);
t.same(snapToNearest(drawable.getAABB()), expected);

drawable.skin.setRotationCenter(100, 25);
drawable.skin.rotationCenter = [100, 25];
drawable.updateProperties({direction: 270, position: [0, 0]});
expected.initFromBounds(-100, 100, -25, 25);
t.same(snapToNearest(drawable.getAABB()), expected);
Expand All @@ -89,7 +89,7 @@ test('rotate by non-right-angles', t => {
const drawable = new Drawable();
drawable.skin = new MockSkin();
drawable.skin.size = [10, 10];
drawable.skin.setRotationCenter(5, 5);
drawable.skin.rotationCenter = [5, 5];

expected.initFromBounds(-5, 5, -5, 5);
t.same(snapToNearest(drawable.getAABB()), expected);
Expand All @@ -111,11 +111,11 @@ test('scale', t => {
expected.initFromBounds(0, 200, -25, 0);
t.same(snapToNearest(drawable.getAABB()), expected);

drawable.skin.setRotationCenter(0, 25);
drawable.skin.rotationCenter = [0, 25];
expected.initFromBounds(0, 200, -12.5, 12.5);
t.same(snapToNearest(drawable.getAABB()), expected);

drawable.skin.setRotationCenter(150, 50);
drawable.skin.rotationCenter = [150, 50];
drawable.updateProperties({scale: [50, 50]});
expected.initFromBounds(-75, 25, 0, 25);
t.same(snapToNearest(drawable.getAABB()), expected);
Expand All @@ -129,12 +129,12 @@ test('rotate and scale', t => {
drawable.skin = new MockSkin();
drawable.skin.size = [100, 1000];

drawable.skin.setRotationCenter(50, 50);
drawable.skin.rotationCenter = [50, 50];
expected.initFromBounds(-50, 50, -950, 50);
t.same(snapToNearest(drawable.getAABB()), expected);

drawable.updateProperties({scale: [40, 60]});
drawable.skin.setRotationCenter(50, 50);
drawable.skin.rotationCenter = [50, 50];
expected.initFromBounds(-20, 20, -570, 30);
t.same(snapToNearest(drawable.getAABB()), expected);

Expand Down