From bc5bc13515b0e7afda20e88003aa7869d32d341b Mon Sep 17 00:00:00 2001 From: Tim Martin Date: Sat, 19 Sep 2015 16:38:00 -0400 Subject: [PATCH 1/4] Remove event callback object when empty When there are no callbacks left attached to a system or entity, that array should be removed from the global handlers object. Failing to do so can cause an unnecessary performance hit when a large number of extant entities have previously bound to that event. Fixes #957. --- src/core/core.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/core/core.js b/src/core/core.js index 59ce8ab0..873b08da 100644 --- a/src/core/core.js +++ b/src/core/core.js @@ -1063,6 +1063,11 @@ Crafty._callbackMethods = { callbacks.splice(i, 1); i--; l--; + // Delete callbacks object if there are no remaining bound events + if (callbacks.length === 0) { + delete this._callbacks[event]; + delete handlers[event][this[0]]; + } } } else { callbacks[i].call(this, data); From 0c9971ca53879db58ceeab2f3fbd1d07ff004090 Mon Sep 17 00:00:00 2001 From: Tim Martin Date: Sat, 19 Sep 2015 17:11:41 -0400 Subject: [PATCH 2/4] Document and rename reorder event - Rename the event for consistency with the existing api - Document it --- src/spatial/2d.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/spatial/2d.js b/src/spatial/2d.js index 4244b7f9..3c8c1752 100644 --- a/src/spatial/2d.js +++ b/src/spatial/2d.js @@ -23,6 +23,7 @@ var M = Math, * @trigger Move - when the entity has moved - { _x:Number, _y:Number, _w:Number, _h:Number } - Old position * @trigger Invalidate - when the entity needs to be redrawn * @trigger Rotate - when the entity is rotated - { cos:Number, sin:Number, deg:Number, rad:Number, o: {x:Number, y:Number}} + * @trigger Reorder - when the entity's z index has changed */ Crafty.c("2D", { /**@ @@ -881,7 +882,7 @@ Crafty.c("2D", { value = value==intValue ? intValue : intValue+1; this._globalZ = value*100000+this[0]; //magic number 10^5 is the max num of entities this[name] = value; - this.trigger("reorder"); + this.trigger("Reorder"); //if the rect bounds change, update the MBR and trigger move } else if (name === '_x' || name === '_y') { // mbr is the minimal bounding rectangle of the entity From 30fe086ed693bed6d4fcf67a27fc4b84226ceab3 Mon Sep 17 00:00:00 2001 From: Tim Martin Date: Sat, 26 Sep 2015 15:22:48 -0400 Subject: [PATCH 3/4] Handle restart more gracefully This fixes a bundle of bugs related to calling `Craft.stop()` and then `Crafty.init()`. - Clear global bound events on stop (regression caused by the new callback methods) - Add a `Crafty._preBind` method for internal use -- anything registered here will be bound when `Crafty.init()` is called (unless we did a soft stop). This works around a problem where we bound some events only once, when crafty.js was loaded. (Currently used in inputs.js and viewport.js) - Make sure that "CraftyStop" is triggered *before* we remove all event handlers! (This was messing up cleanup of dom event handlers, which in turn messed up registering new ones.) - Delete references to previously created graphics layers - Have graphics layers set their initial values in their init functions Some of these are really work arounds for how we initialize graphics layers, the viewport, etc. I think if we switch these to using the new systems idiom we can remove a lot of the changes above. (Definitely the "prebind" hack.) --- src/controls/inputs.js | 5 ++--- src/core/core.js | 42 ++++++++++++++++++++++++++++++++---- src/core/extensions.js | 24 ++++++++------------- src/graphics/canvas-layer.js | 5 +++++ src/graphics/dom-layer.js | 4 ++++ src/graphics/viewport.js | 14 ++++++++---- src/graphics/webgl.js | 3 +++ 7 files changed, 71 insertions(+), 26 deletions(-) diff --git a/src/controls/inputs.js b/src/controls/inputs.js index d216c61f..e9e83a3f 100644 --- a/src/controls/inputs.js +++ b/src/controls/inputs.js @@ -130,7 +130,6 @@ Crafty.extend({ * @see Crafty.multitouch */ mouseDispatch: function (e) { - if (!Crafty.mouseObjs) return; Crafty.lastEvent = e; @@ -592,7 +591,7 @@ Crafty.extend({ }); //initialize the input events onload -Crafty.bind("Load", function () { +Crafty._preBind("Load", function () { Crafty.addEvent(this, "keydown", Crafty.keyboardDispatch); Crafty.addEvent(this, "keyup", Crafty.keyboardDispatch); @@ -616,7 +615,7 @@ Crafty.bind("Load", function () { Crafty.addEvent(this, Crafty.stage.elem, "mousewheel", Crafty.mouseWheelDispatch); }); -Crafty.bind("CraftyStop", function () { +Crafty._preBind("CraftyStop", function () { Crafty.removeEvent(this, "keydown", Crafty.keyboardDispatch); Crafty.removeEvent(this, "keyup", Crafty.keyboardDispatch); diff --git a/src/core/core.js b/src/core/core.js index 59ce8ab0..81deea92 100644 --- a/src/core/core.js +++ b/src/core/core.js @@ -1136,6 +1136,16 @@ Crafty.extend({ * @see Crafty.stop, Crafty.viewport */ init: function (w, h, stage_elem) { + + // If necessary, attach any event handlers registered before Crafty started + if (!this._preBindDone) { + for(var i = 0; i < this._bindOnInit.length; i++) { + + var preBind = this._bindOnInit[i]; + Crafty.bind(preBind.event, preBind.handler); + } + } + Crafty.viewport.init(w, h, stage_elem); //call all arbitrary functions attached to onload @@ -1145,6 +1155,17 @@ Crafty.extend({ return this; }, + // There are some events that need to be bound to Crafty when it's started/restarted, so store them here + // Switching Crafty's internals to use the new system idiom should allow removing this hack + _bindOnInit: [], + _preBindDone: false, + _preBind: function(event, handler) { + this._bindOnInit.push({ + event: event, + handler: handler + }); + }, + /**@ * #Crafty.getVersion * @category Core @@ -1165,7 +1186,7 @@ Crafty.extend({ /**@ * #Crafty.stop * @category Core - * @trigger CraftyStop - when the game is stopped + * @trigger CraftyStop - when the game is stopped - {bool clearState} * @sign public this Crafty.stop([bool clearState]) * @param clearState - if true the stage and all game state is cleared. * @@ -1175,19 +1196,32 @@ Crafty.extend({ * @see Crafty.init */ stop: function (clearState) { + Crafty.trigger("CraftyStop", clearState); + this.timer.stop(); if (clearState) { + // Remove audio Crafty.audio.remove(); + + // Remove the stage element, and re-add a div with the same id if (Crafty.stage && Crafty.stage.elem.parentNode) { var newCrStage = document.createElement('div'); newCrStage.id = Crafty.stage.elem.id; Crafty.stage.elem.parentNode.replaceChild(newCrStage, Crafty.stage.elem); } - initState(); - } - Crafty.trigger("CraftyStop"); + // Reset references to the now destroyed graphics layers + delete Crafty.canvasLayer.context; + delete Crafty.domLayer._div; + delete Crafty.webgl.context; + // reset callbacks, and indicate that prebound functions need to be bound on init again + Crafty._unbindAll(); + Crafty._addCallbackMethods(Crafty); + this._preBindDone = false; + + initState(); + } return this; }, diff --git a/src/core/extensions.js b/src/core/extensions.js index 6e21e399..3482970f 100644 --- a/src/core/extensions.js +++ b/src/core/extensions.js @@ -177,22 +177,18 @@ module.exports = { //save anonymous function to be able to remove var afn = function (e) { - e = e || window.event; - - if (typeof callback === 'function') { - callback.call(ctx, e); - } + callback.call(ctx, e); }, id = ctx[0] || ""; - if (!this._events[id + obj + type + callback]) this._events[id + obj + type + callback] = afn; - else return; - - if (obj.attachEvent) { //IE - obj.attachEvent('on' + type, afn); - } else { //Everyone else - obj.addEventListener(type, afn, false); + if (!this._events[id + obj + type + callback]) + this._events[id + obj + type + callback] = afn; + else { + return; } + + obj.addEventListener(type, afn, false); + }, /**@ @@ -222,9 +218,7 @@ module.exports = { afn = this._events[id + obj + type + callback]; if (afn) { - if (obj.detachEvent) { - obj.detachEvent('on' + type, afn); - } else obj.removeEventListener(type, afn, false); + obj.removeEventListener(type, afn, false); delete this._events[id + obj + type + callback]; } }, diff --git a/src/graphics/canvas-layer.js b/src/graphics/canvas-layer.js index d415c619..c0be936b 100644 --- a/src/graphics/canvas-layer.js +++ b/src/graphics/canvas-layer.js @@ -66,6 +66,11 @@ Crafty.extend({ return; } + // set properties to initial values -- necessary on a restart + this._dirtyRects = []; + this._changedObjs = []; + this.layerCount = 0; + //create an empty canvas element var c; c = document.createElement("canvas"); diff --git a/src/graphics/dom-layer.js b/src/graphics/dom-layer.js index eaaf0703..76687a38 100644 --- a/src/graphics/dom-layer.js +++ b/src/graphics/dom-layer.js @@ -15,6 +15,10 @@ Crafty.extend({ _div: null, init: function () { + // Set properties to initial values -- necessary on a restart + this._changedObjs = []; + this._dirtyViewport = false; + // Create the div that will contain DOM elements var div = this._div = document.createElement("div"); diff --git a/src/graphics/viewport.js b/src/graphics/viewport.js index f0582cb5..0b6103b9 100644 --- a/src/graphics/viewport.js +++ b/src/graphics/viewport.js @@ -189,7 +189,7 @@ Crafty.extend({ Crafty.unbind("EnterFrame", enterFrame); } - Crafty.bind("StopCamera", stopPan); + Crafty._preBind("StopCamera", stopPan); return function (dx, dy, time, easingFn) { // Cancel any current camera control @@ -248,7 +248,7 @@ Crafty.extend({ } } - Crafty.bind("StopCamera", stopFollow); + Crafty._preBind("StopCamera", stopFollow); return function (target, offsetx, offsety) { if (!target || !target.has('2D')) @@ -320,7 +320,7 @@ Crafty.extend({ function stopZoom(){ Crafty.unbind("EnterFrame", enterFrame); } - Crafty.bind("StopCamera", stopZoom); + Crafty._preBind("StopCamera", stopZoom); var startingZoom, finalZoom, finalAmount, startingX, finalX, startingY, finalY, easing; @@ -528,11 +528,17 @@ Crafty.extend({ init: function (w, h, stage_elem) { // setters+getters for the viewport this._defineViewportProperties(); + + // Set initial values -- necessary on restart + this._x = 0; + this._y = 0; + this._scale = 1; + this.bounds = null; + // If no width or height is defined, the width and height is set to fullscreen this._width = w || window.innerWidth; this._height = h || window.innerHeight; - //check if stage exists if (typeof stage_elem === 'undefined') stage_elem = "cr-stage"; diff --git a/src/graphics/webgl.js b/src/graphics/webgl.js index 42bae28d..37105010 100644 --- a/src/graphics/webgl.js +++ b/src/graphics/webgl.js @@ -437,6 +437,9 @@ Crafty.extend({ return; } + // necessary on restart + this.changed_objects = []; + //create an empty canvas element var c; c = document.createElement("canvas"); From 8c954b426b7ee264042128e3a120deafca28e34d Mon Sep 17 00:00:00 2001 From: Tim Martin Date: Sat, 3 Oct 2015 15:11:14 -0400 Subject: [PATCH 4/4] Fix typo in docs --- src/isometric/diamond-iso.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/isometric/diamond-iso.js b/src/isometric/diamond-iso.js index 94bfd0e4..349ceba9 100644 --- a/src/isometric/diamond-iso.js +++ b/src/isometric/diamond-iso.js @@ -80,7 +80,7 @@ Crafty.extend({ * * Use this method to place an entity in an isometric grid. * - * @exampl + * @example * ~~~ * var iso = Crafty.diamondIso.init(64,128,20,20); * isos.place(Crafty.e('2D, DOM, Color').color('red').attr({w:128, h:128}),1,1,2);