From 61c528f3af33a92f72720902c18d8842183069ab Mon Sep 17 00:00:00 2001 From: crschmidt Date: Wed, 30 Jul 2008 21:23:19 +0000 Subject: [PATCH] Change getMousePosition to only be called automatically *if* the 'includeXY' flag on the Events object is set to true. This ends up meaning that we save a lot of unneccesary getMousePosition calls because (for example) the layer doesn't need to include the .xy property. In addition, we add in speed improvements via caching to the getMousePosition, courtesy the work from pgiraud (which was worked on further by tcoulter) -- this results in significantly improved getMousePosition performance improvements in 'real life' situations that are more like the cases that people use OpenLayers, with a higher number of containing divs (and also clearly demonstrate a gain in performance even in the simple case.) The end result is: * In typical map movement over the map, (n / n+1) fewer calls to getMousePosition, where n is the number of active layers when dragging over the map. * In the simple case, 40% faster getMousePosition performance -- and in more complex cases, significantly more performance improvements. To drop the former improvement, which may affect some applications (as described in the includeXY documentation) simply set: OpenLayers.Events.prototype.includeXY = true; This will restore the 'every element has an xy property always' behavior that was the case beore this patch. r=me,tschaub, work by pgiraud related to (See #1509), and (Closes #1459) git-svn-id: http://svn.openlayers.org/trunk/openlayers@7615 dc9f47b5-9b13-0410-9fdd-eb0c1a62fdaf --- lib/OpenLayers/Control/PanZoomBar.js | 6 +- lib/OpenLayers/Events.js | 86 ++++++++++++++++++++++++---- lib/OpenLayers/Map.js | 5 +- 3 files changed, 81 insertions(+), 16 deletions(-) diff --git a/lib/OpenLayers/Control/PanZoomBar.js b/lib/OpenLayers/Control/PanZoomBar.js index df9f352de4..7047e0682c 100644 --- a/lib/OpenLayers/Control/PanZoomBar.js +++ b/lib/OpenLayers/Control/PanZoomBar.js @@ -165,7 +165,8 @@ OpenLayers.Control.PanZoomBar = OpenLayers.Class(OpenLayers.Control.PanZoom, { "absolute"); this.slider = slider; - this.sliderEvents = new OpenLayers.Events(this, slider, null, true); + this.sliderEvents = new OpenLayers.Events(this, slider, null, true, + {includeXY: true}); this.sliderEvents.on({ "mousedown": this.zoomBarDown, "mousemove": this.zoomBarDrag, @@ -197,7 +198,8 @@ OpenLayers.Control.PanZoomBar = OpenLayers.Class(OpenLayers.Control.PanZoom, { this.zoombarDiv = div; - this.divEvents = new OpenLayers.Events(this, div, null, true); + this.divEvents = new OpenLayers.Events(this, div, null, true, + {includeXY: true}); this.divEvents.on({ "mousedown": this.divClick, "mousemove": this.passEventToSlider, diff --git a/lib/OpenLayers/Events.js b/lib/OpenLayers/Events.js index 334c1f6091..a81a3dba1b 100644 --- a/lib/OpenLayers/Events.js +++ b/lib/OpenLayers/Events.js @@ -392,6 +392,34 @@ OpenLayers.Events = OpenLayers.Class({ */ fallThrough: null, + /** + * APIProperty: includeXY + * {Boolean} Should the .xy property automatically be created for browser + * mouse events? In general, this should be false. If it is true, then + * mouse events will automatically generate a '.xy' property on the + * event object that is passed. (Prior to OpenLayers 2.7, this was true + * by default.) Otherwise, you can call the getMousePosition on the + * relevant events handler on the object available via the 'evt.object' + * property of the evt object. So, for most events, you can call: + * function named(evt) { + * this.xy = this.object.events.getMousePosition(evt) + * } + * + * This option typically defaults to false for performance reasons: + * when creating an events object whose primary purpose is to manage + * relatively positioned mouse events within a div, it may make + * sense to set it to true. + * + * This option is also used to control whether the events object caches + * offsets. If this is false, it will not: the reason for this is that + * it is only expected to be called many times if the includeXY property + * is set to true. If you set this to true, you are expected to clear + * the offset cache manually (using this.clearMouseCache()) if: + * the border of the element changes + * the location of the element in the page changes + */ + includeXY: false, + /** * Constructor: OpenLayers.Events * Construct an OpenLayers.Events object. @@ -402,8 +430,10 @@ OpenLayers.Events = OpenLayers.Class({ * eventTypes - {Array(String)} Array of custom application events * fallThrough - {Boolean} Allow events to fall through after these have * been handled? + * options - {Object} Options for the events object. */ - initialize: function (object, element, eventTypes, fallThrough) { + initialize: function (object, element, eventTypes, fallThrough, options) { + OpenLayers.Util.extend(this, options); this.object = object; this.element = element; this.fallThrough = fallThrough; @@ -691,10 +721,24 @@ OpenLayers.Events = OpenLayers.Class({ * evt - {Event} */ handleBrowserEvent: function (evt) { - evt.xy = this.getMousePosition(evt); + if (this.includeXY) { + evt.xy = this.getMousePosition(evt); + } this.triggerEvent(evt.type, evt); }, + /** + * APIMethod: clearMouseCache + * Clear cached data about the mouse position. This should be called any + * time the element that events are registered on changes position + * within the page. + */ + clearMouseCache: function() { + this.element.scrolls = null; + this.element.lefttop = null; + this.element.offsets = null; + }, + /** * Method: getMousePosition * @@ -706,20 +750,38 @@ OpenLayers.Events = OpenLayers.Class({ * for offsets */ getMousePosition: function (evt) { - if (!this.element.offsets) { - this.element.offsets = OpenLayers.Util.pagePosition(this.element); - this.element.offsets[0] += (document.documentElement.scrollLeft + if (!this.includeXY) { + this.clearMouseCache(); + } else if (!this.element.hasScrollEvent) { + OpenLayers.Event.observe(window, 'scroll', + OpenLayers.Function.bind(this.clearMouseCache, this)); + this.element.hasScrollEvent = true; + } + + if (!this.element.scrolls) { + this.element.scrolls = []; + this.element.scrolls[0] = (document.documentElement.scrollLeft || document.body.scrollLeft); - this.element.offsets[1] += (document.documentElement.scrollTop + this.element.scrolls[1] = (document.documentElement.scrollTop || document.body.scrollTop); } + + if (!this.element.lefttop) { + this.element.lefttop = []; + this.element.lefttop[0] = (document.documentElement.clientLeft || 0); + this.element.lefttop[1] = (document.documentElement.clientTop || 0); + } + + if (!this.element.offsets) { + this.element.offsets = OpenLayers.Util.pagePosition(this.element); + this.element.offsets[0] += this.element.scrolls[0]; + this.element.offsets[1] += this.element.scrolls[1]; + } return new OpenLayers.Pixel( - (evt.clientX + (document.documentElement.scrollLeft - || document.body.scrollLeft)) - this.element.offsets[0] - - (document.documentElement.clientLeft || 0), - (evt.clientY + (document.documentElement.scrollTop - || document.body.scrollTop)) - this.element.offsets[1] - - (document.documentElement.clientTop || 0) + (evt.clientX + this.element.scrolls[0]) - this.element.offsets[0] + - this.element.lefttop[0], + (evt.clientY + this.element.scrolls[1]) - this.element.offsets[1] + - this.element.lefttop[1] ); }, diff --git a/lib/OpenLayers/Map.js b/lib/OpenLayers/Map.js index fbd0bce7b7..e6339be061 100644 --- a/lib/OpenLayers/Map.js +++ b/lib/OpenLayers/Map.js @@ -426,7 +426,8 @@ OpenLayers.Map = OpenLayers.Class({ this.events = new OpenLayers.Events(this, this.div, this.EVENT_TYPES, - this.fallThrough); + this.fallThrough, + {includeXY: true}); this.updateSize(); if(this.eventListeners instanceof Object) { this.events.on(this.eventListeners); @@ -1202,7 +1203,7 @@ OpenLayers.Map = OpenLayers.Class({ */ updateSize: function() { // the div might have moved on the page, also - this.events.element.offsets = null; + this.events.clearMouseCache(); var newSize = this.getCurrentSize(); var oldSize = this.getSize(); if (oldSize == null) {