From db590cdd142bd112cc376fd9579b1b92d835a4bd Mon Sep 17 00:00:00 2001 From: David Manthey Date: Thu, 22 Feb 2018 09:28:04 -0500 Subject: [PATCH 1/3] Improve jsdocs for line features. Also correct some minor issues with docs for the polygon feature. --- src/canvas/lineFeature.js | 7 ++- src/d3/lineFeature.js | 17 +++--- src/gl/lineFeature.js | 50 +++++++++++---- src/gl/polygonFeature.js | 3 +- src/lineFeature.js | 124 +++++++++++++++++++++++--------------- src/polygonFeature.js | 14 ++--- 6 files changed, 135 insertions(+), 80 deletions(-) diff --git a/src/canvas/lineFeature.js b/src/canvas/lineFeature.js index 31173af607..3a81544441 100644 --- a/src/canvas/lineFeature.js +++ b/src/canvas/lineFeature.js @@ -3,11 +3,11 @@ var registerFeature = require('../registry').registerFeature; var lineFeature = require('../lineFeature'); /** - * Create a new instance of class lineFeature + * Create a new instance of class lineFeature. * * @class geo.canvas.lineFeature * @extends geo.lineFeature - * @extends geo.canvas.object + * @param {geo.lineFeature.spec} arg * @returns {geo.canvas.lineFeature} */ var canvas_lineFeature = function (arg) { @@ -29,9 +29,10 @@ var canvas_lineFeature = function (arg) { /** * Render the data on the canvas. + * * @protected * @param {object} context2d the canvas context to draw in. - * @param {object} map the parent map object. + * @param {geo.map} map the parent map object. */ this._renderOnCanvas = function (context2d, map) { var data = m_this.data(), diff --git a/src/d3/lineFeature.js b/src/d3/lineFeature.js index 71edbc0529..e809cbfbd3 100644 --- a/src/d3/lineFeature.js +++ b/src/d3/lineFeature.js @@ -3,11 +3,11 @@ var registerFeature = require('../registry').registerFeature; var lineFeature = require('../lineFeature'); /** - * Create a new instance of class lineFeature + * Create a new instance of class lineFeature. * * @class geo.d3.lineFeature * @extends geo.lineFeature - * @extends geo.d3.object + * @param {geo.lineFeature.spec} arg * @returns {geo.d3.lineFeature} */ var d3_lineFeature = function (arg) { @@ -35,7 +35,10 @@ var d3_lineFeature = function (arg) { s_update = this._update; /** - * Initialize + * Initialize. + * + * @param {geo.lineFeature.spec} arg The feature specification. + * @returns {this} */ this._init = function (arg) { s_init.call(m_this, arg); @@ -43,9 +46,9 @@ var d3_lineFeature = function (arg) { }; /** - * Build + * Build. Create the necessary elements to render lines. * - * @override + * @returns {this} */ this._build = function () { var data = m_this.data() || [], @@ -109,9 +112,9 @@ var d3_lineFeature = function (arg) { }; /** - * Update + * Update. Rebuild if necessary. * - * @override + * @returns {this} */ this._update = function () { s_update.call(m_this); diff --git a/src/gl/lineFeature.js b/src/gl/lineFeature.js index 96258c9b09..dc16e5fcd5 100644 --- a/src/gl/lineFeature.js +++ b/src/gl/lineFeature.js @@ -34,10 +34,11 @@ var flagsDebug = { // uses 1 bit }; /** - * Create a new instance of lineFeature + * Create a new instance of lineFeature. * * @class geo.gl.lineFeature * @extends geo.lineFeature + * @param {geo.lineFeature.spec} arg * @returns {geo.gl.lineFeature} */ var gl_lineFeature = function (arg) { @@ -72,6 +73,11 @@ var gl_lineFeature = function (arg) { s_init = this._init, s_update = this._update; + /** + * Create the vertex shader for lines. + * + * @returns {vgl.shader} + */ function createVertexShader() { var vertexShaderSource = [ '#ifdef GL_ES', @@ -244,6 +250,14 @@ var gl_lineFeature = function (arg) { return shader; } + /** + * Create the fragment shader for lines. + * + * @param {boolean} [allowDebug] If truthy, include code that can render + * in debug mode. This is mildly less efficient, even if debugging is + * not turned on. + * @returns {vgl.shader} + */ function createFragmentShader(allowDebug) { var fragmentShaderSource = [ '#ifdef GL_ES', @@ -349,6 +363,9 @@ var gl_lineFeature = function (arg) { return shader; } + /** + * Create and style the data needed to render the lines. + */ function createGLLines() { var data = m_this.data(), i, j, k, v, v2, lidx, @@ -533,12 +550,14 @@ var gl_lineFeature = function (arg) { } /** - * Return the arrangement of vertices used for each line segment. + * Return the arrangement of vertices used for each line segment. Each line + * is rendered by two triangles. This reports how the vertices of those + * triangles are arranged. Each entry is a triple: the line-end number, the + * vertex use, and the side of the line that the vertex is on. * - * @returns {Number} + * @returns {array[]} */ this.featureVertices = function () { - // return [[0, -1], [0, 1], [1, -1], [1, 1], [1, -1], [0, 1]]; return [[0, 'corner', -1], [0, 'near', 1], [1, 'far', -1], [1, 'corner', 1], [1, 'near', -1], [0, 'far', 1]]; }; @@ -546,14 +565,17 @@ var gl_lineFeature = function (arg) { /** * Return the number of vertices used for each line segment. * - * @returns {Number} + * @returns {number} */ this.verticesPerFeature = function () { return m_this.featureVertices().length; }; /** - * Initialize + * Initialize. + * + * @param {geo.lineFeature.spec} arg The feature specification. + * @returns {this} */ this._init = function (arg) { var prog = vgl.shaderProgram(), @@ -640,10 +662,11 @@ var gl_lineFeature = function (arg) { geom.addSource(flagsData); geom.addPrimitive(triangles); m_mapper.setGeometryData(geom); + return m_this; }; /** - * Return list of actors + * Return list of vgl actorss used for rendering. * * @returns {vgl.actor[]} */ @@ -655,9 +678,9 @@ var gl_lineFeature = function (arg) { }; /** - * Build + * Build. Create the necessary elements to render lines. * - * @override + * @returns {this} */ this._build = function () { createGLLines(); @@ -666,12 +689,13 @@ var gl_lineFeature = function (arg) { m_this.renderer().contextRenderer().addActor(m_actor); } m_this.buildTime().modified(); + return m_this; }; /** - * Update + * Update. Rebuild if necessary. * - * @override + * @returns {this} */ this._update = function () { s_update.call(m_this); @@ -687,10 +711,11 @@ var gl_lineFeature = function (arg) { m_actor.setVisible(m_this.visible()); m_actor.material().setBinNumber(m_this.bin()); m_this.updateTime().modified(); + return m_this; }; /** - * Destroy + * Destroy. Free used resources. */ this._exit = function () { m_this.renderer().contextRenderer().removeActor(m_actor); @@ -704,7 +729,6 @@ var gl_lineFeature = function (arg) { inherit(gl_lineFeature, lineFeature); -// Now register it var capabilities = {}; capabilities[lineFeature.capabilities.basic] = true; capabilities[lineFeature.capabilities.multicolor] = true; diff --git a/src/gl/polygonFeature.js b/src/gl/polygonFeature.js index 6ad98efa24..9c6df95027 100644 --- a/src/gl/polygonFeature.js +++ b/src/gl/polygonFeature.js @@ -80,7 +80,7 @@ var gl_polygonFeature = function (arg) { } /** - * Create and style the triangles need to render the polygons. + * Create and style the triangles needed to render the polygons. * * There are several optimizations to do less work when possible. If only * styles have changed, the triangulation is not recomputed, nor is the @@ -315,7 +315,6 @@ var gl_polygonFeature = function (arg) { * @override */ this._build = function () { - createGLPolygons(m_this.dataTime().getMTime() < m_this.buildTime().getMTime() && m_geometry); if (!m_this.renderer().contextRenderer().hasActor(m_actor)) { diff --git a/src/lineFeature.js b/src/lineFeature.js index 0645be9195..0bf988fb31 100644 --- a/src/lineFeature.js +++ b/src/lineFeature.js @@ -4,48 +4,51 @@ var timestamp = require('./timestamp'); var transform = require('./transform'); /** - * Create a new instance of class lineFeature + * Line feature specification. * - * @class geo.lineFeature - * @extends geo.feature - * @param {Object|Function} [arg.position] Position of the data. Default is + * @typedef {geo.feature.spec} geo.lineFeature.spec + * @param {object|Function} [position] Position of the data. Default is * (data). - * @param {Object|Function} [arg.line] Lines from the data. Default is (data). + * @param {object|Function} [line] Lines from the data. Default is (data). * Typically, the data is an array of lines, each of which is an array of * points. Only lines that have at least two points are rendered. The - * position function is called for each point as position(linePoint, - * pointIndex, lineEntry, lineEntryIndex). - * @param {boolean} [arg.selectionAPI=false] True to send selection events on - * mouse move, click, etc. - * @param {boolean} [arg.visible=true] True to show this feature. - * @param {Object} [arg.style] Style object with default style options. - * @param {Object|Function} [arg.style.strokeColor] Color to stroke each - * line. The color can vary by point. Colors can be css names or hex - * values, or an object with r, g, b on a [0-1] scale. - * @param {number|Function} [arg.style.strokeOpacity] Opacity for each line + * position function is called for each point as `position(linePoint, + * pointIndex, lineEntry, lineEntryIndex)`. + * @param {object} [style] Style object with default style options. + * @param {geo.geoColor|Function} [style.strokeColor] Color to stroke each + * line. The color can vary by point. + * @param {number|Function} [style.strokeOpacity] Opacity for each line * stroke. The opacity can vary by point. Opacity is on a [0-1] scale. - * @param {number|Function} [arg.style.strokeWidth] The weight of the line + * @param {number|Function} [style.strokeWidth] The weight of the line * stroke in pixels. The width can vary by point. - * @param {number|Function} [arg.style.strokeOffset] This is a value from -1 + * @param {number|Function} [style.strokeOffset] This is a value from -1 * (left) to 1 (right), with 0 being centered. This can vary by point. - * @param {string|Function} [arg.style.lineCap] One of 'butt' (default), - * 'square', or 'round'. This can vary by point. - * @param {string|Function} [arg.style.lineJoin] One of 'miter' (default), - * 'bevel', 'round', or 'miter-clip'. This can vary by point. - * @param {number|Function} [arg.style.closed] If true and the renderer + * @param {string|Function} [style.lineCap='butt'] One of 'butt', 'square', or + * 'round'. This can vary by point. + * @param {string|Function} [style.lineJoin='miter'] One of 'miter', 'bevel', + * 'round', or 'miter-clip'. This can vary by point. + * @param {boolean|Function} [style.closed=false] If true and the renderer * supports it, connect the first and last points of a line if the line has * more than two points. This applies per line (if a function, it is called - * with (lineEntry, lineEntryIndex). - * @param {number|Function} [arg.style.miterLimit] For lines of more than two - * segments that are mitered, if the miter length exceeds the strokeWidth + * with `(lineEntry, lineEntryIndex)`. + * @param {number|Function} [style.miterLimit=10] For lines of more than two + * segments that are mitered, if the miter length exceeds the `strokeWidth` * divided by the sine of half the angle between segments, then a bevel join * is used instead. This is a single value that applies to all lines. If a - * function, it is called with (data). - * @param {string|Function} [arg.style.antialiasing] Antialiasing distance in + * function, it is called with `(data)`. + * @param {number|Function} [style.antialiasing] Antialiasing distance in * pixels. Values must be non-negative. A value greater than 1 will produce * a visible gradient. This is a single value that applies to all lines. - * @param {string|Function} [arg.style.debug] If 'debug', render lines in debug + * @param {string|Function} [style.debug] If 'debug', render lines in debug * mode. This is a single value that applies to all lines. + */ + +/** + * Create a new instance of class lineFeature. + * + * @class geo.lineFeature + * @extends geo.feature + * @param {geo.lineFeature.spec} arg * @returns {geo.lineFeature} */ var lineFeature = function (arg) { @@ -78,9 +81,11 @@ var lineFeature = function (arg) { }; /** - * Get/Set line accessor + * Get/set lineaccessor. * - * @returns {geo.pointFeature} + * @param {object} [val] if specified, use this for the line accessor + * and return the feature. If not specified, return the current line. + * @returns {object|this} The current line or this feature. */ this.line = function (val) { if (val === undefined) { @@ -94,9 +99,12 @@ var lineFeature = function (arg) { }; /** - * Get/Set position accessor + * Get/Set position accessor. * - * @returns {geo.pointFeature} + * @param {object} [val] if specified, use this for the position accessor + * and return the feature. If not specified, return the current + * position. + * @returns {object|this} The current position or this feature. */ this.position = function (val) { if (val === undefined) { @@ -110,12 +118,19 @@ var lineFeature = function (arg) { }; /** - * Cache information needed for point searches. + * Cache information needed for point searches. The point search + * information record is an array with one entry per line, each entry of + * which is an array with one entry per line segment. These each contain + * an object with the end coordinates (`u`, `v`) of the segment in map gcs + * coordinates and the square of the maximum half-width that needs to be + * considered for the line (`r2`). + * + * @returns {object} The point search information record. */ this._updatePointSearchInfo = function () { if (m_pointSearchTime.getMTime() >= m_this.dataTime().getMTime() && m_pointSearchTime.getMTime() >= m_this.getMTime()) { - return; + return m_pointSearchInfo; } m_pointSearchTime.modified(); m_pointSearchInfo = []; @@ -156,16 +171,18 @@ var lineFeature = function (arg) { }; /** - * Returns an array of datum indices that contain the given point. - * This is a slow implementation with runtime order of the number of - * vertices. A point is considered on a line segment if it is close to the - * line or either end point. Closeness is based on the maximum width of the - * line segement, and is ceil(maxwidth / 2) + 2 pixels. This means that - * corner extensions due to mitering may be outside of the selection area and - * that variable width lines will have a greater selection region than their - * visual size at the narrow end. + * Returns an array of datum indices that contain the given point. This is a + * slow implementation with runtime order of the number of vertices. A point + * is considered on a line segment if it is close to the line or either end + * point. Closeness is based on the maximum width of the line segement, and + * is `ceil(maxwidth / 2) + 2` pixels. This means that corner extensions + * due to mitering may be outside of the selection area and that variable- + * width lines will have a greater selection region than their visual size at + * the narrow end. * * @param {geo.geoPosition} p point to search for in map interface gcs. + * @returns {object} An object with `index`: a list of line indices, and + * `found`: a list of quads that contain the specified coordinate. */ this.pointSearch = function (p) { var data = m_this.data(), indices = [], found = []; @@ -222,7 +239,15 @@ var lineFeature = function (arg) { }; /** - * Returns an array of line indices that are contained in the given box. + * Search for lines contained within a rectangilar region. + * + * @param {geo.geoPosition} lowerLeft Lower-left corner in gcs coordinates. + * @param {geo.geoPosition} upperRight Upper-right corner in gcs coordinates. + * @param {object} [opts] Additional search options. + * @param {boolean} [opts.partial=false] If truthy, include lines that are + * partially in the box, otherwise only include lines that are fully + * within the region. + * @returns {number[]} A list of line indices that are in the box region. */ this.boxSearch = function (lowerLeft, upperRight, opts) { var pos = m_this.position(), @@ -256,7 +281,10 @@ var lineFeature = function (arg) { }; /** - * Initialize + * Initialize. + * + * @param {geo.lineFeature.spec} arg The feature specification. + * @returns {this} */ this._init = function (arg) { arg = arg || {}; @@ -290,6 +318,7 @@ var lineFeature = function (arg) { m_this.style(defaultStyle); m_this.dataTime().modified(); + return m_this; }; this._init(arg); @@ -297,11 +326,12 @@ var lineFeature = function (arg) { }; /** - * Create a lineFeature from an object. + * Create a lineFeature. + * * @see {@link geo.feature.create} * @param {geo.layer} layer The layer to add the feature to - * @param {geo.lineFeature.spec} spec The object specification - * @returns {geo.lineFeature|null} + * @param {geo.lineFeature.spec} spec The feature specification + * @returns {geo.lineFeature|null} The created feature or `null` for failure. */ lineFeature.create = function (layer, spec) { 'use strict'; diff --git a/src/polygonFeature.js b/src/polygonFeature.js index 843823a787..bdb2f10b19 100644 --- a/src/polygonFeature.js +++ b/src/polygonFeature.js @@ -19,16 +19,14 @@ var transform = require('./transform'); * @param {object} [style] Style object with default style options. * @param {boolean|Function} [style.fill] True to fill polygon. Defaults to * true. - * @param {object|Function} [style.fillColor] Color to fill each polygon. The - * color can vary by vertex. Colors can be css names or hex values, or an - * object with r, g, b on a [0-1] scale. + * @param {geo.geoColor|Function} [style.fillColor] Color to fill each polygon. + * The color can vary by vertex. * @param {number|Function} [style.fillOpacity] Opacity for each polygon. The * opacity can vary by vertex. Opacity is on a [0-1] scale. * @param {boolean|Function} [style.stroke] True to stroke polygon. Defaults * to false. - * @param {object|Function} [style.strokeColor] Color to stroke each polygon. - * The color can vary by vertex. Colors can be css names or hex values, or - * an object with r, g, b on a [0-1] scale. + * @param {geo.geoColor|Function} [style.strokeColor] Color to stroke each + * polygon. The color can vary by vertex. * @param {number|Function} [style.strokeOpacity] Opacity for each polygon * stroke. The opacity can vary by vertex. Opacity is on a [0-1] scale. * @param {number|Function} [style.strokeWidth] The weight of the polygon @@ -215,8 +213,8 @@ var polygonFeature = function (arg) { * * @param {geo.geoPosition} coordinate point to search for in map interface * gcs. - * @returns {object} An object with `index`: a list of quad indices, and - * `found`: a list of quads that contain the specified coordinate. + * @returns {object} An object with `index`: a list of polygon indices, and + * `found`: a list of polygons that contain the specified coordinate. */ this.pointSearch = function (coordinate) { var found = [], indices = [], irecord = {}, data = m_this.data(), From 9c36186fca615530b1ad3f8cda6b2c8ed9a3c38e Mon Sep 17 00:00:00 2001 From: David Manthey Date: Thu, 22 Feb 2018 11:56:28 -0500 Subject: [PATCH 2/3] Speed up line style updates. Before, any change to a line style would recompute the geometry information as well. Now, if the position of vertices has not changes, the geometry is not recomputed. Further, many style were being called by function when they were their default values (such as the strokeOffset not being set). These properly are treated as fixed values, avoiding many function calls. The polygon feature, when stroked, is better at passing constant values for stroke opacity to the lines. In practice, this results in a large speed up. For a condition where there are lines with ~100,000 vertices, the line _build call was reduced from 450-500 ms to 12-15 ms. --- src/gl/lineFeature.js | 249 +++++++++++++++++++++++++----------------- src/polygonFeature.js | 19 ++-- 2 files changed, 162 insertions(+), 106 deletions(-) diff --git a/src/gl/lineFeature.js b/src/gl/lineFeature.js index dc16e5fcd5..9259c532a2 100644 --- a/src/gl/lineFeature.js +++ b/src/gl/lineFeature.js @@ -70,6 +70,7 @@ var gl_lineFeature = function (arg) { m_antialiasingUniform, m_flagsUniform, m_dynamicDraw = arg.dynamicDraw === undefined ? false : arg.dynamicDraw, + m_geometry, s_init = this._init, s_update = this._update; @@ -365,16 +366,17 @@ var gl_lineFeature = function (arg) { /** * Create and style the data needed to render the lines. + * + * @param {boolean} onlyStyle if true, use the existing geoemtry and just + * recalculate the style. */ - function createGLLines() { + function createGLLines(onlyStyle) { var data = m_this.data(), i, j, k, v, v2, lidx, numSegments = 0, len, - lineItem, lineItemData, + lineItemList, lineItem, lineItemData, vert = [{}, {}], v1 = vert[1], pos, posIdx3, firstpos, firstPosIdx3, - position = [], - posFunc = m_this.position(), strokeWidthFunc = m_this.style.get('strokeWidth'), strokeWidthVal, strokeColorFunc = m_this.style.get('strokeColor'), strokeColorVal, strokeOpacityFunc = m_this.style.get('strokeOpacity'), strokeOpacityVal, @@ -389,13 +391,14 @@ var gl_lineFeature = function (arg) { strokeWidthBuf, strokeColorBuf, strokeOpacityBuf, dest, dest3, geom = m_mapper.geometryData(), - closedFunc = m_this.style.get('closed'), closedVal, closed = []; + closedFunc = m_this.style.get('closed'), closedVal, closed = [], + updateFlags = true; - closedVal = util.isFunction(m_this.style('closed')) ? undefined : closedFunc(); - lineCapVal = util.isFunction(m_this.style('lineCap')) ? undefined : lineCapFunc(); - lineJoinVal = util.isFunction(m_this.style('lineJoin')) ? undefined : lineJoinFunc(); + closedVal = util.isFunction(m_this.style('closed')) ? undefined : (closedFunc() || false); + lineCapVal = util.isFunction(m_this.style('lineCap')) ? undefined : (lineCapFunc() || 'butt'); + lineJoinVal = util.isFunction(m_this.style('lineJoin')) ? undefined : (lineJoinFunc() || 'miter'); strokeColorVal = util.isFunction(m_this.style('strokeColor')) ? undefined : strokeColorFunc(); - strokeOffsetVal = util.isFunction(m_this.style('strokeOffset')) ? undefined : strokeOffsetFunc(); + strokeOffsetVal = util.isFunction(m_this.style('strokeOffset')) ? undefined : (strokeOffsetFunc() || 0); strokeOpacityVal = util.isFunction(m_this.style('strokeOpacity')) ? undefined : strokeOpacityFunc(); strokeWidthVal = util.isFunction(m_this.style('strokeWidth')) ? undefined : strokeWidthFunc(); @@ -407,54 +410,80 @@ var gl_lineFeature = function (arg) { } m_flagsUniform.set(fixedFlags); m_antialiasingUniform.set(antialiasing); - for (i = 0; i < data.length; i += 1) { - lineItem = m_this.line()(data[i], i); - if (lineItem.length < 2) { - continue; - } - numSegments += lineItem.length - 1; - for (j = 0; j < lineItem.length; j += 1) { - pos = posFunc(lineItem[j], j, lineItem, i); - position.push(pos.x); - position.push(pos.y); - position.push(pos.z || 0.0); - if (!j) { - firstpos = pos; + + if (!onlyStyle) { + var position = [], + posFunc = m_this.position(); + lineItemList = new Array(data.length); + for (i = 0; i < data.length; i += 1) { + lineItem = m_this.line()(data[i], i); + lineItemList[i] = lineItem; + if (lineItem.length < 2) { + continue; } - } - if (lineItem.length > 2 && (closedVal === undefined ? closedFunc(data[i], i) : closedVal)) { - /* line is closed */ - if (pos.x !== firstpos.x || pos.y !== firstpos.y || - pos.z !== firstpos.z) { - numSegments += 1; - closed[i] = 2; /* first and last points are distinct */ - } else { - closed[i] = 1; /* first point is repeated as last point */ + numSegments += lineItem.length - 1; + for (j = 0; j < lineItem.length; j += 1) { + pos = posFunc(lineItem[j], j, lineItem, i); + position.push(pos.x); + position.push(pos.y); + position.push(pos.z || 0.0); + if (!j) { + firstpos = pos; + } + } + if (lineItem.length > 2 && (closedVal === undefined ? closedFunc(data[i], i) : closedVal)) { + /* line is closed */ + if (pos.x !== firstpos.x || pos.y !== firstpos.y || + pos.z !== firstpos.z) { + numSegments += 1; + closed[i] = 2; /* first and last points are distinct */ + } else { + closed[i] = 1; /* first point is repeated as last point */ + } } } - } - position = transform.transformCoordinates( - m_this.gcs(), m_this.layer().map().gcs(), - position, 3); + position = transform.transformCoordinates( + m_this.gcs(), m_this.layer().map().gcs(), position, 3); + len = numSegments * order.length; + posBuf = util.getGeomBuffer(geom, 'pos', len * 3); + prevBuf = util.getGeomBuffer(geom, 'prev', len * 3); + nextBuf = util.getGeomBuffer(geom, 'next', len * 3); + farBuf = util.getGeomBuffer(geom, 'far', len * 3); + + indicesBuf = geom.primitive(0).indices(); + if (!(indicesBuf instanceof Uint16Array) || indicesBuf.length !== len) { + indicesBuf = new Uint16Array(len); + geom.primitive(0).setIndices(indicesBuf); + } + // save some information to be reused when we update only style + m_geometry = { + numSegments: numSegments, + closed: closed, + lineItemList: lineItemList, + lineCapVal: lineCapVal, + lineJoinVal: lineJoinVal, + strokeOffsetVal: strokeOffsetVal + }; + } else { + numSegments = m_geometry.numSegments; + closed = m_geometry.closed; + lineItemList = m_geometry.lineItemList; + len = numSegments * order.length; + updateFlags = ( + (lineCapVal !== m_geometry.lineCapVal || lineCapVal === undefined) || + (lineJoinVal !== m_geometry.lineJoinVal || lineJoinVal === undefined) || + (strokeOffsetVal !== m_geometry.strokeOffsetVal || strokeOffsetVal === undefined) + ); + } - len = numSegments * order.length; - posBuf = util.getGeomBuffer(geom, 'pos', len * 3); - prevBuf = util.getGeomBuffer(geom, 'prev', len * 3); - nextBuf = util.getGeomBuffer(geom, 'next', len * 3); - farBuf = util.getGeomBuffer(geom, 'far', len * 3); flagsBuf = util.getGeomBuffer(geom, 'flags', len); strokeWidthBuf = util.getGeomBuffer(geom, 'strokeWidth', len); strokeColorBuf = util.getGeomBuffer(geom, 'strokeColor', len * 3); strokeOpacityBuf = util.getGeomBuffer(geom, 'strokeOpacity', len); - indicesBuf = geom.primitive(0).indices(); - if (!(indicesBuf instanceof Uint16Array) || indicesBuf.length !== len) { - indicesBuf = new Uint16Array(len); - geom.primitive(0).setIndices(indicesBuf); - } for (i = posIdx3 = dest = dest3 = 0; i < data.length; i += 1) { - lineItem = m_this.line()(data[i], i); + lineItem = lineItemList[i]; if (lineItem.length < 2) { continue; } @@ -473,66 +502,80 @@ var gl_lineFeature = function (arg) { vert[0] = vert[1]; vert[1] = v1; } - v1.pos = j === lidx ? posIdx3 : firstPosIdx3; - v1.prev = lidx ? posIdx3 - 3 : (closed[i] ? - firstPosIdx3 + (lineItem.length - 3 + closed[i]) * 3 : posIdx3); - v1.next = j + 1 < lineItem.length ? posIdx3 + 3 : (closed[i] ? - (j !== lidx ? firstPosIdx3 + 3 : firstPosIdx3 + 6 - closed[i] * 3) : - posIdx3); + if (!onlyStyle) { + v1.pos = j === lidx ? posIdx3 : firstPosIdx3; + v1.prev = lidx ? posIdx3 - 3 : (closed[i] ? + firstPosIdx3 + (lineItem.length - 3 + closed[i]) * 3 : posIdx3); + v1.next = j + 1 < lineItem.length ? posIdx3 + 3 : (closed[i] ? + (j !== lidx ? firstPosIdx3 + 3 : firstPosIdx3 + 6 - closed[i] * 3) : + posIdx3); + } v1.strokeWidth = strokeWidthVal === undefined ? strokeWidthFunc(lineItemData, lidx, lineItem, i) : strokeWidthVal; v1.strokeColor = strokeColorVal === undefined ? strokeColorFunc(lineItemData, lidx, lineItem, i) : strokeColorVal; v1.strokeOpacity = strokeOpacityVal === undefined ? strokeOpacityFunc(lineItemData, lidx, lineItem, i) : strokeOpacityVal; - v1.strokeOffset = (strokeOffsetVal === undefined ? strokeOffsetFunc(lineItemData, lidx, lineItem, i) : strokeOffsetVal) || 0; - if (v1.strokeOffset) { - /* we use 11 bits to store the offset, and we want to store values - * from -1 to 1, so multiply our values by 1023, and use some bit - * manipulation to ensure that it is packed properly */ - v1.posStrokeOffset = Math.round(2048 + 1023 * Math.min(1, Math.max(-1, v1.strokeOffset))) & 0x7FF; - v1.negStrokeOffset = Math.round(2048 - 1023 * Math.min(1, Math.max(-1, v1.strokeOffset))) & 0x7FF; - } else { - v1.posStrokeOffset = v1.negStrokeOffset = 0; - } - if (!closed[i] && (!j || j === lineItem.length - 1)) { - v1.flags = flagsLineCap[lineCapVal === undefined ? lineCapFunc(lineItemData, lidx, lineItem, i) : lineCapVal] || flagsLineCap.butt; - } else { - v1.flags = flagsLineJoin[lineJoinVal === undefined ? lineJoinFunc(lineItemData, lidx, lineItem, i) : lineJoinVal] || flagsLineJoin.miter; + if (updateFlags) { + v1.strokeOffset = (strokeOffsetVal === undefined ? strokeOffsetFunc(lineItemData, lidx, lineItem, i) : strokeOffsetVal) || 0; + if (v1.strokeOffset) { + /* we use 11 bits to store the offset, and we want to store values + * from -1 to 1, so multiply our values by 1023, and use some bit + * manipulation to ensure that it is packed properly */ + v1.posStrokeOffset = Math.round(2048 + 1023 * Math.min(1, Math.max(-1, v1.strokeOffset))) & 0x7FF; + v1.negStrokeOffset = Math.round(2048 - 1023 * Math.min(1, Math.max(-1, v1.strokeOffset))) & 0x7FF; + } else { + v1.posStrokeOffset = v1.negStrokeOffset = 0; + } + if (!closed[i] && (!j || j === lineItem.length - 1)) { + v1.flags = flagsLineCap[lineCapVal === undefined ? lineCapFunc(lineItemData, lidx, lineItem, i) : lineCapVal] || flagsLineCap.butt; + } else { + v1.flags = flagsLineJoin[lineJoinVal === undefined ? lineJoinFunc(lineItemData, lidx, lineItem, i) : lineJoinVal] || flagsLineJoin.miter; + } } if (j) { for (k = 0; k < order.length; k += 1, dest += 1, dest3 += 3) { v = vert[order[k][0]]; v2 = vert[1 - order[k][0]]; - posBuf[dest3] = position[v.pos]; - posBuf[dest3 + 1] = position[v.pos + 1]; - posBuf[dest3 + 2] = position[v.pos + 2]; + if (!onlyStyle) { + posBuf[dest3] = position[v.pos]; + posBuf[dest3 + 1] = position[v.pos + 1]; + posBuf[dest3 + 2] = position[v.pos + 2]; + } if (!order[k][0]) { - prevBuf[dest3] = position[v.prev]; - prevBuf[dest3 + 1] = position[v.prev + 1]; - prevBuf[dest3 + 2] = position[v.prev + 2]; - nextBuf[dest3] = position[v.next]; - nextBuf[dest3 + 1] = position[v.next + 1]; - nextBuf[dest3 + 2] = position[v.next + 2]; - farBuf[dest3] = position[v2.next]; - farBuf[dest3 + 1] = position[v2.next + 1]; - farBuf[dest3 + 2] = position[v2.next + 2]; - flagsBuf[dest] = (flagsVertex[order[k][1]] | - (v.flags << flagsNearLineShift) | - (v2.flags << flagsFarLineShift) | - (v.negStrokeOffset << flagsNearOffsetShift)); + if (!onlyStyle) { + prevBuf[dest3] = position[v.prev]; + prevBuf[dest3 + 1] = position[v.prev + 1]; + prevBuf[dest3 + 2] = position[v.prev + 2]; + nextBuf[dest3] = position[v.next]; + nextBuf[dest3 + 1] = position[v.next + 1]; + nextBuf[dest3 + 2] = position[v.next + 2]; + farBuf[dest3] = position[v2.next]; + farBuf[dest3 + 1] = position[v2.next + 1]; + farBuf[dest3 + 2] = position[v2.next + 2]; + } + if (updateFlags) { + flagsBuf[dest] = (flagsVertex[order[k][1]] | + (v.flags << flagsNearLineShift) | + (v2.flags << flagsFarLineShift) | + (v.negStrokeOffset << flagsNearOffsetShift)); + } } else { - prevBuf[dest3] = position[v.next]; - prevBuf[dest3 + 1] = position[v.next + 1]; - prevBuf[dest3 + 2] = position[v.next + 2]; - nextBuf[dest3] = position[v.prev]; - nextBuf[dest3 + 1] = position[v.prev + 1]; - nextBuf[dest3 + 2] = position[v.prev + 2]; - farBuf[dest3] = position[v2.prev]; - farBuf[dest3 + 1] = position[v2.prev + 1]; - farBuf[dest3 + 2] = position[v2.prev + 2]; - flagsBuf[dest] = (flagsVertex[order[k][1]] | - (v.flags << flagsNearLineShift) | - (v2.flags << flagsFarLineShift) | - (v.posStrokeOffset << flagsNearOffsetShift)); + if (!onlyStyle) { + prevBuf[dest3] = position[v.next]; + prevBuf[dest3 + 1] = position[v.next + 1]; + prevBuf[dest3 + 2] = position[v.next + 2]; + nextBuf[dest3] = position[v.prev]; + nextBuf[dest3 + 1] = position[v.prev + 1]; + nextBuf[dest3 + 2] = position[v.prev + 2]; + farBuf[dest3] = position[v2.prev]; + farBuf[dest3 + 1] = position[v2.prev + 1]; + farBuf[dest3 + 2] = position[v2.prev + 2]; + } + if (updateFlags) { + flagsBuf[dest] = (flagsVertex[order[k][1]] | + (v.flags << flagsNearLineShift) | + (v2.flags << flagsFarLineShift) | + (v.posStrokeOffset << flagsNearOffsetShift)); + } } strokeWidthBuf[dest] = v.strokeWidth; strokeColorBuf[dest3] = v.strokeColor.r; @@ -544,9 +587,11 @@ var gl_lineFeature = function (arg) { } } - geom.boundsDirty(true); m_mapper.modified(); - m_mapper.boundsDirtyTimestamp().modified(); + if (!onlyStyle) { + geom.boundsDirty(true); + m_mapper.boundsDirtyTimestamp().modified(); + } } /** @@ -680,10 +725,18 @@ var gl_lineFeature = function (arg) { /** * Build. Create the necessary elements to render lines. * + * There are several optimizations to do less work when possible. If only + * styles have changed, the geometry is not re-transformed. If styles use + * static values (rather than functions), they are only calculated once. If + * styles have not changed that would affect flags (lineCap, lineJoin, and + * strokeOffset), the vertex flags are not recomputed -- this helps, as it is + * a slow step due to most javascript interpreters not optimizing bit + * operations. + * * @returns {this} */ this._build = function () { - createGLLines(); + createGLLines(m_this.dataTime().getMTime() < m_this.buildTime().getMTime() && m_geometry); if (!m_this.renderer().contextRenderer().hasActor(m_actor)) { m_this.renderer().contextRenderer().addActor(m_actor); diff --git a/src/polygonFeature.js b/src/polygonFeature.js index bdb2f10b19..ca1446c7ad 100644 --- a/src/polygonFeature.js +++ b/src/polygonFeature.js @@ -326,15 +326,18 @@ var polygonFeature = function (arg) { strokeStyle: linePolyStyle(polyStyle.strokeStyle), strokeColor: linePolyStyle(polyStyle.strokeColor), strokeOffset: linePolyStyle(polyStyle.strokeOffset), - strokeOpacity: function (d) { - return m_this.style.get('stroke')(d[2], d[3]) ? m_this.style.get('strokeOpacity')(d[0], d[1], d[2], d[3]) : 0; - } + strokeOpacity: util.isFunction(polyStyle.stroke) || !polyStyle.stroke ? + function (d) { + return m_this.style.get('stroke')(d[2], d[3]) ? m_this.style.get('strokeOpacity')(d[0], d[1], d[2], d[3]) : 0; + } : + linePolyStyle(polyStyle.strokeOpacity) }); var data = this.data(), - posFunc = this.style.get('position'), - polyFunc = this.style.get('polygon'); - if (data !== m_lineFeature._lastData || posFunc !== m_lineFeature._posFunc) { - var lineData = [], i, polygon, loop; + posVal = this.style('position'); + if (data !== m_lineFeature._lastData || posVal !== m_lineFeature._lastPosVal) { + var lineData = [], i, polygon, loop, + posFunc = this.style.get('position'), + polyFunc = this.style.get('polygon'); for (i = 0; i < data.length; i += 1) { polygon = polyFunc(data[i], i); @@ -354,7 +357,7 @@ var polygonFeature = function (arg) { }); m_lineFeature.data(lineData); m_lineFeature._lastData = data; - m_lineFeature._lastPosFunc = posFunc; + m_lineFeature._lastPosVal = posVal; } }; From b06e0f23453b86458d72d004f01940a7a8afeddf Mon Sep 17 00:00:00 2001 From: David Manthey Date: Thu, 22 Feb 2018 14:57:28 -0500 Subject: [PATCH 3/3] Fix a bug in `pointSearch` on closed lines. Some code was in a loop that should have been out of it. This adds a test for the correct behavior. Also, if a polygon is degenerate, it should not generate stroke information. --- src/lineFeature.js | 6 +++--- src/polygonFeature.js | 14 +++++++++----- tests/cases/lineFeature.js | 12 +++++++++++- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/lineFeature.js b/src/lineFeature.js index 0bf988fb31..20eda96e98 100644 --- a/src/lineFeature.js +++ b/src/lineFeature.js @@ -161,10 +161,10 @@ var lineFeature = function (arg) { if (!first && closed) { first = {p: p, r2: r2}; } - if (closed && last.x !== first.p.x && last.y !== first.p.y) { - record.push({u: last, v: first.p, r2: lastr2 > first.r2 ? lastr2 : first.r2}); - } }); + if (closed && first && (last.x !== first.p.x || last.y !== first.p.y)) { + record.push({u: last, v: first.p, r2: lastr2 > first.r2 ? lastr2 : first.r2}); + } m_pointSearchInfo.push(record); }); return m_pointSearchInfo; diff --git a/src/polygonFeature.js b/src/polygonFeature.js index ca1446c7ad..f22eae0cf0 100644 --- a/src/polygonFeature.js +++ b/src/polygonFeature.js @@ -345,11 +345,15 @@ var polygonFeature = function (arg) { continue; } loop = polygon.outer || (Array.isArray(polygon) ? polygon : []); - lineData.push(m_this._getLoopData(data[i], i, loop)); - if (polygon.inner) { - polygon.inner.forEach(function (loop) { - lineData.push(m_this._getLoopData(data[i], i, loop)); - }); + if (loop.length >= 2) { + lineData.push(m_this._getLoopData(data[i], i, loop)); + if (polygon.inner) { + polygon.inner.forEach(function (loop) { + if (loop.length >= 2) { + lineData.push(m_this._getLoopData(data[i], i, loop)); + } + }); + } } } m_lineFeature.position(function (d, i, item, itemIndex) { diff --git a/tests/cases/lineFeature.js b/tests/cases/lineFeature.js index 08c22fd22d..16991bfa28 100644 --- a/tests/cases/lineFeature.js +++ b/tests/cases/lineFeature.js @@ -40,6 +40,9 @@ describe('geo.lineFeature', function () { coord: [{x: 50, y: 10}, {x: 50, y: 10}] }, { coord: [{x: 60, y: 10}] + }, { + coord: [{x: 70, y: 10}, {x: 75, y: 12}, {x: 72, y: 15}, {x: 70, y: 15}], + closed: true } ]; @@ -124,6 +127,13 @@ describe('geo.lineFeature', function () { expect(pt.found.length).toBe(0); pt = line.pointSearch({x: 31, y: 32.5}); expect(pt.found.length).toBe(1); + /* On a closed line, we should find a point between the first and last + * point, but not between the first and a point that isn't the second or + * last. */ + pt = line.pointSearch({x: 70, y: 12.5}); + expect(pt.found.length).toBe(1); + pt = line.pointSearch({x: 71, y: 12.5}); + expect(pt.found.length).toBe(0); /* Variable width should match the widest of either end point */ p = line.featureGcsToDisplay({x: 40, y: 20}); pt = line.pointSearch(map.displayToGcs({x: p.x, y: p.y + 6.95})); @@ -218,7 +228,7 @@ describe('geo.lineFeature', function () { }).data(testLines); line.draw(); stepAnimationFrame(); - expect(layer.node().find('path').length).toBe(7); + expect(layer.node().find('path').length).toBe(8); var paths = layer.node().find('path'); expect(paths.eq(0).css('stroke-linecap')).toBe('butt'); expect(paths.eq(1).css('stroke-linecap')).toBe('round');