From 46ea218d0f250c9e7c808e209eb8173668499cd4 Mon Sep 17 00:00:00 2001 From: Bruno Binet Date: Wed, 7 Aug 2013 15:55:54 +0200 Subject: [PATCH 1/3] Make layer renderers more stupid Layer renderers should not be responsible for listening to layer properties change and triggering a render. Layer change events are now forwarded to the map which will trigger a render. --- src/ol/layer/layerbase.js | 29 +++++++ src/ol/layer/layergroup.js | 26 ++----- src/ol/renderer/layerrenderer.js | 83 --------------------- src/ol/renderer/webgl/webgllayerrenderer.js | 36 --------- 4 files changed, 34 insertions(+), 140 deletions(-) diff --git a/src/ol/layer/layerbase.js b/src/ol/layer/layerbase.js index 1b7d0abdcd..1d154fd40a 100644 --- a/src/ol/layer/layerbase.js +++ b/src/ol/layer/layerbase.js @@ -1,3 +1,5 @@ +goog.require('goog.events'); +goog.require('goog.events.EventType'); goog.provide('ol.layer.LayerBase'); goog.provide('ol.layer.LayerProperty'); goog.provide('ol.layer.LayerState'); @@ -59,10 +61,29 @@ ol.layer.LayerBase = function(options) { this.setValues(values); + goog.events.listen(this, [ + ol.Object.getChangeEventType(ol.layer.LayerProperty.BRIGHTNESS), + ol.Object.getChangeEventType(ol.layer.LayerProperty.CONTRAST), + ol.Object.getChangeEventType(ol.layer.LayerProperty.HUE), + ol.Object.getChangeEventType(ol.layer.LayerProperty.OPACITY), + ol.Object.getChangeEventType(ol.layer.LayerProperty.SATURATION), + ol.Object.getChangeEventType(ol.layer.LayerProperty.VISIBLE), + goog.events.EventType.LOAD + ], + this.handleLayerChange, false, this); + }; goog.inherits(ol.layer.LayerBase, ol.Object); +/** + * @protected + */ +ol.layer.LayerBase.prototype.dispatchChangeEvent = function() { + this.dispatchEvent(goog.events.EventType.CHANGE); +}; + + /** * @return {number} Brightness. */ @@ -179,6 +200,14 @@ goog.exportProperty( ol.layer.LayerBase.prototype.getVisible); +/** + * @protected + */ +ol.layer.LayerBase.prototype.handleLayerChange = function() { + this.dispatchChangeEvent(); +}; + + /** * @return {boolean} Is ready. */ diff --git a/src/ol/layer/layergroup.js b/src/ol/layer/layergroup.js index 85dd6c85bd..163500305f 100644 --- a/src/ol/layer/layergroup.js +++ b/src/ol/layer/layergroup.js @@ -92,11 +92,11 @@ ol.layer.LayerGroup.prototype.handleLayersChanged_ = function(event) { layer = layersArray[i]; this.listenerKeys_[goog.getUid(layer).toString()] = goog.events.listen(layer, goog.events.EventType.CHANGE, - this.handleLayerChange_, false, this); + this.handleLayerChange, false, this); } } - this.dispatchChangeEvent_(); + this.dispatchChangeEvent(); }; @@ -107,9 +107,9 @@ ol.layer.LayerGroup.prototype.handleLayersChanged_ = function(event) { ol.layer.LayerGroup.prototype.handleLayersAdd_ = function(collectionEvent) { var layer = /** @type {ol.layer.LayerBase} */ (collectionEvent.elem); this.listenerKeys_[goog.getUid(layer).toString()] = goog.events.listen( - layer, goog.events.EventType.CHANGE, this.handleLayerChange_, false, + layer, goog.events.EventType.CHANGE, this.handleLayerChange, false, this); - this.dispatchChangeEvent_(); + this.dispatchChangeEvent(); }; @@ -122,23 +122,7 @@ ol.layer.LayerGroup.prototype.handleLayersRemove_ = function(collectionEvent) { var key = goog.getUid(layer).toString(); goog.events.unlistenByKey(this.listenerKeys_[key]); delete this.listenerKeys_[key]; - this.dispatchChangeEvent_(); -}; - - -/** - * @private - */ -ol.layer.LayerGroup.prototype.handleLayerChange_ = function() { - this.dispatchChangeEvent_(); -}; - - -/** - * @private - */ -ol.layer.LayerGroup.prototype.dispatchChangeEvent_ = function() { - this.dispatchEvent(goog.events.EventType.CHANGE); + this.dispatchChangeEvent(); }; diff --git a/src/ol/renderer/layerrenderer.js b/src/ol/renderer/layerrenderer.js index 0eff0ddd05..94605f5c3d 100644 --- a/src/ol/renderer/layerrenderer.js +++ b/src/ol/renderer/layerrenderer.js @@ -1,19 +1,13 @@ goog.provide('ol.renderer.Layer'); goog.require('goog.Disposable'); -goog.require('goog.events'); -goog.require('goog.events.EventType'); -goog.require('ol.Attribution'); -goog.require('ol.Coordinate'); goog.require('ol.FrameState'); goog.require('ol.Image'); goog.require('ol.ImageState'); -goog.require('ol.Object'); goog.require('ol.Tile'); goog.require('ol.TileRange'); goog.require('ol.TileState'); goog.require('ol.layer.Layer'); -goog.require('ol.layer.LayerProperty'); goog.require('ol.layer.LayerState'); goog.require('ol.source.Source'); goog.require('ol.source.TileSource'); @@ -42,32 +36,6 @@ ol.renderer.Layer = function(mapRenderer, layer) { */ this.layer_ = layer; - goog.events.listen(this.layer_, - ol.Object.getChangeEventType(ol.layer.LayerProperty.BRIGHTNESS), - this.handleLayerBrightnessChange, false, this); - - goog.events.listen(this.layer_, - ol.Object.getChangeEventType(ol.layer.LayerProperty.CONTRAST), - this.handleLayerContrastChange, false, this); - - goog.events.listen(this.layer_, - ol.Object.getChangeEventType(ol.layer.LayerProperty.HUE), - this.handleLayerHueChange, false, this); - - goog.events.listen(this.layer_, goog.events.EventType.LOAD, - this.handleLayerLoad, false, this); - - goog.events.listen(this.layer_, - ol.Object.getChangeEventType(ol.layer.LayerProperty.OPACITY), - this.handleLayerOpacityChange, false, this); - - goog.events.listen(this.layer_, - ol.Object.getChangeEventType(ol.layer.LayerProperty.SATURATION), - this.handleLayerSaturationChange, false, this); - - goog.events.listen(this.layer_, - ol.Object.getChangeEventType(ol.layer.LayerProperty.VISIBLE), - this.handleLayerVisibleChange, false, this); }; goog.inherits(ol.renderer.Layer, goog.Disposable); @@ -120,24 +88,6 @@ ol.renderer.Layer.prototype.getMapRenderer = function() { }; -/** - * @protected - */ -ol.renderer.Layer.prototype.handleLayerBrightnessChange = goog.nullFunction; - - -/** - * @protected - */ -ol.renderer.Layer.prototype.handleLayerContrastChange = goog.nullFunction; - - -/** - * @protected - */ -ol.renderer.Layer.prototype.handleLayerHueChange = goog.nullFunction; - - /** * Handle changes in image state. * @param {goog.events.Event} event Image change event. @@ -151,39 +101,6 @@ ol.renderer.Layer.prototype.handleImageChange = function(event) { }; -/** - * @protected - */ -ol.renderer.Layer.prototype.handleLayerLoad = function() { - this.renderIfReadyAndVisible(); -}; - - -/** - * @protected - */ -ol.renderer.Layer.prototype.handleLayerOpacityChange = function() { - this.renderIfReadyAndVisible(); -}; - - -/** - * @protected - */ -ol.renderer.Layer.prototype.handleLayerSaturationChange = goog.nullFunction; - - -/** - * @protected - */ -ol.renderer.Layer.prototype.handleLayerVisibleChange = function() { - var layer = this.getLayer(); - if (layer.isReady()) { - this.getMap().render(); - } -}; - - /** * @param {ol.FrameState} frameState Frame state. * @param {ol.layer.LayerState} layerState Layer state. diff --git a/src/ol/renderer/webgl/webgllayerrenderer.js b/src/ol/renderer/webgl/webgllayerrenderer.js index 9b9d1e944a..578985681f 100644 --- a/src/ol/renderer/webgl/webgllayerrenderer.js +++ b/src/ol/renderer/webgl/webgllayerrenderer.js @@ -187,42 +187,6 @@ ol.renderer.webgl.Layer.prototype.getProjectionMatrix = function() { }; -/** - * @inheritDoc - */ -ol.renderer.webgl.Layer.prototype.handleLayerBrightnessChange = function() { - this.updateBrightnessMatrix_(); - this.renderIfReadyAndVisible(); -}; - - -/** - * @inheritDoc - */ -ol.renderer.webgl.Layer.prototype.handleLayerContrastChange = function() { - this.updateContrastMatrix_(); - this.renderIfReadyAndVisible(); -}; - - -/** - * @inheritDoc - */ -ol.renderer.webgl.Layer.prototype.handleLayerHueChange = function() { - this.updateHueMatrix_(); - this.renderIfReadyAndVisible(); -}; - - -/** - * @inheritDoc - */ -ol.renderer.webgl.Layer.prototype.handleLayerSaturationChange = function() { - this.updateSaturationMatrix_(); - this.renderIfReadyAndVisible(); -}; - - /** * Handle webglcontextlost. */ From 8b435059f7a869e56b1b5289e403fa290cf556f7 Mon Sep 17 00:00:00 2001 From: Bruno Binet Date: Mon, 12 Aug 2013 11:16:52 +0200 Subject: [PATCH 2/3] Update color matrices based on framestate values In getColorMatrix method, we'll update the color matrices only if layerstate color properties has changed in the framestate. --- src/ol/renderer/webgl/webgllayerrenderer.js | 97 ++++++++++----------- src/ol/renderer/webgl/webglmaprenderer.js | 7 +- 2 files changed, 54 insertions(+), 50 deletions(-) diff --git a/src/ol/renderer/webgl/webgllayerrenderer.js b/src/ol/renderer/webgl/webgllayerrenderer.js index 578985681f..f99258937b 100644 --- a/src/ol/renderer/webgl/webgllayerrenderer.js +++ b/src/ol/renderer/webgl/webgllayerrenderer.js @@ -59,37 +59,51 @@ ol.renderer.webgl.Layer = function(mapRenderer, layer) { /** * @private - * @type {boolean} + * @type {number|undefined} */ - this.colorMatrixDirty_ = true; + this.brightness_ = undefined; /** * @private * @type {!goog.vec.Mat4.Float32} */ this.brightnessMatrix_ = goog.vec.Mat4.createFloat32(); - this.updateBrightnessMatrix_(); + + /** + * @private + * @type {number|undefined} + */ + this.contrast_ = undefined; /** * @private * @type {!goog.vec.Mat4.Float32} */ this.contrastMatrix_ = goog.vec.Mat4.createFloat32(); - this.updateContrastMatrix_(); + + /** + * @private + * @type {number|undefined} + */ + this.hue_ = undefined; /** * @private * @type {!goog.vec.Mat4.Float32} */ this.hueMatrix_ = goog.vec.Mat4.createFloat32(); - this.updateHueMatrix_(); + + /** + * @private + * @type {number|undefined} + */ + this.saturation_ = undefined; /** * @private * @type {!goog.vec.Mat4.Float32} */ this.saturationMatrix_ = goog.vec.Mat4.createFloat32(); - this.updateSaturationMatrix_(); }; goog.inherits(ol.renderer.webgl.Layer, ol.renderer.Layer); @@ -144,10 +158,36 @@ ol.renderer.webgl.Layer.prototype.bindFramebuffer = /** + * @param {number} brightness Brightness. + * @param {number} contrast Contrast. + * @param {number} hue Hue. + * @param {number} saturation Saturation. * @return {!goog.vec.Mat4.Float32} Color matrix. */ -ol.renderer.webgl.Layer.prototype.getColorMatrix = function() { - if (this.colorMatrixDirty_) { +ol.renderer.webgl.Layer.prototype.getColorMatrix = function( + brightness, contrast, hue, saturation) { + var colorMatrixDirty = false; + if (brightness !== this.brightness_) { + ol.vec.Mat4.makeBrightness(this.brightnessMatrix_, brightness); + this.brightness_ = brightness; + colorMatrixDirty = true; + } + if (contrast !== this.contrast_) { + ol.vec.Mat4.makeContrast(this.contrastMatrix_, contrast); + this.contrast_ = contrast; + colorMatrixDirty = true; + } + if (hue !== this.hue_) { + ol.vec.Mat4.makeHue(this.hueMatrix_, hue); + this.hue_ = hue; + colorMatrixDirty = true; + } + if (saturation !== this.saturation_) { + ol.vec.Mat4.makeSaturation(this.saturationMatrix_, saturation); + this.saturation_ = saturation; + colorMatrixDirty = true; + } + if (colorMatrixDirty) { this.updateColorMatrix_(); } return this.colorMatrix_; @@ -197,16 +237,6 @@ ol.renderer.webgl.Layer.prototype.handleWebGLContextLost = function() { }; -/** - * @private - */ -ol.renderer.webgl.Layer.prototype.updateBrightnessMatrix_ = function() { - var brightness = this.getLayer().getBrightness(); - ol.vec.Mat4.makeBrightness(this.brightnessMatrix_, brightness); - this.colorMatrixDirty_ = true; -}; - - /** * @private */ @@ -217,35 +247,4 @@ ol.renderer.webgl.Layer.prototype.updateColorMatrix_ = function() { goog.vec.Mat4.multMat(colorMatrix, this.brightnessMatrix_, colorMatrix); goog.vec.Mat4.multMat(colorMatrix, this.saturationMatrix_, colorMatrix); goog.vec.Mat4.multMat(colorMatrix, this.hueMatrix_, colorMatrix); - this.colorMatrixDirty_ = false; -}; - - -/** - * @private - */ -ol.renderer.webgl.Layer.prototype.updateContrastMatrix_ = function() { - var contrast = this.getLayer().getContrast(); - ol.vec.Mat4.makeContrast(this.contrastMatrix_, contrast); - this.colorMatrixDirty_ = true; -}; - - -/** - * @private - */ -ol.renderer.webgl.Layer.prototype.updateHueMatrix_ = function() { - var hue = this.getLayer().getHue(); - ol.vec.Mat4.makeHue(this.hueMatrix_, hue); - this.colorMatrixDirty_ = true; -}; - - -/** - * @private - */ -ol.renderer.webgl.Layer.prototype.updateSaturationMatrix_ = function() { - var saturation = this.getLayer().getSaturation(); - ol.vec.Mat4.makeSaturation(this.saturationMatrix_, saturation); - this.colorMatrixDirty_ = true; }; diff --git a/src/ol/renderer/webgl/webglmaprenderer.js b/src/ol/renderer/webgl/webglmaprenderer.js index d3d117e174..085ce33ab7 100644 --- a/src/ol/renderer/webgl/webglmaprenderer.js +++ b/src/ol/renderer/webgl/webglmaprenderer.js @@ -643,7 +643,12 @@ ol.renderer.webgl.Map.prototype.renderFrame = function(frameState) { layerRenderer.getProjectionMatrix()); if (useColor) { gl.uniformMatrix4fv(locations.u_colorMatrix, false, - layerRenderer.getColorMatrix()); + layerRenderer.getColorMatrix( + layerState.brightness, + layerState.contrast, + layerState.hue, + layerState.saturation + )); } gl.uniform1f(locations.u_opacity, layerState.opacity); gl.bindTexture(goog.webgl.TEXTURE_2D, layerRenderer.getTexture()); From 88da6da3a7bbea5d30e17861e1e683b4a9a68656 Mon Sep 17 00:00:00 2001 From: Bruno Binet Date: Mon, 12 Aug 2013 11:51:42 +0200 Subject: [PATCH 3/3] Trigger CHANGE event only when required For example only when layer is both ready and visible. --- src/ol/layer/layerbase.js | 19 +++++++++++++++++-- src/ol/layer/layergroup.js | 18 ++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/ol/layer/layerbase.js b/src/ol/layer/layerbase.js index 1d154fd40a..3273e744be 100644 --- a/src/ol/layer/layerbase.js +++ b/src/ol/layer/layerbase.js @@ -67,11 +67,14 @@ ol.layer.LayerBase = function(options) { ol.Object.getChangeEventType(ol.layer.LayerProperty.HUE), ol.Object.getChangeEventType(ol.layer.LayerProperty.OPACITY), ol.Object.getChangeEventType(ol.layer.LayerProperty.SATURATION), - ol.Object.getChangeEventType(ol.layer.LayerProperty.VISIBLE), goog.events.EventType.LOAD ], this.handleLayerChange, false, this); + goog.events.listen(this, + ol.Object.getChangeEventType(ol.layer.LayerProperty.VISIBLE), + this.handleLayerVisibleChange, false, this); + }; goog.inherits(ol.layer.LayerBase, ol.Object); @@ -204,7 +207,19 @@ goog.exportProperty( * @protected */ ol.layer.LayerBase.prototype.handleLayerChange = function() { - this.dispatchChangeEvent(); + if (this.getVisible() && this.isReady()) { + this.dispatchChangeEvent(); + } +}; + + +/** + * @protected + */ +ol.layer.LayerBase.prototype.handleLayerVisibleChange = function() { + if (this.isReady()) { + this.dispatchChangeEvent(); + } }; diff --git a/src/ol/layer/layergroup.js b/src/ol/layer/layergroup.js index 163500305f..b93c0e40b2 100644 --- a/src/ol/layer/layergroup.js +++ b/src/ol/layer/layergroup.js @@ -66,6 +66,24 @@ ol.layer.LayerGroup = function(opt_options) { goog.inherits(ol.layer.LayerGroup, ol.layer.LayerBase); +/** + * @inheritDoc + */ +ol.layer.LayerGroup.prototype.handleLayerChange = function() { + if (this.getVisible()) { + this.dispatchChangeEvent(); + } +}; + + +/** + * @inheritDoc + */ +ol.layer.LayerGroup.prototype.handleLayerVisibleChange = function() { + this.dispatchChangeEvent(); +}; + + /** * @param {goog.events.Event} event Event. * @private