From a028e1033e458effbe19ef02543b5b63925997c2 Mon Sep 17 00:00:00 2001 From: archmoj Date: Wed, 12 Feb 2020 09:58:34 -0500 Subject: [PATCH 1/8] declare aspectmode variable --- src/plots/gl3d/scene.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/plots/gl3d/scene.js b/src/plots/gl3d/scene.js index f4071c4b63b..2b1736cd54f 100644 --- a/src/plots/gl3d/scene.js +++ b/src/plots/gl3d/scene.js @@ -725,6 +725,8 @@ proto.plot = function(sceneData, fullLayout, layout) { }); } + var aspectmode = fullSceneLayout.aspectmode; + var axesScaleRatio = [1, 1, 1]; // Compute axis scale per category @@ -741,7 +743,7 @@ proto.plot = function(sceneData, fullLayout, layout) { var axisAutoScaleFactor = 4; var aspectRatio; - if(fullSceneLayout.aspectmode === 'auto') { + if(aspectmode === 'auto') { if(Math.max.apply(null, axesScaleRatio) / Math.min.apply(null, axesScaleRatio) <= axisAutoScaleFactor) { /* * USE DATA MODE WHEN AXIS RANGE DIMENSIONS ARE RELATIVELY EQUAL @@ -754,11 +756,11 @@ proto.plot = function(sceneData, fullLayout, layout) { */ aspectRatio = [1, 1, 1]; } - } else if(fullSceneLayout.aspectmode === 'cube') { + } else if(aspectmode === 'cube') { aspectRatio = [1, 1, 1]; - } else if(fullSceneLayout.aspectmode === 'data') { + } else if(aspectmode === 'data') { aspectRatio = axesScaleRatio; - } else if(fullSceneLayout.aspectmode === 'manual') { + } else if(aspectmode === 'manual') { var userRatio = fullSceneLayout.aspectratio; aspectRatio = [userRatio.x, userRatio.y, userRatio.z]; } else { From ac22cd307ebcf78831f346c160c2b83f33da3696 Mon Sep 17 00:00:00 2001 From: archmoj Date: Wed, 12 Feb 2020 10:06:46 -0500 Subject: [PATCH 2/8] do not compute axis scale when not needed --- src/plots/gl3d/scene.js | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/plots/gl3d/scene.js b/src/plots/gl3d/scene.js index 2b1736cd54f..7644b24c881 100644 --- a/src/plots/gl3d/scene.js +++ b/src/plots/gl3d/scene.js @@ -726,24 +726,26 @@ proto.plot = function(sceneData, fullLayout, layout) { } var aspectmode = fullSceneLayout.aspectmode; - - var axesScaleRatio = [1, 1, 1]; - - // Compute axis scale per category - for(i = 0; i < 3; ++i) { - axis = fullSceneLayout[axisProperties[i]]; - axisType = axis.type; - var axisRatio = axisTypeRatios[axisType]; - axesScaleRatio[i] = Math.pow(axisRatio.acc, 1.0 / axisRatio.count) / dataScale[i]; + var axesScaleRatio; + if(aspectmode === 'auto' || aspectmode === 'data') { + axesScaleRatio = [1, 1, 1]; + + // Compute axis scale per category + for(i = 0; i < 3; ++i) { + axis = fullSceneLayout[axisProperties[i]]; + axisType = axis.type; + var axisRatio = axisTypeRatios[axisType]; + axesScaleRatio[i] = Math.pow(axisRatio.acc, 1.0 / axisRatio.count) / dataScale[i]; + } } /* * Dynamically set the aspect ratio depending on the users aspect settings */ - var axisAutoScaleFactor = 4; var aspectRatio; - if(aspectmode === 'auto') { + var axisAutoScaleFactor = 4; + if(Math.max.apply(null, axesScaleRatio) / Math.min.apply(null, axesScaleRatio) <= axisAutoScaleFactor) { /* * USE DATA MODE WHEN AXIS RANGE DIMENSIONS ARE RELATIVELY EQUAL From a7a6c66f11c0c5fad782b1782b05d1c5983c1bd8 Mon Sep 17 00:00:00 2001 From: archmoj Date: Wed, 12 Feb 2020 10:09:51 -0500 Subject: [PATCH 3/8] move basic cube and manual aspectmode options to the top --- src/plots/gl3d/scene.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/plots/gl3d/scene.js b/src/plots/gl3d/scene.js index 7644b24c881..73b3609d89c 100644 --- a/src/plots/gl3d/scene.js +++ b/src/plots/gl3d/scene.js @@ -725,6 +725,10 @@ proto.plot = function(sceneData, fullLayout, layout) { }); } + /* + * Dynamically set the aspect ratio depending on the users aspect settings + */ + var aspectRatio; var aspectmode = fullSceneLayout.aspectmode; var axesScaleRatio; if(aspectmode === 'auto' || aspectmode === 'data') { @@ -739,11 +743,12 @@ proto.plot = function(sceneData, fullLayout, layout) { } } - /* - * Dynamically set the aspect ratio depending on the users aspect settings - */ - var aspectRatio; - if(aspectmode === 'auto') { + if(aspectmode === 'cube') { + aspectRatio = [1, 1, 1]; + } else if(aspectmode === 'manual') { + var userRatio = fullSceneLayout.aspectratio; + aspectRatio = [userRatio.x, userRatio.y, userRatio.z]; + } else if(aspectmode === 'auto') { var axisAutoScaleFactor = 4; if(Math.max.apply(null, axesScaleRatio) / Math.min.apply(null, axesScaleRatio) <= axisAutoScaleFactor) { @@ -758,13 +763,8 @@ proto.plot = function(sceneData, fullLayout, layout) { */ aspectRatio = [1, 1, 1]; } - } else if(aspectmode === 'cube') { - aspectRatio = [1, 1, 1]; } else if(aspectmode === 'data') { aspectRatio = axesScaleRatio; - } else if(aspectmode === 'manual') { - var userRatio = fullSceneLayout.aspectratio; - aspectRatio = [userRatio.x, userRatio.y, userRatio.z]; } else { throw new Error('scene.js aspectRatio was not one of the enumerated types'); } From 580b8196d4ea503a4f619f20a5ba0268cb8f2134 Mon Sep 17 00:00:00 2001 From: archmoj Date: Wed, 12 Feb 2020 10:15:24 -0500 Subject: [PATCH 4/8] compute axesScaleRatio inside auto and data case --- src/plots/gl3d/scene.js | 46 ++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/src/plots/gl3d/scene.js b/src/plots/gl3d/scene.js index 73b3609d89c..02cf077d71f 100644 --- a/src/plots/gl3d/scene.js +++ b/src/plots/gl3d/scene.js @@ -730,10 +730,13 @@ proto.plot = function(sceneData, fullLayout, layout) { */ var aspectRatio; var aspectmode = fullSceneLayout.aspectmode; - var axesScaleRatio; - if(aspectmode === 'auto' || aspectmode === 'data') { - axesScaleRatio = [1, 1, 1]; - + if(aspectmode === 'cube') { + aspectRatio = [1, 1, 1]; + } else if(aspectmode === 'manual') { + var userRatio = fullSceneLayout.aspectratio; + aspectRatio = [userRatio.x, userRatio.y, userRatio.z]; + } else if(aspectmode === 'auto' || aspectmode === 'data') { + var axesScaleRatio = [1, 1, 1]; // Compute axis scale per category for(i = 0; i < 3; ++i) { axis = fullSceneLayout[axisProperties[i]]; @@ -741,30 +744,25 @@ proto.plot = function(sceneData, fullLayout, layout) { var axisRatio = axisTypeRatios[axisType]; axesScaleRatio[i] = Math.pow(axisRatio.acc, 1.0 / axisRatio.count) / dataScale[i]; } - } - if(aspectmode === 'cube') { - aspectRatio = [1, 1, 1]; - } else if(aspectmode === 'manual') { - var userRatio = fullSceneLayout.aspectratio; - aspectRatio = [userRatio.x, userRatio.y, userRatio.z]; - } else if(aspectmode === 'auto') { - var axisAutoScaleFactor = 4; + if(aspectmode === 'data') { + aspectRatio = axesScaleRatio; + } else { // i.e. 'auto' option + var axisAutoScaleFactor = 4; - if(Math.max.apply(null, axesScaleRatio) / Math.min.apply(null, axesScaleRatio) <= axisAutoScaleFactor) { - /* - * USE DATA MODE WHEN AXIS RANGE DIMENSIONS ARE RELATIVELY EQUAL - */ + if(Math.max.apply(null, axesScaleRatio) / Math.min.apply(null, axesScaleRatio) <= axisAutoScaleFactor) { + /* + * USE DATA MODE WHEN AXIS RANGE DIMENSIONS ARE RELATIVELY EQUAL + */ - aspectRatio = axesScaleRatio; - } else { - /* - * USE EQUAL MODE WHEN AXIS RANGE DIMENSIONS ARE HIGHLY UNEQUAL - */ - aspectRatio = [1, 1, 1]; + aspectRatio = axesScaleRatio; + } else { + /* + * USE EQUAL MODE WHEN AXIS RANGE DIMENSIONS ARE HIGHLY UNEQUAL + */ + aspectRatio = [1, 1, 1]; + } } - } else if(aspectmode === 'data') { - aspectRatio = axesScaleRatio; } else { throw new Error('scene.js aspectRatio was not one of the enumerated types'); } From 9ba5c968878bfca262cd06044d6114d8789fc015 Mon Sep 17 00:00:00 2001 From: archmoj Date: Wed, 12 Feb 2020 10:34:24 -0500 Subject: [PATCH 5/8] refactor long if statement --- src/plots/gl3d/scene.js | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/plots/gl3d/scene.js b/src/plots/gl3d/scene.js index 02cf077d71f..53190ef4754 100644 --- a/src/plots/gl3d/scene.js +++ b/src/plots/gl3d/scene.js @@ -748,18 +748,14 @@ proto.plot = function(sceneData, fullLayout, layout) { if(aspectmode === 'data') { aspectRatio = axesScaleRatio; } else { // i.e. 'auto' option - var axisAutoScaleFactor = 4; - - if(Math.max.apply(null, axesScaleRatio) / Math.min.apply(null, axesScaleRatio) <= axisAutoScaleFactor) { - /* - * USE DATA MODE WHEN AXIS RANGE DIMENSIONS ARE RELATIVELY EQUAL - */ - + if( + Math.max.apply(null, axesScaleRatio) / + Math.min.apply(null, axesScaleRatio) <= 4 + ) { + // USE DATA MODE WHEN AXIS RANGE DIMENSIONS ARE RELATIVELY EQUAL aspectRatio = axesScaleRatio; } else { - /* - * USE EQUAL MODE WHEN AXIS RANGE DIMENSIONS ARE HIGHLY UNEQUAL - */ + // USE EQUAL MODE WHEN AXIS RANGE DIMENSIONS ARE HIGHLY UNEQUAL aspectRatio = [1, 1, 1]; } } From e29aceb930208840dc27e5d2bdac782332fab505 Mon Sep 17 00:00:00 2001 From: archmoj Date: Wed, 12 Feb 2020 14:44:08 -0500 Subject: [PATCH 6/8] preserve aspectratio & aspectmode changes by orthographic scroll zoom for restyle interactions --- src/plots/gl3d/scene.js | 4 +- test/jasmine/tests/gl3d_plot_interact_test.js | 92 +++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/src/plots/gl3d/scene.js b/src/plots/gl3d/scene.js index 53190ef4754..e27d82d8adf 100644 --- a/src/plots/gl3d/scene.js +++ b/src/plots/gl3d/scene.js @@ -246,6 +246,7 @@ proto.initializeGLPlot = function() { y: s * o.y, z: s * o.z }); + scene._aspectmode = 'manual'; } relayoutCallback(scene); @@ -729,7 +730,7 @@ proto.plot = function(sceneData, fullLayout, layout) { * Dynamically set the aspect ratio depending on the users aspect settings */ var aspectRatio; - var aspectmode = fullSceneLayout.aspectmode; + var aspectmode = scene._aspectmode || fullSceneLayout.aspectmode; if(aspectmode === 'cube') { aspectRatio = [1, 1, 1]; } else if(aspectmode === 'manual') { @@ -762,6 +763,7 @@ proto.plot = function(sceneData, fullLayout, layout) { } else { throw new Error('scene.js aspectRatio was not one of the enumerated types'); } + scene._aspectmode = aspectmode; /* * Write aspect Ratio back to user data and fullLayout so that it is modifies as user diff --git a/test/jasmine/tests/gl3d_plot_interact_test.js b/test/jasmine/tests/gl3d_plot_interact_test.js index 5f60b2e3130..8b611d706ef 100644 --- a/test/jasmine/tests/gl3d_plot_interact_test.js +++ b/test/jasmine/tests/gl3d_plot_interact_test.js @@ -1384,6 +1384,98 @@ describe('Test gl3d drag and wheel interactions', function() { .catch(failTest) .then(done); }); + + it('@gl should preserve aspectratio values when orthographic scroll zoom i.e. after restyle', function(done) { + var coords = { + x: [1, 2, 10, 4, 5], + y: [10, 2, 4, 4, 2], + z: [10, 2, 4, 8, 16], + }; + + var mock = { + data: [{ + type: 'scatter3d', + x: coords.x, + y: coords.y, + z: coords.z, + mode: 'markers', + marker: { + color: 'red', + size: 16, + } + }, { + type: 'scatter3d', + x: [coords.x[0]], + y: [coords.y[0]], + z: [coords.z[0]], + mode: 'markers', + marker: { + color: 'blue', + size: 32, + } + }], + layout: { + width: 400, + height: 400, + scene: { + camera: { + projection: { + type: 'orthographic' + } + }, + } + } + }; + + var sceneTarget; + var relayoutEvent; + var relayoutCnt = 0; + + Plotly.plot(gd, mock) + .then(function() { + gd.on('plotly_relayout', function(e) { + relayoutCnt++; + relayoutEvent = e; + }); + + sceneTarget = gd.querySelector('.svg-container .gl-container #scene canvas'); + }) + .then(function() { + var aspectratio = gd._fullLayout.scene.aspectratio; + expect(aspectratio.x).toBeCloseTo(0.898, 3, 'aspectratio.x'); + expect(aspectratio.y).toBeCloseTo(0.798, 3, 'aspectratio.y'); + expect(aspectratio.z).toBeCloseTo(1.396, 3, 'aspectratio.z'); + }) + .then(function() { + return scroll(sceneTarget); + }) + .then(function() { + expect(relayoutCnt).toEqual(1); + + var aspectratio = relayoutEvent['scene.aspectratio']; + expect(aspectratio.x).toBeCloseTo(0.816, 3, 'aspectratio.x'); + expect(aspectratio.y).toBeCloseTo(0.725, 3, 'aspectratio.y'); + expect(aspectratio.z).toBeCloseTo(1.269, 3, 'aspectratio.z'); + }) + .then(function() { + // select a point + var i = 2; + + return Plotly.restyle(gd, { + x: [[coords.x[i]]], + y: [[coords.y[i]]], + z: [[coords.z[i]]], + }, 1); + }) + .then(function() { + var aspectratio = gd._fullLayout.scene.aspectratio; + expect(aspectratio.x).toBeCloseTo(0.816, 3, 'aspectratio.x'); + expect(aspectratio.y).toBeCloseTo(0.725, 3, 'aspectratio.y'); + expect(aspectratio.z).toBeCloseTo(1.269, 3, 'aspectratio.z'); + }) + .catch(failTest) + .then(done); + }); }); describe('Test gl3d relayout calls', function() { From 078d80035d91cdddccf1542f48be1f5fb878ccda Mon Sep 17 00:00:00 2001 From: archmoj Date: Wed, 12 Feb 2020 16:22:34 -0500 Subject: [PATCH 7/8] do not use sceneLayout variable name for fullSceneLayout --- src/plots/gl3d/scene.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/plots/gl3d/scene.js b/src/plots/gl3d/scene.js index e27d82d8adf..7c414f104d4 100644 --- a/src/plots/gl3d/scene.js +++ b/src/plots/gl3d/scene.js @@ -471,12 +471,12 @@ proto.recoverContext = function() { var axisProperties = [ 'xaxis', 'yaxis', 'zaxis' ]; function computeTraceBounds(scene, trace, bounds) { - var sceneLayout = scene.fullSceneLayout; + var fullSceneLayout = scene.fullSceneLayout; for(var d = 0; d < 3; d++) { var axisName = axisProperties[d]; var axLetter = axisName.charAt(0); - var ax = sceneLayout[axisName]; + var ax = fullSceneLayout[axisName]; var coords = trace[axLetter]; var calendar = trace[axLetter + 'calendar']; var len = trace['_' + axLetter + 'length']; @@ -509,13 +509,13 @@ function computeTraceBounds(scene, trace, bounds) { } function computeAnnotationBounds(scene, bounds) { - var sceneLayout = scene.fullSceneLayout; - var annotations = sceneLayout.annotations || []; + var fullSceneLayout = scene.fullSceneLayout; + var annotations = fullSceneLayout.annotations || []; for(var d = 0; d < 3; d++) { var axisName = axisProperties[d]; var axLetter = axisName.charAt(0); - var ax = sceneLayout[axisName]; + var ax = fullSceneLayout[axisName]; for(var j = 0; j < annotations.length; j++) { var ann = annotations[j]; From 1828797e27f6394dc4f27a7be1fe0f6a762d5037 Mon Sep 17 00:00:00 2001 From: archmoj Date: Wed, 12 Feb 2020 16:31:31 -0500 Subject: [PATCH 8/8] avoid using temporary _aspectmode --- src/plots/gl3d/scene.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/plots/gl3d/scene.js b/src/plots/gl3d/scene.js index 7c414f104d4..5efb94d4bc4 100644 --- a/src/plots/gl3d/scene.js +++ b/src/plots/gl3d/scene.js @@ -246,7 +246,7 @@ proto.initializeGLPlot = function() { y: s * o.y, z: s * o.z }); - scene._aspectmode = 'manual'; + scene.fullSceneLayout.aspectmode = layout[scene.id].aspectmode = 'manual'; } relayoutCallback(scene); @@ -730,7 +730,7 @@ proto.plot = function(sceneData, fullLayout, layout) { * Dynamically set the aspect ratio depending on the users aspect settings */ var aspectRatio; - var aspectmode = scene._aspectmode || fullSceneLayout.aspectmode; + var aspectmode = fullSceneLayout.aspectmode; if(aspectmode === 'cube') { aspectRatio = [1, 1, 1]; } else if(aspectmode === 'manual') { @@ -763,7 +763,6 @@ proto.plot = function(sceneData, fullLayout, layout) { } else { throw new Error('scene.js aspectRatio was not one of the enumerated types'); } - scene._aspectmode = aspectmode; /* * Write aspect Ratio back to user data and fullLayout so that it is modifies as user