diff --git a/examples/icon-sprite-webgl.js b/examples/icon-sprite-webgl.js index 45d78a80fd..ce2adb7649 100644 --- a/examples/icon-sprite-webgl.js +++ b/examples/icon-sprite-webgl.js @@ -59,7 +59,7 @@ const style = { rotateWithView: false, offset: [ 0, - 9 + 0 ], textureCoord: [ 'match', @@ -144,7 +144,7 @@ map.addLayer( const info = document.getElementById('info'); map.on('pointermove', function(evt) { - if (map.getView().getInteracting()) { + if (map.getView().getInteracting() || map.getView().getAnimating()) { return; } const pixel = evt.pixel; diff --git a/examples/webgl-points-layer.js b/examples/webgl-points-layer.js index ae04790a71..3d3c980a41 100644 --- a/examples/webgl-points-layer.js +++ b/examples/webgl-points-layer.js @@ -124,6 +124,7 @@ function refreshLayer(newStyle) { if (previousLayer) { map.removeLayer(previousLayer); + previousLayer.dispose(); } literalStyle = newStyle; } diff --git a/src/ol/layer/Base.js b/src/ol/layer/Base.js index 7155040289..21a44fdc36 100644 --- a/src/ol/layer/Base.js +++ b/src/ol/layer/Base.js @@ -320,6 +320,17 @@ class BaseLayer extends BaseObject { setZIndex(zindex) { this.set(LayerProperty.Z_INDEX, zindex); } + + /** + * @inheritDoc + */ + disposeInternal() { + if (this.state_) { + this.state_.layer = null; + this.state_ = null; + } + super.disposeInternal(); + } } diff --git a/src/ol/layer/Layer.js b/src/ol/layer/Layer.js index 0291785b19..bef87a803e 100644 --- a/src/ol/layer/Layer.js +++ b/src/ol/layer/Layer.js @@ -287,6 +287,13 @@ class Layer extends BaseLayer { return null; } + /** + * @inheritDoc + */ + disposeInternal() { + this.setSource(null); + super.disposeInternal(); + } } diff --git a/src/ol/layer/WebGLPoints.js b/src/ol/layer/WebGLPoints.js index c3bdb80b10..a49ff9e21e 100644 --- a/src/ol/layer/WebGLPoints.js +++ b/src/ol/layer/WebGLPoints.js @@ -57,6 +57,9 @@ import Layer from './Layer.js'; * } * ``` * + * **Important: a `WebGLPoints` layer must be manually disposed when removed, otherwise the underlying WebGL context + * will not be garbage collected.** + * * Note that any property set in the options is set as a {@link module:ol/Object~BaseObject} * property on the layer object; for example, setting `title: 'My Title'` in the * options means that `title` is observable, and has get/set accessors. @@ -100,6 +103,15 @@ class WebGLPointsLayer extends Layer { attributes: this.parseResult_.attributes }); } + + /** + * + * @inheritDoc + */ + disposeInternal() { + this.renderer_.dispose(); + super.disposeInternal(); + } } export default WebGLPointsLayer; diff --git a/src/ol/renderer/webgl/Layer.js b/src/ol/renderer/webgl/Layer.js index f7f1848268..b3f6072fec 100644 --- a/src/ol/renderer/webgl/Layer.js +++ b/src/ol/renderer/webgl/Layer.js @@ -71,7 +71,7 @@ class WebGLLayerRenderer extends LayerRenderer { * @inheritDoc */ disposeInternal() { - this.helper.disposeInternal(); + this.helper.dispose(); super.disposeInternal(); } diff --git a/src/ol/renderer/webgl/PointsLayer.js b/src/ol/renderer/webgl/PointsLayer.js index d0ae7f1140..868f9b7473 100644 --- a/src/ol/renderer/webgl/PointsLayer.js +++ b/src/ol/renderer/webgl/PointsLayer.js @@ -19,13 +19,23 @@ import {getUid} from '../../util.js'; import WebGLRenderTarget from '../../webgl/RenderTarget.js'; import {assert} from '../../asserts.js'; import BaseVector from '../../layer/BaseVector.js'; +import {listen, unlistenByKey} from '../../events.js'; +import VectorEventType from '../../source/VectorEventType.js'; /** * @typedef {Object} CustomAttribute A description of a custom attribute to be passed on to the GPU, with a value different * for each feature. * @property {string} name Attribute name. - * @property {function(import("../../Feature").default):number} callback This callback computes the numerical value of the - * attribute for a given feature. + * @property {function(import("../../Feature").default, Object):number} callback This callback computes the numerical value of the + * attribute for a given feature (properties are available as 2nd arg for quicker access). + */ + +/** + * @typedef {Object} FeatureCacheItem Object that holds a reference to a feature, its geometry and properties. Used to optimize + * rebuildBuffers by accessing these objects quicker. + * @property {import("../../Feature").default} feature Feature + * @property {Object} properties Feature properties + * @property {import("../../geom").Geometry} geometry Feature geometry */ /** @@ -269,6 +279,72 @@ class WebGLPointsLayerRenderer extends WebGLLayerRenderer { this.getLayer().changed(); } }.bind(this)); + + /** + * This object will be updated when the source changes. Key is uid. + * @type {Object} + * @private + */ + this.featureCache_ = {}; + + /** + * Amount of features in the cache. + * @type {number} + * @private + */ + this.featureCount_ = 0; + + const source = this.getLayer().getSource(); + this.sourceListenKeys_ = [ + listen(source, VectorEventType.ADDFEATURE, this.handleSourceFeatureAdded_, this), + listen(source, VectorEventType.CHANGEFEATURE, this.handleSourceFeatureChanged_, this), + listen(source, VectorEventType.REMOVEFEATURE, this.handleSourceFeatureDelete_, this) + ]; + source.forEachFeature(function(feature) { + this.featureCache_[getUid(feature)] = { + feature: feature, + properties: feature.getProperties(), + geometry: feature.getGeometry() + }; + this.featureCount_++; + }.bind(this)); + } + + /** + * @param {import("../../source/Vector.js").VectorSourceEvent} event Event. + * @private + */ + handleSourceFeatureAdded_(event) { + const feature = event.feature; + this.featureCache_[getUid(feature)] = { + feature: feature, + properties: feature.getProperties(), + geometry: feature.getGeometry() + }; + this.featureCount_++; + } + + /** + * @param {import("../../source/Vector.js").VectorSourceEvent} event Event. + * @private + */ + handleSourceFeatureChanged_(event) { + const feature = event.feature; + this.featureCache_[getUid(feature)] = { + feature: feature, + properties: feature.getProperties(), + geometry: feature.getGeometry() + }; + } + + /** + * @param {import("../../source/Vector.js").VectorSourceEvent} event Event. + * @private + */ + handleSourceFeatureDelete_(event) { + const feature = event.feature; + delete this.featureCache_[getUid(feature)]; + this.featureCount_--; } /** @@ -343,45 +419,41 @@ class WebGLPointsLayerRenderer extends WebGLLayerRenderer { * @private */ rebuildBuffers_(frameState) { - const layer = this.getLayer(); - const vectorSource = layer.getSource(); - // saves the projection transform for the current frame state const projectionTransform = createTransform(); this.helper.makeProjectionTransform(frameState, projectionTransform); - const features = vectorSource.getFeatures(); - // here we anticipate the amount of render instructions that we well generate // this can be done since we know that for normal render we only have x, y as base instructions, // and x, y, r, g, b, a and featureUid for hit render instructions // and we also know the amount of custom attributes to append to these - const totalInstructionsCount = (2 + this.customAttributes.length) * features.length; + const totalInstructionsCount = (2 + this.customAttributes.length) * this.featureCount_; if (!this.renderInstructions_ || this.renderInstructions_.length !== totalInstructionsCount) { this.renderInstructions_ = new Float32Array(totalInstructionsCount); } if (this.hitDetectionEnabled_) { - const totalHitInstructionsCount = (7 + this.customAttributes.length) * features.length; + const totalHitInstructionsCount = (7 + this.customAttributes.length) * this.featureCount_; if (!this.hitRenderInstructions_ || this.hitRenderInstructions_.length !== totalHitInstructionsCount) { this.hitRenderInstructions_ = new Float32Array(totalHitInstructionsCount); } } // loop on features to fill the buffer - let feature; + let featureCache, geometry; const tmpCoords = []; const tmpColor = []; let renderIndex = 0; let hitIndex = 0; let hitColor; - for (let i = 0; i < features.length; i++) { - feature = features[i]; - if (!feature.getGeometry() || feature.getGeometry().getType() !== GeometryType.POINT) { + for (const featureUid in this.featureCache_) { + featureCache = this.featureCache_[featureUid]; + geometry = /** @type {import("../../geom").Point} */(featureCache.geometry); + if (!geometry || geometry.getType() !== GeometryType.POINT) { continue; } - tmpCoords[0] = feature.getGeometry().getFlatCoordinates()[0]; - tmpCoords[1] = feature.getGeometry().getFlatCoordinates()[1]; + tmpCoords[0] = geometry.getFlatCoordinates()[0]; + tmpCoords[1] = geometry.getFlatCoordinates()[1]; applyTransform(projectionTransform, tmpCoords); hitColor = colorEncodeId(hitIndex + 6, tmpColor); @@ -398,13 +470,13 @@ class WebGLPointsLayerRenderer extends WebGLLayerRenderer { this.hitRenderInstructions_[hitIndex++] = hitColor[1]; this.hitRenderInstructions_[hitIndex++] = hitColor[2]; this.hitRenderInstructions_[hitIndex++] = hitColor[3]; - this.hitRenderInstructions_[hitIndex++] = Number(getUid(feature)); + this.hitRenderInstructions_[hitIndex++] = Number(featureUid); } // pushing custom attributes let value; for (let j = 0; j < this.customAttributes.length; j++) { - value = this.customAttributes[j].callback(feature); + value = this.customAttributes[j].callback(featureCache.feature, featureCache.properties); this.renderInstructions_[renderIndex++] = value; if (this.hitDetectionEnabled_) { this.hitRenderInstructions_[hitIndex++] = value; @@ -448,7 +520,7 @@ class WebGLPointsLayerRenderer extends WebGLLayerRenderer { const pixel = applyTransform(frameState.coordinateToPixelTransform, coordinate.slice()); - const data = this.hitRenderTarget_.readPixel(pixel[0], pixel[1]); + const data = this.hitRenderTarget_.readPixel(pixel[0] / 2, pixel[1] / 2); const color = [ data[0] / 255, data[1] / 255, @@ -476,7 +548,10 @@ class WebGLPointsLayerRenderer extends WebGLLayerRenderer { return; } - this.hitRenderTarget_.setSize(frameState.size); + this.hitRenderTarget_.setSize([ + Math.floor(frameState.size[0] / 2), + Math.floor(frameState.size[1] / 2) + ]); this.helper.useProgram(this.hitProgram_); this.helper.prepareDrawToRenderTarget(frameState, this.hitRenderTarget_, true); @@ -495,6 +570,11 @@ class WebGLPointsLayerRenderer extends WebGLLayerRenderer { */ disposeInternal() { this.worker_.terminate(); + this.layer_ = null; + this.sourceListenKeys_.forEach(function(key) { + unlistenByKey(key); + }); + this.sourceListenKeys_ = null; super.disposeInternal(); } } diff --git a/src/ol/style/expressions.js b/src/ol/style/expressions.js index 889e33b921..6527164bf2 100644 --- a/src/ol/style/expressions.js +++ b/src/ol/style/expressions.js @@ -203,6 +203,19 @@ export function colorToGlsl(color) { ); } +/** + * Returns a stable equivalent number for the string literal. + * @param {ParsingContext} context Parsing context + * @param {string} string String literal value + * @returns {number} Number equivalent + */ +export function getStringNumberEquivalent(context, string) { + if (context.stringLiteralsMap[string] === undefined) { + context.stringLiteralsMap[string] = Object.keys(context.stringLiteralsMap).length; + } + return context.stringLiteralsMap[string]; +} + /** * Returns a stable equivalent number for the string literal, for use in shaders. This number is then * converted to be a GLSL-compatible string. @@ -211,10 +224,7 @@ export function colorToGlsl(color) { * @returns {string} GLSL-compatible string containing a number */ export function stringToGlsl(context, string) { - if (context.stringLiteralsMap[string] === undefined) { - context.stringLiteralsMap[string] = Object.keys(context.stringLiteralsMap).length; - } - return numberToGlsl(context.stringLiteralsMap[string]); + return numberToGlsl(getStringNumberEquivalent(context, string)); } /** diff --git a/src/ol/webgl/Helper.js b/src/ol/webgl/Helper.js index b44944b4ff..cf2ceeab3a 100644 --- a/src/ol/webgl/Helper.js +++ b/src/ol/webgl/Helper.js @@ -266,18 +266,6 @@ class WebGLHelper extends Disposable { */ this.bufferCache_ = {}; - /** - * @private - * @type {!Array} - */ - this.shaderCache_ = []; - - /** - * @private - * @type {!Array} - */ - this.programCache_ = []; - /** * @private * @type {WebGLProgram} @@ -419,18 +407,6 @@ class WebGLHelper extends Disposable { disposeInternal() { this.canvas_.removeEventListener(ContextEventType.LOST, this.boundHandleWebGLContextLost_); this.canvas_.removeEventListener(ContextEventType.RESTORED, this.boundHandleWebGLContextRestored_); - const gl = this.getGL(); - if (!gl.isContextLost()) { - for (const key in this.bufferCache_) { - gl.deleteBuffer(this.bufferCache_[key].buffer); - } - for (const key in this.programCache_) { - gl.deleteProgram(this.programCache_[key]); - } - for (const key in this.shaderCache_) { - gl.deleteShader(this.shaderCache_[key]); - } - } } /** @@ -480,9 +456,10 @@ class WebGLHelper extends Disposable { */ prepareDrawToRenderTarget(frameState, renderTarget, opt_disableAlphaBlend) { const gl = this.getGL(); + const size = renderTarget.getSize(); gl.bindFramebuffer(gl.FRAMEBUFFER, renderTarget.getFramebuffer()); - gl.viewport(0, 0, frameState.size[0], frameState.size[1]); + gl.viewport(0, 0, size[0], size[1]); gl.bindTexture(gl.TEXTURE_2D, renderTarget.getTexture()); gl.clearColor(0.0, 0.0, 0.0, 0.0); gl.clear(gl.COLOR_BUFFER_BIT); @@ -652,7 +629,6 @@ class WebGLHelper extends Disposable { const shader = gl.createShader(type); gl.shaderSource(shader, source); gl.compileShader(shader); - this.shaderCache_.push(shader); return shader; } @@ -684,7 +660,6 @@ class WebGLHelper extends Disposable { gl.attachShader(program, fragmentShader); gl.attachShader(program, vertexShader); gl.linkProgram(program); - this.programCache_.push(program); return program; } @@ -811,8 +786,6 @@ class WebGLHelper extends Disposable { */ handleWebGLContextLost() { clear(this.bufferCache_); - clear(this.shaderCache_); - clear(this.programCache_); this.currentProgram_ = null; } @@ -823,8 +796,6 @@ class WebGLHelper extends Disposable { handleWebGLContextRestored() { } - // TODO: shutdown program - /** * Will create or reuse a given webgl texture and apply the given size. If no image data * specified, the texture will be empty, otherwise image data will be used and the `size` diff --git a/src/ol/webgl/RenderTarget.js b/src/ol/webgl/RenderTarget.js index 4500495dc1..28e156f237 100644 --- a/src/ol/webgl/RenderTarget.js +++ b/src/ol/webgl/RenderTarget.js @@ -115,12 +115,21 @@ class WebGLRenderTarget { /** * Reads one pixel of the frame buffer as an array of r, g, b, a components * in the 0-255 range (unsigned byte). + * If x and/or y are outside of existing data, an array filled with 0 is returned. * @param {number} x Pixel coordinate * @param {number} y Pixel coordinate * @returns {Uint8Array} Integer array with one color value (4 components) * @api */ readPixel(x, y) { + if (x < 0 || y < 0 || x > this.size_[0] || y >= this.size_[1]) { + tmpArray4[0] = 0; + tmpArray4[1] = 0; + tmpArray4[2] = 0; + tmpArray4[3] = 0; + return tmpArray4; + } + this.readAll(); const index = Math.floor(x) + (this.size_[1] - Math.floor(y) - 1) * this.size_[0]; tmpArray4[0] = this.data_[index * 4]; diff --git a/src/ol/webgl/ShaderBuilder.js b/src/ol/webgl/ShaderBuilder.js index bea9a05926..1ccd598583 100644 --- a/src/ol/webgl/ShaderBuilder.js +++ b/src/ol/webgl/ShaderBuilder.js @@ -3,7 +3,7 @@ * @module ol/webgl/ShaderBuilder */ -import {expressionToGlsl, stringToGlsl, ValueTypes} from '../style/expressions.js'; +import {expressionToGlsl, getStringNumberEquivalent, ValueTypes} from '../style/expressions.js'; /** * @typedef {Object} VaryingDescription @@ -450,7 +450,7 @@ export function parseLiteralStyle(style) { } let value = style.variables[varName]; if (typeof value === 'string') { - value = parseFloat(stringToGlsl(vertContext, value)); + value = getStringNumberEquivalent(vertContext, value); } return value !== undefined ? value : -9999999; // to avoid matching with the first string literal }; @@ -484,10 +484,10 @@ export function parseLiteralStyle(style) { attributes: vertContext.attributes.map(function(attributeName) { return { name: attributeName, - callback: function(feature) { - let value = feature.get(attributeName); + callback: function(feature, props) { + let value = props[attributeName]; if (typeof value === 'string') { - value = parseFloat(stringToGlsl(vertContext, value)); + value = getStringNumberEquivalent(vertContext, value); } return value !== undefined ? value : -9999999; // to avoid matching with the first string literal } diff --git a/test/spec/ol/renderer/webgl/pointslayer.test.js b/test/spec/ol/renderer/webgl/pointslayer.test.js index 25d6d230b9..583f90992a 100644 --- a/test/spec/ol/renderer/webgl/pointslayer.test.js +++ b/test/spec/ol/renderer/webgl/pointslayer.test.js @@ -7,6 +7,7 @@ import {get as getProjection} from '../../../../../src/ol/proj.js'; import ViewHint from '../../../../../src/ol/ViewHint.js'; import {WebGLWorkerMessageType} from '../../../../../src/ol/renderer/webgl/Layer.js'; import {compose as composeTransform, create as createTransform} from '../../../../../src/ol/transform.js'; +import {getUid} from '../../../../../src/ol/util.js'; const baseFrameState = { viewHints: [], @@ -388,4 +389,120 @@ describe('ol.renderer.webgl.PointsLayer', function() { }); }); + describe('featureCache_', function() { + let source, layer, features; + + function getCache(feature, renderer) { + return renderer.featureCache_[getUid(feature)]; + } + + beforeEach(function() { + source = new VectorSource(); + layer = new VectorLayer({ + source + }); + features = [ + new Feature({ + id: 'A', + test: 'abcd', + geometry: new Point([0, 1]) + }), + new Feature({ + id: 'D', + test: 'efgh', + geometry: new Point([2, 3]) + }), + new Feature({ + id: 'C', + test: 'ijkl', + geometry: new Point([4, 5]) + }) + ]; + }); + + it('contains no features initially', function() { + const renderer = new WebGLPointsLayerRenderer(layer, { + vertexShader: simpleVertexShader, + fragmentShader: simpleFragmentShader + }); + expect(renderer.featureCount_).to.be(0); + }); + + it('contains the features initially present in the source', function() { + source.addFeatures(features); + const renderer = new WebGLPointsLayerRenderer(layer, { + vertexShader: simpleVertexShader, + fragmentShader: simpleFragmentShader + }); + expect(renderer.featureCount_).to.be(3); + expect(getCache(features[0], renderer).feature).to.be(features[0]); + expect(getCache(features[0], renderer).geometry).to.be(features[0].getGeometry()); + expect(getCache(features[0], renderer).properties['test']).to.be(features[0].get('test')); + expect(getCache(features[1], renderer).feature).to.be(features[1]); + expect(getCache(features[1], renderer).geometry).to.be(features[1].getGeometry()); + expect(getCache(features[1], renderer).properties['test']).to.be(features[1].get('test')); + expect(getCache(features[2], renderer).feature).to.be(features[2]); + expect(getCache(features[2], renderer).geometry).to.be(features[2].getGeometry()); + expect(getCache(features[2], renderer).properties['test']).to.be(features[2].get('test')); + }); + + it('contains the features added to the source', function() { + const renderer = new WebGLPointsLayerRenderer(layer, { + vertexShader: simpleVertexShader, + fragmentShader: simpleFragmentShader + }); + + source.addFeature(features[0]); + expect(renderer.featureCount_).to.be(1); + + source.addFeature(features[1]); + expect(renderer.featureCount_).to.be(2); + + expect(getCache(features[0], renderer).feature).to.be(features[0]); + expect(getCache(features[0], renderer).geometry).to.be(features[0].getGeometry()); + expect(getCache(features[0], renderer).properties['test']).to.be(features[0].get('test')); + expect(getCache(features[1], renderer).feature).to.be(features[1]); + expect(getCache(features[1], renderer).geometry).to.be(features[1].getGeometry()); + expect(getCache(features[1], renderer).properties['test']).to.be(features[1].get('test')); + }); + + it('does not contain the features removed to the source', function() { + const renderer = new WebGLPointsLayerRenderer(layer, { + vertexShader: simpleVertexShader, + fragmentShader: simpleFragmentShader + }); + + source.addFeatures(features); + expect(renderer.featureCount_).to.be(3); + + source.removeFeature(features[1]); + expect(renderer.featureCount_).to.be(2); + + expect(getCache(features[0], renderer).feature).to.be(features[0]); + expect(getCache(features[0], renderer).geometry).to.be(features[0].getGeometry()); + expect(getCache(features[0], renderer).properties['test']).to.be(features[0].get('test')); + expect(getCache(features[2], renderer).feature).to.be(features[2]); + expect(getCache(features[2], renderer).geometry).to.be(features[2].getGeometry()); + expect(getCache(features[2], renderer).properties['test']).to.be(features[2].get('test')); + }); + + it('contains up to date properties and geometry', function() { + const renderer = new WebGLPointsLayerRenderer(layer, { + vertexShader: simpleVertexShader, + fragmentShader: simpleFragmentShader + }); + + source.addFeatures(features); + features[0].set('test', 'updated'); + features[0].set('added', true); + features[0].getGeometry().setCoordinates([10, 20]); + expect(renderer.featureCount_).to.be(3); + + expect(getCache(features[0], renderer).feature).to.be(features[0]); + expect(getCache(features[0], renderer).geometry.getCoordinates()).to.eql([10, 20]); + expect(getCache(features[0], renderer).properties['test']).to.be(features[0].get('test')); + expect(getCache(features[0], renderer).properties['added']).to.be(features[0].get('added')); + }); + }); + }); diff --git a/test/spec/ol/webgl/rendertarget.test.js b/test/spec/ol/webgl/rendertarget.test.js index 3468b16c26..550aae73d0 100644 --- a/test/spec/ol/webgl/rendertarget.test.js +++ b/test/spec/ol/webgl/rendertarget.test.js @@ -123,6 +123,26 @@ describe('ol.webgl.RenderTarget', function() { expect(spy.callCount).to.eql(2); }); + it('returns an array filled with 0 if outside of range', function() { + const rt = new WebGLRenderTarget(helper, [4, 4]); + helper.createTexture([4, 4], testImage_4x4, rt.getTexture()); + + let data = rt.readPixel(-1, 0); + expect(data).to.eql([0, 0, 0, 0]); + + data = rt.readPixel(3, -1); + expect(data).to.eql([0, 0, 0, 0]); + + data = rt.readPixel(6, 2); + expect(data).to.eql([0, 0, 0, 0]); + + data = rt.readPixel(2, 7); + expect(data).to.eql([0, 0, 0, 0]); + + data = rt.readPixel(2, 3); + expect(data).not.to.eql([0, 0, 0, 0]); + }); + }); });