From b433cfb792429a63a460d4c3d2ce5a21a026d168 Mon Sep 17 00:00:00 2001 From: David Manthey Date: Wed, 26 Jun 2019 08:54:19 -0400 Subject: [PATCH 1/3] Explicitly exit retired renderers. Before, this was waiting for GC to clean up unused renderers, which might not happen if renderers were created and destroyed without a major GC event. If GC didn't occur, the webgl context wouldn't be released. This explicitly exits the renderers (and explicitly loses the webgl context) as soon as the renderer is no longer in use (due to autosharing). --- src/webgl/layer.js | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/webgl/layer.js b/src/webgl/layer.js index 80faf8789e..1de01d8644 100644 --- a/src/webgl/layer.js +++ b/src/webgl/layer.js @@ -136,8 +136,6 @@ var webgl_layer = function () { map._updateAutoshareRenderers = function () { var layers = map.sortedLayers(), renderer, - used_canvases = [], - canvases = [], rerender_list = [], opacity; layers.forEach(function (layer) { @@ -149,12 +147,10 @@ var webgl_layer = function () { rerender_list.push(layer.renderer()); } renderer = layer.renderer(); - used_canvases.push(renderer.canvas()[0]); opacity = layer.opacity(); } else { if (layer.renderer() !== renderer) { rerender_list.push(layer.renderer()); - canvases.push(layer.renderer().canvas()[0]); layer.switchRenderer(renderer, false); rerender_list.push(layer.renderer()); } @@ -169,15 +165,13 @@ var webgl_layer = function () { }); layers.forEach(function (layer) { if (rerender_list.indexOf(layer.renderer()) >= 0) { - layer.renderer()._render(); + layer.renderer()._renderFrame(); rerender_list = rerender_list.filter((val) => val !== layer.renderer()); } }); - canvases.forEach(function (canvas) { - if (used_canvases.indexOf(canvas) < 0) { - canvas.remove(); - used_canvases.push(canvas); - } + /* explicitly exit any renderers we no longer use */ + rerender_list.forEach(function (renderer) { + renderer._exit(); }); }; From 8545ce9fddc2ada4485ebdaea89f7aef0cf1db83 Mon Sep 17 00:00:00 2001 From: David Manthey Date: Fri, 5 Jul 2019 12:37:18 -0400 Subject: [PATCH 2/3] Clear quads on layer addition and removal. Previously, this was done on z-index changes. Since layers can be rendered in shared canvases, this is needed on layer removal or addition since that can effect the overall computed tile layer offsets. --- src/webgl/tileLayer.js | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/webgl/tileLayer.js b/src/webgl/tileLayer.js index 4c4244a67f..f91fdd90b3 100644 --- a/src/webgl/tileLayer.js +++ b/src/webgl/tileLayer.js @@ -3,6 +3,9 @@ var tileLayer = require('../tileLayer'); var webgl_tileLayer = function () { 'use strict'; + + var geo_event = require('../event'); + var m_this = this, s_init = this._init, s_exit = this._exit, @@ -132,16 +135,28 @@ var webgl_tileLayer = function () { */ this.zIndex = function (zIndex, allowDuplicate) { if (zIndex !== undefined) { - /* If the z-index has changed, clear the quads so they are composited in - * the correct order. */ - m_this.clear(); - if (m_quadFeature) { - m_quadFeature.modified(); - } + this._clearQuads(true); } return s_zIndex.apply(m_this, arguments); }; + /** + * If the z-index has changed or layers are added or removed, clear the quads + * so they are composited in the correct order. + * + * @param {boolean} [noredraw] If truthy, don't redraw after clearing the + * quads. + */ + this._clearQuads = function (noredraw) { + m_this.clear(); + if (m_quadFeature) { + m_quadFeature.modified(); + } + if (noredraw !== true) { + this.draw(); + } + }; + /** * Update layer. * @@ -197,6 +212,10 @@ var webgl_tileLayer = function () { m_quadFeature.gcs(m_this._options.gcs || m_this.map().gcs()); m_quadFeature.data(m_tiles); m_quadFeature._update(); + + var map = m_this.map(); + map.geoOn(geo_event.layerAdd, this._clearQuads); + map.geoOn(geo_event.layerRemove, this._clearQuads); }; /* These functions don't need to do anything. */ From 9963aec09ce472b54527ee127114878c933130fd Mon Sep 17 00:00:00 2001 From: David Manthey Date: Wed, 10 Jul 2019 08:29:37 -0400 Subject: [PATCH 3/3] Add a third state for autoshareRenderer. The default (`true` setting) will only autoshare if it is likely to not result in a visual difference (layer opacity 0 or 1, tile layers below non-tile layers). The `"more"` setting will perform as before this PR -- autosharing if the opacities are identical and irrespective of the mixing of tile and non-tile layers. `false`, as before, won't autoshare. This also adds a map level setting of `autoshareRenderer` which sets the defaults for newly created levels. --- src/layer.js | 21 ++++++++------ src/map.js | 23 +++++++++++++++ src/webgl/layer.js | 12 ++++++-- tests/cases/layer.js | 68 ++++++++++++++++++++++++++++++++++++++++++-- tests/cases/map.js | 12 ++++++++ 5 files changed, 121 insertions(+), 15 deletions(-) diff --git a/src/layer.js b/src/layer.js index 907a68edce..c949f7e5a5 100644 --- a/src/layer.js +++ b/src/layer.js @@ -17,14 +17,17 @@ var rendererForAnnotations = require('./registry').rendererForAnnotations; * to determine the renderer. If a {@link geo.renderer} instance, the * renderer is not recreated; not all renderers can be shared by multiple * layers. - * @property {boolean} [autoshareRenderer=true] If truthy and the renderer - * supports it, auto-share renderers between layers. Currently, auto-sharing - * can only occur for webgl renderers, and will only occur between adjacent - * layers than have the same opacity. Shared renderers has slightly - * different behavior than non-shared renderers: changing z-index may result - * in rerendering and be slightly slower; only one DOM canvas is used for all - * shared renderers. Some features have slight z-stacking differences in - * shared versus non-shared renderers. + * @property {boolean|string} [autoshareRenderer=true] If truthy and the + * renderer supports it, auto-share renderers between layers. Currently, + * auto-sharing can only occur for webgl renderers and adjacent layers. If + * `true`, sharing will only occur if the layers have the same opacity and it + * is 1 or 0 and any tile layers are below non-tile layers. If `"more"`, + * sharing will occur for any adjacent layers that have the same opacity. + * Shared renderers has slightly different behavior than non-shared + * renderers: changing z-index may result in rerendering and be slightly + * slower; only one DOM canvas is used for all shared renderers. Some + * features have slight z-stacking differences in shared versus non-shared + * renderers. * @property {HTMLElement} [canvas] If specified, use this canvas rather than * a canvas associaied with the renderer directly. Renderers may not support * sharing a canvas. @@ -137,7 +140,7 @@ var layer = function (arg) { /** * Get the setting of autoshareRenderer. * - * @returns {boolean} + * @returns {boolean|string} */ this.autoshareRenderer = function () { return m_autoshareRenderer; diff --git a/src/map.js b/src/map.js index 39fca1d5fb..f292668c9e 100644 --- a/src/map.js +++ b/src/map.js @@ -56,6 +56,9 @@ var sceneObject = require('./sceneObject'); * maximum bounds in the vertical direction. * @property {boolean} [clampZoom=true] Prevent zooming out so that the map * area is smaller than the window. + * @property {boolean|string} [autoshareRenderer] If specified, pass this value + * to layers when they are created. See + * {@link geo.layer.spec#autoshareRenderer} for valid values. */ /** @@ -133,6 +136,7 @@ var map = function (arg) { m_clampZoom, m_animationQueue = arg.animationQueue || [], m_autoResize = arg.autoResize === undefined ? true : arg.autoResize, + m_autoshareRenderer = arg.autoshareRenderer, m_origin; /* Compute the maximum bounds on our map projection. By default, x ranges @@ -614,6 +618,9 @@ var map = function (arg) { */ this.createLayer = function (layerName, arg) { arg = arg || {}; + if (m_this.autoshareRenderer() !== undefined) { + arg = Object.assign({autoshareRenderer: m_this.autoshareRenderer()}, arg); + } var newLayer = registry.createLayer( layerName, m_this, arg); @@ -1873,6 +1880,22 @@ var map = function (arg) { return zoom; }; + /** + * Get or set the setting of autoshareRenderer. + * + * @param {boolean|string|null} [arg] If specified, the new value for + * autoshareRender that gets passed to created layers. `null` will clear + * the value. + * @returns {boolean|string|this} + */ + this.autoshareRenderer = function (arg) { + if (arg === undefined) { + return m_autoshareRenderer; + } + m_autoshareRenderer = arg === null ? undefined : arg; + return m_this; + }; + /** * Draw a layer image to a canvas context. The layer's opacity and transform * are applied. This is used as part of making a screenshot. diff --git a/src/webgl/layer.js b/src/webgl/layer.js index 1de01d8644..f14146d839 100644 --- a/src/webgl/layer.js +++ b/src/webgl/layer.js @@ -6,6 +6,7 @@ var webgl_layer = function () { var createRenderer = require('../registry').createRenderer; var geo_event = require('../event'); var webglRenderer = require('./webglRenderer'); + var tileLayer = require('../tileLayer'); var m_this = this, s_init = this._init, @@ -137,23 +138,28 @@ var webgl_layer = function () { var layers = map.sortedLayers(), renderer, rerender_list = [], - opacity; + opacity, + lowerTileLayers; layers.forEach(function (layer) { - if (!layer.autoshareRenderer() || !layer.renderer() || layer.renderer().api() !== webglRenderer.apiname) { + let autoshare = layer.autoshareRenderer(), + isTileLayer = layer instanceof tileLayer; + if (!autoshare || !layer.renderer() || layer.renderer().api() !== webglRenderer.apiname) { renderer = null; - } else if (!renderer || layer.opacity() !== opacity) { + } else if (!renderer || layer.opacity() !== opacity || (autoshare !== 'more' && ((layer.opacity() > 0 && layer.opacity() < 1) || (isTileLayer && !lowerTileLayers)))) { if (!layer.node()[0].contains(layer.renderer().canvas()[0])) { layer.switchRenderer(createRenderer(webglRenderer.apiname, layer), false); rerender_list.push(layer.renderer()); } renderer = layer.renderer(); opacity = layer.opacity(); + lowerTileLayers = isTileLayer; } else { if (layer.renderer() !== renderer) { rerender_list.push(layer.renderer()); layer.switchRenderer(renderer, false); rerender_list.push(layer.renderer()); } + lowerTileLayers &= isTileLayer; } }); layers.forEach(function (layer) { diff --git a/tests/cases/layer.js b/tests/cases/layer.js index ba1ba88402..e74d9e9b08 100644 --- a/tests/cases/layer.js +++ b/tests/cases/layer.js @@ -323,7 +323,7 @@ describe('geo.webgl.layer', function () { restoreWebglRenderer(); }); }); - describe('autoshareRenderer is true', function () { + describe('autoshareRenderer is true"', function () { var map, layer1, layer2, layer3; it('_init', function (done) { mockWebglRenderer(); @@ -348,6 +348,68 @@ describe('geo.webgl.layer', function () { layer2.visible(true); expect($('canvas', map.node()).length).toBe(1); }); + it('opacity', function () { + layer1.opacity(0.5); + expect($('canvas', map.node()).length).toBe(2); + layer2.opacity(0.5); + expect($('canvas', map.node()).length).toBe(3); + layer1.opacity(1); + expect($('canvas', map.node()).length).toBe(3); + layer2.opacity(1); + expect($('canvas', map.node()).length).toBe(1); + }); + it('zIndex', function () { + expect($('canvas', layer1.node()).length).toBe(1); + expect($('canvas', layer2.node()).length).toBe(0); + expect($('canvas', layer3.node()).length).toBe(0); + layer1.moveUp(); + expect($('canvas', map.node()).length).toBe(1); + expect($('canvas', layer1.node()).length).toBe(0); + expect($('canvas', layer2.node()).length).toBe(1); + expect($('canvas', layer3.node()).length).toBe(0); + layer1.moveUp(); + expect($('canvas', map.node()).length).toBe(2); + expect($('canvas', layer1.node()).length).toBe(1); + expect($('canvas', layer2.node()).length).toBe(1); + expect($('canvas', layer3.node()).length).toBe(0); + layer1.moveToBottom(); + expect($('canvas', map.node()).length).toBe(1); + expect($('canvas', layer1.node()).length).toBe(1); + expect($('canvas', layer2.node()).length).toBe(0); + expect($('canvas', layer3.node()).length).toBe(0); + }); + it('cleanup', function () { + console.log.restore(); + destroyMap(); + restoreWebglRenderer(); + }); + }); + describe('autoshareRenderer is "more"', function () { + var map, layer1, layer2, layer3; + it('_init', function (done) { + mockWebglRenderer(); + sinon.stub(console, 'log', function () {}); + map = createMap(); + map.autoshareRenderer('more'); + layer1 = map.createLayer('osm', {renderer: 'webgl', url: '/testdata/white.jpg'}); + layer2 = map.createLayer('osm', {renderer: 'webgl', url: '/testdata/weather.png', keepLower: false}); + layer3 = map.createLayer('feature', {renderer: 'webgl'}); + layer3.createFeature('point', {}).data([{x: 2, y: 1}]); + map.onIdle(function () { + expect($('canvas', map.node()).length).toBe(1); + done(); + }); + }); + it('visible', function () { + layer1.visible(false); + expect($('canvas', map.node()).length).toBe(1); + layer1.visible(true); + expect($('canvas', map.node()).length).toBe(1); + layer2.visible(false); + expect($('canvas', map.node()).length).toBe(1); + layer2.visible(true); + expect($('canvas', map.node()).length).toBe(1); + }); it('opacity', function () { layer1.opacity(0.5); expect($('canvas', map.node()).length).toBe(2); @@ -390,9 +452,9 @@ describe('geo.webgl.layer', function () { mockWebglRenderer(); sinon.stub(console, 'log', function () {}); map = createMap(); - layer1 = map.createLayer('osm', {renderer: 'webgl', url: '/testdata/white.jpg'}); + layer1 = map.createLayer('osm', {renderer: 'webgl', url: '/testdata/white.jpg', autoshareRenderer: 'more'}); layer2 = map.createLayer('osm', {renderer: 'webgl', url: '/testdata/weather.png', keepLower: false, autoshareRenderer: false}); - layer3 = map.createLayer('feature', {renderer: 'webgl'}); + layer3 = map.createLayer('feature', {renderer: 'webgl', autoshareRenderer: 'more'}); layer3.createFeature('point', {}).data([{x: 2, y: 1}]); map.onIdle(function () { expect($('canvas', map.node()).length).toBe(3); diff --git a/tests/cases/map.js b/tests/cases/map.js index e610663398..314abba943 100644 --- a/tests/cases/map.js +++ b/tests/cases/map.js @@ -408,6 +408,18 @@ describe('geo.core.map', function () { expect(m.fileReader(r)).toBe(m); expect(m.fileReader()).toBe(r); }); + it('autoshareRenderer', function () { + var m = createMap(); + expect(m.autoshareRenderer()).toBe(undefined); + expect(m.autoshareRenderer(false)).toBe(m); + expect(m.autoshareRenderer()).toBe(false); + expect(m.autoshareRenderer(true)).toBe(m); + expect(m.autoshareRenderer()).toBe(true); + expect(m.autoshareRenderer(null)).toBe(m); + expect(m.autoshareRenderer()).toBe(undefined); + m = createMap({autoshareRenderer: 'more'}); + expect(m.autoshareRenderer()).toBe('more'); + }); }); describe('Public utility methods', function () {