Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for Scattergl animation bug #6452

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

fix for Scattergl animation bug #6452

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
369977f
Fix for https://github.com/plotly/plotly.js/issues/6251
alexturcea Jan 29, 2023
544ba78
Perhaps this fix is more general?
alexturcea Jan 29, 2023
53e4a73
Update plot.js
alexturcea Jan 29, 2023
9743ef5
Made changes more evident
alexturcea Jan 29, 2023
1129bc3
Merge https://github.com/plotly/plotly.js into fix-issues-6251
alexturcea Oct 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 13 additions & 14 deletions src/traces/scattergl/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ var exports = module.exports = function plot(gd, subplot, cdata) {
return;
}

var count = scene.count;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to just var count = cdata.length in that case - and remove all the other changes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! @eiriklv Could you please fetch upstream/master on your fork and test it by opening a PR?

var regl = fullLayout._glcanvas.data()[0].regl;

// that is needed for fills
Expand All @@ -74,28 +73,28 @@ var exports = module.exports = function plot(gd, subplot, cdata) {
scene.fill2d = createLine(regl);
}
if(scene.glText === true) {
scene.glText = new Array(count);
for(i = 0; i < count; i++) {
scene.glText = new Array(cdata.length);
for(i = 0; i < cdata.length; i++) {
scene.glText[i] = new Text(regl);
}
}

// update main marker options
if(scene.glText) {
if(count > scene.glText.length) {
if(cdata.length > scene.glText.length) {
// add gl text marker
var textsToAdd = count - scene.glText.length;
var textsToAdd = cdata.length - scene.glText.length;
for(i = 0; i < textsToAdd; i++) {
scene.glText.push(new Text(regl));
}
} else if(count < scene.glText.length) {
} else if(cdata.length < scene.glText.length) {
// remove gl text marker
var textsToRemove = scene.glText.length - count;
var removedTexts = scene.glText.splice(count, textsToRemove);
var textsToRemove = scene.glText.length - cdata.length;
var removedTexts = scene.glText.splice(cdata.length, textsToRemove);
removedTexts.forEach(function(text) { text.destroy(); });
}

for(i = 0; i < count; i++) {
for(i = 0; i < cdata.length; i++) {
scene.glText[i].update(scene.textOptions[i]);
}
}
Expand Down Expand Up @@ -128,7 +127,7 @@ var exports = module.exports = function plot(gd, subplot, cdata) {
}

// fill requires linked traces, so we generate it's positions here
scene.fillOrder = Lib.repeat(null, count);
scene.fillOrder = Lib.repeat(null, cdata.length);
if(scene.fill2d) {
scene.fillOptions = scene.fillOptions.map(function(fillOptions, i) {
var cdscatter = cdata[i];
Expand Down Expand Up @@ -255,7 +254,7 @@ var exports = module.exports = function plot(gd, subplot, cdata) {
var isSelectMode = selectMode(dragmode);
var clickSelectEnabled = fullLayout.clickmode.indexOf('select') > -1;

for(i = 0; i < count; i++) {
for(i = 0; i < cdata.length; i++) {
var cd0 = cdata[i][0];
var trace = cd0.trace;
var stash = cd0.t;
Expand Down Expand Up @@ -306,8 +305,8 @@ var exports = module.exports = function plot(gd, subplot, cdata) {

// use unselected styles on 'context' canvas
if(scene.scatter2d) {
var unselOpts = new Array(count);
for(i = 0; i < count; i++) {
var unselOpts = new Array(cdata.length);
for(i = 0; i < cdata.length; i++) {
unselOpts[i] = scene.selectBatch[i].length || scene.unselectBatch[i].length ?
scene.markerUnselectedOptions[i] :
{};
Expand Down Expand Up @@ -348,7 +347,7 @@ var exports = module.exports = function plot(gd, subplot, cdata) {
(yaxis._rl || yaxis.range)[1]
]
};
var vpRange = Lib.repeat(vpRange0, scene.count);
var vpRange = Lib.repeat(vpRange0, cdata.length);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this one

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change required?


// upload viewport/range data to GPU
if(scene.fill2d) {
Expand Down