From e78c14c061a5800790fd49e64c120f1d3d9f5959 Mon Sep 17 00:00:00 2001 From: Olivier Guyot Date: Thu, 31 Oct 2019 10:48:28 +0100 Subject: [PATCH 1/7] Webgl points renderer / add a cache for features in the source This allows quicker access to features as well as their geometries and properties, reducing the time taken by a rebuildBuffers call. --- src/ol/renderer/webgl/PointsLayer.js | 85 ++++++++++--- .../ol/renderer/webgl/pointslayer.test.js | 117 ++++++++++++++++++ 2 files changed, 186 insertions(+), 16 deletions(-) diff --git a/src/ol/renderer/webgl/PointsLayer.js b/src/ol/renderer/webgl/PointsLayer.js index d0ae7f1140..7e8ddb7874 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} 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,50 @@ 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_ = {}; + + const source = this.getLayer().getSource(); + listen(source, VectorEventType.ADDFEATURE, this.handleSourceFeatureChanged_, this); + listen(source, VectorEventType.CHANGEFEATURE, this.handleSourceFeatureChanged_, this); + listen(source, VectorEventType.REMOVEFEATURE, this.handleSourceFeatureDelete_, this); + source.getFeatures().forEach(function(feature) { + const uid = getUid(feature); + this.featureCache_[uid] = { + feature: feature, + properties: feature.getProperties(), + geometry: feature.getGeometry() + }; + }.bind(this)); + } + + /** + * @param {import("../../source/Vector.js").VectorSourceEvent} event Event. + * @private + */ + handleSourceFeatureChanged_(event) { + const feature = event.feature; + const uid = getUid(feature); + this.featureCache_[uid] = { + feature: feature, + properties: feature.getProperties(), + geometry: feature.getGeometry() + }; + } + + /** + * @param {import("../../source/Vector.js").VectorSourceEvent} event Event. + * @private + */ + handleSourceFeatureDelete_(event) { + const feature = event.feature; + const uid = getUid(feature); + delete this.featureCache_[uid]; } /** @@ -343,45 +397,44 @@ 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(); + const featureUids = Object.keys(this.featureCache_); // 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) * featureUids.length; 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) * featureUids.length; if (!this.hitRenderInstructions_ || this.hitRenderInstructions_.length !== totalHitInstructionsCount) { this.hitRenderInstructions_ = new Float32Array(totalHitInstructionsCount); } } // loop on features to fill the buffer - let feature; + let featureUid, 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 (let i = 0; i < featureUids.length; i++) { + featureUid = featureUids[i]; + 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 +451,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; diff --git a/test/spec/ol/renderer/webgl/pointslayer.test.js b/test/spec/ol/renderer/webgl/pointslayer.test.js index 25d6d230b9..d68fc78c0d 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(Object.keys(renderer.featureCache_).length).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(Object.keys(renderer.featureCache_).length).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(Object.keys(renderer.featureCache_).length).to.be(1); + + source.addFeature(features[1]); + expect(Object.keys(renderer.featureCache_).length).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(Object.keys(renderer.featureCache_).length).to.be(3); + + source.removeFeature(features[1]); + expect(Object.keys(renderer.featureCache_).length).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(Object.keys(renderer.featureCache_).length).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')); + }); + }); + }); From e5e03d46a0ce92655f96077d679ec01977ebc37f Mon Sep 17 00:00:00 2001 From: Olivier Guyot Date: Thu, 31 Oct 2019 10:50:24 +0100 Subject: [PATCH 2/7] Webgl points renderer / more optimizations Simplify calls in the attributes callback, also less stress on garbage collection. --- src/ol/style/expressions.js | 18 ++++++++++++++---- src/ol/webgl/ShaderBuilder.js | 10 +++++----- 2 files changed, 19 insertions(+), 9 deletions(-) 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/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 } From f7b0f6750be73bddcb150b24bb46942dddb383db Mon Sep 17 00:00:00 2001 From: jahow Date: Wed, 30 Oct 2019 21:59:57 +0100 Subject: [PATCH 3/7] Resolve memory leak when deleting a webgl layer Various references were kept, preventing the layer and underlying renderer and webgl context to be garbage collected. Also, the Helper was simplified because it turns out deleting manually all Webgl objects is useless: these objects will be released when the context is garbage collected anyway. Note: this touches the Layer and BaseLayer classes, as the following were preventing the layer from being garbage collected: * layer reference in the `state_` object in BaseLayer * dangling listener for source change in Layer --- examples/webgl-points-layer.js | 1 + src/ol/layer/Base.js | 11 ++++++++++ src/ol/layer/Layer.js | 7 +++++++ src/ol/layer/WebGLPoints.js | 12 +++++++++++ src/ol/renderer/webgl/Layer.js | 2 +- src/ol/renderer/webgl/PointsLayer.js | 15 ++++++++++---- src/ol/webgl/Helper.js | 30 ---------------------------- 7 files changed, 43 insertions(+), 35 deletions(-) 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 7e8ddb7874..f0dbdb7452 100644 --- a/src/ol/renderer/webgl/PointsLayer.js +++ b/src/ol/renderer/webgl/PointsLayer.js @@ -19,7 +19,7 @@ import {getUid} from '../../util.js'; import WebGLRenderTarget from '../../webgl/RenderTarget.js'; import {assert} from '../../asserts.js'; import BaseVector from '../../layer/BaseVector.js'; -import {listen} from '../../events.js'; +import {listen, unlistenByKey} from '../../events.js'; import VectorEventType from '../../source/VectorEventType.js'; /** @@ -288,9 +288,11 @@ class WebGLPointsLayerRenderer extends WebGLLayerRenderer { this.featureCache_ = {}; const source = this.getLayer().getSource(); - listen(source, VectorEventType.ADDFEATURE, this.handleSourceFeatureChanged_, this); - listen(source, VectorEventType.CHANGEFEATURE, this.handleSourceFeatureChanged_, this); - listen(source, VectorEventType.REMOVEFEATURE, this.handleSourceFeatureDelete_, this); + this.sourceListenKeys_ = [ + listen(source, VectorEventType.ADDFEATURE, this.handleSourceFeatureChanged_, this), + listen(source, VectorEventType.CHANGEFEATURE, this.handleSourceFeatureChanged_, this), + listen(source, VectorEventType.REMOVEFEATURE, this.handleSourceFeatureDelete_, this) + ]; source.getFeatures().forEach(function(feature) { const uid = getUid(feature); this.featureCache_[uid] = { @@ -548,6 +550,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/webgl/Helper.js b/src/ol/webgl/Helper.js index b44944b4ff..27434f79ea 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]); - } - } } /** @@ -652,7 +628,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 +659,6 @@ class WebGLHelper extends Disposable { gl.attachShader(program, fragmentShader); gl.attachShader(program, vertexShader); gl.linkProgram(program); - this.programCache_.push(program); return program; } @@ -811,8 +785,6 @@ class WebGLHelper extends Disposable { */ handleWebGLContextLost() { clear(this.bufferCache_); - clear(this.shaderCache_); - clear(this.programCache_); this.currentProgram_ = null; } @@ -823,8 +795,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` From 43010c8934735d2526460507794d033413e20002 Mon Sep 17 00:00:00 2001 From: Olivier Guyot Date: Thu, 31 Oct 2019 13:27:57 +0100 Subject: [PATCH 4/7] Webgl / return 0 if doing renderTarget.read outside of data width/height This would happen when WebGLPointsLayerRenderer.forEachFeatureAtCoordinate is called on "warped" worlds at -360/+360 degrees, and may produce false positives. --- src/ol/webgl/RenderTarget.js | 6 ++++++ test/spec/ol/webgl/rendertarget.test.js | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/ol/webgl/RenderTarget.js b/src/ol/webgl/RenderTarget.js index 4500495dc1..a3933ae414 100644 --- a/src/ol/webgl/RenderTarget.js +++ b/src/ol/webgl/RenderTarget.js @@ -115,12 +115,18 @@ 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] = tmpArray4[1] = tmpArray4[2] = 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/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]); + }); + }); }); From 600e1a4647334c3685b1d599dc3cfe82eb0cf849 Mon Sep 17 00:00:00 2001 From: Olivier Guyot Date: Thu, 31 Oct 2019 13:40:51 +0100 Subject: [PATCH 5/7] Webgl points renderer / use a smaller canvas for hit detection render The hit detection render is now done against a canvas with half the width/height of the main render. This still provides sufficient precision while requiring a much smaller memory allocation (especially for retina devices). --- src/ol/renderer/webgl/PointsLayer.js | 7 +++++-- src/ol/webgl/Helper.js | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/ol/renderer/webgl/PointsLayer.js b/src/ol/renderer/webgl/PointsLayer.js index f0dbdb7452..811925df7a 100644 --- a/src/ol/renderer/webgl/PointsLayer.js +++ b/src/ol/renderer/webgl/PointsLayer.js @@ -503,7 +503,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, @@ -531,7 +531,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); diff --git a/src/ol/webgl/Helper.js b/src/ol/webgl/Helper.js index 27434f79ea..cf2ceeab3a 100644 --- a/src/ol/webgl/Helper.js +++ b/src/ol/webgl/Helper.js @@ -456,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); From af15cfb815c7ce9086a42bf2e96178345e9ad0f6 Mon Sep 17 00:00:00 2001 From: Olivier Guyot Date: Thu, 31 Oct 2019 14:19:39 +0100 Subject: [PATCH 6/7] Icon webgl example / avoid doing hit detection when view is moving --- examples/icon-sprite-webgl.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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; From 7da86ae71fdedc41c334c7f882fb458f8ab49e08 Mon Sep 17 00:00:00 2001 From: Olivier Guyot Date: Mon, 4 Nov 2019 09:41:12 +0100 Subject: [PATCH 7/7] Webgl points renderer / slight improvements following review Also fixes a lint error. --- src/ol/renderer/webgl/PointsLayer.js | 47 +++++++++++++------ src/ol/webgl/RenderTarget.js | 5 +- .../ol/renderer/webgl/pointslayer.test.js | 14 +++--- 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/src/ol/renderer/webgl/PointsLayer.js b/src/ol/renderer/webgl/PointsLayer.js index 811925df7a..868f9b7473 100644 --- a/src/ol/renderer/webgl/PointsLayer.js +++ b/src/ol/renderer/webgl/PointsLayer.js @@ -287,30 +287,50 @@ class WebGLPointsLayerRenderer extends WebGLLayerRenderer { */ 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.handleSourceFeatureChanged_, this), + listen(source, VectorEventType.ADDFEATURE, this.handleSourceFeatureAdded_, this), listen(source, VectorEventType.CHANGEFEATURE, this.handleSourceFeatureChanged_, this), listen(source, VectorEventType.REMOVEFEATURE, this.handleSourceFeatureDelete_, this) ]; - source.getFeatures().forEach(function(feature) { - const uid = getUid(feature); - this.featureCache_[uid] = { + 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; - const uid = getUid(feature); - this.featureCache_[uid] = { + this.featureCache_[getUid(feature)] = { feature: feature, properties: feature.getProperties(), geometry: feature.getGeometry() @@ -323,8 +343,8 @@ class WebGLPointsLayerRenderer extends WebGLLayerRenderer { */ handleSourceFeatureDelete_(event) { const feature = event.feature; - const uid = getUid(feature); - delete this.featureCache_[uid]; + delete this.featureCache_[getUid(feature)]; + this.featureCount_--; } /** @@ -403,32 +423,29 @@ class WebGLPointsLayerRenderer extends WebGLLayerRenderer { const projectionTransform = createTransform(); this.helper.makeProjectionTransform(frameState, projectionTransform); - const featureUids = Object.keys(this.featureCache_); - // 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) * featureUids.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) * featureUids.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 featureUid, featureCache, geometry; + let featureCache, geometry; const tmpCoords = []; const tmpColor = []; let renderIndex = 0; let hitIndex = 0; let hitColor; - for (let i = 0; i < featureUids.length; i++) { - featureUid = featureUids[i]; + for (const featureUid in this.featureCache_) { featureCache = this.featureCache_[featureUid]; geometry = /** @type {import("../../geom").Point} */(featureCache.geometry); if (!geometry || geometry.getType() !== GeometryType.POINT) { diff --git a/src/ol/webgl/RenderTarget.js b/src/ol/webgl/RenderTarget.js index a3933ae414..28e156f237 100644 --- a/src/ol/webgl/RenderTarget.js +++ b/src/ol/webgl/RenderTarget.js @@ -123,7 +123,10 @@ class WebGLRenderTarget { */ readPixel(x, y) { if (x < 0 || y < 0 || x > this.size_[0] || y >= this.size_[1]) { - tmpArray4[0] = tmpArray4[1] = tmpArray4[2] = tmpArray4[3] = 0; + tmpArray4[0] = 0; + tmpArray4[1] = 0; + tmpArray4[2] = 0; + tmpArray4[3] = 0; return tmpArray4; } diff --git a/test/spec/ol/renderer/webgl/pointslayer.test.js b/test/spec/ol/renderer/webgl/pointslayer.test.js index d68fc78c0d..583f90992a 100644 --- a/test/spec/ol/renderer/webgl/pointslayer.test.js +++ b/test/spec/ol/renderer/webgl/pointslayer.test.js @@ -425,7 +425,7 @@ describe('ol.renderer.webgl.PointsLayer', function() { vertexShader: simpleVertexShader, fragmentShader: simpleFragmentShader }); - expect(Object.keys(renderer.featureCache_).length).to.be(0); + expect(renderer.featureCount_).to.be(0); }); it('contains the features initially present in the source', function() { @@ -434,7 +434,7 @@ describe('ol.renderer.webgl.PointsLayer', function() { vertexShader: simpleVertexShader, fragmentShader: simpleFragmentShader }); - expect(Object.keys(renderer.featureCache_).length).to.be(3); + 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')); @@ -453,10 +453,10 @@ describe('ol.renderer.webgl.PointsLayer', function() { }); source.addFeature(features[0]); - expect(Object.keys(renderer.featureCache_).length).to.be(1); + expect(renderer.featureCount_).to.be(1); source.addFeature(features[1]); - expect(Object.keys(renderer.featureCache_).length).to.be(2); + 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()); @@ -473,10 +473,10 @@ describe('ol.renderer.webgl.PointsLayer', function() { }); source.addFeatures(features); - expect(Object.keys(renderer.featureCache_).length).to.be(3); + expect(renderer.featureCount_).to.be(3); source.removeFeature(features[1]); - expect(Object.keys(renderer.featureCache_).length).to.be(2); + 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()); @@ -496,7 +496,7 @@ describe('ol.renderer.webgl.PointsLayer', function() { features[0].set('test', 'updated'); features[0].set('added', true); features[0].getGeometry().setCoordinates([10, 20]); - expect(Object.keys(renderer.featureCache_).length).to.be(3); + 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]);