From 48149833066ccd5fca245ae073f6172cd28ac4a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Lemoine?= Date: Thu, 8 Jan 2015 16:41:51 +0100 Subject: [PATCH 1/2] Also listen on loading images This fixes a bug that occured when an image source was used by multiple maps. In that case the map that didn't load the image wouldn't register a load listener on that image and would therefore not call render to request a re-render of the map. --- .../canvas/canvasimagelayerrenderer.js | 11 ++---- .../canvas/canvasvectorlayerrenderer.js | 4 +-- src/ol/renderer/dom/domimagelayerrenderer.js | 11 ++---- src/ol/renderer/dom/domvectorlayerrenderer.js | 4 +-- src/ol/renderer/layerrenderer.js | 34 +++++++++++++++++-- .../renderer/webgl/webglimagelayerrenderer.js | 11 ++---- .../webgl/webglvectorlayerrenderer.js | 4 +-- 7 files changed, 44 insertions(+), 35 deletions(-) diff --git a/src/ol/renderer/canvas/canvasimagelayerrenderer.js b/src/ol/renderer/canvas/canvasimagelayerrenderer.js index 6fa2e36188..834f0e771f 100644 --- a/src/ol/renderer/canvas/canvasimagelayerrenderer.js +++ b/src/ol/renderer/canvas/canvasimagelayerrenderer.js @@ -1,11 +1,8 @@ goog.provide('ol.renderer.canvas.ImageLayer'); goog.require('goog.asserts'); -goog.require('goog.events'); -goog.require('goog.events.EventType'); goog.require('goog.vec.Mat4'); goog.require('ol.ImageBase'); -goog.require('ol.ImageState'); goog.require('ol.ViewHint'); goog.require('ol.extent'); goog.require('ol.layer.Image'); @@ -117,12 +114,8 @@ ol.renderer.canvas.ImageLayer.prototype.prepareFrame = image = imageSource.getImage( renderedExtent, viewResolution, pixelRatio, projection); if (!goog.isNull(image)) { - var imageState = image.getState(); - if (imageState == ol.ImageState.IDLE) { - goog.events.listenOnce(image, goog.events.EventType.CHANGE, - this.handleImageChange, false, this); - image.load(); - } else if (imageState == ol.ImageState.LOADED) { + var loaded = this.loadImage(image); + if (loaded) { this.image_ = image; } } diff --git a/src/ol/renderer/canvas/canvasvectorlayerrenderer.js b/src/ol/renderer/canvas/canvasvectorlayerrenderer.js index 3fe8912969..b9491a7641 100644 --- a/src/ol/renderer/canvas/canvasvectorlayerrenderer.js +++ b/src/ol/renderer/canvas/canvasvectorlayerrenderer.js @@ -149,7 +149,7 @@ ol.renderer.canvas.VectorLayer.prototype.forEachFeatureAtPixel = * @param {goog.events.Event} event Image style change event. * @private */ -ol.renderer.canvas.VectorLayer.prototype.handleImageChange_ = +ol.renderer.canvas.VectorLayer.prototype.handleStyleImageChange_ = function(event) { this.renderIfReadyAndVisible(); }; @@ -273,7 +273,7 @@ ol.renderer.canvas.VectorLayer.prototype.renderFeature = loading = ol.renderer.vector.renderFeature( replayGroup, feature, styles[i], ol.renderer.vector.getSquaredTolerance(resolution, pixelRatio), - this.handleImageChange_, this) || loading; + this.handleStyleImageChange_, this) || loading; } return loading; }; diff --git a/src/ol/renderer/dom/domimagelayerrenderer.js b/src/ol/renderer/dom/domimagelayerrenderer.js index f41162b049..624fa8f0c2 100644 --- a/src/ol/renderer/dom/domimagelayerrenderer.js +++ b/src/ol/renderer/dom/domimagelayerrenderer.js @@ -3,11 +3,8 @@ goog.provide('ol.renderer.dom.ImageLayer'); goog.require('goog.asserts'); goog.require('goog.dom'); goog.require('goog.dom.TagName'); -goog.require('goog.events'); -goog.require('goog.events.EventType'); goog.require('goog.vec.Mat4'); goog.require('ol.ImageBase'); -goog.require('ol.ImageState'); goog.require('ol.ViewHint'); goog.require('ol.dom'); goog.require('ol.extent'); @@ -113,12 +110,8 @@ ol.renderer.dom.ImageLayer.prototype.prepareFrame = var image_ = imageSource.getImage(renderedExtent, viewResolution, frameState.pixelRatio, projection); if (!goog.isNull(image_)) { - var imageState = image_.getState(); - if (imageState == ol.ImageState.IDLE) { - goog.events.listenOnce(image_, goog.events.EventType.CHANGE, - this.handleImageChange, false, this); - image_.load(); - } else if (imageState == ol.ImageState.LOADED) { + var loaded = this.loadImage(image_); + if (loaded) { image = image_; } } diff --git a/src/ol/renderer/dom/domvectorlayerrenderer.js b/src/ol/renderer/dom/domvectorlayerrenderer.js index 5d2d802d21..7b0026fd42 100644 --- a/src/ol/renderer/dom/domvectorlayerrenderer.js +++ b/src/ol/renderer/dom/domvectorlayerrenderer.js @@ -189,7 +189,7 @@ ol.renderer.dom.VectorLayer.prototype.forEachFeatureAtPixel = * @param {goog.events.Event} event Image style change event. * @private */ -ol.renderer.dom.VectorLayer.prototype.handleImageChange_ = +ol.renderer.dom.VectorLayer.prototype.handleStyleImageChange_ = function(event) { this.renderIfReadyAndVisible(); }; @@ -313,7 +313,7 @@ ol.renderer.dom.VectorLayer.prototype.renderFeature = loading = ol.renderer.vector.renderFeature( replayGroup, feature, styles[i], ol.renderer.vector.getSquaredTolerance(resolution, pixelRatio), - this.handleImageChange_, this) || loading; + this.handleStyleImageChange_, this) || loading; } return loading; }; diff --git a/src/ol/renderer/layerrenderer.js b/src/ol/renderer/layerrenderer.js index 356561bb0f..af79a3d9a2 100644 --- a/src/ol/renderer/layerrenderer.js +++ b/src/ol/renderer/layerrenderer.js @@ -2,6 +2,8 @@ goog.provide('ol.renderer.Layer'); goog.require('goog.Disposable'); goog.require('goog.asserts'); +goog.require('goog.events'); +goog.require('goog.events.EventType'); goog.require('ol.ImageState'); goog.require('ol.TileRange'); goog.require('ol.TileState'); @@ -84,9 +86,9 @@ ol.renderer.Layer.prototype.getMapRenderer = function() { /** * Handle changes in image state. * @param {goog.events.Event} event Image change event. - * @protected + * @private */ -ol.renderer.Layer.prototype.handleImageChange = function(event) { +ol.renderer.Layer.prototype.handleImageChange_ = function(event) { var image = /** @type {ol.Image} */ (event.target); if (image.getState() === ol.ImageState.LOADED) { this.renderIfReadyAndVisible(); @@ -94,6 +96,34 @@ ol.renderer.Layer.prototype.handleImageChange = function(event) { }; +/** + * Load the image if not already loaded, and register the image change + * listener if needed. + * @param {ol.ImageBase} image Image. + * @return {boolean} `true` if the image is already loaded, `false` + * otherwise. + * @protected + */ +ol.renderer.Layer.prototype.loadImage = function(image) { + var imageState = image.getState(); + if (imageState != ol.ImageState.LOADED && + imageState != ol.ImageState.ERROR) { + // the image is either "idle" or "loading", register the change + // listener (a noop if the listener was already registered) + goog.asserts.assert(imageState == ol.ImageState.IDLE || + imageState == ol.ImageState.LOADING); + goog.events.listenOnce(image, goog.events.EventType.CHANGE, + this.handleImageChange_, false, this); + } + if (imageState == ol.ImageState.IDLE) { + image.load(); + imageState = image.getState(); + goog.asserts.assert(imageState == ol.ImageState.LOADING); + } + return imageState == ol.ImageState.LOADED; +}; + + /** * @protected */ diff --git a/src/ol/renderer/webgl/webglimagelayerrenderer.js b/src/ol/renderer/webgl/webglimagelayerrenderer.js index 61dd3f6a2c..d1f9d44dd8 100644 --- a/src/ol/renderer/webgl/webglimagelayerrenderer.js +++ b/src/ol/renderer/webgl/webglimagelayerrenderer.js @@ -1,14 +1,11 @@ goog.provide('ol.renderer.webgl.ImageLayer'); goog.require('goog.asserts'); -goog.require('goog.events'); -goog.require('goog.events.EventType'); goog.require('goog.vec.Mat4'); goog.require('goog.webgl'); goog.require('ol.Coordinate'); goog.require('ol.Extent'); goog.require('ol.ImageBase'); -goog.require('ol.ImageState'); goog.require('ol.ViewHint'); goog.require('ol.extent'); goog.require('ol.layer.Image'); @@ -133,12 +130,8 @@ ol.renderer.webgl.ImageLayer.prototype.prepareFrame = var image_ = imageSource.getImage(renderedExtent, viewResolution, frameState.pixelRatio, projection); if (!goog.isNull(image_)) { - var imageState = image_.getState(); - if (imageState == ol.ImageState.IDLE) { - goog.events.listenOnce(image_, goog.events.EventType.CHANGE, - this.handleImageChange, false, this); - image_.load(); - } else if (imageState == ol.ImageState.LOADED) { + var loaded = this.loadImage(image_); + if (loaded) { image = image_; texture = this.createTexture_(image_); if (!goog.isNull(this.texture)) { diff --git a/src/ol/renderer/webgl/webglvectorlayerrenderer.js b/src/ol/renderer/webgl/webglvectorlayerrenderer.js index e2a685a633..4bb6da40c1 100644 --- a/src/ol/renderer/webgl/webglvectorlayerrenderer.js +++ b/src/ol/renderer/webgl/webglvectorlayerrenderer.js @@ -108,7 +108,7 @@ ol.renderer.webgl.VectorLayer.prototype.forEachFeatureAtPixel = * @param {goog.events.Event} event Image style change event. * @private */ -ol.renderer.webgl.VectorLayer.prototype.handleImageChange_ = +ol.renderer.webgl.VectorLayer.prototype.handleStyleImageChange_ = function(event) { this.renderIfReadyAndVisible(); }; @@ -232,7 +232,7 @@ ol.renderer.webgl.VectorLayer.prototype.renderFeature = loading = ol.renderer.vector.renderFeature( replayGroup, feature, styles[i], ol.renderer.vector.getSquaredTolerance(resolution, pixelRatio), - this.handleImageChange_, this) || loading; + this.handleStyleImageChange_, this) || loading; } return loading; }; From 0f486e86f7cceecfdd2b42e919e59c83fc192da3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Lemoine?= Date: Fri, 9 Jan 2015 09:45:25 +0100 Subject: [PATCH 2/2] Add tests for ol.renderer.Layer#loadImage --- test/spec/ol/renderer/layerrenderer.test.js | 89 +++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 test/spec/ol/renderer/layerrenderer.test.js diff --git a/test/spec/ol/renderer/layerrenderer.test.js b/test/spec/ol/renderer/layerrenderer.test.js new file mode 100644 index 0000000000..d16f242ecb --- /dev/null +++ b/test/spec/ol/renderer/layerrenderer.test.js @@ -0,0 +1,89 @@ +goog.provide('ol.test.renderer.Layer'); + +describe('ol.renderer.Layer', function() { + var renderer; + var eventType = goog.events.EventType.CHANGE; + + beforeEach(function() { + var map = new ol.Map({}); + var mapRenderer = map.getRenderer(); + var layer = new ol.layer.Layer({}); + renderer = new ol.renderer.Layer(mapRenderer, layer); + }); + + describe('#loadImage', function() { + var image; + var imageLoadFunction; + + beforeEach(function() { + var extent = []; + var resolution = 1; + var pixelRatio = 1; + var attributions = []; + var src = ''; + var crossOrigin = ''; + imageLoadFunction = sinon.spy(); + image = new ol.Image(extent, resolution, pixelRatio, attributions, + src, crossOrigin, imageLoadFunction); + }); + + describe('load IDLE image', function() { + + it('returns false', function() { + var loaded = renderer.loadImage(image); + expect(loaded).to.be(false); + }); + + it('registers a listener', function() { + renderer.loadImage(image); + var listeners = image.getListeners(eventType, false); + expect(listeners).to.have.length(1); + }); + + }); + + describe('load LOADED image', function() { + + it('returns true', function() { + image.state = ol.ImageState.LOADED; + var loaded = renderer.loadImage(image); + expect(loaded).to.be(true); + }); + + it('does not register a listener', function() { + image.state = ol.ImageState.LOADED; + var loaded = renderer.loadImage(image); + expect(loaded).to.be(true); + }); + + }); + + describe('load LOADING image', function() { + + beforeEach(function() { + renderer.loadImage(image); + expect(image.getState()).to.be(ol.ImageState.LOADING); + }); + + it('returns false', function() { + var loaded = renderer.loadImage(image); + expect(loaded).to.be(false); + }); + + it('does not register a new listener', function() { + renderer.loadImage(image); + var listeners = image.getListeners(eventType, false); + expect(listeners).to.have.length(1); + }); + + }); + + }); +}); + +goog.require('goog.events.EventType'); +goog.require('ol.Image'); +goog.require('ol.ImageState'); +goog.require('ol.Map'); +goog.require('ol.layer.Layer'); +goog.require('ol.renderer.Layer');