azchohfi / LottieUWP

UWP port of Lottie(https://github.com/airbnb/lottie-android)
Apache License 2.0
273 stars 45 forks source link

Memory leak in BitmapCanvas.DrawPath #14

Closed jasonwurzel closed 6 years ago

jasonwurzel commented 6 years ago

Hi Alexandre ,

I'm using your nice port extensively. When having several LottieAnimationView on screen I observed severe memory consumption of my app ( 10 AnimationViews resulting in about 3mb / sec Memory growth). So I dug into the source code and I think I found the issue (though I'm not 100% sure of the root cause). In BitmapCanvas.DrawPath a Geometry and a Brush get created but never get disposed (after getting handed over to _drawingSession.DrawGeometry respectively _drawingSession.FillGeometry). Please see the following excerpt of BitmapCanvas:

        public void DrawPath(Path path, Paint paint)
        {
            UpdateDrawingSessionWithFlags(paint.Flags);

            _drawingSession.Transform = GetCurrentTransform();

            var geometry = path.GetGeometry(_device);

            var gradient = paint.Shader as Gradient;
            var brush = gradient != null ? gradient.GetBrush(_device, paint.Alpha) : new CanvasSolidColorBrush(_device, paint.Color);
            brush = paint.ColorFilter?.Apply(this, brush) ?? brush;

            if (paint.Style == Paint.PaintStyle.Stroke)
                _drawingSession.DrawGeometry(geometry, brush, paint.StrokeWidth, GetCanvasStrokeStyle(paint));
            else
                _drawingSession.FillGeometry(geometry, brush);

            geometry.Dispose();
            brush.Dispose();
        }

of course this has to be changed to using statements and there are other places where this is applicable, I think (DrawRect for example). What do you think? should I create a pull request for this?

Greetings, Michael

azchohfi commented 6 years ago

This is something that I've noticed on the masks branch (not merged yet) and I was about to investigate further. Thank you very much for the bug report!

This is very simple for the DrawRect methods, but not that much for the ones that support gradient (DrawPath and DrawText), since they cache things on the GradientFillContent and GradientStrokeContent. This should actually be a property of the Paint class.

I will look further into this.

azchohfi commented 6 years ago

Please, get the my get latest and see if the bug is fixed.

jasonwurzel commented 6 years ago

Hi Alexandre, yes, this looks much better :-)

azchohfi commented 6 years ago

I'm going to mark this as closed. Again, thank you very much for the report!