From 12f3e1f83ad9a89e5480a2172d65999a983240d5 Mon Sep 17 00:00:00 2001 From: archmoj Date: Fri, 21 Dec 2018 18:18:33 -0500 Subject: [PATCH 01/15] removed indices convert --- src/traces/mesh3d/defaults.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/traces/mesh3d/defaults.js b/src/traces/mesh3d/defaults.js index 56fc6ae1821..67290c75c83 100644 --- a/src/traces/mesh3d/defaults.js +++ b/src/traces/mesh3d/defaults.js @@ -41,13 +41,6 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout return; } - if(indices) { - // otherwise, convert all face indices to ints - indices.forEach(function(index) { - for(var i = 0; i < index.length; ++i) index[i] |= 0; - }); - } - var handleCalendarDefaults = Registry.getComponentMethod('calendars', 'handleTraceDefaults'); handleCalendarDefaults(traceIn, traceOut, ['x', 'y', 'z'], layout); From 33e2b9a085f0732a7f2b096278e9e25843f3d9aa Mon Sep 17 00:00:00 2001 From: archmoj Date: Fri, 21 Dec 2018 18:34:45 -0500 Subject: [PATCH 02/15] if no indices then trace visible to false --- src/traces/mesh3d/defaults.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/traces/mesh3d/defaults.js b/src/traces/mesh3d/defaults.js index 67290c75c83..f4917913058 100644 --- a/src/traces/mesh3d/defaults.js +++ b/src/traces/mesh3d/defaults.js @@ -36,7 +36,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout var coords = readComponents(['x', 'y', 'z']); var indices = readComponents(['i', 'j', 'k']); - if(!coords) { + if(!coords || !indices) { traceOut.visible = false; return; } From f1772a21acaa6a45d70cf635fb591baf27ff70cf Mon Sep 17 00:00:00 2001 From: archmoj Date: Fri, 21 Dec 2018 18:55:46 -0500 Subject: [PATCH 03/15] check for vertex data only --- src/traces/mesh3d/defaults.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/traces/mesh3d/defaults.js b/src/traces/mesh3d/defaults.js index f4917913058..62a679b071a 100644 --- a/src/traces/mesh3d/defaults.js +++ b/src/traces/mesh3d/defaults.js @@ -34,9 +34,9 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout } var coords = readComponents(['x', 'y', 'z']); - var indices = readComponents(['i', 'j', 'k']); + readComponents(['i', 'j', 'k']); - if(!coords || !indices) { + if(!coords) { traceOut.visible = false; return; } From 65a5806eae7a593a5ff6fbcaba072f0f885599d6 Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 3 Jan 2019 12:25:52 -0500 Subject: [PATCH 04/15] revised code after 1st review 3369 --- package-lock.json | 15 +++++++++++---- src/traces/mesh3d/convert.js | 27 +++++++++++++-------------- src/traces/mesh3d/defaults.js | 15 +++++++++++++-- 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/package-lock.json b/package-lock.json index 71b3cc0d2e5..4aa1b5c0443 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4088,7 +4088,8 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "concat-map": { "version": "0.0.1", @@ -4098,7 +4099,8 @@ "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "core-util-is": { "version": "1.0.2", @@ -4215,7 +4217,8 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "ini": { "version": "1.3.5", @@ -4227,6 +4230,7 @@ "version": "1.0.0", "bundled": true, "dev": true, + "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -4352,7 +4356,8 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "object-assign": { "version": "4.1.1", @@ -4364,6 +4369,7 @@ "version": "1.4.0", "bundled": true, "dev": true, + "optional": true, "requires": { "wrappy": "1" } @@ -4485,6 +4491,7 @@ "version": "1.0.2", "bundled": true, "dev": true, + "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", diff --git a/src/traces/mesh3d/convert.js b/src/traces/mesh3d/convert.js index 713f252282e..f2d804cfc07 100644 --- a/src/traces/mesh3d/convert.js +++ b/src/traces/mesh3d/convert.js @@ -71,19 +71,21 @@ proto.update = function(data) { var positions = zip3( toDataCoords(layout.xaxis, data.x, scene.dataScale[0], data.xcalendar), toDataCoords(layout.yaxis, data.y, scene.dataScale[1], data.ycalendar), - toDataCoords(layout.zaxis, data.z, scene.dataScale[2], data.zcalendar)); + toDataCoords(layout.zaxis, data.z, scene.dataScale[2], data.zcalendar) + ); var cells; if(data.i && data.j && data.k) { - cells = zip3(data.i, data.j, data.k); - } - else if(data.alphahull === 0) { + cells = zip3( + data.i.map(function(e) { return Math.round(e); }), + data.j.map(function(e) { return Math.round(e); }), + data.k.map(function(e) { return Math.round(e); }) + ); + } else if(data.alphahull === 0) { cells = convexHull(positions); - } - else if(data.alphahull > 0) { + } else if(data.alphahull > 0) { cells = alphaShape(data.alphahull, positions); - } - else { + } else { var d = ['x', 'y', 'z'].indexOf(data.delaunayaxis); cells = triangulate(positions.map(function(c) { return [c[(d + 1) % 3], c[(d + 2) % 3]]; @@ -113,16 +115,13 @@ proto.update = function(data) { config.vertexIntensity = data.intensity; config.vertexIntensityBounds = [data.cmin, data.cmax]; config.colormap = parseColorScale(data.colorscale); - } - else if(data.vertexcolor) { + } else if(data.vertexcolor) { this.color = data.vertexcolor[0]; config.vertexColors = parseColorArray(data.vertexcolor); - } - else if(data.facecolor) { + } else if(data.facecolor) { this.color = data.facecolor[0]; config.cellColors = parseColorArray(data.facecolor); - } - else { + } else { this.color = data.color; config.meshColor = str2RgbaArray(data.color); } diff --git a/src/traces/mesh3d/defaults.js b/src/traces/mesh3d/defaults.js index eed7911bef7..ee54c60384f 100644 --- a/src/traces/mesh3d/defaults.js +++ b/src/traces/mesh3d/defaults.js @@ -34,12 +34,23 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout } var coords = readComponents(['x', 'y', 'z']); - readComponents(['i', 'j', 'k']); - if(!coords) { traceOut.visible = false; return; } + var numVertices = coords[0].length; + + var indices = readComponents(['i', 'j', 'k']); + if(indices) { + indices.forEach(function(arr) { + for(var i = 0; i < arr.length; ++i) { + if(!Lib.isIndex(arr, numVertices)) { + traceOut.visible = false; + return; + } + } + }); + } var handleCalendarDefaults = Registry.getComponentMethod('calendars', 'handleTraceDefaults'); handleCalendarDefaults(traceIn, traceOut, ['x', 'y', 'z'], layout); From e97f5edbf2bf6874c7ecaf8fde79d2580b3745bd Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 3 Jan 2019 13:39:41 -0500 Subject: [PATCH 05/15] improved condition checks and added jasmine tests for mesh3d --- src/traces/mesh3d/defaults.js | 30 +++- test/jasmine/tests/mesh3d_test.js | 266 ++++++++++++++++++++++++------ 2 files changed, 241 insertions(+), 55 deletions(-) diff --git a/src/traces/mesh3d/defaults.js b/src/traces/mesh3d/defaults.js index ee54c60384f..1ed668e5891 100644 --- a/src/traces/mesh3d/defaults.js +++ b/src/traces/mesh3d/defaults.js @@ -38,18 +38,34 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout traceOut.visible = false; return; } - var numVertices = coords[0].length; - var indices = readComponents(['i', 'j', 'k']); - if(indices) { - indices.forEach(function(arr) { - for(var i = 0; i < arr.length; ++i) { - if(!Lib.isIndex(arr, numVertices)) { + var allIndices = readComponents(['i', 'j', 'k']); + if(allIndices === false) { + traceOut.visible = false; + return; + } + if(allIndices) { + var numVertices = coords[0].length; + allIndices.forEach(function(indices) { + indices.forEach(function(index) { + if(!Lib.isIndex(index, numVertices)) { traceOut.visible = false; return; } - } + }); }); + + var numFaces = allIndices[0].length; + for(var q = 0; q < numFaces; q++) { + if( + allIndices[0][q] === allIndices[1][q] || + allIndices[0][q] === allIndices[2][q] || + allIndices[1][q] === allIndices[2][q] + ) { + traceOut.visible = false; + return; + } + } } var handleCalendarDefaults = Registry.getComponentMethod('calendars', 'handleTraceDefaults'); diff --git a/test/jasmine/tests/mesh3d_test.js b/test/jasmine/tests/mesh3d_test.js index e268057caaf..9423e92ade1 100644 --- a/test/jasmine/tests/mesh3d_test.js +++ b/test/jasmine/tests/mesh3d_test.js @@ -3,55 +3,225 @@ var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); var failTest = require('../assets/fail_test'); -describe('Test mesh3d restyle', function() { - afterEach(destroyGraphDiv); - - it('should clear *cauto* when restyle *cmin* and/or *cmax*', function(done) { - var gd = createGraphDiv(); - - function _assert(user, full) { - var trace = gd.data[0]; - var fullTrace = gd._fullData[0]; - - expect(trace.cauto).toBe(user[0], 'user cauto'); - expect(trace.cmin).toBe(user[1], 'user cmin'); - expect(trace.cmax).toBe(user[2], 'user cmax'); - expect(fullTrace.cauto).toBe(full[0], 'full cauto'); - expect(fullTrace.cmin).toBe(full[1], 'full cmin'); - expect(fullTrace.cmax).toBe(full[2], 'full cmax'); +describe('Test mesh3d', function() { + 'use strict'; + + describe('restyle', function() { + afterEach(destroyGraphDiv); + + it('should clear *cauto* when restyle *cmin* and/or *cmax*', function(done) { + var gd = createGraphDiv(); + + function _assert(user, full) { + var trace = gd.data[0]; + var fullTrace = gd._fullData[0]; + + expect(trace.cauto).toBe(user[0], 'user cauto'); + expect(trace.cmin).toBe(user[1], 'user cmin'); + expect(trace.cmax).toBe(user[2], 'user cmax'); + expect(fullTrace.cauto).toBe(full[0], 'full cauto'); + expect(fullTrace.cmin).toBe(full[1], 'full cmin'); + expect(fullTrace.cmax).toBe(full[2], 'full cmax'); + } + + Plotly.plot(gd, [{ + type: 'mesh3d', + x: [0, 1, 2, 0], + y: [0, 0, 1, 2], + z: [0, 2, 0, 1], + i: [0, 0, 0, 1], + j: [1, 2, 3, 2], + k: [2, 3, 1, 3], + intensity: [0, 0.33, 0.66, 3] + }]) + .then(function() { + _assert([undefined, undefined, undefined], [true, 0, 3]); + + return Plotly.restyle(gd, 'cmin', 0); + }) + .then(function() { + _assert([false, 0, undefined], [false, 0, 3]); + + return Plotly.restyle(gd, 'cmax', 10); + }) + .then(function() { + _assert([false, 0, 10], [false, 0, 10]); + + return Plotly.restyle(gd, 'cauto', true); + }) + .then(function() { + _assert([true, 0, 10], [true, 0, 3]); + + return Plotly.purge(gd); + }) + .catch(failTest) + .then(done); + }); + }); + + describe('dimension and expected visibility tests', function() { + var gd; + + beforeEach(function() { + gd = createGraphDiv(); + }); + + afterEach(function() { + Plotly.purge(gd); + destroyGraphDiv(); + }); + + function assertVisibility(exp, msg) { + expect(gd._fullData[0]).not.toBe(undefined, 'no visibility!'); + expect(gd._fullData[0].visible).toBe(exp, msg); } - Plotly.plot(gd, [{ - type: 'mesh3d', - x: [0, 1, 2, 0], - y: [0, 0, 1, 2], - z: [0, 2, 0, 1], - i: [0, 0, 0, 1], - j: [1, 2, 3, 2], - k: [2, 3, 1, 3], - intensity: [0, 0.33, 0.66, 3] - }]) - .then(function() { - _assert([undefined, undefined, undefined], [true, 0, 3]); - - return Plotly.restyle(gd, 'cmin', 0); - }) - .then(function() { - _assert([false, 0, undefined], [false, 0, 3]); - - return Plotly.restyle(gd, 'cmax', 10); - }) - .then(function() { - _assert([false, 0, 10], [false, 0, 10]); - - return Plotly.restyle(gd, 'cauto', true); - }) - .then(function() { - _assert([true, 0, 10], [true, 0, 3]); - - return Plotly.purge(gd); - }) - .catch(failTest) - .then(done); + it('@gl mesh3d should be visible when the vertex array are empty', function(done) { + Plotly.plot(gd, [{ + x: [], + y: [], + z: [], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(false, 'not to be visible'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be visible when the index arrays are not provided', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: [0, 0.5, 0.5, 1], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(false, 'not to be visible'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be invisible when the indices are not integer', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: [0, 0.5, 0.5, 1], + i: [0, 0, 0, 1.00001], + j: [1, 1, 2, 2], + k: [2, 3, 3, 2.99999], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(false, 'not to be visible'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be invisible when the indices are equal or greater than the number of vertices', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: [0, 0.5, 0.5, 1], + i: [0, 0, 0, 1], + j: [1, 1, 2, 2], + k: [2, 3, 3, 4], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(false, 'not to be visible'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be invisible when the indices are negative', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: [0, 0.5, 0.5, 1], + i: [0, 0, 0, -1], + j: [1, 1, 2, 2], + k: [2, 3, 3, 3], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(false, 'not to be visible'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be invisible when the indices have different sizes', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: [0, 0.5, 0.5, 1], + i: [0, 0, 0, 1], + j: [1, 1, 2], + k: [2, 3, 3, 3], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(false, 'not to be visible'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be invisible when the indices of a triangle point to identical vertex twice', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: [0, 0.5, 0.5, 1], + i: [0, 0, 0, 1], + j: [1, 1, 2, 3], + k: [2, 3, 3, 3], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(false, 'not to be visible'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be visible when the indices are provided and OK', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: [0, 0.5, 0.5, 1], + i: [0, 0, 0, 1], + j: [1, 1, 2, 2], + k: [2, 3, 3, 3], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(true, 'to be visible'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be visible when the index arrays are empty', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: [0, 0.5, 0.5, 1], + i: [], + j: [], + k: [], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(true, 'to be visible'); + }) + .catch(failTest) + .then(done); + }); }); + }); From 55e5e121edede25a7358d96c7d2b28fbb08e455d Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 3 Jan 2019 14:50:06 -0500 Subject: [PATCH 06/15] added more logic --- src/traces/mesh3d/defaults.js | 24 +++++++++++-- test/jasmine/tests/mesh3d_test.js | 56 +++++++++++++++---------------- 2 files changed, 50 insertions(+), 30 deletions(-) diff --git a/src/traces/mesh3d/defaults.js b/src/traces/mesh3d/defaults.js index 1ed668e5891..13ac7eebceb 100644 --- a/src/traces/mesh3d/defaults.js +++ b/src/traces/mesh3d/defaults.js @@ -39,11 +39,31 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout return; } - var allIndices = readComponents(['i', 'j', 'k']); - if(allIndices === false) { + // three indices should be all provided or not + if( + (traceIn.i && (!traceIn.j || !traceIn.k)) || + (traceIn.j && (!traceIn.k || !traceIn.i)) || + (traceIn.k && (!traceIn.i || !traceIn.j)) + ) { traceOut.visible = false; return; } + + // test for size of indices + if( + traceIn.i && Lib.isArrayOrTypedArray(traceIn.i) && + traceIn.j && Lib.isArrayOrTypedArray(traceIn.j) && + traceIn.k && Lib.isArrayOrTypedArray(traceIn.k) + ) { + if( traceIn.k.length !== 0 && ( + traceIn.i.length !== traceIn.j.length || + traceIn.j.length !== traceIn.k.length)) { + traceOut.visible = false; + return; + } + } + + var allIndices = readComponents(['i', 'j', 'k']); if(allIndices) { var numVertices = coords[0].length; allIndices.forEach(function(indices) { diff --git a/test/jasmine/tests/mesh3d_test.js b/test/jasmine/tests/mesh3d_test.js index 9423e92ade1..f642b1ddbb2 100644 --- a/test/jasmine/tests/mesh3d_test.js +++ b/test/jasmine/tests/mesh3d_test.js @@ -76,34 +76,6 @@ describe('Test mesh3d', function() { expect(gd._fullData[0].visible).toBe(exp, msg); } - it('@gl mesh3d should be visible when the vertex array are empty', function(done) { - Plotly.plot(gd, [{ - x: [], - y: [], - z: [], - type: 'mesh3d' - }]) - .then(function() { - assertVisibility(false, 'not to be visible'); - }) - .catch(failTest) - .then(done); - }); - - it('@gl mesh3d should be visible when the index arrays are not provided', function(done) { - Plotly.plot(gd, [{ - x: [0, 1, 0.5, 0.5], - y: [0, 0.5, 1, 0.5], - z: [0, 0.5, 0.5, 1], - type: 'mesh3d' - }]) - .then(function() { - assertVisibility(false, 'not to be visible'); - }) - .catch(failTest) - .then(done); - }); - it('@gl mesh3d should be invisible when the indices are not integer', function(done) { Plotly.plot(gd, [{ x: [0, 1, 0.5, 0.5], @@ -222,6 +194,34 @@ describe('Test mesh3d', function() { .catch(failTest) .then(done); }); + + it('@gl mesh3d should be visible when the index arrays are not provided', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: [0, 0.5, 0.5, 1], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(true, 'to be visible'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be visible when the vertex array are empty', function(done) { + Plotly.plot(gd, [{ + x: [], + y: [], + z: [], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(true, 'not to be visible'); + }) + .catch(failTest) + .then(done); + }); }); }); From 79deeae44e94222446058f7c963a849b0b15f785 Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 3 Jan 2019 14:53:46 -0500 Subject: [PATCH 07/15] revert optional deps --- package-lock.json | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4aa1b5c0443..71b3cc0d2e5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4088,8 +4088,7 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", @@ -4099,8 +4098,7 @@ "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -4217,8 +4215,7 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -4230,7 +4227,6 @@ "version": "1.0.0", "bundled": true, "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -4356,8 +4352,7 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -4369,7 +4364,6 @@ "version": "1.4.0", "bundled": true, "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -4491,7 +4485,6 @@ "version": "1.0.2", "bundled": true, "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", From bf51940d9294c928bbb310ef58c728cf9c03e736 Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 3 Jan 2019 15:34:28 -0500 Subject: [PATCH 08/15] fixed syntax --- src/traces/mesh3d/defaults.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/traces/mesh3d/defaults.js b/src/traces/mesh3d/defaults.js index 13ac7eebceb..a0aeb6aea36 100644 --- a/src/traces/mesh3d/defaults.js +++ b/src/traces/mesh3d/defaults.js @@ -55,7 +55,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout traceIn.j && Lib.isArrayOrTypedArray(traceIn.j) && traceIn.k && Lib.isArrayOrTypedArray(traceIn.k) ) { - if( traceIn.k.length !== 0 && ( + if(traceIn.k.length !== 0 && ( traceIn.i.length !== traceIn.j.length || traceIn.j.length !== traceIn.k.length)) { traceOut.visible = false; From 83060bc00a5cc17b1144f486a01e02903b1b9524 Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 3 Jan 2019 20:59:47 -0500 Subject: [PATCH 09/15] using loops instead of map in mesh3d convert --- src/traces/mesh3d/convert.js | 56 ++++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/src/traces/mesh3d/convert.js b/src/traces/mesh3d/convert.js index f2d804cfc07..d4039862ca6 100644 --- a/src/traces/mesh3d/convert.js +++ b/src/traces/mesh3d/convert.js @@ -52,7 +52,43 @@ proto.handlePick = function(selection) { }; function parseColorArray(colors) { - return colors.map(str2RgbaArray); + var b = []; + var len = colors.length; + for(var i = 0; i < len; i++) { + b[i] = str2RgbaArray(colors[i]); + } + return b; +} + +// Unpack position data +function toDataCoords(axis, coord, scale, calendar) { + var b = []; + var len = coord.length; + for(var i = 0; i < len; i++) { + b[i] = axis.d2l(coord[i], 0, calendar) * scale; + } + return b; +} + +// round indices if passed as floats +function toIndex(a) { + var b = []; + var len = a.length; + for(var i = 0; i < len; i++) { + b[i] = Math.round(a[i]); + } + return b; +} + +// create cells +function delaunayCells(delaunayaxis, positions) { + var d = ['x', 'y', 'z'].indexOf(delaunayaxis); + var b = []; + var len = positions.length; + for(var i = 0; i < len; i++) { + b[i] = [positions[i][(d + 1) % 3], positions[i][(d + 2) % 3]]; + } + return triangulate(b); } proto.update = function(data) { @@ -61,13 +97,6 @@ proto.update = function(data) { this.data = data; - // Unpack position data - function toDataCoords(axis, coord, scale, calendar) { - return coord.map(function(x) { - return axis.d2l(x, 0, calendar) * scale; - }); - } - var positions = zip3( toDataCoords(layout.xaxis, data.x, scene.dataScale[0], data.xcalendar), toDataCoords(layout.yaxis, data.y, scene.dataScale[1], data.ycalendar), @@ -77,19 +106,16 @@ proto.update = function(data) { var cells; if(data.i && data.j && data.k) { cells = zip3( - data.i.map(function(e) { return Math.round(e); }), - data.j.map(function(e) { return Math.round(e); }), - data.k.map(function(e) { return Math.round(e); }) + toIndex(data.i), + toIndex(data.j), + toIndex(data.k) ); } else if(data.alphahull === 0) { cells = convexHull(positions); } else if(data.alphahull > 0) { cells = alphaShape(data.alphahull, positions); } else { - var d = ['x', 'y', 'z'].indexOf(data.delaunayaxis); - cells = triangulate(positions.map(function(c) { - return [c[(d + 1) % 3], c[(d + 2) % 3]]; - })); + cells = delaunayCells(data.delaunayaxis, positions); } var config = { From a041e51366a6c5b281ec8f7e50026ce2acafbc94 Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 3 Jan 2019 22:26:16 -0500 Subject: [PATCH 10/15] moved certain condition checks to convert step --- src/traces/mesh3d/convert.js | 41 +++++++++++++++++++++++++++++++ src/traces/mesh3d/defaults.js | 40 +----------------------------- test/jasmine/tests/mesh3d_test.js | 2 +- 3 files changed, 43 insertions(+), 40 deletions(-) diff --git a/src/traces/mesh3d/convert.js b/src/traces/mesh3d/convert.js index d4039862ca6..e51485d1023 100644 --- a/src/traces/mesh3d/convert.js +++ b/src/traces/mesh3d/convert.js @@ -14,6 +14,7 @@ var triangulate = require('delaunay-triangulate'); var alphaShape = require('alpha-shape'); var convexHull = require('convex-hull'); +var isIndex = require('../../lib').isIndex; var parseColorScale = require('../../lib/gl_format_color').parseColorScale; var str2RgbaArray = require('../../lib/str2rgbarray'); var zip3 = require('../../plots/gl3d/zip3'); @@ -91,12 +92,40 @@ function delaunayCells(delaunayaxis, positions) { return triangulate(b); } +// validate indices +function hasValidIndices(list, numVertices) { + var len = list.length; + for(var i = 0; i < len; i++) { + if(!isIndex(list[i], numVertices)) { + return false; + } + } + return true; +} + +// avoid pointing to an identical vertex twice +function isTriangle(lists) { + var len = lists[0].length; + for(var i = 0; i < len; i++) { + if( + lists[0][i] === lists[1][i] || + lists[1][i] === lists[2][i] || + lists[2][i] === lists[0][i] + ) { + return false; + } + } + return true; +} + proto.update = function(data) { var scene = this.scene; var layout = scene.fullSceneLayout; this.data = data; + var numVertices = data.x.length; + var positions = zip3( toDataCoords(layout.xaxis, data.x, scene.dataScale[0], data.xcalendar), toDataCoords(layout.yaxis, data.y, scene.dataScale[1], data.ycalendar), @@ -105,6 +134,18 @@ proto.update = function(data) { var cells; if(data.i && data.j && data.k) { + + if( + data.i.length !== data.j.length || + data.j.length !== data.k.length || + !hasValidIndices(data.i, numVertices) || + !hasValidIndices(data.j, numVertices) || + !hasValidIndices(data.k, numVertices) || + !isTriangle([data.i, data.j, data.k]) + ) { + data.visible = false; + return; + } cells = zip3( toIndex(data.i), toIndex(data.j), diff --git a/src/traces/mesh3d/defaults.js b/src/traces/mesh3d/defaults.js index a0aeb6aea36..91cdfc9044a 100644 --- a/src/traces/mesh3d/defaults.js +++ b/src/traces/mesh3d/defaults.js @@ -48,45 +48,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout traceOut.visible = false; return; } - - // test for size of indices - if( - traceIn.i && Lib.isArrayOrTypedArray(traceIn.i) && - traceIn.j && Lib.isArrayOrTypedArray(traceIn.j) && - traceIn.k && Lib.isArrayOrTypedArray(traceIn.k) - ) { - if(traceIn.k.length !== 0 && ( - traceIn.i.length !== traceIn.j.length || - traceIn.j.length !== traceIn.k.length)) { - traceOut.visible = false; - return; - } - } - - var allIndices = readComponents(['i', 'j', 'k']); - if(allIndices) { - var numVertices = coords[0].length; - allIndices.forEach(function(indices) { - indices.forEach(function(index) { - if(!Lib.isIndex(index, numVertices)) { - traceOut.visible = false; - return; - } - }); - }); - - var numFaces = allIndices[0].length; - for(var q = 0; q < numFaces; q++) { - if( - allIndices[0][q] === allIndices[1][q] || - allIndices[0][q] === allIndices[2][q] || - allIndices[1][q] === allIndices[2][q] - ) { - traceOut.visible = false; - return; - } - } - } + readComponents(['i', 'j', 'k']); var handleCalendarDefaults = Registry.getComponentMethod('calendars', 'handleTraceDefaults'); handleCalendarDefaults(traceIn, traceOut, ['x', 'y', 'z'], layout); diff --git a/test/jasmine/tests/mesh3d_test.js b/test/jasmine/tests/mesh3d_test.js index f642b1ddbb2..aaf06ecbae0 100644 --- a/test/jasmine/tests/mesh3d_test.js +++ b/test/jasmine/tests/mesh3d_test.js @@ -76,7 +76,7 @@ describe('Test mesh3d', function() { expect(gd._fullData[0].visible).toBe(exp, msg); } - it('@gl mesh3d should be invisible when the indices are not integer', function(done) { + it('@gl mesh3d should be visible when the indices are not integer', function(done) { Plotly.plot(gd, [{ x: [0, 1, 0.5, 0.5], y: [0, 0.5, 1, 0.5], From 2b54f1438e4f3e10ea9076d32e97b537e35abfbc Mon Sep 17 00:00:00 2001 From: archmoj Date: Fri, 4 Jan 2019 11:01:49 -0500 Subject: [PATCH 11/15] pass 2 removed some extra checks --- src/traces/mesh3d/convert.js | 35 +++++---------------- test/jasmine/tests/mesh3d_test.js | 52 +++++++++++++++---------------- 2 files changed, 34 insertions(+), 53 deletions(-) diff --git a/src/traces/mesh3d/convert.js b/src/traces/mesh3d/convert.js index e51485d1023..dc25cd715d6 100644 --- a/src/traces/mesh3d/convert.js +++ b/src/traces/mesh3d/convert.js @@ -14,7 +14,6 @@ var triangulate = require('delaunay-triangulate'); var alphaShape = require('alpha-shape'); var convexHull = require('convex-hull'); -var isIndex = require('../../lib').isIndex; var parseColorScale = require('../../lib/gl_format_color').parseColorScale; var str2RgbaArray = require('../../lib/str2rgbarray'); var zip3 = require('../../plots/gl3d/zip3'); @@ -71,8 +70,8 @@ function toDataCoords(axis, coord, scale, calendar) { return b; } -// round indices if passed as floats -function toIndex(a) { +// Round indices if passed as floats +function toRoundIndex(a) { var b = []; var len = a.length; for(var i = 0; i < len; i++) { @@ -81,7 +80,6 @@ function toIndex(a) { return b; } -// create cells function delaunayCells(delaunayaxis, positions) { var d = ['x', 'y', 'z'].indexOf(delaunayaxis); var b = []; @@ -92,26 +90,11 @@ function delaunayCells(delaunayaxis, positions) { return triangulate(b); } -// validate indices +// Validate indices function hasValidIndices(list, numVertices) { var len = list.length; for(var i = 0; i < len; i++) { - if(!isIndex(list[i], numVertices)) { - return false; - } - } - return true; -} - -// avoid pointing to an identical vertex twice -function isTriangle(lists) { - var len = lists[0].length; - for(var i = 0; i < len; i++) { - if( - lists[0][i] === lists[1][i] || - lists[1][i] === lists[2][i] || - lists[2][i] === lists[0][i] - ) { + if(list[i] < 0 || list[i] >= numVertices) { return false; } } @@ -140,16 +123,14 @@ proto.update = function(data) { data.j.length !== data.k.length || !hasValidIndices(data.i, numVertices) || !hasValidIndices(data.j, numVertices) || - !hasValidIndices(data.k, numVertices) || - !isTriangle([data.i, data.j, data.k]) + !hasValidIndices(data.k, numVertices) ) { - data.visible = false; return; } cells = zip3( - toIndex(data.i), - toIndex(data.j), - toIndex(data.k) + toRoundIndex(data.i), + toRoundIndex(data.j), + toRoundIndex(data.k) ); } else if(data.alphahull === 0) { cells = convexHull(positions); diff --git a/test/jasmine/tests/mesh3d_test.js b/test/jasmine/tests/mesh3d_test.js index aaf06ecbae0..990ea6b179d 100644 --- a/test/jasmine/tests/mesh3d_test.js +++ b/test/jasmine/tests/mesh3d_test.js @@ -82,18 +82,18 @@ describe('Test mesh3d', function() { y: [0, 0.5, 1, 0.5], z: [0, 0.5, 0.5, 1], i: [0, 0, 0, 1.00001], - j: [1, 1, 2, 2], - k: [2, 3, 3, 2.99999], + j: [1, 1, 2, 1.99999], + k: [2, 3, 3, 3.00001], type: 'mesh3d' }]) .then(function() { - assertVisibility(false, 'not to be visible'); + assertVisibility(true, 'to be visible'); }) .catch(failTest) .then(done); }); - it('@gl mesh3d should be invisible when the indices are equal or greater than the number of vertices', function(done) { + it('@gl mesh3d should be visible when the indices are equal or greater than the number of vertices', function(done) { Plotly.plot(gd, [{ x: [0, 1, 0.5, 0.5], y: [0, 0.5, 1, 0.5], @@ -104,13 +104,13 @@ describe('Test mesh3d', function() { type: 'mesh3d' }]) .then(function() { - assertVisibility(false, 'not to be visible'); + assertVisibility(true, 'to be visible'); }) .catch(failTest) .then(done); }); - it('@gl mesh3d should be invisible when the indices are negative', function(done) { + it('@gl mesh3d should be visible when the indices are negative', function(done) { Plotly.plot(gd, [{ x: [0, 1, 0.5, 0.5], y: [0, 0.5, 1, 0.5], @@ -121,13 +121,13 @@ describe('Test mesh3d', function() { type: 'mesh3d' }]) .then(function() { - assertVisibility(false, 'not to be visible'); + assertVisibility(true, 'to be visible'); }) .catch(failTest) .then(done); }); - it('@gl mesh3d should be invisible when the indices have different sizes', function(done) { + it('@gl mesh3d should be visible when the indices have different sizes', function(done) { Plotly.plot(gd, [{ x: [0, 1, 0.5, 0.5], y: [0, 0.5, 1, 0.5], @@ -138,24 +138,7 @@ describe('Test mesh3d', function() { type: 'mesh3d' }]) .then(function() { - assertVisibility(false, 'not to be visible'); - }) - .catch(failTest) - .then(done); - }); - - it('@gl mesh3d should be invisible when the indices of a triangle point to identical vertex twice', function(done) { - Plotly.plot(gd, [{ - x: [0, 1, 0.5, 0.5], - y: [0, 0.5, 1, 0.5], - z: [0, 0.5, 0.5, 1], - i: [0, 0, 0, 1], - j: [1, 1, 2, 3], - k: [2, 3, 3, 3], - type: 'mesh3d' - }]) - .then(function() { - assertVisibility(false, 'not to be visible'); + assertVisibility(true, 'to be visible'); }) .catch(failTest) .then(done); @@ -222,6 +205,23 @@ describe('Test mesh3d', function() { .catch(failTest) .then(done); }); + + it('@gl mesh3d should be visible when values are passed in string format', function(done) { + Plotly.plot(gd, [{ + x: ['0', '1', '0.5', '0.5'], + y: ['0', '0.5', '1', '0.5'], + z: ['0', '0.5', '0.5', '1'], + i: ['0', '0', '0', '1'], + j: ['1', '1', '2', '2'], + k: ['2', '3', '3', '3'], + type: 'mesh3d' + }]).then(function() { + assertVisibility(true, 'not to be visible'); + }) + .catch(failTest) + .then(done); + }); + }); }); From a9ab05fdd45d581fea2770ddc6ea96fd6300d300 Mon Sep 17 00:00:00 2001 From: archmoj Date: Fri, 4 Jan 2019 13:58:06 -0500 Subject: [PATCH 12/15] fixed jasmine tests to also check for position and cell data --- test/jasmine/tests/mesh3d_test.js | 95 +++++++++++++++++++++++++++---- 1 file changed, 84 insertions(+), 11 deletions(-) diff --git a/test/jasmine/tests/mesh3d_test.js b/test/jasmine/tests/mesh3d_test.js index 990ea6b179d..2407568b709 100644 --- a/test/jasmine/tests/mesh3d_test.js +++ b/test/jasmine/tests/mesh3d_test.js @@ -59,7 +59,7 @@ describe('Test mesh3d', function() { }); }); - describe('dimension and expected visibility tests', function() { + describe('dimension and expected visibility check and cell/position tests', function() { var gd; beforeEach(function() { @@ -76,6 +76,14 @@ describe('Test mesh3d', function() { expect(gd._fullData[0].visible).toBe(exp, msg); } + function assertPositions(exp, msg) { + expect(gd._fullLayout.scene._scene.glplot.objects[0].positions.length !== undefined).toBe(exp, msg); + } + + function assertCells(exp, msg) { + expect(gd._fullLayout.scene._scene.glplot.objects[0].cells.length !== undefined).toBe(exp, msg); + } + it('@gl mesh3d should be visible when the indices are not integer', function(done) { Plotly.plot(gd, [{ x: [0, 1, 0.5, 0.5], @@ -89,6 +97,12 @@ describe('Test mesh3d', function() { .then(function() { assertVisibility(true, 'to be visible'); }) + .then(function() { + assertPositions(true, 'not to be false'); + }) + .then(function() { + assertCells(true, 'not to be false'); + }) .catch(failTest) .then(done); }); @@ -106,6 +120,12 @@ describe('Test mesh3d', function() { .then(function() { assertVisibility(true, 'to be visible'); }) + .then(function() { + assertPositions(true, 'not to be false'); + }) + .then(function() { + assertCells(true, 'not to be false'); + }) .catch(failTest) .then(done); }); @@ -123,6 +143,12 @@ describe('Test mesh3d', function() { .then(function() { assertVisibility(true, 'to be visible'); }) + .then(function() { + assertPositions(true, 'not to be false'); + }) + .then(function() { + assertCells(true, 'not to be false'); + }) .catch(failTest) .then(done); }); @@ -140,6 +166,12 @@ describe('Test mesh3d', function() { .then(function() { assertVisibility(true, 'to be visible'); }) + .then(function() { + assertPositions(true, 'not to be false'); + }) + .then(function() { + assertCells(true, 'not to be false'); + }) .catch(failTest) .then(done); }); @@ -157,6 +189,34 @@ describe('Test mesh3d', function() { .then(function() { assertVisibility(true, 'to be visible'); }) + .then(function() { + assertPositions(true, 'not to be false'); + }) + .then(function() { + assertCells(true, 'not to be false'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be visible when values are passed in string format', function(done) { + Plotly.plot(gd, [{ + x: ['0', '1', '0.5', '0.5'], + y: ['0', '0.5', '1', '0.5'], + z: ['0', '0.5', '0.5', '1'], + i: ['0', '0', '0', '1'], + j: ['1', '1', '2', '2'], + k: ['2', '3', '3', '3'], + type: 'mesh3d' + }]).then(function() { + assertVisibility(true, 'not to be visible'); + }) + .then(function() { + assertPositions(true, 'not to be false'); + }) + .then(function() { + assertCells(true, 'not to be false'); + }) .catch(failTest) .then(done); }); @@ -174,6 +234,12 @@ describe('Test mesh3d', function() { .then(function() { assertVisibility(true, 'to be visible'); }) + .then(function() { + assertPositions(true, 'not to be false'); + }) + .then(function() { + assertCells(true, 'not to be false'); + }) .catch(failTest) .then(done); }); @@ -188,11 +254,17 @@ describe('Test mesh3d', function() { .then(function() { assertVisibility(true, 'to be visible'); }) + .then(function() { + assertPositions(true, 'not to be false'); + }) + .then(function() { + assertCells(true, 'not to be false'); + }) .catch(failTest) .then(done); }); - it('@gl mesh3d should be visible when the vertex array are empty', function(done) { + it('@gl mesh3d should be visible when the vertex arrays are empty', function(done) { Plotly.plot(gd, [{ x: [], y: [], @@ -202,21 +274,22 @@ describe('Test mesh3d', function() { .then(function() { assertVisibility(true, 'not to be visible'); }) + .then(function() { + assertPositions(true, 'not to be false'); + }) + .then(function() { + assertCells(true, 'not to be false'); + }) .catch(failTest) .then(done); }); - it('@gl mesh3d should be visible when values are passed in string format', function(done) { + it('@gl mesh3d should be invisible when the vertex arrays missing', function(done) { Plotly.plot(gd, [{ - x: ['0', '1', '0.5', '0.5'], - y: ['0', '0.5', '1', '0.5'], - z: ['0', '0.5', '0.5', '1'], - i: ['0', '0', '0', '1'], - j: ['1', '1', '2', '2'], - k: ['2', '3', '3', '3'], type: 'mesh3d' - }]).then(function() { - assertVisibility(true, 'not to be visible'); + }]) + .then(function() { + assertVisibility(false, 'to be visible'); }) .catch(failTest) .then(done); From ec502ee990de42e6062ad63a25595e10b3d5eb1c Mon Sep 17 00:00:00 2001 From: archmoj Date: Fri, 4 Jan 2019 16:09:39 -0500 Subject: [PATCH 13/15] improved jasmine tests and -0.49 > 0 round edge case --- src/traces/mesh3d/convert.js | 2 +- test/jasmine/tests/mesh3d_test.js | 63 +++++++++++++++++++++---------- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/src/traces/mesh3d/convert.js b/src/traces/mesh3d/convert.js index dc25cd715d6..9b2c926f738 100644 --- a/src/traces/mesh3d/convert.js +++ b/src/traces/mesh3d/convert.js @@ -94,7 +94,7 @@ function delaunayCells(delaunayaxis, positions) { function hasValidIndices(list, numVertices) { var len = list.length; for(var i = 0; i < len; i++) { - if(list[i] < 0 || list[i] >= numVertices) { + if(list[i] <= -0.5 || list[i] >= numVertices - 0.5) { // Note: the indices would be rounded -0.49 is valid. return false; } } diff --git a/test/jasmine/tests/mesh3d_test.js b/test/jasmine/tests/mesh3d_test.js index 2407568b709..68b055352c9 100644 --- a/test/jasmine/tests/mesh3d_test.js +++ b/test/jasmine/tests/mesh3d_test.js @@ -77,11 +77,11 @@ describe('Test mesh3d', function() { } function assertPositions(exp, msg) { - expect(gd._fullLayout.scene._scene.glplot.objects[0].positions.length !== undefined).toBe(exp, msg); + expect(gd._fullLayout.scene._scene.glplot.objects[0].positions.length).toBe(exp, msg); } function assertCells(exp, msg) { - expect(gd._fullLayout.scene._scene.glplot.objects[0].cells.length !== undefined).toBe(exp, msg); + expect(gd._fullLayout.scene._scene.glplot.objects[0].cells.length).toBe(exp, msg); } it('@gl mesh3d should be visible when the indices are not integer', function(done) { @@ -98,10 +98,33 @@ describe('Test mesh3d', function() { assertVisibility(true, 'to be visible'); }) .then(function() { - assertPositions(true, 'not to be false'); + assertPositions(4, 'to be OK positions'); }) .then(function() { - assertCells(true, 'not to be false'); + assertCells(4, 'to be OK cells'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be visible when the indices could be rounded to be in vertex range', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: [0, 0.5, 0.5, 1], + i: [-0.49, 0, 0, 1], + j: [1, 1, 2, 2], + k: [2, 3, 3, 3.49], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(true, 'to be visible'); + }) + .then(function() { + assertPositions(4, 'to be OK positions'); + }) + .then(function() { + assertCells(4, 'to be OK cells'); }) .catch(failTest) .then(done); @@ -121,10 +144,10 @@ describe('Test mesh3d', function() { assertVisibility(true, 'to be visible'); }) .then(function() { - assertPositions(true, 'not to be false'); + assertPositions(0, 'to be OK positions'); }) .then(function() { - assertCells(true, 'not to be false'); + assertCells(0, 'to be OK cells'); }) .catch(failTest) .then(done); @@ -144,10 +167,10 @@ describe('Test mesh3d', function() { assertVisibility(true, 'to be visible'); }) .then(function() { - assertPositions(true, 'not to be false'); + assertPositions(0, 'to be OK positions'); }) .then(function() { - assertCells(true, 'not to be false'); + assertCells(0, 'to be OK cells'); }) .catch(failTest) .then(done); @@ -167,10 +190,10 @@ describe('Test mesh3d', function() { assertVisibility(true, 'to be visible'); }) .then(function() { - assertPositions(true, 'not to be false'); + assertPositions(0, 'to be OK positions'); }) .then(function() { - assertCells(true, 'not to be false'); + assertCells(0, 'to be OK cells'); }) .catch(failTest) .then(done); @@ -190,10 +213,10 @@ describe('Test mesh3d', function() { assertVisibility(true, 'to be visible'); }) .then(function() { - assertPositions(true, 'not to be false'); + assertPositions(4, 'to be OK positions'); }) .then(function() { - assertCells(true, 'not to be false'); + assertCells(4, 'to be OK cells'); }) .catch(failTest) .then(done); @@ -212,10 +235,10 @@ describe('Test mesh3d', function() { assertVisibility(true, 'not to be visible'); }) .then(function() { - assertPositions(true, 'not to be false'); + assertPositions(4, 'to be OK positions'); }) .then(function() { - assertCells(true, 'not to be false'); + assertCells(4, 'to be OK cells'); }) .catch(failTest) .then(done); @@ -235,10 +258,10 @@ describe('Test mesh3d', function() { assertVisibility(true, 'to be visible'); }) .then(function() { - assertPositions(true, 'not to be false'); + assertPositions(4, 'to be OK positions'); }) .then(function() { - assertCells(true, 'not to be false'); + assertCells(0, 'to be OK cells'); }) .catch(failTest) .then(done); @@ -255,10 +278,10 @@ describe('Test mesh3d', function() { assertVisibility(true, 'to be visible'); }) .then(function() { - assertPositions(true, 'not to be false'); + assertPositions(4, 'to be OK positions'); }) .then(function() { - assertCells(true, 'not to be false'); + assertCells(3, 'to be OK cells'); }) .catch(failTest) .then(done); @@ -275,10 +298,10 @@ describe('Test mesh3d', function() { assertVisibility(true, 'not to be visible'); }) .then(function() { - assertPositions(true, 'not to be false'); + assertPositions(0, 'to be OK positions'); }) .then(function() { - assertCells(true, 'not to be false'); + assertCells(0, 'to be OK cells'); }) .catch(failTest) .then(done); From fadce01bf1350fc7d5b499e61eeaa9f13b4fc6e1 Mon Sep 17 00:00:00 2001 From: archmoj Date: Mon, 7 Jan 2019 10:23:18 -0500 Subject: [PATCH 14/15] traceIn > traceOut and more jasmine tests --- src/traces/mesh3d/defaults.js | 6 +- test/jasmine/tests/mesh3d_test.js | 161 +++++++++++++++++++++++++++++- 2 files changed, 163 insertions(+), 4 deletions(-) diff --git a/src/traces/mesh3d/defaults.js b/src/traces/mesh3d/defaults.js index 91cdfc9044a..227154a0022 100644 --- a/src/traces/mesh3d/defaults.js +++ b/src/traces/mesh3d/defaults.js @@ -41,9 +41,9 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout // three indices should be all provided or not if( - (traceIn.i && (!traceIn.j || !traceIn.k)) || - (traceIn.j && (!traceIn.k || !traceIn.i)) || - (traceIn.k && (!traceIn.i || !traceIn.j)) + (traceOut.i && (!traceOut.j || !traceOut.k)) || + (traceOut.j && (!traceOut.k || !traceOut.i)) || + (traceOut.k && (!traceOut.i || !traceOut.j)) ) { traceOut.visible = false; return; diff --git a/test/jasmine/tests/mesh3d_test.js b/test/jasmine/tests/mesh3d_test.js index 68b055352c9..197977951c6 100644 --- a/test/jasmine/tests/mesh3d_test.js +++ b/test/jasmine/tests/mesh3d_test.js @@ -318,6 +318,165 @@ describe('Test mesh3d', function() { .then(done); }); - }); + it('@gl mesh3d should be invisible when the vertex arrays are not arrays - number case', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: 1, + i: [0, 0, 0, 1], + j: [1, 1, 2, 2], + k: [2, 3, 3, 3], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(false, 'to be visible'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be invisible when the vertex arrays are not arrays - boolean case', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: true, + i: [0, 0, 0, 1], + j: [1, 1, 2, 2], + k: [2, 3, 3, 3], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(false, 'to be visible'); + }) + .catch(failTest) + .then(done); + }); + it('@gl mesh3d should be invisible when the vertex arrays are not arrays - object case', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: {}, + i: [0, 0, 0, 1], + j: [1, 1, 2, 2], + k: [2, 3, 3, 3], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(false, 'to be visible'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be invisible when the vertex arrays are not arrays - string case', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: '[0, 0.5, 0.5, 1]', + i: [0, 0, 0, 1], + j: [1, 1, 2, 2], + k: [2, 3, 3, 3], + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(false, 'to be visible'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be visible when the index arrays are not arrays - string case', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: [0, 0.5, 0.5, 1], + i: [0, 0, 0, 1], + j: [1, 1, 2, 2], + k: '[2, 3, 3, 3]', + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(true, 'to be invisible'); + }) + .then(function() { + assertPositions(4, 'to be OK positions'); + }) + .then(function() { + assertCells(3, 'to be OK cells'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be visible when the index arrays are not arrays - object case', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: [0, 0.5, 0.5, 1], + i: [0, 0, 0, 1], + j: [1, 1, 2, 2], + k: {}, + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(true, 'to be invisible'); + }) + .then(function() { + assertPositions(4, 'to be OK positions'); + }) + .then(function() { + assertCells(3, 'to be OK cells'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be visible when the index arrays are not arrays - boolean case', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: [0, 0.5, 0.5, 1], + i: [0, 0, 0, 1], + j: [1, 1, 2, 2], + k: true, + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(true, 'to be invisible'); + }) + .then(function() { + assertPositions(4, 'to be OK positions'); + }) + .then(function() { + assertCells(3, 'to be OK cells'); + }) + .catch(failTest) + .then(done); + }); + + it('@gl mesh3d should be visible when the index arrays are not arrays - number case', function(done) { + Plotly.plot(gd, [{ + x: [0, 1, 0.5, 0.5], + y: [0, 0.5, 1, 0.5], + z: [0, 0.5, 0.5, 1], + i: [0, 0, 0, 1], + j: [1, 1, 2, 2], + k: 1, + type: 'mesh3d' + }]) + .then(function() { + assertVisibility(true, 'to be invisible'); + }) + .then(function() { + assertPositions(4, 'to be OK positions'); + }) + .then(function() { + assertCells(3, 'to be OK cells'); + }) + .catch(failTest) + .then(done); + }); + + }); }); From 29f5a06c2aa5be4101c33ed649f8ef74a19a0d71 Mon Sep 17 00:00:00 2001 From: archmoj Date: Mon, 7 Jan 2019 11:38:38 -0500 Subject: [PATCH 15/15] must have tested traceOut after coerce fix --- src/traces/mesh3d/defaults.js | 2 +- test/jasmine/tests/mesh3d_test.js | 40 +++++++------------------------ 2 files changed, 9 insertions(+), 33 deletions(-) diff --git a/src/traces/mesh3d/defaults.js b/src/traces/mesh3d/defaults.js index 227154a0022..952f8cd73c5 100644 --- a/src/traces/mesh3d/defaults.js +++ b/src/traces/mesh3d/defaults.js @@ -39,6 +39,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout return; } + readComponents(['i', 'j', 'k']); // three indices should be all provided or not if( (traceOut.i && (!traceOut.j || !traceOut.k)) || @@ -48,7 +49,6 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout traceOut.visible = false; return; } - readComponents(['i', 'j', 'k']); var handleCalendarDefaults = Registry.getComponentMethod('calendars', 'handleTraceDefaults'); handleCalendarDefaults(traceIn, traceOut, ['x', 'y', 'z'], layout); diff --git a/test/jasmine/tests/mesh3d_test.js b/test/jasmine/tests/mesh3d_test.js index 197977951c6..6ee4d407bb3 100644 --- a/test/jasmine/tests/mesh3d_test.js +++ b/test/jasmine/tests/mesh3d_test.js @@ -386,7 +386,7 @@ describe('Test mesh3d', function() { .then(done); }); - it('@gl mesh3d should be visible when the index arrays are not arrays - string case', function(done) { + it('@gl mesh3d should be invisible when the index arrays are not arrays - string case', function(done) { Plotly.plot(gd, [{ x: [0, 1, 0.5, 0.5], y: [0, 0.5, 1, 0.5], @@ -397,19 +397,13 @@ describe('Test mesh3d', function() { type: 'mesh3d' }]) .then(function() { - assertVisibility(true, 'to be invisible'); - }) - .then(function() { - assertPositions(4, 'to be OK positions'); - }) - .then(function() { - assertCells(3, 'to be OK cells'); + assertVisibility(false, 'to be visible'); }) .catch(failTest) .then(done); }); - it('@gl mesh3d should be visible when the index arrays are not arrays - object case', function(done) { + it('@gl mesh3d should be invisible when the index arrays are not arrays - object case', function(done) { Plotly.plot(gd, [{ x: [0, 1, 0.5, 0.5], y: [0, 0.5, 1, 0.5], @@ -420,19 +414,13 @@ describe('Test mesh3d', function() { type: 'mesh3d' }]) .then(function() { - assertVisibility(true, 'to be invisible'); - }) - .then(function() { - assertPositions(4, 'to be OK positions'); - }) - .then(function() { - assertCells(3, 'to be OK cells'); + assertVisibility(false, 'to be visible'); }) .catch(failTest) .then(done); }); - it('@gl mesh3d should be visible when the index arrays are not arrays - boolean case', function(done) { + it('@gl mesh3d should be invisible when the index arrays are not arrays - boolean case', function(done) { Plotly.plot(gd, [{ x: [0, 1, 0.5, 0.5], y: [0, 0.5, 1, 0.5], @@ -443,19 +431,13 @@ describe('Test mesh3d', function() { type: 'mesh3d' }]) .then(function() { - assertVisibility(true, 'to be invisible'); - }) - .then(function() { - assertPositions(4, 'to be OK positions'); - }) - .then(function() { - assertCells(3, 'to be OK cells'); + assertVisibility(false, 'to be visible'); }) .catch(failTest) .then(done); }); - it('@gl mesh3d should be visible when the index arrays are not arrays - number case', function(done) { + it('@gl mesh3d should be invisible when the index arrays are not arrays - number case', function(done) { Plotly.plot(gd, [{ x: [0, 1, 0.5, 0.5], y: [0, 0.5, 1, 0.5], @@ -466,13 +448,7 @@ describe('Test mesh3d', function() { type: 'mesh3d' }]) .then(function() { - assertVisibility(true, 'to be invisible'); - }) - .then(function() { - assertPositions(4, 'to be OK positions'); - }) - .then(function() { - assertCells(3, 'to be OK cells'); + assertVisibility(false, 'to be visible'); }) .catch(failTest) .then(done);