diff --git a/draftlogs/8107_fix.md b/draftlogs/8107_fix.md new file mode 100644 index 00000000000..64b73ab0dac --- /dev/null +++ b/draftlogs/8107_fix.md @@ -0,0 +1 @@ +- Fix scattergl text hidden by selected markers [[#8107](https://github.com/plotly/plotly.js/pull/8107)] diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 4a00e816cf7..24e28cefd52 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -29,6 +29,8 @@ var manageArrays = require('./manage_arrays'); var helpers = require('./helpers'); var subroutines = require('./subroutines'); var editTypes = require('./edit_types'); +const selectMode = require('../components/dragelement/helpers').selectMode; +const hasText = require('../traces/scatter/subtypes').hasText; var AX_NAME_PATTERN = require('../plots/cartesian/constants').AX_NAME_PATTERN; @@ -2166,8 +2168,8 @@ function _relayout(gd, aobj) { fullLayout._has('scatter-like') && fullLayout._has('regl') && ai === 'dragmode' && - (vi === 'lasso' || vi === 'select') && - !(vOld === 'lasso' || vOld === 'select') + selectMode(vi) !== selectMode(vOld) && + (selectMode(vi) || gd._fullData.some(trace => trace.visible === true && Registry.traceIs(trace, 'regl') && hasText(trace))) ) { flags.plot = true; } else if (valObject) editTypes.update(flags, valObject); diff --git a/src/traces/scattergl/plot.js b/src/traces/scattergl/plot.js index 19d944ab376..f42df5a3abd 100644 --- a/src/traces/scattergl/plot.js +++ b/src/traces/scattergl/plot.js @@ -81,32 +81,6 @@ var exports = module.exports = function plot(gd, subplot, cdata) { if(scene.fill2d === true) { scene.fill2d = createLine(regl); } - if(scene.glText === true) { - scene.glText = new Array(count); - for(i = 0; i < count; i++) { - scene.glText[i] = new Text(regl); - } - } - - // update main marker options - if(scene.glText) { - if(count > scene.glText.length) { - // add gl text marker - var textsToAdd = count - scene.glText.length; - for(i = 0; i < textsToAdd; i++) { - scene.glText.push(new Text(regl)); - } - } else if(count < scene.glText.length) { - // remove gl text marker - var textsToRemove = scene.glText.length - count; - var removedTexts = scene.glText.splice(count, textsToRemove); - removedTexts.forEach(function(text) { text.destroy(); }); - } - - for(i = 0; i < count; i++) { - scene.glText[i].update(scene.textOptions[i]); - } - } if(scene.line2d) { scene.line2d.update(scene.lineOptions); scene.lineOptions = scene.lineOptions.map(function(lineOptions) { @@ -309,6 +283,16 @@ var exports = module.exports = function plot(gd, subplot, cdata) { } } + if (scene.glText && (scene.dirty || scene.glTextOnFocus !== isSelectMode)) { + const textRegl = isSelectMode ? fullLayout._glcanvas.data()[1].regl : regl; + // Keep one set per canvas so mode changes reuse WebGL text buffers. + scene.glText = scene.glTextLayers[isSelectMode ? 1 : 0]; + scene.glTextOnFocus = isSelectMode; + while (scene.glText.length < count) scene.glText.push(new Text(textRegl)); + if (scene.glText.length > count) scene.glText.splice(count).forEach(text => text.destroy()); + for (let i = 0; i < count; i++) scene.glText[i].update(scene.textOptions[i]); + } + if(isSelectMode) { // create scatter instance by cloning scatter2d if(!scene.select2d) { diff --git a/src/traces/scattergl/scene_update.js b/src/traces/scattergl/scene_update.js index 080c524525c..c38d0c68ce3 100644 --- a/src/traces/scattergl/scene_update.js +++ b/src/traces/scattergl/scene_update.js @@ -34,6 +34,8 @@ module.exports = function sceneUpdate(gd, subplot) { error2d: false, line2d: false, glText: false, + glTextLayers: [[], []], + glTextOnFocus: false, select2d: false }; @@ -94,7 +96,7 @@ module.exports = function sceneUpdate(gd, subplot) { scatter2d.draw(i); } } - if(glText[i] && scene.textOptions[i]) { + if (!scene.glTextOnFocus && glText[i] && scene.textOptions[i]) { glText[i].render(); } } @@ -102,6 +104,12 @@ module.exports = function sceneUpdate(gd, subplot) { if(select2d) { select2d.draw(selectBatch); } + // Selection markers use the focus canvas, so draw their text on that canvas last. + if (scene.glTextOnFocus) { + for (let i = 0; i < count; i++) { + if (glText[i] && scene.textOptions[i]) glText[i].render(); + } + } scene.dirty = false; }; @@ -113,11 +121,7 @@ module.exports = function sceneUpdate(gd, subplot) { if(scene.error2d && scene.error2d.destroy) scene.error2d.destroy(); if(scene.line2d && scene.line2d.destroy) scene.line2d.destroy(); if(scene.select2d && scene.select2d.destroy) scene.select2d.destroy(); - if(scene.glText) { - scene.glText.forEach(function(text) { - if(text.destroy) text.destroy(); - }); - } + scene.glTextLayers.forEach(texts => texts.forEach(text => text.destroy())); scene.lineOptions = null; scene.fillOptions = null; diff --git a/test/image/mocks/gl2d_point-selection.json b/test/image/mocks/gl2d_point-selection.json index 32b4fc84308..06664bf7165 100644 --- a/test/image/mocks/gl2d_point-selection.json +++ b/test/image/mocks/gl2d_point-selection.json @@ -11,7 +11,7 @@ "color": "#67353E", "size": 12 }, - "textposition": "top left", + "textposition": "middle center", "selectedpoints": [1, 2, 3], "selected": { "marker": { diff --git a/test/jasmine/tests/scattergl_test.js b/test/jasmine/tests/scattergl_test.js index 8e3595ffd15..8e55bfbc9ad 100644 --- a/test/jasmine/tests/scattergl_test.js +++ b/test/jasmine/tests/scattergl_test.js @@ -325,6 +325,52 @@ describe('end-to-end scattergl tests', function() { .then(done, done.fail); }); + it('@gl should keep text above selected markers and restore normal trace ordering', async () => { + function redPixels(selector) { + const pixels = readPixel(gd.querySelector(selector), 170, 170, 60, 60); + let count = 0; + for (let i = 0; i < pixels.length; i += 4) { + if (pixels[i] > 150 && pixels[i + 1] < 100 && pixels[i + 2] < 100) count++; + } + return count; + } + + await Plotly.newPlot(gd, [{ + type: 'scattergl', mode: 'markers+text', x: [0], y: [0], + text: ['M'], textposition: 'middle center', textfont: { color: 'red', size: 40 }, + marker: { color: 'blue', size: 80 }, selectedpoints: [0] + }], { + width: 400, height: 400, margin: { l: 0, r: 0, t: 0, b: 0 }, + xaxis: { range: [-1, 1] }, yaxis: { range: [-1, 1] }, dragmode: 'pan', showlegend: false + }, { plotGlPixelRatio: 1 }); + expect(redPixels('.gl-canvas-focus')).toBeGreaterThan(30); + expect(redPixels('.gl-canvas-context')).toBe(0); + const scene = gd._fullLayout._plots.xy._scene; + const focusText = scene.glText[0]; + + await Plotly.restyle(gd, 'selectedpoints', null); + expect(redPixels('.gl-canvas-context')).toBeGreaterThan(30); + expect(redPixels('.gl-canvas-focus')).toBe(0); + const contextText = scene.glText[0]; + + await Plotly.relayout(gd, 'dragmode', 'select'); + await Plotly.restyle(gd, 'selectedpoints', [[0]]); + expect(redPixels('.gl-canvas-focus')).toBeGreaterThan(30); + expect(scene.glText[0]).toBe(focusText, 'reuse text buffers when returning to selection'); + await Plotly.restyle(gd, 'selectedpoints', null); + await Plotly.relayout(gd, 'dragmode', 'pan'); + expect(scene.glText[0]).toBe(contextText, 'reuse text buffers when returning to pan'); + expect(redPixels('.gl-canvas-context')).toBeGreaterThan(30); + expect(redPixels('.gl-canvas-focus')).toBe(0); + await Plotly.addTraces(gd, { + type: 'scattergl', mode: 'markers', x: [0], y: [0], marker: { color: 'blue', size: 80 } + }); + expect(redPixels('.gl-canvas-context')).toBe(0, 'later traces still cover earlier text outside selection mode'); + expect(redPixels('.gl-canvas-focus')).toBe(0); + await Plotly.deleteTraces(gd, [1]); + expect(redPixels('.gl-canvas-context')).toBeGreaterThan(30); + }); + it('@gl should update selected points', function(done) { // #2298 var dat = [{