Skip to content

Commit e8b7e0c

Browse files
authored
Merge pull request #724 from monfera/memory-leak-fixes
Memory leak fixes
2 parents 5c20d5d + 8937668 commit e8b7e0c

File tree

3 files changed

+126
-22
lines changed

3 files changed

+126
-22
lines changed

src/plot_api/plot_api.js

+4
Original file line numberDiff line numberDiff line change
@@ -835,6 +835,10 @@ Plotly.redraw = function(gd) {
835835
*/
836836
Plotly.newPlot = function(gd, data, layout, config) {
837837
gd = getGraphDiv(gd);
838+
839+
// remove gl contexts
840+
Plots.cleanPlot([], {}, gd._fullData || {}, gd._fullLayout || {});
841+
838842
Plots.purge(gd);
839843
return Plotly.plot(gd, data, layout, config);
840844
};

src/plots/gl2d/scene2d.js

+10
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,16 @@ proto.cameraChanged = function() {
310310
};
311311

312312
proto.destroy = function() {
313+
314+
var traces = this.traces;
315+
316+
if(traces) {
317+
Object.keys(traces).map(function(key) {
318+
traces[key].dispose();
319+
delete traces[key];
320+
});
321+
}
322+
313323
this.glplot.dispose();
314324

315325
if(!this.staticPlot) this.container.removeChild(this.canvas);

test/jasmine/tests/gl_plot_interact_test.js

+112-22
Original file line numberDiff line numberDiff line change
@@ -507,41 +507,131 @@ describe('Test gl plot interactions', function() {
507507
});
508508
});
509509

510-
describe('Plots.cleanPlot', function() {
510+
describe('Removal of gl contexts', function() {
511511

512-
it('should remove gl context from the graph div of a gl3d plot', function(done) {
513-
gd = createGraphDiv();
512+
var mockData2d = [{
513+
type: 'scattergl',
514+
x: [1, 2, 3],
515+
y: [2, 1, 3]
516+
}];
514517

515-
var mockData = [{
516-
type: 'scatter3d'
517-
}];
518518

519-
Plotly.plot(gd, mockData).then(function() {
520-
expect(gd._fullLayout.scene._scene.glplot).toBeDefined();
519+
var mockData3d = [{
520+
type: 'scatter3d',
521+
x: [1, 2, 3],
522+
y: [2, 1, 3],
523+
z: [3, 2, 1]
524+
}];
521525

522-
Plots.cleanPlot([], {}, gd._fullData, gd._fullLayout);
523-
expect(gd._fullLayout.scene._scene.glplot).toBe(null);
526+
describe('Plots.cleanPlot', function() {
524527

525-
done();
528+
it('should remove gl context from the graph div of a gl3d plot', function(done) {
529+
gd = createGraphDiv();
530+
531+
Plotly.plot(gd, mockData3d).then(function() {
532+
expect(gd._fullLayout.scene._scene.glplot).toBeDefined();
533+
534+
Plots.cleanPlot([], {}, gd._fullData, gd._fullLayout);
535+
expect(gd._fullLayout.scene._scene.glplot).toBe(null);
536+
537+
done();
538+
});
526539
});
527-
});
528540

529-
it('should remove gl context from the graph div of a gl2d plot', function(done) {
530-
gd = createGraphDiv();
541+
it('should remove gl context from the graph div of a gl2d plot', function(done) {
542+
gd = createGraphDiv();
531543

532-
var mockData = [{
544+
Plotly.plot(gd, mockData2d).then(function() {
545+
expect(gd._fullLayout._plots.xy._scene2d.glplot).toBeDefined();
546+
547+
Plots.cleanPlot([], {}, gd._fullData, gd._fullLayout);
548+
expect(gd._fullLayout._plots).toEqual({});
549+
550+
done();
551+
});
552+
});
553+
});
554+
describe('Plotly.newPlot', function() {
555+
556+
var mockData2dNew = [{
533557
type: 'scattergl',
534-
x: [1, 2, 3],
535-
y: [1, 2, 3]
558+
x: [1, 3, 2],
559+
y: [2, 3, 1]
536560
}];
537561

538-
Plotly.plot(gd, mockData).then(function() {
539-
expect(gd._fullLayout._plots.xy._scene2d.glplot).toBeDefined();
540562

541-
Plots.cleanPlot([], {}, gd._fullData, gd._fullLayout);
542-
expect(gd._fullLayout._plots).toEqual({});
563+
var mockData3dNew = [{
564+
type: 'scatter3d',
565+
x: [2, 1, 3],
566+
y: [1, 2, 3],
567+
z: [2, 1, 3]
568+
}];
543569

544-
done();
570+
571+
it('should remove gl context from the graph div of a gl3d plot', function(done) {
572+
gd = createGraphDiv();
573+
574+
Plotly.plot(gd, mockData3d).then(function() {
575+
576+
var firstGlplotObject = gd._fullLayout.scene._scene.glplot;
577+
var firstGlContext = firstGlplotObject.gl;
578+
var firstCanvas = firstGlContext.canvas;
579+
580+
expect(firstGlplotObject).toBeDefined();
581+
582+
Plotly.newPlot(gd, mockData3dNew, {}).then(function() {
583+
584+
var secondGlplotObject = gd._fullLayout.scene._scene.glplot;
585+
var secondGlContext = secondGlplotObject.gl;
586+
var secondCanvas = secondGlContext.canvas;
587+
588+
expect(secondGlplotObject).not.toBe(firstGlplotObject);
589+
expect(firstGlplotObject.gl === null);
590+
expect(secondGlContext instanceof WebGLRenderingContext);
591+
expect(secondGlContext).not.toBe(firstGlContext);
592+
593+
// The same canvas can't possibly be reassinged a new WebGL context, but let's leave room
594+
// for the implementation to make the context get lost and have the old canvas stick around
595+
// in a disused state.
596+
expect(firstCanvas.parentNode === null ||
597+
firstCanvas !== secondCanvas && firstGlContext.isContextLost());
598+
599+
done();
600+
601+
});
602+
});
603+
});
604+
605+
it('should remove gl context from the graph div of a gl2d plot', function(done) {
606+
gd = createGraphDiv();
607+
608+
Plotly.plot(gd, mockData2d).then(function() {
609+
610+
var firstGlplotObject = gd._fullLayout._plots.xy._scene2d.glplot;
611+
var firstGlContext = firstGlplotObject.gl;
612+
var firstCanvas = firstGlContext.canvas;
613+
614+
expect(firstGlplotObject).toBeDefined();
615+
expect(firstGlContext).toBeDefined();
616+
expect(firstGlContext instanceof WebGLRenderingContext);
617+
618+
Plotly.newPlot(gd, mockData2dNew, {}).then(function() {
619+
620+
var secondGlplotObject = gd._fullLayout._plots.xy._scene2d.glplot;
621+
var secondGlContext = secondGlplotObject.gl;
622+
var secondCanvas = secondGlContext.canvas;
623+
624+
expect(Object.keys(gd._fullLayout._plots).length === 1);
625+
expect(secondGlplotObject).not.toBe(firstGlplotObject);
626+
expect(firstGlplotObject.gl === null);
627+
expect(secondGlContext instanceof WebGLRenderingContext);
628+
expect(secondGlContext).not.toBe(firstGlContext);
629+
expect(firstCanvas.parentNode === null ||
630+
firstCanvas !== secondCanvas && firstGlContext.isContextLost());
631+
632+
done();
633+
});
634+
});
545635
});
546636
});
547637
});

0 commit comments

Comments
 (0)