From 8fb74cbfd9c4cd1744655bd80f80af73aecd6ec4 Mon Sep 17 00:00:00 2001 From: Tim Martin Date: Sat, 24 Jun 2017 15:58:10 -0400 Subject: [PATCH] Optimize hit() and _SAT() The main win here is reducing the number of allocations. hit() was also doing an intermediate check between the broad-phase search and the _SAT collision test. I believe this increases the amount of work in the common case - Allow results object to be passed to hit() - Flatten the results of _SAT to reduce allocations a bit when hits are found. - Optimize hit a bit to reduce the amount of work done - Remove an intermediate overlap check; it's unnecessary when we're already using broad-phase + SAT - Assume that the collision component always has map - Return as early as possible --- src/spatial/collision.js | 74 ++++++++++++++++++++------------------- tests/unit/spatial/sat.js | 48 ++++++++++++------------- 2 files changed, 62 insertions(+), 60 deletions(-) diff --git a/src/spatial/collision.js b/src/spatial/collision.js index efc7123c..2a116478 100644 --- a/src/spatial/collision.js +++ b/src/spatial/collision.js @@ -425,11 +425,13 @@ Crafty.c("Collision", { * @comp Collision * @kind Method * - * @sign public Array .hit(String component) + * @sign public Array .hit(String component[, Array results]) * @param component - Check collision with entities that have this component * applied to them. + * @param results - If a results array is supplied, any collisions will be appended to it * @return `null` if there is no collision. If a collision is detected, * returns an Array of collision data objects (see below). + * If the results parameter was passed, it will be used as the return value. * * Tests for collisions with entities that have the specified component * applied to them. @@ -444,7 +446,9 @@ Crafty.c("Collision", { * [{ * obj: [entity], * type: ["MBR" or "SAT"], - * overlap: [number] + * overlap: [number], + * nx: [number], + * ny: [number] * }] * ~~~ * @@ -471,8 +475,8 @@ Crafty.c("Collision", { * hitData = hitDatas[0]; // resolving collision for just one collider * if (hitData.type === 'SAT') { // SAT, advanced collision resolution * // move player back by amount of overlap - * this.x -= hitData.overlap * hitData.normal.x; - * this.y -= hitData.overlap * hitData.normal.y; + * this.x -= hitData.overlap * hitData.nx; + * this.y -= hitData.overlap * hitData.ny; * } else { // MBR, simple collision resolution * // move player to position before he moved (on respective axis) * this[evt.axis] = evt.oldValue; @@ -483,46 +487,46 @@ Crafty.c("Collision", { * * @see Crafty.map#Crafty.map.search */ - hit: function (component) { - var area = this._cbr || this._mbr || this, - results = Crafty.map.unfilteredSearch(area), - i = 0, - l = results.length, - dupes = {}, - id, obj, oarea, key, - overlap = Crafty.rectManager.overlap, - hasMap = ('map' in this && 'containsPoint' in this.map), - finalresult = []; - + _collisionHitDupes: [], + _collisionHitResults: [], + hit: function (component, finalresult) { + var area = this._cbr || this._mbr || this; + var results = this._collisionHitResults; + results.length = 0; + results = Crafty.map.unfilteredSearch(area, results); + var l = results.length; if (!l) { return null; } + var i = 0, + dupes = this._collisionHitDupes, + id, obj; + + finalresult = finalresult || []; + dupes.length = 0; for (; i < l; ++i) { obj = results[i]; - oarea = obj._cbr || obj._mbr || obj; //use the mbr if (!obj) continue; id = obj[0]; //check if not added to hash and that actually intersects - if (!dupes[id] && this[0] !== id && obj.__c[component] && overlap(oarea, area)) + if (!dupes[id] && this[0] !== id && obj.__c[component]){ dupes[id] = obj; - } - - for (key in dupes) { - obj = dupes[key]; - - if (hasMap && 'map' in obj) { - var SAT = this._SAT(this.map, obj.map); - SAT.obj = obj; - SAT.type = "SAT"; - if (SAT) finalresult.push(SAT); - } else { - finalresult.push({ - obj: obj, - type: "MBR" - }); + if (obj.map) { + var SAT = this._SAT(this.map, obj.map); + if (SAT) { + finalresult.push(SAT); + SAT.obj = obj; + SAT.type = "SAT"; + } + } else if (Crafty.rectManager.overlap(area, this._cbr || this._mbr || this)){ + finalresult.push({ + obj: obj, + type: "MBR" + }); + } } } @@ -934,10 +938,8 @@ Crafty.c("Collision", { return { overlap: MTV, - normal: { - x: MNx, - y: MNy - } + nx: MNx, + ny: MNy }; } }); diff --git a/tests/unit/spatial/sat.js b/tests/unit/spatial/sat.js index 4ebb5daa..c724d49f 100644 --- a/tests/unit/spatial/sat.js +++ b/tests/unit/spatial/sat.js @@ -13,15 +13,15 @@ var o = e._SAT(poly1, poly2); _.notEqual(o, false, "Overlap exists"); _.strictEqual(o.overlap, -1, "Overlap by 1 unit"); - _.strictEqual(o.normal.x, -1, "normal.x is -1"); - _.strictEqual(o.normal.y, 0, "normal.y is 0"); + _.strictEqual(o.nx, -1, "nx is -1"); + _.strictEqual(o.ny, 0, "ny is 0"); // order 2 o = e._SAT(poly2, poly1); _.notEqual(o, false, "Overlap exists"); _.strictEqual(o.overlap, -1, "Overlap by 1 unit"); - _.strictEqual(o.normal.x, 1, "normal.x is 1"); - _.strictEqual(o.normal.y, 0, "normal.y is 0"); + _.strictEqual(o.nx, 1, "nx is 1"); + _.strictEqual(o.ny, 0, "ny is 0"); }); @@ -32,15 +32,15 @@ var o = e._SAT(poly1, poly2); _.notEqual(o, false, "Overlap exists"); _.strictEqual(o.overlap, -1, "Overlap by 1 unit"); - _.strictEqual(o.normal.y, -1, "normal.y is -1"); - _.strictEqual(o.normal.x, 0, "normal.x is 0"); + _.strictEqual(o.ny, -1, "ny is -1"); + _.strictEqual(o.nx, 0, "nx is 0"); // order 2 o = e._SAT(poly2, poly1); _.notEqual(o, false, "Overlap exists"); _.strictEqual(o.overlap, -1, "Overlap by 1 unit"); - _.strictEqual(o.normal.y, 1, "normal.y is 1"); - _.strictEqual(o.normal.x, 0, "normal.x is 0"); + _.strictEqual(o.ny, 1, "ny is 1"); + _.strictEqual(o.nx, 0, "nx is 0"); }); @@ -53,15 +53,15 @@ var o = e._SAT(poly1, poly2); _.notEqual(o, false, "Overlap exists"); _.strictEqual(o.overlap, -1, "Overlap by 1 unit"); - _.strictEqual(o.normal.x, -1, "normal.x is -1"); - _.strictEqual(o.normal.y, 0, "normal.y is 0"); + _.strictEqual(o.nx, -1, "nx is -1"); + _.strictEqual(o.ny, 0, "ny is 0"); // order 2 o = e._SAT(poly2, poly1); _.notEqual(o, false, "Overlap exists"); _.strictEqual(o.overlap, -1, "Overlap by 1 unit"); - _.strictEqual(o.normal.x, 1, "normal.x is 1"); - _.strictEqual(o.normal.y, 0, "normal.y is 0"); + _.strictEqual(o.nx, 1, "nx is 1"); + _.strictEqual(o.ny, 0, "ny is 0"); }); @@ -72,15 +72,15 @@ var o = e._SAT(poly1, poly2); _.notEqual(o, false, "Overlap exists"); _.strictEqual(o.overlap, -1, "Overlap by 1 unit"); - _.strictEqual(o.normal.y, -1, "normal.y is -1"); - _.strictEqual(o.normal.x, 0, "normal.x is 0"); + _.strictEqual(o.ny, -1, "ny is -1"); + _.strictEqual(o.nx, 0, "nx is 0"); // order 2 o = e._SAT(poly2, poly1); _.notEqual(o, false, "Overlap exists"); _.strictEqual(o.overlap, -1, "Overlap by 1 unit"); - _.strictEqual(o.normal.y, 1, "normal.y is 1"); - _.strictEqual(o.normal.x, 0, "normal.x is 0"); + _.strictEqual(o.ny, 1, "ny is 1"); + _.strictEqual(o.nx, 0, "nx is 0"); }); test("overlap with non parallel faces, but axis-aligned normal", function(_){ @@ -90,15 +90,15 @@ var o = e._SAT(poly1, poly2); _.notEqual(o, false, "Overlap exists"); _.strictEqual(o.overlap, -1, "Overlap by 1 unit"); - _.strictEqual(o.normal.x, -1, "normal.x is -1"); - _.strictEqual(o.normal.y, 0, "normal.x is 0"); + _.strictEqual(o.nx, -1, "nx is -1"); + _.strictEqual(o.ny, 0, "nx is 0"); // order 2 o = e._SAT(poly2, poly1); _.notEqual(o, false, "Overlap exists"); _.strictEqual(o.overlap, -1, "Overlap by 1 unit"); - _.strictEqual(o.normal.x, 1, "normal.x is 1"); - _.strictEqual(o.normal.y, 0, "normal.x is 0"); + _.strictEqual(o.nx, 1, "nx is 1"); + _.strictEqual(o.ny, 0, "nx is 0"); }); test("overlap with non parallel faces, non-axis-aligned normal", function(_){ @@ -112,15 +112,15 @@ var o = e._SAT(poly1, poly2); _.notEqual(o, false, "Overlap exists"); _.ok(is_inverse_sqrt2(-o.overlap), "Overlap by 1/sqrt(2)"); - _.ok( is_inverse_sqrt2(-o.normal.x), "normal.x is -1/sqrt(2)"); - _.ok( is_inverse_sqrt2(-o.normal.y), "normal.y is -1/sqrt(2)"); + _.ok( is_inverse_sqrt2(-o.nx), "nx is -1/sqrt(2)"); + _.ok( is_inverse_sqrt2(-o.ny), "ny is -1/sqrt(2)"); // order 2 o = e._SAT(poly2, poly1); _.notEqual(o, false, "Overlap exists"); _.ok(is_inverse_sqrt2(-o.overlap), "Overlap by 1/sqrt(2)"); - _.ok( is_inverse_sqrt2(o.normal.x), "normal.x is +1/sqrt(2)"); - _.ok( is_inverse_sqrt2(o.normal.y), "normal.y is +1/sqrt(2)"); + _.ok( is_inverse_sqrt2(o.nx), "nx is +1/sqrt(2)"); + _.ok( is_inverse_sqrt2(o.ny), "ny is +1/sqrt(2)"); }); })();