Merge pull request #16525 from apache/fix-memory-leak

perf(lines): fix potential memory leak in the effect line when setOption with notMerge
apache · Feb 17, 2022 · 949d160 · 949d160
2 parents e4bdd30 + d6f07f1
commit 949d160
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 53 deletions.
diff --git a/src/chart/helper/EffectLine.ts b/src/chart/helper/EffectLine.ts
@@ -117,8 +117,6 @@ class EffectLine extends graphic.Group {
             return;
         }
 
-        const self = this;
-
         const points = lineData.getItemLayout(idx);
 
         let period = effectModel.get('period') * 1000;
@@ -138,42 +136,48 @@ class EffectLine extends graphic.Group {
         }
 
         if (period !== this._period || loop !== this._loop) {
-
             symbol.stopAnimation();
-
-            if (period > 0) {
-                let delayNum: number;
-                if (zrUtil.isFunction(delayExpr)) {
-                    delayNum = delayExpr(idx);
-                }
-                else {
-                    delayNum = delayExpr;
-                }
-                if (symbol.__t > 0) {
-                    delayNum = -period * symbol.__t;
-                }
-                symbol.__t = 0;
-                const animator = symbol.animate('', loop)
-                    .when(period, {
-                        __t: 1
-                    })
-                    .delay(delayNum)
-                    .during(function () {
-                        self._updateSymbolPosition(symbol);
-                    });
-                if (!loop) {
-                    animator.done(function () {
-                        self.remove(symbol);
-                    });
-                }
-                animator.start();
+            let delayNum: number;
+            if (zrUtil.isFunction(delayExpr)) {
+                delayNum = delayExpr(idx);
             }
+            else {
+                delayNum = delayExpr;
+            }
+            if (symbol.__t > 0) {
+                delayNum = -period * symbol.__t;
+            }
+
+            this._animateSymbol(
+                symbol, period, delayNum, loop
+            );
         }
 
         this._period = period;
         this._loop = loop;
     }
 
+    private _animateSymbol(symbol: ECSymbolOnEffectLine, period: number, delayNum: number, loop: boolean) {
+        if (period > 0) {
+            symbol.__t = 0;
+            const self = this;
+            const animator = symbol.animate('', loop)
+                .when(period, {
+                    __t: 1
+                })
+                .delay(delayNum)
+                .during(function () {
+                    self._updateSymbolPosition(symbol);
+                });
+            if (!loop) {
+                animator.done(function () {
+                    self.remove(symbol);
+                });
+            }
+            animator.start();
+        }
+    }
+
     protected _getLineLength(symbol: ECSymbolOnEffectLine) {
         // Not so accurate
         return (vec2.dist(symbol.__p1, symbol.__cp1)

diff --git a/src/chart/helper/Symbol.ts b/src/chart/helper/Symbol.ts
@@ -23,14 +23,15 @@ import {getECData} from '../../util/innerStore';
 import { enterEmphasis, leaveEmphasis, toggleHoverEmphasis } from '../../util/states';
 import {getDefaultLabel} from './labelHelper';
 import SeriesData from '../../data/SeriesData';
-import { ColorString, BlurScope, AnimationOption, ZRColor } from '../../util/types';
+import { ColorString, BlurScope, AnimationOption, ZRColor, AnimationOptionMixin } from '../../util/types';
 import SeriesModel from '../../model/Series';
 import { PathProps } from 'zrender/src/graphic/Path';
 import { SymbolDrawSeriesScope, SymbolDrawItemModelOption } from './SymbolDraw';
 import { extend } from 'zrender/src/core/util';
 import { setLabelStyle, getLabelStatesModels } from '../../label/labelStyle';
 import ZRImage from 'zrender/src/graphic/Image';
 import { saveOldStyle } from '../../animation/basicTrasition';
+import Model from '../../model/Model';
 
 type ECSymbol = ReturnType<typeof createSymbol>;
 
@@ -43,8 +44,6 @@ interface SymbolOpts {
 
 class Symbol extends graphic.Group {
 
-    private _seriesModel: SeriesModel;
-
     private _symbolType: string;
 
     /**
@@ -201,8 +200,6 @@ class Symbol extends graphic.Group {
             // Must stop leave transition manually if don't call initProps or updateProps.
             this.childAt(0).stopAnimation('leave');
         }
-
-        this._seriesModel = seriesModel;
     }
 
     _updateCommon(
@@ -353,12 +350,11 @@ class Symbol extends graphic.Group {
         this.scaleX = this.scaleY = scale;
     }
 
-    fadeOut(cb: () => void, opt?: {
+    fadeOut(cb: () => void, seriesModel: Model<AnimationOptionMixin>, opt?: {
         fadeLabel: boolean,
         animation?: AnimationOption
     }) {
         const symbolPath = this.childAt(0) as ECSymbol;
-        const seriesModel = this._seriesModel;
         const dataIndex = getECData(this).dataIndex;
         const animationOpt = opt && opt.animation;
         // Avoid mistaken hover when fading out

diff --git a/src/chart/helper/SymbolDraw.ts b/src/chart/helper/SymbolDraw.ts
@@ -40,6 +40,7 @@ import Model from '../../model/Model';
 import { ScatterSeriesOption } from '../scatter/ScatterSeries';
 import { getLabelStatesModels } from '../../label/labelStyle';
 import Element from 'zrender/src/Element';
+import SeriesModel from '../../model/Series';
 
 interface UpdateOpt {
     isIgnore?(idx: number): boolean
@@ -51,7 +52,7 @@ interface UpdateOpt {
 
 interface SymbolLike extends graphic.Group {
     updateData(data: SeriesData, idx: number, scope?: SymbolDrawSeriesScope, opt?: UpdateOpt): void
-    fadeOut?(cb: () => void): void
+    fadeOut?(cb: () => void, seriesModel: SeriesModel): void
 }
 
 interface SymbolLikeCtor {
@@ -252,7 +253,7 @@ class SymbolDraw {
                 const el = oldData.getItemGraphicEl(oldIdx) as SymbolLike;
                 el && el.fadeOut(function () {
                     group.remove(el);
-                });
+                }, seriesModel as SeriesModel);
             })
             .execute();
 
@@ -319,7 +320,7 @@ class SymbolDraw {
             data.eachItemGraphicEl(function (el: SymbolLike) {
                 el.fadeOut(function () {
                     group.remove(el);
-                });
+                }, data.hostModel as SeriesModel);
             });
         }
         else {

diff --git a/src/chart/tree/TreeView.ts b/src/chart/tree/TreeView.ts
@@ -706,7 +706,7 @@ function removeNode(
         removeOpt: removeAnimationOpt
     });
 
-    symbolEl.fadeOut(null, {
+    symbolEl.fadeOut(null, data.hostModel as TreeSeriesModel, {
         fadeLabel: true,
         animation: removeAnimationOpt
     });

diff --git a/test/lib/testHelper.js b/test/lib/testHelper.js
diff --git a/test/node/ssr.js b/test/node/ssr.js