From 9ca2a0fc8ba4a444b2ab070160e10a5d0d415dc5 Mon Sep 17 00:00:00 2001 From: Tim Martin Date: Sat, 8 Dec 2018 14:17:25 -0500 Subject: [PATCH 1/5] Fix a bug with attached entities and origin change When the origin of a rotated entity changes, we need to update the position of any attached entities. Rotation around an origin is equivalent to shifting to the origin, rotating, and then shifting back. When the origin changes, conceptually we can handle this by reversing the current rotation, changing the origin, and then rotating back. However, this turns out to be equivavent to a simple shift in position, dependent on the current rotation, and the shift in origin. So th implementation calculates this shift and propagates it to child entities. --- src/spatial/2d.js | 56 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/src/spatial/2d.js b/src/spatial/2d.js index fdddb03a..7439859a 100644 --- a/src/spatial/2d.js +++ b/src/spatial/2d.js @@ -6,6 +6,20 @@ var M = Math, PI = M.PI, DEG_TO_RAD = PI / 180; +// These versions of cos and sin ensure that rotations very close to 0 return 0. +// This avoids some problems where entities are not quite aligned with the grid. +var rounded_cos = function(drad) { + var cos = Math.cos(drad); + cos = -1e-10 < cos && cos < 1e-10 ? 0 : cos; + return cos; +}; + +var rounded_sin = function(drad) { + var sin = Math.sin(drad); + sin = -1e-10 < sin && sin < 1e-10 ? 0 : sin; + return sin; +}; + /**@ * #2D * @category 2D @@ -364,11 +378,8 @@ Crafty.c("2D", { dy1 = this._y - this._by1 - oy, dy2 = this._y + this._h + this._by2 - oy; - var ct = Math.cos(rad), - st = Math.sin(rad); - // Special case 90 degree rotations to prevent rounding problems - ct = ct < 1e-10 && ct > -1e-10 ? 0 : ct; - st = st < 1e-10 && st > -1e-10 ? 0 : st; + var ct = rounded_cos(rad), + st = rounded_sin(rad); // Calculate the new points relative to the origin, then find the new (absolute) bounding coordinates! var x0 = dx1 * ct + dy1 * st, @@ -695,11 +706,7 @@ Crafty.c("2D", { * moves in the same way. */ _cascade: function(e) { - if (!e) return; //no change in position - var i = 0, - children = this._children, - l = children.length, - obj; + if (!e) return; //use current position var dx = this._x - e._x, @@ -707,6 +714,14 @@ Crafty.c("2D", { dw = this._w - e._w, dh = this._h - e._h; + this._cascadeInner(dx, dy, dw, dh); + }, + + _cascadeInner: function(dx, dy, dw, dh) { + var i = 0, + children = this._children, + l = children.length, + obj; for (; i < l; ++i) { obj = children[i]; if (obj.__frozen) continue; @@ -736,11 +751,8 @@ Crafty.c("2D", { obj; // precalculate rotation info var drad = deg * DEG_TO_RAD; - var cos = Math.cos(drad); - var sin = Math.sin(drad); - // Avoid some rounding problems - cos = -1e-10 < cos && cos < 1e-10 ? 0 : cos; - sin = -1e-10 < sin && sin < 1e-10 ? 0 : sin; + var cos = rounded_cos(drad); + var sin = rounded_sin(drad); var ox = this._origin.x + this._x; var oy = this._origin.y + this._y; @@ -892,8 +904,22 @@ Crafty.c("2D", { } } + // When entity is rotated, we'll need to shift attached entities on origin change + // The below transformation is equivalent to unrotating, shifting the origin, + // and then rotating around this new origin + if (this.rotation) { + var rad = -this._rotation * DEG_TO_RAD; + var d_ox = x - this._origin.x; + var d_oy = y - this._origin.y; + var cos = rounded_cos(rad); + var sin = rounded_sin(rad); + var dx = d_ox * (1 - cos) - d_oy * sin; + var dy = d_oy * (1 - cos) + d_ox * sin; + this._cascadeInner(dx, dy, 0, 0); + } this._origin.x = x; this._origin.y = y; + this.trigger("OriginChanged"); return this; }, From bab5a0c569a81a5ffb7cfcd94922384f6deafe5c Mon Sep 17 00:00:00 2001 From: Tim Martin Date: Sat, 8 Dec 2018 14:41:55 -0500 Subject: [PATCH 2/5] Recalculate MBR on origin change --- src/spatial/2d.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/spatial/2d.js b/src/spatial/2d.js index 7439859a..d2940f84 100644 --- a/src/spatial/2d.js +++ b/src/spatial/2d.js @@ -920,6 +920,10 @@ Crafty.c("2D", { this._origin.x = x; this._origin.y = y; + if (this._mbr) { + this._calculateMBR(); + } + this.trigger("OriginChanged"); return this; }, From 5c8796ca0a02c6f636f3f14c4b40997dcfa0ee1e Mon Sep 17 00:00:00 2001 From: Tim Martin Date: Sat, 8 Dec 2018 15:11:27 -0500 Subject: [PATCH 3/5] Tests for child/mbr after origin changes When the origin changes, this affects attached entities and the MBR. Add tests for this behavior. --- tests/unit/spatial/2d.js | 74 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/tests/unit/spatial/2d.js b/tests/unit/spatial/2d.js index 9b54c764..c1f188ef 100644 --- a/tests/unit/spatial/2d.js +++ b/tests/unit/spatial/2d.js @@ -361,6 +361,80 @@ _.strictEqual(child._rotation, 90, "child also rotates 90deg"); }); + test("origin change affects children", function(_) { + var parent = Crafty.e("2D").attr({ + x: 0, + y: 0, + w: 50, + h: 50 + }); + var child = Crafty.e("2D").attr({ + x: 100, + y: 0, + w: 50, + h: 50 + }); + parent.attach(child); + parent.rotation = 90; + _.strictEqual( + child._y, + 100, + "child rotates around top left corner (x position)" + ); + _.strictEqual( + child._x, + 0, + "child rotates around top left corner (y position)" + ); + + parent.origin(50, 0); + _.strictEqual( + child._y, + 50, + "child rotates around top right corner (x position)" + ); + _.strictEqual( + child._x, + 50, + "child rotates around top right corner (y position)" + ); + }); + + test("origin change affects MBR", function(_) { + var parent = Crafty.e("2D").attr({ + x: 0, + y: 0, + w: 50, + h: 50 + }); + + parent.rotation = 90; + var mbr1 = parent.mbr(); + _.strictEqual( + mbr1._x, + -50, + "MBR rotates around top left corner (x position)" + ); + _.strictEqual( + mbr1._y, + 0, + "MBR rotates around top left corner (y position)" + ); + + parent.origin(50, 0); + var mbr2 = parent.mbr(); + _.strictEqual( + mbr2._x, + 0, + "MBR rotates around top right corner (x position)" + ); + _.strictEqual( + mbr2._y, + -50, + "MBR rotates around top right corner (y position)" + ); + }); + test("origin properties", function(_) { var player = Crafty.e("2D, Centered").attr({ x: 0, From 2d7569f4b57e83b240e21f4164dff4fb7d3540ef Mon Sep 17 00:00:00 2001 From: Tim Martin Date: Sat, 8 Dec 2018 15:16:09 -0500 Subject: [PATCH 4/5] Invalidate the entity when the origin changes --- src/spatial/2d.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/spatial/2d.js b/src/spatial/2d.js index d2940f84..2961faa3 100644 --- a/src/spatial/2d.js +++ b/src/spatial/2d.js @@ -925,6 +925,7 @@ Crafty.c("2D", { } this.trigger("OriginChanged"); + this.trigger("Invalidate"); return this; }, From 27c03c4cc820775bc0e73b2e03642f366f423bb3 Mon Sep 17 00:00:00 2001 From: Tim Martin Date: Sat, 8 Dec 2018 15:18:31 -0500 Subject: [PATCH 5/5] Origin change should update map/rendering --- src/spatial/2d.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/spatial/2d.js b/src/spatial/2d.js index 2961faa3..658a65bd 100644 --- a/src/spatial/2d.js +++ b/src/spatial/2d.js @@ -924,6 +924,9 @@ Crafty.c("2D", { this._calculateMBR(); } + // Update position in spatial map + this._entry.update(this._cbr || this._mbr || this); + this.trigger("OriginChanged"); this.trigger("Invalidate"); return this;