Merge pull request #9864 from tschaub/opacity

Properly update frameState.animate and deal with non-numeric layer opacity

doc/errors/index.md

@@ -236,3 +236,7 @@ A `WebGLArrayBuffer` must either be of type `ELEMENT_ARRAY_BUFFER` or `ARRAY_BUF
 ### 63
 
 Support for the `OES_element_index_uint` WebGL extension is mandatory for WebGL layers.
+
+### 64
+
+Layer opacity must be a number.

src/ol/layer/Base.js

@@ -6,6 +6,7 @@ import BaseObject from '../Object.js';
 import LayerProperty from './Property.js';
 import {clamp} from '../math.js';
 import {assign} from '../obj.js';
+import {assert} from '../asserts.js';
 
 
 /**
@@ -52,8 +53,11 @@ class BaseLayer extends BaseObject {
      * @type {Object<string, *>}
      */
     const properties = assign({}, options);
+
     properties[LayerProperty.OPACITY] =
-       options.opacity !== undefined ? options.opacity : 1;
+        options.opacity !== undefined ? options.opacity : 1;
+    assert(typeof properties[LayerProperty.OPACITY] === 'number', 64); // Layer opacity must be a number
+
     properties[LayerProperty.VISIBLE] =
        options.visible !== undefined ? options.visible : true;
     properties[LayerProperty.Z_INDEX] = options.zIndex;
@@ -292,6 +296,7 @@ class BaseLayer extends BaseObject {
    * @api
    */
   setOpacity(opacity) {
+    assert(typeof opacity === 'number', 64); // Layer opacity must be a number
     this.set(LayerProperty.OPACITY, opacity);
   }
 

src/ol/renderer/canvas/TileLayer.js

@@ -377,7 +377,8 @@ class CanvasTileLayerRenderer extends CanvasLayerRenderer {
       return;
     }
     const uid = getUid(this);
-    const alpha = opacity * (transition ? tile.getAlpha(uid, frameState.time) : 1);
+    const tileAlpha = transition ? tile.getAlpha(uid, frameState.time) : 1;
+    const alpha = opacity * tileAlpha;
     const alphaChanged = alpha !== this.context.globalAlpha;
     if (alphaChanged) {
       this.context.save();
@@ -389,7 +390,7 @@ class CanvasTileLayerRenderer extends CanvasLayerRenderer {
     if (alphaChanged) {
       this.context.restore();
     }
-    if (alpha !== 1) {
+    if (tileAlpha !== 1) {
       frameState.animate = true;
     } else if (transition) {
       tile.endTransition(uid);

test/spec/ol/layer/layer.test.js

@@ -112,6 +112,19 @@ describe('ol.layer.Layer', function() {
       layer.dispose();
     });
 
+    it('throws on non-numeric opacity', function() {
+      function create() {
+        new Layer({
+          source: new Source({
+            projection: 'EPSG:4326'
+          }),
+          opacity: 'foo'
+        });
+      }
+
+      expect(create).to.throwException();
+    });
+
     it('accepts a custom render function', function() {
       let called = false;
       const layer = new Layer({
@@ -511,6 +524,13 @@ describe('ol.layer.Layer', function() {
       expect(layer.getOpacity()).to.be(0.3);
     });
 
+    it('throws on types other than number', function() {
+      function set() {
+        layer.setOpacity('foo');
+      }
+      expect(set).to.throwException();
+    });
+
     it('triggers a change event', function() {
       const listener = sinon.spy();
       layer.on('propertychange', listener);

test/spec/ol/layer/tile.test.js

@@ -1,5 +1,6 @@
+import {Map, View} from '../../../../src/ol/index.js';
 import TileLayer from '../../../../src/ol/layer/Tile.js';
-import OSM from '../../../../src/ol/source/OSM.js';
+import {OSM, XYZ} from '../../../../src/ol/source.js';
 
 
 describe('ol.layer.Tile', function() {
@@ -32,4 +33,73 @@ describe('ol.layer.Tile', function() {
 
   });
 
+  describe('frameState.animate after tile transition with layer opacity', function() {
+    let target, map;
+
+    beforeEach(function(done) {
+      target = document.createElement('div');
+      Object.assign(target.style, {
+        position: 'absolute',
+        left: '-1000px',
+        top: '-1000px',
+        width: '256px',
+        height: '256px'
+      });
+      document.body.appendChild(target);
+
+      map = new Map({
+        target: target,
+        view: new View({center: [0, 0], zoom: 1})
+      });
+
+      map.once('rendercomplete', function() {
+        done();
+      });
+    });
+
+    afterEach(function() {
+      map.dispose();
+      document.body.removeChild(target);
+    });
+
+    it('sets frameState.animate to false when opacity is 1', function(done) {
+      let lastFrameState;
+      const layer = new TileLayer({
+        opacity: 1,
+        source: new XYZ({
+          url: 'spec/ol/data/osm-0-0-0.png'
+        })
+      });
+      layer.on('postrender', function(event) {
+        lastFrameState = event.frameState;
+      });
+
+      map.once('rendercomplete', function() {
+        expect(lastFrameState.animate).to.be(false);
+        done();
+      });
+
+      map.addLayer(layer);
+    });
+
+    it('sets frameState.animate to false when opacity is 0.5', function(done) {
+      let lastFrameState;
+      const layer = new TileLayer({
+        opacity: 0.5,
+        source: new XYZ({
+          url: 'spec/ol/data/osm-0-0-0.png'
+        })
+      });
+      layer.on('postrender', function(event) {
+        lastFrameState = event.frameState;
+      });
+
+      map.once('rendercomplete', function() {
+        expect(lastFrameState.animate).to.be(false);
+        done();
+      });
+
+      map.addLayer(layer);
+    });
+  });
 });

test/spec/ol/map.test.js

@@ -203,6 +203,7 @@ describe('ol.Map', function() {
         target: target,
         layers: [
           new TileLayer({
+            opacity: 0.5,
             source: new XYZ({
               url: 'spec/ol/data/osm-{z}-{x}-{y}.png'
             })