From c02330c95dc3b820f2d24592969a32529034b272 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 2 Oct 2018 12:32:11 -0400 Subject: [PATCH] fix #2797 - clear full canvas and use redrawReglTraces on selections - no longer need _module.styleOnSelect! - selections on overlaid subplots now work! - remove clearViewport (for now), we could bring it back again to optimize selections on disjoint subplots. --- src/plots/cartesian/select.js | 47 +++++++++------------------ src/traces/scattergl/index.js | 34 ------------------- src/traces/scatterpolargl/index.js | 1 - src/traces/splom/index.js | 26 --------------- test/jasmine/tests/gl2d_click_test.js | 43 ++++++++++++++++++++++++ test/jasmine/tests/splom_test.js | 15 +++++---- 6 files changed, 67 insertions(+), 99 deletions(-) diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js index 31200f8fb1f..35d39d285a6 100644 --- a/src/plots/cartesian/select.js +++ b/src/plots/cartesian/select.js @@ -19,7 +19,8 @@ var polygon = require('../../lib/polygon'); var throttle = require('../../lib/throttle'); var makeEventData = require('../../components/fx/helpers').makeEventData; var getFromId = require('./axis_ids').getFromId; -var sortModules = require('../sort_modules').sortModules; +var clearGlCanvases = require('../../lib/clear_gl_canvases'); +var redrawReglTraces = require('../../plot_api/subroutines').redrawReglTraces; var constants = require('./constants'); var MINSELECT = constants.MINSELECT; @@ -663,7 +664,7 @@ function isOnlyOnePointSelected(searchTraces) { } function updateSelectedState(gd, searchTraces, eventData) { - var i, j, searchInfo, trace; + var i, searchInfo, cd, trace; if(eventData) { var pts = eventData.points || []; @@ -696,43 +697,25 @@ function updateSelectedState(gd, searchTraces, eventData) { } } - // group searchInfo traces by trace modules - var lookup = {}; + var hasRegl = false; for(i = 0; i < searchTraces.length; i++) { searchInfo = searchTraces[i]; + cd = searchInfo.cd; + trace = cd[0].trace; - var name = searchInfo._module.name; - if(lookup[name]) { - lookup[name].push(searchInfo); - } else { - lookup[name] = [searchInfo]; + if(Registry.traceIs(trace, 'regl')) { + hasRegl = true; } + + var _module = searchInfo._module; + var fn = _module.styleOnSelect || _module.style; + if(fn) fn(gd, cd); } - var keys = Object.keys(lookup).sort(sortModules); - - for(i = 0; i < keys.length; i++) { - var items = lookup[keys[i]]; - var len = items.length; - var item0 = items[0]; - var trace0 = item0.cd[0].trace; - var _module = item0._module; - var styleSelection = _module.styleOnSelect || _module.style; - - if(Registry.traceIs(trace0, 'regl')) { - // plot regl traces per module - var cds = new Array(len); - for(j = 0; j < len; j++) { - cds[j] = items[j].cd; - } - styleSelection(gd, cds); - } else { - // plot svg trace per trace - for(j = 0; j < len; j++) { - styleSelection(gd, items[j].cd); - } - } + if(hasRegl) { + clearGlCanvases(gd); + redrawReglTraces(gd); } } diff --git a/src/traces/scattergl/index.js b/src/traces/scattergl/index.js index 9de593704b9..8b4d7dc4f7f 100644 --- a/src/traces/scattergl/index.js +++ b/src/traces/scattergl/index.js @@ -294,18 +294,6 @@ function sceneUpdate(gd, subplot) { scene.dirty = false; }; - scene.clear = function clear() { - var vp = getViewport(gd._fullLayout, subplot.xaxis, subplot.yaxis); - - if(scene.select2d) { - clearViewport(scene.select2d, vp); - } - - var anyComponent = scene.scatter2d || scene.line2d || - (scene.glText || [])[0] || scene.fill2d || scene.error2d; - if(anyComponent) clearViewport(anyComponent, vp); - }; - // remove scene resources scene.destroy = function destroy() { if(scene.fill2d) scene.fill2d.destroy(); @@ -357,14 +345,6 @@ function getViewport(fullLayout, xaxis, yaxis) { ]; } -function clearViewport(comp, vp) { - var gl = comp.regl._gl; - gl.enable(gl.SCISSOR_TEST); - gl.scissor(vp[0], vp[1], vp[2] - vp[0], vp[3] - vp[1]); - gl.clearColor(0, 0, 0, 0); - gl.clear(gl.COLOR_BUFFER_BIT); -} - function plot(gd, subplot, cdata) { if(!cdata.length) return; @@ -889,19 +869,6 @@ function selectPoints(searchInfo, selectionTester) { return selection; } -function styleOnSelect(gd, cds) { - var stash = cds[0][0].t; - var scene = stash._scene; - - // don't clear the subplot if there are splom traces - // on the graph - if(!gd._fullLayout._has('splom')) { - scene.clear(); - } - - scene.draw(); -} - function styleTextSelection(cd) { var cd0 = cd[0]; var stash = cd0.t; @@ -951,7 +918,6 @@ module.exports = { calc: calc, plot: plot, hoverPoints: hoverPoints, - styleOnSelect: styleOnSelect, selectPoints: selectPoints, sceneOptions: sceneOptions, diff --git a/src/traces/scatterpolargl/index.js b/src/traces/scatterpolargl/index.js index 70024feb649..f317682eb90 100644 --- a/src/traces/scatterpolargl/index.js +++ b/src/traces/scatterpolargl/index.js @@ -183,7 +183,6 @@ module.exports = { calc: calc, plot: plot, hoverPoints: hoverPoints, - styleOnSelect: ScatterGl.styleOnSelect, selectPoints: ScatterGl.selectPoints, meta: { diff --git a/src/traces/splom/index.js b/src/traces/splom/index.js index ea759a89915..a08363f2f96 100644 --- a/src/traces/splom/index.js +++ b/src/traces/splom/index.js @@ -438,31 +438,6 @@ function selectPoints(searchInfo, selectionTester) { return selection; } -function styleOnSelect(gd, cds) { - var fullLayout = gd._fullLayout; - var cd0 = cds[0]; - var scene0 = fullLayout._splomScenes[cd0[0].trace.uid]; - scene0.matrix.regl.clear({color: true, depth: true}); - - if(fullLayout._splomGrid) { - fullLayout._splomGrid.draw(); - } - - for(var i = 0; i < cds.length; i++) { - var scene = fullLayout._splomScenes[cds[i][0].trace.uid]; - scene.draw(); - } - - // redraw all subplot with scattergl traces, - // as we cleared the whole canvas above - if(fullLayout._has('cartesian')) { - for(var k in fullLayout._plots) { - var sp = fullLayout._plots[k]; - if(sp._scene) sp._scene.draw(); - } - } -} - function getDimIndex(trace, ax) { var axId = ax._id; var axLetter = axId.charAt(0); @@ -491,7 +466,6 @@ module.exports = { plot: plot, hoverPoints: hoverPoints, selectPoints: selectPoints, - styleOnSelect: styleOnSelect, editStyle: editStyle, meta: { diff --git a/test/jasmine/tests/gl2d_click_test.js b/test/jasmine/tests/gl2d_click_test.js index c2b2121e386..34a826faca9 100644 --- a/test/jasmine/tests/gl2d_click_test.js +++ b/test/jasmine/tests/gl2d_click_test.js @@ -1135,4 +1135,47 @@ describe('@noCI Test gl2d lasso/select:', function() { .catch(failTest) .then(done); }); + + it('should work on overlaid subplots', function(done) { + gd = createGraphDiv(); + + var scene, scene2; + + Plotly.plot(gd, [{ + x: [1, 2, 3], + y: [40, 50, 60], + type: 'scattergl', + mode: 'markers' + }, { + x: [2, 3, 4], + y: [4, 5, 6], + yaxis: 'y2', + type: 'scattergl', + mode: 'markers' + }], { + xaxis: {domain: [0.2, 1]}, + yaxis2: {overlaying: 'y', side: 'left', position: 0}, + showlegend: false, + margin: {l: 0, t: 0, b: 0, r: 0}, + width: 400, + height: 400, + dragmode: 'select' + }) + .then(delay(100)) + .then(function() { + scene = gd._fullLayout._plots.xy._scene; + scene2 = gd._fullLayout._plots.xy2._scene; + + spyOn(scene.scatter2d, 'draw'); + spyOn(scene2.scatter2d, 'draw'); + }) + .then(function() { return select([[20, 20], [380, 250]]); }) + .then(function() { + expect(scene.scatter2d.draw).toHaveBeenCalledTimes(1); + expect(scene2.scatter2d.draw).toHaveBeenCalledTimes(1); + }) + .catch(failTest) + .then(done); + + }); }); diff --git a/test/jasmine/tests/splom_test.js b/test/jasmine/tests/splom_test.js index 5760f4bb528..0babe3221fa 100644 --- a/test/jasmine/tests/splom_test.js +++ b/test/jasmine/tests/splom_test.js @@ -1298,23 +1298,26 @@ describe('Test splom select:', function() { var cnt = 0; var scatterGlCnt = 0; var splomCnt = 0; + var scatterglScene, splomScene; Plotly.newPlot(gd, fig).then(function() { - // 'scattergl' trace module - spyOn(gd._fullLayout._modules[0], 'styleOnSelect').and.callFake(function() { + var fullLayout = gd._fullLayout; + scatterglScene = fullLayout._plots.xy._scene; + splomScene = fullLayout._splomScenes[gd._fullData[1].uid]; + + spyOn(scatterglScene, 'draw').and.callFake(function() { cnt++; scatterGlCnt = cnt; }); - // 'splom' trace module - spyOn(gd._fullLayout._modules[1], 'styleOnSelect').and.callFake(function() { + spyOn(splomScene, 'draw').and.callFake(function() { cnt++; splomCnt = cnt; }); }) .then(function() { return _select([[20, 395], [195, 205]]); }) .then(function() { - expect(gd._fullLayout._modules[0].styleOnSelect).toHaveBeenCalledTimes(1); - expect(gd._fullLayout._modules[1].styleOnSelect).toHaveBeenCalledTimes(1); + expect(scatterglScene.draw).toHaveBeenCalledTimes(1); + expect(splomScene.draw).toHaveBeenCalledTimes(1); expect(cnt).toBe(2); expect(splomCnt).toBe(1, 'splom redraw before scattergl');