From ed38e70667ef1da043559083e25a55d60fda2ce4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 3 Mar 2017 16:09:46 -0500 Subject: [PATCH 1/5] don't create scattergl trace objects on initialization - instead, create them on update when needed - this should speed up scattergl plotting especially when multiple traces are present - TODO: we'll need to reorder the trace object when adding a a 'mode' or an error bar to make sure e.g. lines are always drawn below scatter pts. --- src/traces/scattergl/convert.js | 188 +++++++++++++++++--------------- 1 file changed, 98 insertions(+), 90 deletions(-) diff --git a/src/traces/scattergl/convert.js b/src/traces/scattergl/convert.js index fcfa54eb6d4..65acdc8af64 100644 --- a/src/traces/scattergl/convert.js +++ b/src/traces/scattergl/convert.js @@ -49,8 +49,13 @@ function LineWithMarkers(scene, uid) { this.idToIndex = []; this.bounds = [0, 0, 0, 0]; + this.isVisible = false; this.hasLines = false; - this.lineOptions = { + this.hasErrorX = false; + this.hasErrorY = false; + this.hasMarkers = false; + + this.line = this.initObject(createLine, { positions: new Float64Array(0), color: [0, 0, 0, 1], width: 1, @@ -61,34 +66,25 @@ function LineWithMarkers(scene, uid) { [0, 0, 0, 1], [0, 0, 0, 1]], dashes: [1] - }; - this.line = createLine(scene.glplot, this.lineOptions); - this.line._trace = this; + }); - this.hasErrorX = false; - this.errorXOptions = { + this.errorX = this.initObject(createError, { positions: new Float64Array(0), errors: new Float64Array(0), lineWidth: 1, capSize: 0, color: [0, 0, 0, 1] - }; - this.errorX = createError(scene.glplot, this.errorXOptions); - this.errorX._trace = this; + }); - this.hasErrorY = false; - this.errorYOptions = { + this.errorY = this.initObject(createError, { positions: new Float64Array(0), errors: new Float64Array(0), lineWidth: 1, capSize: 0, color: [0, 0, 0, 1] - }; - this.errorY = createError(scene.glplot, this.errorYOptions); - this.errorY._trace = this; + }); - this.hasMarkers = false; - this.scatterOptions = { + var scatterOptions0 = { positions: new Float64Array(0), sizes: [], colors: [], @@ -100,16 +96,48 @@ function LineWithMarkers(scene, uid) { borderSize: 1, borderColor: [0, 0, 0, 1] }; - this.scatter = createScatter(scene.glplot, this.scatterOptions); - this.scatter._trace = this; - this.fancyScatter = createFancyScatter(scene.glplot, this.scatterOptions); - this.fancyScatter._trace = this; - this.isVisible = false; + this.scatter = this.initObject(createScatter, scatterOptions0); + this.fancyScatter = this.initObject(createFancyScatter, scatterOptions0); } var proto = LineWithMarkers.prototype; +proto.initObject = function(createFn, options) { + var _this = this; + var glplot = _this.scene.glplot; + var options0 = Lib.extendFlat({}, options); + var obj = null; + + // TODO how to handle ordering ??? + + function update() { + if(!obj) { + obj = createFn(glplot, options); + obj._trace = _this; + } + obj.update(options); + return obj; + } + + function clear() { + if(obj) obj.update(options0); + return obj; + } + + function dispose() { + if(obj) obj.dispose(); + return obj; + } + + return { + options: options, + update: update, + clear: clear, + dispose: dispose + }; +}; + proto.handlePick = function(pickResult) { var index = pickResult.pointId; @@ -233,6 +261,7 @@ function _convertColor(colors, opacities, count) { * - markers */ proto.update = function(options) { + if(options.visible !== true) { this.isVisible = false; this.hasLines = false; @@ -255,7 +284,11 @@ proto.update = function(options) { this.connectgaps = !!options.connectgaps; if(!this.isVisible) { - this.clear(); + this.line.clear(); + this.errorX.clear(); + this.errorY.clear(); + this.scatter.clear(); + this.fancyScatter.clear(); } else if(this.isFancy(options)) { this.updateFancy(options); @@ -292,22 +325,6 @@ function allFastTypesLikely(a) { return true; } -proto.clear = function() { - this.lineOptions.positions = new Float64Array(0); - this.line.update(this.lineOptions); - - this.errorXOptions.positions = new Float64Array(0); - this.errorX.update(this.errorXOptions); - - this.errorYOptions.positions = new Float64Array(0); - this.errorY.update(this.errorYOptions); - - this.scatterOptions.positions = new Float64Array(0); - this.scatterOptions.glyphs = []; - this.scatter.update(this.scatterOptions); - this.fancyScatter.update(this.scatterOptions); -}; - proto.updateFast = function(options) { var x = this.xData = this.pickXData = options.x; var y = this.yData = this.pickYData = options.y; @@ -363,34 +380,30 @@ proto.updateFast = function(options) { var markerSize; if(this.hasMarkers) { - this.scatterOptions.positions = positions; + this.scatter.options.positions = positions; var markerColor = str2RGBArray(options.marker.color), borderColor = str2RGBArray(options.marker.line.color), opacity = (options.opacity) * (options.marker.opacity); markerColor[3] *= opacity; - this.scatterOptions.color = markerColor; + this.scatter.options.color = markerColor; borderColor[3] *= opacity; - this.scatterOptions.borderColor = borderColor; + this.scatter.options.borderColor = borderColor; markerSize = options.marker.size; - this.scatterOptions.size = markerSize; - this.scatterOptions.borderSize = options.marker.line.width; + this.scatter.options.size = markerSize; + this.scatter.options.borderSize = options.marker.line.width; - this.scatter.update(this.scatterOptions); + this.scatter.update(); } else { - this.scatterOptions.positions = new Float64Array(0); - this.scatterOptions.glyphs = []; - this.scatter.update(this.scatterOptions); + this.scatter.clear(); } // turn off fancy scatter plot - this.scatterOptions.positions = new Float64Array(0); - this.scatterOptions.glyphs = []; - this.fancyScatter.update(this.scatterOptions); + this.fancyScatter.clear(); // add item for autorange routine this.expandAxesFast(bounds, markerSize); @@ -464,16 +477,16 @@ proto.updateFancy = function(options) { var sizes; if(this.hasMarkers) { - this.scatterOptions.positions = positions; + this.scatter.options.positions = positions; // TODO rewrite convert function so that // we don't have to loop through the data another time - this.scatterOptions.sizes = new Array(pId); - this.scatterOptions.glyphs = new Array(pId); - this.scatterOptions.borderWidths = new Array(pId); - this.scatterOptions.colors = new Array(pId * 4); - this.scatterOptions.borderColors = new Array(pId * 4); + this.scatter.options.sizes = new Array(pId); + this.scatter.options.glyphs = new Array(pId); + this.scatter.options.borderWidths = new Array(pId); + this.scatter.options.colors = new Array(pId * 4); + this.scatter.options.borderColors = new Array(pId * 4); var markerSizeFunc = makeBubbleSizeFn(options), markerOpts = options.marker, @@ -490,28 +503,24 @@ proto.updateFancy = function(options) { for(i = 0; i < pId; ++i) { index = idToIndex[i]; - this.scatterOptions.sizes[i] = 4.0 * sizes[index]; - this.scatterOptions.glyphs[i] = glyphs[index]; - this.scatterOptions.borderWidths[i] = 0.5 * borderWidths[index]; + this.scatter.options.sizes[i] = 4.0 * sizes[index]; + this.scatter.options.glyphs[i] = glyphs[index]; + this.scatter.options.borderWidths[i] = 0.5 * borderWidths[index]; for(j = 0; j < 4; ++j) { - this.scatterOptions.colors[4 * i + j] = colors[4 * index + j]; - this.scatterOptions.borderColors[4 * i + j] = borderColors[4 * index + j]; + this.scatter.options.colors[4 * i + j] = colors[4 * index + j]; + this.scatter.options.borderColors[4 * i + j] = borderColors[4 * index + j]; } } - this.fancyScatter.update(this.scatterOptions); + this.fancyScatter.update(); } else { - this.scatterOptions.positions = new Float64Array(0); - this.scatterOptions.glyphs = []; - this.fancyScatter.update(this.scatterOptions); + this.fancyScatter.clear(); } // turn off fast scatter plot - this.scatterOptions.positions = new Float64Array(0); - this.scatterOptions.glyphs = []; - this.scatter.update(this.scatterOptions); + this.scatter.clear(); // add item for autorange routine this.expandAxesFancy(x, y, sizes); @@ -535,61 +544,60 @@ proto.updateLines = function(options, positions) { } } - this.lineOptions.positions = linePositions; + this.line.options.positions = linePositions; var lineColor = convertColor(options.line.color, options.opacity, 1), - lineWidth = Math.round(0.5 * this.lineOptions.width), + lineWidth = Math.round(0.5 * this.line.options.width), dashes = (DASHES[options.line.dash] || [1]).slice(); for(i = 0; i < dashes.length; ++i) dashes[i] *= lineWidth; switch(options.fill) { case 'tozeroy': - this.lineOptions.fill = [false, true, false, false]; + this.line.options.fill = [false, true, false, false]; break; case 'tozerox': - this.lineOptions.fill = [true, false, false, false]; + this.line.options.fill = [true, false, false, false]; break; default: - this.lineOptions.fill = [false, false, false, false]; + this.line.options.fill = [false, false, false, false]; break; } var fillColor = str2RGBArray(options.fillcolor); - this.lineOptions.color = lineColor; - this.lineOptions.width = 2.0 * options.line.width; - this.lineOptions.dashes = dashes; - this.lineOptions.fillColor = [fillColor, fillColor, fillColor, fillColor]; + this.line.options.color = lineColor; + this.line.options.width = 2.0 * options.line.width; + this.line.options.dashes = dashes; + this.line.options.fillColor = [fillColor, fillColor, fillColor, fillColor]; + + this.line.update(); } else { - this.lineOptions.positions = new Float64Array(0); + this.line.clear(); } - - this.line.update(this.lineOptions); }; proto.updateError = function(axLetter, options, positions, errors) { var errorObj = this['error' + axLetter], - errorOptions = options['error_' + axLetter.toLowerCase()], - errorObjOptions = this['error' + axLetter + 'Options']; + errorOptions = options['error_' + axLetter.toLowerCase()]; if(axLetter.toLowerCase() === 'x' && errorOptions.copy_ystyle) { errorOptions = options.error_y; } if(this['hasError' + axLetter]) { - errorObjOptions.positions = positions; - errorObjOptions.errors = errors; - errorObjOptions.capSize = errorOptions.width; - errorObjOptions.lineWidth = errorOptions.thickness / 2; // ballpark rescaling - errorObjOptions.color = convertColor(errorOptions.color, 1, 1); + errorObj.options.positions = positions; + errorObj.options.errors = errors; + errorObj.options.capSize = errorOptions.width; + errorObj.options.lineWidth = errorOptions.thickness / 2; // ballpark rescaling + errorObj.options.color = convertColor(errorOptions.color, 1, 1); + + errorObj.update(); } else { - errorObjOptions.positions = new Float64Array(0); + errorObj.clear(); } - - errorObj.update(errorObjOptions); }; proto.expandAxesFast = function(bounds, markerSize) { From f1278725b8b0c685bb4bfa966206241e2c576812 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 3 Mar 2017 16:10:09 -0500 Subject: [PATCH 2/5] fixup gl2d looking for scattergl trace objects --- .../tests/gl2d_date_axis_render_test.js | 26 ++++++++++++------- test/jasmine/tests/gl_plot_interact_test.js | 4 ++- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/test/jasmine/tests/gl2d_date_axis_render_test.js b/test/jasmine/tests/gl2d_date_axis_render_test.js index 53392de3e76..3dcc9214b20 100644 --- a/test/jasmine/tests/gl2d_date_axis_render_test.js +++ b/test/jasmine/tests/gl2d_date_axis_render_test.js @@ -1,4 +1,4 @@ -var PlotlyInternal = require('@src/plotly'); +var Plotly = require('@lib'); var hasWebGLSupport = require('../assets/has_webgl_support'); @@ -18,7 +18,7 @@ describe('date axis', function() { afterEach(destroyGraphDiv); it('should use the fancy gl-vis/gl-scatter2d', function() { - PlotlyInternal.plot(gd, [{ + Plotly.plot(gd, [{ type: 'scattergl', 'marker': { 'color': 'rgb(31, 119, 180)', @@ -38,11 +38,13 @@ describe('date axis', function() { expect(gd._fullData[0]._module.basePlotModule.name).toBe('gl2d'); // one way of check which renderer - fancy vs not - we're using - expect(gd._fullLayout._plots.xy._scene2d.glplot.objects[3].pointCount).toBe(0); + var objs = gd._fullLayout._plots.xy._scene2d.glplot.objects; + expect(objs.length).toEqual(2); + expect(objs[1].points.length).toEqual(4); }); it('should use the fancy gl-vis/gl-scatter2d once again', function() { - PlotlyInternal.plot(gd, [{ + Plotly.plot(gd, [{ type: 'scattergl', 'marker': { 'color': 'rgb(31, 119, 180)', @@ -62,11 +64,13 @@ describe('date axis', function() { expect(gd._fullData[0]._module.basePlotModule.name).toBe('gl2d'); // one way of check which renderer - fancy vs not - we're using - expect(gd._fullLayout._plots.xy._scene2d.glplot.objects[3].pointCount).toBe(0); + var objs = gd._fullLayout._plots.xy._scene2d.glplot.objects; + expect(objs.length).toEqual(2); + expect(objs[1].points.length).toEqual(4); }); it('should now use the non-fancy gl-vis/gl-scatter2d', function() { - PlotlyInternal.plot(gd, [{ + Plotly.plot(gd, [{ type: 'scattergl', mode: 'markers', // important, as otherwise lines are assumed (which needs fancy) x: [new Date('2016-10-10'), new Date('2016-10-11')], @@ -78,11 +82,13 @@ describe('date axis', function() { expect(gd._fullData[0].type).toBe('scattergl'); expect(gd._fullData[0]._module.basePlotModule.name).toBe('gl2d'); - expect(gd._fullLayout._plots.xy._scene2d.glplot.objects[3].pointCount).toBe(2); + var objs = gd._fullLayout._plots.xy._scene2d.glplot.objects; + expect(objs.length).toEqual(1); + expect(objs[0].pointCount).toEqual(2); }); it('should use the non-fancy gl-vis/gl-scatter2d with string dates', function() { - PlotlyInternal.plot(gd, [{ + Plotly.plot(gd, [{ type: 'scattergl', mode: 'markers', // important, as otherwise lines are assumed (which needs fancy) x: ['2016-10-10', '2016-10-11'], @@ -94,6 +100,8 @@ describe('date axis', function() { expect(gd._fullData[0].type).toBe('scattergl'); expect(gd._fullData[0]._module.basePlotModule.name).toBe('gl2d'); - expect(gd._fullLayout._plots.xy._scene2d.glplot.objects[3].pointCount).toBe(2); + var objs = gd._fullLayout._plots.xy._scene2d.glplot.objects; + expect(objs.length).toEqual(1); + expect(objs[0].pointCount).toEqual(2); }); }); diff --git a/test/jasmine/tests/gl_plot_interact_test.js b/test/jasmine/tests/gl_plot_interact_test.js index db4a57922a2..8a1717af26a 100644 --- a/test/jasmine/tests/gl_plot_interact_test.js +++ b/test/jasmine/tests/gl_plot_interact_test.js @@ -375,7 +375,9 @@ describe('Test gl plot interactions', function() { }); it('should be able to toggle visibility', function(done) { - var OBJECT_PER_TRACE = 5; + + // a line object + scatter fancy + var OBJECT_PER_TRACE = 2; var objects = function() { return gd._fullLayout._plots.xy._scene2d.glplot.objects; From 18e2d2bbd1b2f99ada37649b144195bfacee410a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 3 Mar 2017 16:47:11 -0500 Subject: [PATCH 3/5] sort scattergl objects - at end of .update step - to preserve order on updates --- src/traces/scattergl/convert.js | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/traces/scattergl/convert.js b/src/traces/scattergl/convert.js index 65acdc8af64..c67fb2df327 100644 --- a/src/traces/scattergl/convert.js +++ b/src/traces/scattergl/convert.js @@ -65,8 +65,8 @@ function LineWithMarkers(scene, uid) { [0, 0, 0, 1], [0, 0, 0, 1], [0, 0, 0, 1]], - dashes: [1] - }); + dashes: [1], + }, 0); this.errorX = this.initObject(createError, { positions: new Float64Array(0), @@ -74,7 +74,7 @@ function LineWithMarkers(scene, uid) { lineWidth: 1, capSize: 0, color: [0, 0, 0, 1] - }); + }, 1); this.errorY = this.initObject(createError, { positions: new Float64Array(0), @@ -82,7 +82,7 @@ function LineWithMarkers(scene, uid) { lineWidth: 1, capSize: 0, color: [0, 0, 0, 1] - }); + }, 2); var scatterOptions0 = { positions: new Float64Array(0), @@ -97,24 +97,23 @@ function LineWithMarkers(scene, uid) { borderColor: [0, 0, 0, 1] }; - this.scatter = this.initObject(createScatter, scatterOptions0); - this.fancyScatter = this.initObject(createFancyScatter, scatterOptions0); + this.scatter = this.initObject(createScatter, scatterOptions0, 3); + this.fancyScatter = this.initObject(createFancyScatter, scatterOptions0, 4); } var proto = LineWithMarkers.prototype; -proto.initObject = function(createFn, options) { +proto.initObject = function(createFn, options, index) { var _this = this; var glplot = _this.scene.glplot; var options0 = Lib.extendFlat({}, options); var obj = null; - // TODO how to handle ordering ??? - function update() { if(!obj) { obj = createFn(glplot, options); obj._trace = _this; + obj._index = index; } obj.update(options); return obj; @@ -254,12 +253,6 @@ function _convertColor(colors, opacities, count) { return result; } -/* Order is important here to get the correct laying: - * - lines - * - errorX - * - errorY - * - markers - */ proto.update = function(options) { if(options.visible !== true) { @@ -297,6 +290,15 @@ proto.update = function(options) { this.updateFast(options); } + // sort objects so that order is preserve on updates: + // - lines + // - errorX + // - errorY + // - markers + this.scene.glplot.objects.sort(function(a, b) { + return a._index - b._index; + }); + // not quite on-par with 'scatter', but close enough for now // does not handle the colorscale case this.color = getTraceColor(options, {}); From 81d3bf68d483c2d702a7108026893745a09baaf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 6 Mar 2017 10:29:03 -0500 Subject: [PATCH 4/5] sort gl-vis object per trace in scene2d update traces --- src/plots/gl2d/scene2d.js | 6 ++++++ src/traces/scattergl/convert.js | 8 ++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/plots/gl2d/scene2d.js b/src/plots/gl2d/scene2d.js index 7d53c2cf800..935c6951e05 100644 --- a/src/plots/gl2d/scene2d.js +++ b/src/plots/gl2d/scene2d.js @@ -494,6 +494,12 @@ proto.updateTraces = function(fullData, calcData) { this.traces[fullTrace.uid] = traceObj; } } + + // order object per traces + this.glplot.objects.sort(function(a, b) { + return a._trace.index - b._trace.index; + }); + }; proto.emitPointAction = function(nextSelection, eventType) { diff --git a/src/traces/scattergl/convert.js b/src/traces/scattergl/convert.js index c67fb2df327..0845118dbdc 100644 --- a/src/traces/scattergl/convert.js +++ b/src/traces/scattergl/convert.js @@ -46,6 +46,7 @@ function LineWithMarkers(scene, uid) { this.hoverinfo = 'all'; this.connectgaps = true; + this.index = null; this.idToIndex = []; this.bounds = [0, 0, 0, 0]; @@ -103,7 +104,7 @@ function LineWithMarkers(scene, uid) { var proto = LineWithMarkers.prototype; -proto.initObject = function(createFn, options, index) { +proto.initObject = function(createFn, options, objIndex) { var _this = this; var glplot = _this.scene.glplot; var options0 = Lib.extendFlat({}, options); @@ -113,7 +114,7 @@ proto.initObject = function(createFn, options, index) { if(!obj) { obj = createFn(glplot, options); obj._trace = _this; - obj._index = index; + obj._index = objIndex; } obj.update(options); return obj; @@ -299,6 +300,9 @@ proto.update = function(options) { return a._index - b._index; }); + // set trace index so that scene2d can sort object per traces + this.index = options.index; + // not quite on-par with 'scatter', but close enough for now // does not handle the colorscale case this.color = getTraceColor(options, {}); From 34ea31e9d58790928020f4fa7a6b982d6985ae16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 13 Mar 2017 18:32:20 -0400 Subject: [PATCH 5/5] :hocho: useless return statements --- src/traces/scattergl/convert.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/traces/scattergl/convert.js b/src/traces/scattergl/convert.js index 0845118dbdc..31ead16a5b8 100644 --- a/src/traces/scattergl/convert.js +++ b/src/traces/scattergl/convert.js @@ -117,17 +117,14 @@ proto.initObject = function(createFn, options, objIndex) { obj._index = objIndex; } obj.update(options); - return obj; } function clear() { if(obj) obj.update(options0); - return obj; } function dispose() { if(obj) obj.dispose(); - return obj; } return {