Merge pull request #10235 from jahow/webgl-optimizations

Various optimizations and fixes for the WebGL points renderer

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;

examples/webgl-points-layer.js

@@ -124,6 +124,7 @@ function refreshLayer(newStyle) {
 
   if (previousLayer) {
     map.removeLayer(previousLayer);
+    previousLayer.dispose();
   }
   literalStyle = newStyle;
 }

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();
+  }
 }
 
 

src/ol/layer/Layer.js

@@ -287,6 +287,13 @@ class Layer extends BaseLayer {
     return null;
   }
 
+  /**
+   * @inheritDoc
+   */
+  disposeInternal() {
+    this.setSource(null);
+    super.disposeInternal();
+  }
 }
 
 

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;

src/ol/renderer/webgl/Layer.js

@@ -71,7 +71,7 @@ class WebGLLayerRenderer extends LayerRenderer {
    * @inheritDoc
    */
   disposeInternal() {
-    this.helper.disposeInternal();
+    this.helper.dispose();
     super.disposeInternal();
   }
 

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<string, *>):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<string, *>} 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<string, FeatureCacheItem>}
+     * @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();
   }
 }

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));
 }
 
 /**

src/ol/webgl/Helper.js

@@ -266,18 +266,6 @@ class WebGLHelper extends Disposable {
      */
     this.bufferCache_ = {};
 
-    /**
-     * @private
-     * @type {!Array<WebGLShader>}
-     */
-    this.shaderCache_ = [];
-
-    /**
-     * @private
-     * @type {!Array<WebGLProgram>}
-     */
-    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`

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];

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
         }

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'));
+    });
+  });
+
 });

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]);
+    });
+
   });
 
 });