From aeca29acb29d62c57663786dd8b9eba7fcc959a6 Mon Sep 17 00:00:00 2001 From: Robert Konigsberg Date: Sat, 2 Mar 2013 16:15:41 -0500 Subject: [PATCH] Fix bug 382. Simplify resizing by not recreating everything from scratch. This makes it simpler to deal with i328, since we don't have to worry about unremoved event handlers. Since we're not building everything from scratch, we have to make sure the clipping area is set correctly. Although there's a resetClip method in the HTML5 spec, it's too new. But, man, if we had it things would be great. So instead there's a conditional restore (no restore on the first operation) and a save, for the subsequent restore. Almost all tests pass, but the ones that test save/restore balances are unsurprisingly broken, as there's one additional save. This can be addressed with calls to g.destroy prior to the assertion, or make a special assertion that verifies there's one more context save than restore. This has an additional benefit of being slightly faster. Using /Users/konigsberg/git/kberg-dygraphs/tests/resize.html and the following snippet var start = new Date().getTime(); for (var idx = 0; idx < 500; idx++) { g.resize(100 + idx, 100 + idx); } console.log(new Date().getTime() - start); I frequently saw a difference of about 200ms (2200-range to 2400-range.) How much of that was drawing a mildly complex image? Who knows? I also did some minor refactoring with regards to events, and event method names to make things easier to track. There may be a need for a plug-in event for resizing. --- dygraph-utils.js | 13 ++++++++- dygraph.js | 70 ++++++++++++++++++++++------------------------- extras/unzoom.js | 4 +-- plugins/annotations.js | 8 +++--- plugins/range-selector.js | 12 ++++---- 5 files changed, 57 insertions(+), 50 deletions(-) diff --git a/dygraph-utils.js b/dygraph-utils.js index 11a2fba..5bd58d7 100644 --- a/dygraph-utils.js +++ b/dygraph-utils.js @@ -193,7 +193,7 @@ Dygraph.addEvent = function addEvent(elem, type, fn) { * on the event. The function takes one parameter: the event object. * @private */ -Dygraph.prototype.addEvent = function(elem, type, fn) { +Dygraph.prototype.addAndTrackEvent = function(elem, type, fn) { Dygraph.addEvent(elem, type, fn); this.registeredEvents_.push({ elem : elem, type : type, fn : fn }); }; @@ -221,6 +221,17 @@ Dygraph.removeEvent = function(elem, type, fn) { } }; +Dygraph.prototype.removeTrackedEvents_ = function() { + if (this.registeredEvents_) { + for (var idx = 0; idx < this.registeredEvents_.length; idx++) { + var reg = this.registeredEvents_[idx]; + Dygraph.removeEvent(reg.elem, reg.type, reg.fn); + } + } + + this.registeredEvents_ = []; +} + /** * Cancels further processing of an event. This is useful to prevent default * browser actions, e.g. highlighting text on a double-click. diff --git a/dygraph.js b/dygraph.js index 4316ac3..dc908e6 100644 --- a/dygraph.js +++ b/dygraph.js @@ -979,8 +979,7 @@ Dygraph.prototype.createInterface_ = function() { var enclosing = this.maindiv_; this.graphDiv = document.createElement("div"); - this.graphDiv.style.width = this.width_ + "px"; - this.graphDiv.style.height = this.height_ + "px"; + // TODO(danvk): any other styles that are useful to set here? this.graphDiv.style.textAlign = 'left'; // This is a CSS "reset" enclosing.appendChild(this.graphDiv); @@ -988,10 +987,8 @@ Dygraph.prototype.createInterface_ = function() { // Create the canvas for interactive parts of the chart. this.canvas_ = Dygraph.createCanvas(); this.canvas_.style.position = "absolute"; - this.canvas_.width = this.width_; - this.canvas_.height = this.height_; - this.canvas_.style.width = this.width_ + "px"; // for IE - this.canvas_.style.height = this.height_ + "px"; // for IE + + this.resizeElements_(); this.canvas_ctx_ = Dygraph.getContext(this.canvas_); @@ -1025,8 +1022,8 @@ Dygraph.prototype.createInterface_ = function() { } }; - this.addEvent(window, 'mouseout', this.mouseOutHandler_); - this.addEvent(this.mouseEventElement_, 'mousemove', this.mouseMoveHandler_); + this.addAndTrackEvent(window, 'mouseout', this.mouseOutHandler_); + this.addAndTrackEvent(this.mouseEventElement_, 'mousemove', this.mouseMoveHandler_); // Don't recreate and register the resize handler on subsequent calls. // This happens when the graph is resized. @@ -1037,16 +1034,28 @@ Dygraph.prototype.createInterface_ = function() { // Update when the window is resized. // TODO(danvk): drop frames depending on complexity of the chart. - this.addEvent(window, 'resize', this.resizeHandler_); + this.addAndTrackEvent(window, 'resize', this.resizeHandler_); } }; +Dygraph.prototype.resizeElements_ = function() { + this.graphDiv.style.width = this.width_ + "px"; + this.graphDiv.style.height = this.height_ + "px"; + this.canvas_.width = this.width_; + this.canvas_.height = this.height_; + this.canvas_.style.width = this.width_ + "px"; // for IE + this.canvas_.style.height = this.height_ + "px"; // for IE +} + /** * Detach DOM elements in the dygraph and null out all data references. * Calling this when you're done with a dygraph can dramatically reduce memory * usage. See, e.g., the tests/perf.html example. */ Dygraph.prototype.destroy = function() { + this.canvas_ctx_.restore(); + this.hidden_ctx_.restore(); + var removeRecursive = function(node) { while (node.hasChildNodes()) { removeRecursive(node.firstChild); @@ -1054,19 +1063,11 @@ Dygraph.prototype.destroy = function() { } }; - if (this.registeredEvents_) { - for (var idx = 0; idx < this.registeredEvents_.length; idx++) { - var reg = this.registeredEvents_[idx]; - Dygraph.removeEvent(reg.elem, reg.type, reg.fn); - } - } - - this.registeredEvents_ = []; + this.removeTrackedEvents_(); // remove mouse event handlers (This may not be necessary anymore) Dygraph.removeEvent(window, 'mouseout', this.mouseOutHandler_); Dygraph.removeEvent(this.mouseEventElement_, 'mousemove', this.mouseMoveHandler_); - Dygraph.removeEvent(this.mouseEventElement_, 'mouseup', this.mouseUpHandler_); // remove window handlers Dygraph.removeEvent(window,'resize',this.resizeHandler_); @@ -1338,19 +1339,13 @@ Dygraph.prototype.createDragInterface_ = function() { for (var eventName in interactionModel) { if (!interactionModel.hasOwnProperty(eventName)) continue; - this.addEvent(this.mouseEventElement_, eventName, + this.addAndTrackEvent(this.mouseEventElement_, eventName, bindHandler(interactionModel[eventName])); } - // unregister the handler on subsequent calls. - // This happens when the graph is resized. - if (this.mouseUpHandler_) { - Dygraph.removeEvent(document, 'mouseup', this.mouseUpHandler_); - } - // If the user releases the mouse button during a drag, but not over the // canvas, then it doesn't count as a zooming action. - this.mouseUpHandler_ = function(event) { + var mouseUpHandler = function(event) { if (context.isZooming || context.isPanning) { context.isZooming = false; context.dragStartX = null; @@ -1370,7 +1365,7 @@ Dygraph.prototype.createDragInterface_ = function() { context.tarp.uncover(); }; - this.addEvent(document, 'mouseup', this.mouseUpHandler_); + this.addAndTrackEvent(document, 'mouseup', this.mouseUpHandler); }; /** @@ -2230,6 +2225,15 @@ Dygraph.prototype.predraw_ = function() { this.cascadeEvents_('clearChart'); this.plotter_.clear(); } + + if(!this.is_initial_draw_) { + this.canvas_ctx_.restore(); + this.hidden_ctx_.restore(); + } + + this.canvas_ctx_.save(); + this.hidden_ctx_.save(); + this.plotter_ = new DygraphCanvasRenderer(this, this.hidden_, this.hidden_ctx_, @@ -3532,17 +3536,9 @@ Dygraph.prototype.resize = function(width, height) { this.height_ = this.maindiv_.clientHeight; } + this.resizeElements_(); + if (old_width != this.width_ || old_height != this.height_) { - // TODO(danvk): there should be a clear() method. - this.maindiv_.innerHTML = ""; - this.roller_ = null; - this.attrs_.labelsDiv = null; - this.createInterface_(); - if (this.annotations_.length) { - // createInterface_ reset the layout, so we need to do this. - this.layout_.setAnnotations(this.annotations_); - } - this.createDragInterface_(); this.predraw_(); } diff --git a/extras/unzoom.js b/extras/unzoom.js index 802c31b..fac1bbc 100644 --- a/extras/unzoom.js +++ b/extras/unzoom.js @@ -79,14 +79,14 @@ Dygraph.Plugins.Unzoom = (function() { g.resetZoom(); }; - g.addEvent(parent, 'mouseover', function() { + g.addAndTrackEvent(parent, 'mouseover', function() { if (g.isZoomed()) { self.show(true); } self.over_ = true; }); - g.addEvent(parent, 'mouseout', function() { + g.addAndTrackEvent(parent, 'mouseout', function() { self.show(false); self.over_ = false; }); diff --git a/plugins/annotations.js b/plugins/annotations.js index 04ebb8f..8576104 100644 --- a/plugins/annotations.js +++ b/plugins/annotations.js @@ -143,13 +143,13 @@ annotations.prototype.didDrawChart = function(e) { div.style.borderColor = g.colorsMap_[p.name]; a.div = div; - g.addEvent(div, 'click', + g.addAndTrackEvent(div, 'click', bindEvt('clickHandler', 'annotationClickHandler', p, this)); - g.addEvent(div, 'mouseover', + g.addAndTrackEvent(div, 'mouseover', bindEvt('mouseOverHandler', 'annotationMouseOverHandler', p, this)); - g.addEvent(div, 'mouseout', + g.addAndTrackEvent(div, 'mouseout', bindEvt('mouseOutHandler', 'annotationMouseOutHandler', p, this)); - g.addEvent(div, 'dblclick', + g.addAndTrackEvent(div, 'dblclick', bindEvt('dblClickHandler', 'annotationDblClickHandler', p, this)); containerDiv.appendChild(div); diff --git a/plugins/range-selector.js b/plugins/range-selector.js index df8f867..f00a87d 100644 --- a/plugins/range-selector.js +++ b/plugins/range-selector.js @@ -504,7 +504,7 @@ rangeSelector.prototype.initInteraction_ = function() { addTouchEvents = function(elem, fn) { var types = ['touchstart', 'touchend', 'touchmove', 'touchcancel']; for (var i = 0; i < types.length; i++) { - self.dygraph_.addEvent(elem, types[i], fn); + self.dygraph_.addAndTrackEvent(elem, types[i], fn); } }; @@ -512,14 +512,14 @@ rangeSelector.prototype.initInteraction_ = function() { this.setDefaultOption_('panEdgeFraction', 0.0001); var dragStartEvent = window.opera ? 'mousedown' : 'dragstart'; - this.dygraph_.addEvent(this.leftZoomHandle_, dragStartEvent, onZoomStart); - this.dygraph_.addEvent(this.rightZoomHandle_, dragStartEvent, onZoomStart); + this.dygraph_.addAndTrackEvent(this.leftZoomHandle_, dragStartEvent, onZoomStart); + this.dygraph_.addAndTrackEvent(this.rightZoomHandle_, dragStartEvent, onZoomStart); if (this.isUsingExcanvas_) { - this.dygraph_.addEvent(this.iePanOverlay_, 'mousedown', onPanStart); + this.dygraph_.addAndTrackEvent(this.iePanOverlay_, 'mousedown', onPanStart); } else { - this.dygraph_.addEvent(this.fgcanvas_, 'mousedown', onPanStart); - this.dygraph_.addEvent(this.fgcanvas_, 'mousemove', onCanvasHover); + this.dygraph_.addAndTrackEvent(this.fgcanvas_, 'mousedown', onPanStart); + this.dygraph_.addAndTrackEvent(this.fgcanvas_, 'mousemove', onCanvasHover); } // Touch events -- 2.7.4