From 72f766e5d6c72ea0637694365ece8391686d52ff Mon Sep 17 00:00:00 2001 From: Nikita Tsukanov Date: Tue, 14 Nov 2017 17:27:03 +0300 Subject: [PATCH 01/12] Ref-counting infrastructure for bitmaps --- src/Avalonia.Base/Utilities/Ref.cs | 143 ++++++++++++++++++ src/Avalonia.Controls/WindowIcon.cs | 2 +- src/Avalonia.Visuals/Media/Imaging/Bitmap.cs | 41 +++-- src/Avalonia.Visuals/Media/Imaging/IBitmap.cs | 6 +- .../Media/Imaging/RenderTargetBitmap.cs | 16 +- .../Media/Imaging/WritableBitmap.cs | 5 +- src/Avalonia.Visuals/Platform/IBitmapImpl.cs | 3 +- .../Platform/IDrawingContextImpl.cs | 5 +- .../Rendering/DeferredRenderer.cs | 21 +-- src/Avalonia.Visuals/Rendering/RenderLayer.cs | 9 +- .../SceneGraph/DeferredDrawingContextImpl.cs | 5 +- .../Rendering/SceneGraph/ImageNode.cs | 18 ++- src/Skia/Avalonia.Skia/DrawingContextImpl.cs | 13 +- .../Media/DrawingContextImpl.cs | 9 +- .../Media/ImageBrushImpl.cs | 4 +- 15 files changed, 241 insertions(+), 59 deletions(-) create mode 100644 src/Avalonia.Base/Utilities/Ref.cs diff --git a/src/Avalonia.Base/Utilities/Ref.cs b/src/Avalonia.Base/Utilities/Ref.cs new file mode 100644 index 0000000000..eb73488bd0 --- /dev/null +++ b/src/Avalonia.Base/Utilities/Ref.cs @@ -0,0 +1,143 @@ +using System; +using System.Runtime.ConstrainedExecution; +using System.Threading; + +namespace Avalonia.Utilities +{ + public interface IRef : IDisposable where T : class + { + T Item { get; } + IRef Clone(); + IRef CloneAs() where TResult : class; + } + + + + public static class RefCountable + { + public static IRef Create(T item) where T : class, IDisposable + { + return new Ref(item, new RefCounter(item)); + } + + public static IRef CreateUnownedNotClonable(T item) where T : class + => new TempRef(item); + + class TempRef : IRef where T : class + { + public void Dispose() + { + + } + + public TempRef(T item) + { + Item = item; + } + + public T Item { get; } + public IRef Clone() => throw new NotSupportedException(); + + public IRef CloneAs() where TResult : class + => throw new NotSupportedException(); + } + + class RefCounter + { + private IDisposable _item; + private volatile int _refs; + private object _lock = new object(); + + public RefCounter(IDisposable item) + { + _item = item; + } + + public void AddRef() + { + Interlocked.Increment(ref _refs); + } + + public void Release() + { + if (Interlocked.Decrement(ref _refs) == 0) + { + lock (_lock) + { + _item?.Dispose(); + _item = null; + } + } + + } + } + + class Ref : CriticalFinalizerObject, IRef where T : class + { + private T _item; + private RefCounter _counter; + private object _lock = new object(); + + public Ref(T item, RefCounter counter) + { + _item = item; + _counter = counter; + Thread.MemoryBarrier(); + _counter.AddRef(); + } + + public void Dispose() + { + lock (_lock) + { + if (_item != null) + { + _counter.Release(); + _item = null; + } + GC.SuppressFinalize(this); + } + } + + ~Ref() + { + _counter?.Release(); + } + + public T Item + { + get + { + lock (_lock) + { + return _item; + } + } + } + + public IRef Clone() + { + lock (_lock) + { + if (_item != null) + return new Ref(_item, _counter); + throw new ObjectDisposedException("Ref<" + typeof(T) + ">"); + } + } + + public IRef CloneAs() where TResult : class + { + lock (_lock) + { + lock (_lock) + { + if (_item != null) + return new Ref((TResult) (object) _item, _counter); + throw new ObjectDisposedException("Ref<" + typeof(T) + ">"); + } + } + } + } + } + +} \ No newline at end of file diff --git a/src/Avalonia.Controls/WindowIcon.cs b/src/Avalonia.Controls/WindowIcon.cs index dff84ff6ef..26195e6d76 100644 --- a/src/Avalonia.Controls/WindowIcon.cs +++ b/src/Avalonia.Controls/WindowIcon.cs @@ -16,7 +16,7 @@ namespace Avalonia.Controls { public WindowIcon(IBitmap bitmap) { - PlatformImpl = AvaloniaLocator.Current.GetService().LoadIcon(bitmap.PlatformImpl); + PlatformImpl = AvaloniaLocator.Current.GetService().LoadIcon(bitmap.PlatformImpl.Item); } public WindowIcon(string fileName) diff --git a/src/Avalonia.Visuals/Media/Imaging/Bitmap.cs b/src/Avalonia.Visuals/Media/Imaging/Bitmap.cs index c9f7e0f7ac..892cd935cf 100644 --- a/src/Avalonia.Visuals/Media/Imaging/Bitmap.cs +++ b/src/Avalonia.Visuals/Media/Imaging/Bitmap.cs @@ -4,6 +4,7 @@ using System; using System.IO; using Avalonia.Platform; +using Avalonia.Utilities; namespace Avalonia.Media.Imaging { @@ -19,7 +20,7 @@ namespace Avalonia.Media.Imaging public Bitmap(string fileName) { IPlatformRenderInterface factory = AvaloniaLocator.Current.GetService(); - PlatformImpl = factory.LoadBitmap(fileName); + PlatformImpl = RefCountable.Create(factory.LoadBitmap(fileName)); } /// @@ -29,18 +30,33 @@ namespace Avalonia.Media.Imaging public Bitmap(Stream stream) { IPlatformRenderInterface factory = AvaloniaLocator.Current.GetService(); - PlatformImpl = factory.LoadBitmap(stream); + PlatformImpl = RefCountable.Create(factory.LoadBitmap(stream)); } /// /// Initializes a new instance of the class. /// /// A platform-specific bitmap implementation. + protected Bitmap(IRef impl) + { + PlatformImpl = impl.Clone(); + } + + /// + /// Initializes a new instance of the class. + /// + /// A platform-specific bitmap implementation. Bitmap class takes the ownership. protected Bitmap(IBitmapImpl impl) { - PlatformImpl = impl; + PlatformImpl = RefCountable.Create(impl); } - + + /// + public virtual void Dispose() + { + PlatformImpl.Dispose(); + } + /// /// Initializes a new instance of the class. /// @@ -51,27 +67,24 @@ namespace Avalonia.Media.Imaging /// Bytes per row public Bitmap(PixelFormat format, IntPtr data, int width, int height, int stride) { - PlatformImpl = AvaloniaLocator.Current.GetService() - .LoadBitmap(format, data, width, height, stride); + PlatformImpl = RefCountable.Create(AvaloniaLocator.Current.GetService() + .LoadBitmap(format, data, width, height, stride)); } /// /// Gets the width of the bitmap, in pixels. /// - public int PixelWidth => PlatformImpl.PixelWidth; + public int PixelWidth => PlatformImpl.Item.PixelWidth; /// /// Gets the height of the bitmap, in pixels. /// - public int PixelHeight => PlatformImpl.PixelHeight; + public int PixelHeight => PlatformImpl.Item.PixelHeight; /// /// Gets the platform-specific bitmap implementation. /// - public IBitmapImpl PlatformImpl - { - get; - } + public IRef PlatformImpl { get; } /// /// Saves the bitmap to a file. @@ -79,12 +92,12 @@ namespace Avalonia.Media.Imaging /// The filename. public void Save(string fileName) { - PlatformImpl.Save(fileName); + PlatformImpl.Item.Save(fileName); } public void Save(Stream stream) { - PlatformImpl.Save(stream); + PlatformImpl.Item.Save(stream); } } } diff --git a/src/Avalonia.Visuals/Media/Imaging/IBitmap.cs b/src/Avalonia.Visuals/Media/Imaging/IBitmap.cs index 5a10145d84..465173059e 100644 --- a/src/Avalonia.Visuals/Media/Imaging/IBitmap.cs +++ b/src/Avalonia.Visuals/Media/Imaging/IBitmap.cs @@ -1,15 +1,17 @@ // Copyright (c) The Avalonia Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. +using System; using System.IO; using Avalonia.Platform; +using Avalonia.Utilities; namespace Avalonia.Media.Imaging { /// /// Represents a bitmap image. /// - public interface IBitmap + public interface IBitmap : IDisposable { /// /// Gets the width of the bitmap, in pixels. @@ -24,7 +26,7 @@ namespace Avalonia.Media.Imaging /// /// Gets the platform-specific bitmap implementation. /// - IBitmapImpl PlatformImpl { get; } + IRef PlatformImpl { get; } /// /// Saves the bitmap to a file. diff --git a/src/Avalonia.Visuals/Media/Imaging/RenderTargetBitmap.cs b/src/Avalonia.Visuals/Media/Imaging/RenderTargetBitmap.cs index 9d3f15f69a..3968c05efb 100644 --- a/src/Avalonia.Visuals/Media/Imaging/RenderTargetBitmap.cs +++ b/src/Avalonia.Visuals/Media/Imaging/RenderTargetBitmap.cs @@ -2,8 +2,10 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; +using System.Runtime.CompilerServices; using Avalonia.Platform; using Avalonia.Rendering; +using Avalonia.Utilities; using Avalonia.VisualTree; namespace Avalonia.Media.Imaging @@ -21,14 +23,19 @@ namespace Avalonia.Media.Imaging /// The horizontal DPI of the bitmap. /// The vertical DPI of the bitmap. public RenderTargetBitmap(int pixelWidth, int pixelHeight, double dpiX = 96, double dpiY = 96) - : base(CreateImpl(pixelWidth, pixelHeight, dpiX, dpiY)) + : this(RefCountable.Create(CreateImpl(pixelWidth, pixelHeight, dpiX, dpiY))) { } + private RenderTargetBitmap(IRef impl) : base(impl) + { + PlatformImpl = impl; + } + /// /// Gets the platform-specific bitmap implementation. /// - public new IRenderTargetBitmapImpl PlatformImpl => (IRenderTargetBitmapImpl)base.PlatformImpl; + public new IRef PlatformImpl { get; } /// /// Disposes of the bitmap. @@ -36,6 +43,7 @@ namespace Avalonia.Media.Imaging public void Dispose() { PlatformImpl.Dispose(); + base.Dispose(); } /// @@ -52,13 +60,13 @@ namespace Avalonia.Media.Imaging /// The horizontal DPI of the bitmap. /// The vertical DPI of the bitmap. /// The platform-specific implementation. - private static IBitmapImpl CreateImpl(int width, int height, double dpiX, double dpiY) + private static IRenderTargetBitmapImpl CreateImpl(int width, int height, double dpiX, double dpiY) { IPlatformRenderInterface factory = AvaloniaLocator.Current.GetService(); return factory.CreateRenderTargetBitmap(width, height, dpiX, dpiY); } /// - public IDrawingContextImpl CreateDrawingContext(IVisualBrushRenderer vbr) => PlatformImpl.CreateDrawingContext(vbr); + public IDrawingContextImpl CreateDrawingContext(IVisualBrushRenderer vbr) => PlatformImpl.Item.CreateDrawingContext(vbr); } } diff --git a/src/Avalonia.Visuals/Media/Imaging/WritableBitmap.cs b/src/Avalonia.Visuals/Media/Imaging/WritableBitmap.cs index 5c5b516ddd..df3e71dc85 100644 --- a/src/Avalonia.Visuals/Media/Imaging/WritableBitmap.cs +++ b/src/Avalonia.Visuals/Media/Imaging/WritableBitmap.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Text; using System.Threading.Tasks; using Avalonia.Platform; +using Avalonia.Utilities; namespace Avalonia.Media.Imaging { @@ -16,7 +17,7 @@ namespace Avalonia.Media.Imaging : base(AvaloniaLocator.Current.GetService().CreateWritableBitmap(width, height, format)) { } - - public ILockedFramebuffer Lock() => ((IWritableBitmapImpl) PlatformImpl).Lock(); + + public ILockedFramebuffer Lock() => ((IWritableBitmapImpl) PlatformImpl.Item).Lock(); } } diff --git a/src/Avalonia.Visuals/Platform/IBitmapImpl.cs b/src/Avalonia.Visuals/Platform/IBitmapImpl.cs index 12ac28e880..d60838452e 100644 --- a/src/Avalonia.Visuals/Platform/IBitmapImpl.cs +++ b/src/Avalonia.Visuals/Platform/IBitmapImpl.cs @@ -1,6 +1,7 @@ // Copyright (c) The Avalonia Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. +using System; using System.IO; namespace Avalonia.Platform @@ -8,7 +9,7 @@ namespace Avalonia.Platform /// /// Defines the platform-specific interface for a . /// - public interface IBitmapImpl + public interface IBitmapImpl : IDisposable { /// /// Gets the width of the bitmap, in pixels. diff --git a/src/Avalonia.Visuals/Platform/IDrawingContextImpl.cs b/src/Avalonia.Visuals/Platform/IDrawingContextImpl.cs index 3db4527bfb..04bbe713f2 100644 --- a/src/Avalonia.Visuals/Platform/IDrawingContextImpl.cs +++ b/src/Avalonia.Visuals/Platform/IDrawingContextImpl.cs @@ -3,6 +3,7 @@ using System; using Avalonia.Media; +using Avalonia.Utilities; namespace Avalonia.Platform { @@ -29,7 +30,7 @@ namespace Avalonia.Platform /// The opacity to draw with. /// The rect in the image to draw. /// The rect in the output to draw to. - void DrawImage(IBitmapImpl source, double opacity, Rect sourceRect, Rect destRect); + void DrawImage(IRef source, double opacity, Rect sourceRect, Rect destRect); /// /// Draws a bitmap image. @@ -38,7 +39,7 @@ namespace Avalonia.Platform /// The opacity mask to draw with. /// The destination rect for the opacity mask. /// The rect in the output to draw to. - void DrawImage(IBitmapImpl source, IBrush opacityMask, Rect opacityMaskRect, Rect destRect); + void DrawImage(IRef source, IBrush opacityMask, Rect opacityMaskRect, Rect destRect); /// /// Draws a line. diff --git a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs index f7befa646a..26b5244b00 100644 --- a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs +++ b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs @@ -12,6 +12,7 @@ using System.IO; using Avalonia.Media.Immutable; using System.Threading; using System.Linq; +using Avalonia.Utilities; namespace Avalonia.Rendering { @@ -31,7 +32,7 @@ namespace Avalonia.Rendering private Scene _scene; private IRenderTarget _renderTarget; private DirtyVisuals _dirty; - private IRenderTargetBitmapImpl _overlay; + private IRef _overlay; private bool _updateQueued; private object _rendering = new object(); private int _lastSceneId = -1; @@ -267,7 +268,7 @@ namespace Avalonia.Rendering if (node != null) { - using (var context = renderTarget.CreateDrawingContext(this)) + using (var context = renderTarget.Item.CreateDrawingContext(this)) { foreach (var rect in layer.Dirty) { @@ -294,7 +295,7 @@ namespace Avalonia.Rendering { var overlay = GetOverlay(parentContent, scene.Size, scene.Scaling); - using (var context = overlay.CreateDrawingContext(this)) + using (var context = overlay.Item.CreateDrawingContext(this)) { context.Clear(Colors.Transparent); RenderDirtyRects(context); @@ -323,7 +324,7 @@ namespace Avalonia.Rendering foreach (var layer in scene.Layers) { var bitmap = _layers[layer.LayerRoot].Bitmap; - var sourceRect = new Rect(0, 0, bitmap.PixelWidth, bitmap.PixelHeight); + var sourceRect = new Rect(0, 0, bitmap.Item.PixelWidth, bitmap.Item.PixelHeight); if (layer.GeometryClip != null) { @@ -347,7 +348,7 @@ namespace Avalonia.Rendering if (_overlay != null) { - var sourceRect = new Rect(0, 0, _overlay.PixelWidth, _overlay.PixelHeight); + var sourceRect = new Rect(0, 0, _overlay.Item.PixelWidth, _overlay.Item.PixelHeight); context.DrawImage(_overlay, 0.5, sourceRect, clientRect); } @@ -420,7 +421,7 @@ namespace Avalonia.Rendering } } - private IRenderTargetBitmapImpl GetOverlay( + private IRef GetOverlay( IDrawingContextImpl parentContext, Size size, double scaling) @@ -428,11 +429,11 @@ namespace Avalonia.Rendering var pixelSize = size * scaling; if (_overlay == null || - _overlay.PixelWidth != pixelSize.Width || - _overlay.PixelHeight != pixelSize.Height) + _overlay.Item.PixelWidth != pixelSize.Width || + _overlay.Item.PixelHeight != pixelSize.Height) { _overlay?.Dispose(); - _overlay = parentContext.CreateLayer(size); + _overlay = RefCountable.Create(parentContext.CreateLayer(size)); } return _overlay; @@ -445,7 +446,7 @@ namespace Avalonia.Rendering foreach (var layer in _layers) { var fileName = Path.Combine(DebugFramesPath, $"frame-{id}-layer-{index++}.png"); - layer.Bitmap.Save(fileName); + layer.Bitmap.Item.Save(fileName); } } } diff --git a/src/Avalonia.Visuals/Rendering/RenderLayer.cs b/src/Avalonia.Visuals/Rendering/RenderLayer.cs index ed33295db6..d1b1f1440a 100644 --- a/src/Avalonia.Visuals/Rendering/RenderLayer.cs +++ b/src/Avalonia.Visuals/Rendering/RenderLayer.cs @@ -1,6 +1,7 @@ using System; using Avalonia.Media; using Avalonia.Platform; +using Avalonia.Utilities; using Avalonia.VisualTree; namespace Avalonia.Rendering @@ -16,13 +17,13 @@ namespace Avalonia.Rendering IVisual layerRoot) { _drawingContext = drawingContext; - Bitmap = drawingContext.CreateLayer(size); + Bitmap = RefCountable.Create(drawingContext.CreateLayer(size)); Size = size; Scaling = scaling; LayerRoot = layerRoot; } - public IRenderTargetBitmapImpl Bitmap { get; private set; } + public IRef Bitmap { get; private set; } public double Scaling { get; private set; } public Size Size { get; private set; } public IVisual LayerRoot { get; } @@ -31,9 +32,9 @@ namespace Avalonia.Rendering { if (Size != size || Scaling != scaling) { - var resized = _drawingContext.CreateLayer(size); + var resized = RefCountable.Create(_drawingContext.CreateLayer(size)); - using (var context = resized.CreateDrawingContext(null)) + using (var context = resized.Item.CreateDrawingContext(null)) { context.Clear(Colors.Transparent); context.DrawImage(Bitmap, 1, new Rect(Size), new Rect(Size)); diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs index 29c482c336..38a10a7b7f 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using Avalonia.Media; using Avalonia.Platform; +using Avalonia.Utilities; using Avalonia.VisualTree; namespace Avalonia.Rendering.SceneGraph @@ -113,7 +114,7 @@ namespace Avalonia.Rendering.SceneGraph } /// - public void DrawImage(IBitmapImpl source, double opacity, Rect sourceRect, Rect destRect) + public void DrawImage(IRef source, double opacity, Rect sourceRect, Rect destRect) { var next = NextDrawAs(); @@ -128,7 +129,7 @@ namespace Avalonia.Rendering.SceneGraph } /// - public void DrawImage(IBitmapImpl source, IBrush opacityMask, Rect opacityMaskRect, Rect sourceRect) + public void DrawImage(IRef source, IBrush opacityMask, Rect opacityMaskRect, Rect sourceRect) { // This method is currently only used to composite layers so shouldn't be called here. throw new NotSupportedException(); diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/ImageNode.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/ImageNode.cs index 8291d1c0bb..8a1a5cb481 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/ImageNode.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/ImageNode.cs @@ -3,13 +3,14 @@ using System; using Avalonia.Platform; +using Avalonia.Utilities; namespace Avalonia.Rendering.SceneGraph { /// /// A node in the scene graph which represents an image draw. /// - internal class ImageNode : DrawOperation + internal class ImageNode : DrawOperation, IDisposable { /// /// Initializes a new instance of the class. @@ -19,11 +20,11 @@ namespace Avalonia.Rendering.SceneGraph /// The draw opacity. /// The source rect. /// The destination rect. - public ImageNode(Matrix transform, IBitmapImpl source, double opacity, Rect sourceRect, Rect destRect) + public ImageNode(Matrix transform, IRef source, double opacity, Rect sourceRect, Rect destRect) : base(destRect, transform, null) { Transform = transform; - Source = source; + Source = source.Clone(); Opacity = opacity; SourceRect = sourceRect; DestRect = destRect; @@ -37,7 +38,7 @@ namespace Avalonia.Rendering.SceneGraph /// /// Gets the image to draw. /// - public IBitmapImpl Source { get; } + public IRef Source { get; } /// /// Gets the draw opacity. @@ -67,10 +68,10 @@ namespace Avalonia.Rendering.SceneGraph /// The properties of the other draw operation are passed in as arguments to prevent /// allocation of a not-yet-constructed draw operation object. /// - public bool Equals(Matrix transform, IBitmapImpl source, double opacity, Rect sourceRect, Rect destRect) + public bool Equals(Matrix transform, IRef source, double opacity, Rect sourceRect, Rect destRect) { return transform == Transform && - Equals(source, Source) && + Equals(source.Item, Source.Item) && opacity == Opacity && sourceRect == SourceRect && destRect == DestRect; @@ -87,5 +88,10 @@ namespace Avalonia.Rendering.SceneGraph /// public override bool HitTest(Point p) => Bounds.Contains(p); + + public void Dispose() + { + Source?.Dispose(); + } } } diff --git a/src/Skia/Avalonia.Skia/DrawingContextImpl.cs b/src/Skia/Avalonia.Skia/DrawingContextImpl.cs index dd3ced1d89..0bc133e9df 100644 --- a/src/Skia/Avalonia.Skia/DrawingContextImpl.cs +++ b/src/Skia/Avalonia.Skia/DrawingContextImpl.cs @@ -6,6 +6,7 @@ using System.Linq; using Avalonia.Platform; using Avalonia.Rendering; using Avalonia.Rendering.Utilities; +using Avalonia.Utilities; namespace Avalonia.Skia { @@ -39,9 +40,9 @@ namespace Avalonia.Skia Canvas.Clear(color.ToSKColor()); } - public void DrawImage(IBitmapImpl source, double opacity, Rect sourceRect, Rect destRect) + public void DrawImage(IRef source, double opacity, Rect sourceRect, Rect destRect) { - var impl = (BitmapImpl)source; + var impl = (BitmapImpl)source.Item; var s = sourceRect.ToSKRect(); var d = destRect.ToSKRect(); using (var paint = new SKPaint() @@ -51,10 +52,10 @@ namespace Avalonia.Skia } } - public void DrawImage(IBitmapImpl source, IBrush opacityMask, Rect opacityMaskRect, Rect destRect) + public void DrawImage(IRef source, IBrush opacityMask, Rect opacityMaskRect, Rect destRect) { PushOpacityMask(opacityMask, opacityMaskRect); - DrawImage(source, 1, new Rect(0, 0, source.PixelWidth, source.PixelHeight), destRect); + DrawImage(source, 1, new Rect(0, 0, source.Item.PixelWidth, source.Item.PixelHeight), destRect); PopOpacityMask(); } @@ -237,7 +238,7 @@ namespace Avalonia.Skia } else { - tileBrushImage = (BitmapImpl)((tileBrush as IImageBrush)?.Source?.PlatformImpl); + tileBrushImage = (BitmapImpl)((tileBrush as IImageBrush)?.Source?.PlatformImpl.Item); } if (tileBrush != null && tileBrushImage != null) @@ -252,7 +253,7 @@ namespace Avalonia.Skia context.Clear(Colors.Transparent); context.PushClip(calc.IntermediateClip); context.Transform = calc.IntermediateTransform; - context.DrawImage(tileBrushImage, 1, rect, rect); + context.DrawImage(RefCountable.CreateUnownedNotClonable(tileBrushImage), 1, rect, rect); context.PopClip(); } diff --git a/src/Windows/Avalonia.Direct2D1/Media/DrawingContextImpl.cs b/src/Windows/Avalonia.Direct2D1/Media/DrawingContextImpl.cs index 6a72923ce3..afbca14346 100644 --- a/src/Windows/Avalonia.Direct2D1/Media/DrawingContextImpl.cs +++ b/src/Windows/Avalonia.Direct2D1/Media/DrawingContextImpl.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using Avalonia.Media; using Avalonia.Platform; using Avalonia.Rendering; +using Avalonia.Utilities; using SharpDX; using SharpDX.Direct2D1; using SharpDX.Mathematics.Interop; @@ -100,9 +101,9 @@ namespace Avalonia.Direct2D1.Media /// The opacity to draw with. /// The rect in the image to draw. /// The rect in the output to draw to. - public void DrawImage(IBitmapImpl source, double opacity, Rect sourceRect, Rect destRect) + public void DrawImage(IRef source, double opacity, Rect sourceRect, Rect destRect) { - using (var d2d = ((BitmapImpl)source).GetDirect2DBitmap(_renderTarget)) + using (var d2d = ((BitmapImpl)source.Item).GetDirect2DBitmap(_renderTarget)) { _renderTarget.DrawBitmap( d2d.Value, @@ -120,9 +121,9 @@ namespace Avalonia.Direct2D1.Media /// The opacity mask to draw with. /// The destination rect for the opacity mask. /// The rect in the output to draw to. - public void DrawImage(IBitmapImpl source, IBrush opacityMask, Rect opacityMaskRect, Rect destRect) + public void DrawImage(IRef source, IBrush opacityMask, Rect opacityMaskRect, Rect destRect) { - using (var d2dSource = ((BitmapImpl)source).GetDirect2DBitmap(_renderTarget)) + using (var d2dSource = ((BitmapImpl)source.Item).GetDirect2DBitmap(_renderTarget)) using (var sourceBrush = new BitmapBrush(_renderTarget, d2dSource.Value)) using (var d2dOpacityMask = CreateBrush(opacityMask, opacityMaskRect.Size)) using (var geometry = new SharpDX.Direct2D1.RectangleGeometry(_renderTarget.Factory, destRect.ToDirect2D())) diff --git a/src/Windows/Avalonia.Direct2D1/Media/ImageBrushImpl.cs b/src/Windows/Avalonia.Direct2D1/Media/ImageBrushImpl.cs index 08cfed2ace..75b397edd6 100644 --- a/src/Windows/Avalonia.Direct2D1/Media/ImageBrushImpl.cs +++ b/src/Windows/Avalonia.Direct2D1/Media/ImageBrushImpl.cs @@ -4,6 +4,7 @@ using System; using Avalonia.Media; using Avalonia.Rendering.Utilities; +using Avalonia.Utilities; using SharpDX.Direct2D1; namespace Avalonia.Direct2D1.Media @@ -100,7 +101,8 @@ namespace Avalonia.Direct2D1.Media context.Clear(Colors.Transparent); context.PushClip(calc.IntermediateClip); context.Transform = calc.IntermediateTransform; - context.DrawImage(bitmap, 1, rect, rect); + + context.DrawImage(RefCountable.CreateUnownedNotClonable(bitmap), 1, rect, rect); context.PopClip(); } From 401ea4fe99b07d1812dbbfe667305e27892203c0 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 15 Dec 2017 20:53:48 -0600 Subject: [PATCH 02/12] Fix concurrency primative usage in Ref.cs --- src/Avalonia.Base/Utilities/Ref.cs | 42 +++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/src/Avalonia.Base/Utilities/Ref.cs b/src/Avalonia.Base/Utilities/Ref.cs index eb73488bd0..f78544b416 100644 --- a/src/Avalonia.Base/Utilities/Ref.cs +++ b/src/Avalonia.Base/Utilities/Ref.cs @@ -46,7 +46,6 @@ namespace Avalonia.Utilities { private IDisposable _item; private volatile int _refs; - private object _lock = new object(); public RefCounter(IDisposable item) { @@ -55,20 +54,39 @@ namespace Avalonia.Utilities public void AddRef() { - Interlocked.Increment(ref _refs); + var old = _refs; + while (true) + { + var current = Interlocked.CompareExchange(ref _refs, old + 1, old); + if (current == old) + { + if (current == 0) + { + throw new ObjectDisposedException("Cannot add a reference to a 0-referenced item"); + } + } + old = current; + } } public void Release() { - if (Interlocked.Decrement(ref _refs) == 0) + var old = _refs; + while (true) { - lock (_lock) + var current = Interlocked.CompareExchange(ref _refs, old - 1, old); + + if (current == old) { - _item?.Dispose(); - _item = null; + if (current == 1) + { + _item.Dispose(); + _item = null; + } + return; } + old = current; } - } } @@ -82,7 +100,6 @@ namespace Avalonia.Utilities { _item = item; _counter = counter; - Thread.MemoryBarrier(); _counter.AddRef(); } @@ -129,12 +146,13 @@ namespace Avalonia.Utilities { lock (_lock) { - lock (_lock) + if (_item != null) { - if (_item != null) - return new Ref((TResult) (object) _item, _counter); - throw new ObjectDisposedException("Ref<" + typeof(T) + ">"); + TResult castItem = null; + Volatile.Write(ref castItem, (TResult)(object)_item); + return new Ref(castItem, _counter); } + throw new ObjectDisposedException("Ref<" + typeof(T) + ">"); } } } From acaedd866ba3184eba7c60f87196e28847116bd8 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 20 Dec 2017 13:34:50 -0600 Subject: [PATCH 03/12] Fix PR feedback. --- src/Avalonia.Base/Utilities/Ref.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Avalonia.Base/Utilities/Ref.cs b/src/Avalonia.Base/Utilities/Ref.cs index f78544b416..98077cd4fd 100644 --- a/src/Avalonia.Base/Utilities/Ref.cs +++ b/src/Avalonia.Base/Utilities/Ref.cs @@ -57,13 +57,14 @@ namespace Avalonia.Utilities var old = _refs; while (true) { + if (old == 0) + { + throw new ObjectDisposedException("Cannot add a reference to a nonreferenced item"); + } var current = Interlocked.CompareExchange(ref _refs, old + 1, old); if (current == old) { - if (current == 0) - { - throw new ObjectDisposedException("Cannot add a reference to a 0-referenced item"); - } + break; } old = current; } @@ -78,12 +79,12 @@ namespace Avalonia.Utilities if (current == old) { - if (current == 1) + if (old == 1) { _item.Dispose(); _item = null; } - return; + break; } old = current; } @@ -100,6 +101,7 @@ namespace Avalonia.Utilities { _item = item; _counter = counter; + Interlocked.MemoryBarrier(); _counter.AddRef(); } @@ -148,9 +150,7 @@ namespace Avalonia.Utilities { if (_item != null) { - TResult castItem = null; - Volatile.Write(ref castItem, (TResult)(object)_item); - return new Ref(castItem, _counter); + return new Ref((TResult)(object)_item, _counter); } throw new ObjectDisposedException("Ref<" + typeof(T) + ">"); } From 617f35ce53ad0d0ded0d23d831c129828c707a4b Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Sun, 28 Jan 2018 02:12:14 -0600 Subject: [PATCH 04/12] Add ref-counting checks throughout the SceneGraph nodes. Because IDrawOperations can be shared between IVisualNodes, they are now also ref-counted. --- src/Avalonia.Base/Utilities/Ref.cs | 14 ++-- .../Media/Imaging/RenderTargetBitmap.cs | 9 --- .../Rendering/DeferredRenderer.cs | 8 +-- .../Rendering/SceneGraph/ClipNode.cs | 4 ++ .../SceneGraph/DeferredDrawingContextImpl.cs | 66 +++++++++---------- .../Rendering/SceneGraph/DrawOperation.cs | 4 ++ .../Rendering/SceneGraph/GeometryClipNode.cs | 3 + .../Rendering/SceneGraph/IDrawOperation.cs | 2 +- .../Rendering/SceneGraph/IVisualNode.cs | 5 +- .../Rendering/SceneGraph/ImageNode.cs | 4 +- .../Rendering/SceneGraph/OpacityNode.cs | 4 ++ .../Rendering/SceneGraph/VisualNode.cs | 55 ++++++++++++---- .../Media/DrawingContextImpl.cs | 2 +- 13 files changed, 113 insertions(+), 67 deletions(-) diff --git a/src/Avalonia.Base/Utilities/Ref.cs b/src/Avalonia.Base/Utilities/Ref.cs index 98077cd4fd..f9e8b29b95 100644 --- a/src/Avalonia.Base/Utilities/Ref.cs +++ b/src/Avalonia.Base/Utilities/Ref.cs @@ -50,6 +50,7 @@ namespace Avalonia.Utilities public RefCounter(IDisposable item) { _item = item; + _refs = 1; } public void AddRef() @@ -101,8 +102,6 @@ namespace Avalonia.Utilities { _item = item; _counter = counter; - Interlocked.MemoryBarrier(); - _counter.AddRef(); } public void Dispose() @@ -139,7 +138,11 @@ namespace Avalonia.Utilities lock (_lock) { if (_item != null) - return new Ref(_item, _counter); + { + var newRef = new Ref(_item, _counter); + _counter.AddRef(); + return newRef; + } throw new ObjectDisposedException("Ref<" + typeof(T) + ">"); } } @@ -150,7 +153,10 @@ namespace Avalonia.Utilities { if (_item != null) { - return new Ref((TResult)(object)_item, _counter); + var castRef = new Ref((TResult)(object)_item, _counter); + Interlocked.MemoryBarrier(); + _counter.AddRef(); + return castRef; } throw new ObjectDisposedException("Ref<" + typeof(T) + ">"); } diff --git a/src/Avalonia.Visuals/Media/Imaging/RenderTargetBitmap.cs b/src/Avalonia.Visuals/Media/Imaging/RenderTargetBitmap.cs index 3968c05efb..52bd8a3c04 100644 --- a/src/Avalonia.Visuals/Media/Imaging/RenderTargetBitmap.cs +++ b/src/Avalonia.Visuals/Media/Imaging/RenderTargetBitmap.cs @@ -37,15 +37,6 @@ namespace Avalonia.Media.Imaging /// public new IRef PlatformImpl { get; } - /// - /// Disposes of the bitmap. - /// - public void Dispose() - { - PlatformImpl.Dispose(); - base.Dispose(); - } - /// /// Renders a visual to the . /// diff --git a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs index 850c0a607f..989e2eacdf 100644 --- a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs +++ b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs @@ -35,7 +35,7 @@ namespace Avalonia.Rendering private object _rendering = new object(); private int _lastSceneId = -1; private DisplayDirtyRects _dirtyRectsDisplay = new DisplayDirtyRects(); - private IDrawOperation _currentDraw; + private IRef _currentDraw; /// /// Initializes a new instance of the class. @@ -159,13 +159,13 @@ namespace Avalonia.Rendering /// Size IVisualBrushRenderer.GetRenderTargetSize(IVisualBrush brush) { - return (_currentDraw as BrushDrawOperation)?.ChildScenes?[brush.Visual]?.Size ?? Size.Empty; + return (_currentDraw as IRef)?.Item.ChildScenes?[brush.Visual]?.Size ?? Size.Empty; } /// void IVisualBrushRenderer.RenderVisualBrush(IDrawingContextImpl context, IVisualBrush brush) { - var childScene = (_currentDraw as BrushDrawOperation)?.ChildScenes?[brush.Visual]; + var childScene = (_currentDraw as IRef)?.Item.ChildScenes?[brush.Visual]; if (childScene != null) { @@ -253,7 +253,7 @@ namespace Avalonia.Rendering foreach (var operation in node.DrawOperations) { _currentDraw = operation; - operation.Render(context); + operation.Item.Render(context); _currentDraw = null; } diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/ClipNode.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/ClipNode.cs index 1cda1d5461..14e6b4ab63 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/ClipNode.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/ClipNode.cs @@ -60,5 +60,9 @@ namespace Avalonia.Rendering.SceneGraph context.PopClip(); } } + + public void Dispose() + { + } } } diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs index 38a10a7b7f..d6ab4bd5cb 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs @@ -81,7 +81,7 @@ namespace Avalonia.Rendering.SceneGraph /// public void Dispose() { - // Nothing to do here as we allocate no unmanaged resources. + _node?.Dispose(); } /// @@ -103,9 +103,9 @@ namespace Avalonia.Rendering.SceneGraph { var next = NextDrawAs(); - if (next == null || !next.Equals(Transform, brush, pen, geometry)) + if (next == null || !next.Item.Equals(Transform, brush, pen, geometry)) { - Add(new GeometryNode(Transform, brush, pen, geometry, CreateChildScene(brush))); + Add(RefCountable.Create(new GeometryNode(Transform, brush, pen, geometry, CreateChildScene(brush)))); } else { @@ -118,9 +118,9 @@ namespace Avalonia.Rendering.SceneGraph { var next = NextDrawAs(); - if (next == null || !next.Equals(Transform, source, opacity, sourceRect, destRect)) + if (next == null || !next.Item.Equals(Transform, source, opacity, sourceRect, destRect)) { - Add(new ImageNode(Transform, source, opacity, sourceRect, destRect)); + Add(RefCountable.Create(new ImageNode(Transform, source, opacity, sourceRect, destRect))); } else { @@ -140,9 +140,9 @@ namespace Avalonia.Rendering.SceneGraph { var next = NextDrawAs(); - if (next == null || !next.Equals(Transform, pen, p1, p2)) + if (next == null || !next.Item.Equals(Transform, pen, p1, p2)) { - Add(new LineNode(Transform, pen, p1, p2, CreateChildScene(pen.Brush))); + Add(RefCountable.Create(new LineNode(Transform, pen, p1, p2, CreateChildScene(pen.Brush)))); } else { @@ -155,9 +155,9 @@ namespace Avalonia.Rendering.SceneGraph { var next = NextDrawAs(); - if (next == null || !next.Equals(Transform, null, pen, rect, cornerRadius)) + if (next == null || !next.Item.Equals(Transform, null, pen, rect, cornerRadius)) { - Add(new RectangleNode(Transform, null, pen, rect, cornerRadius, CreateChildScene(pen.Brush))); + Add(RefCountable.Create(new RectangleNode(Transform, null, pen, rect, cornerRadius, CreateChildScene(pen.Brush)))); } else { @@ -170,9 +170,9 @@ namespace Avalonia.Rendering.SceneGraph { var next = NextDrawAs(); - if (next == null || !next.Equals(Transform, foreground, origin, text)) + if (next == null || !next.Item.Equals(Transform, foreground, origin, text)) { - Add(new TextNode(Transform, foreground, origin, text, CreateChildScene(foreground))); + Add(RefCountable.Create(new TextNode(Transform, foreground, origin, text, CreateChildScene(foreground)))); } else { @@ -185,9 +185,9 @@ namespace Avalonia.Rendering.SceneGraph { var next = NextDrawAs(); - if (next == null || !next.Equals(Transform, brush, null, rect, cornerRadius)) + if (next == null || !next.Item.Equals(Transform, brush, null, rect, cornerRadius)) { - Add(new RectangleNode(Transform, brush, null, rect, cornerRadius, CreateChildScene(brush))); + Add(RefCountable.Create(new RectangleNode(Transform, brush, null, rect, cornerRadius, CreateChildScene(brush)))); } else { @@ -205,9 +205,9 @@ namespace Avalonia.Rendering.SceneGraph { var next = NextDrawAs(); - if (next == null || !next.Equals(null)) + if (next == null || !next.Item.Equals(null)) { - Add(new ClipNode()); + Add(RefCountable.Create(new ClipNode())); } else { @@ -220,9 +220,9 @@ namespace Avalonia.Rendering.SceneGraph { var next = NextDrawAs(); - if (next == null || !next.Equals(null)) + if (next == null || !next.Item.Equals(null)) { - Add(new GeometryClipNode()); + Add(RefCountable.Create((new GeometryClipNode()))); } else { @@ -235,9 +235,9 @@ namespace Avalonia.Rendering.SceneGraph { var next = NextDrawAs(); - if (next == null || !next.Equals(null)) + if (next == null || !next.Item.Equals(null)) { - Add(new OpacityNode()); + Add(RefCountable.Create(new OpacityNode())); } else { @@ -250,9 +250,9 @@ namespace Avalonia.Rendering.SceneGraph { var next = NextDrawAs(); - if (next == null || !next.Equals(null, null)) + if (next == null || !next.Item.Equals(null, null)) { - Add(new OpacityMaskNode()); + Add(RefCountable.Create(new OpacityMaskNode())); } else { @@ -265,9 +265,9 @@ namespace Avalonia.Rendering.SceneGraph { var next = NextDrawAs(); - if (next == null || !next.Equals(clip)) + if (next == null || !next.Item.Equals(clip)) { - Add(new ClipNode(clip)); + Add(RefCountable.Create(new ClipNode(clip))); } else { @@ -280,9 +280,9 @@ namespace Avalonia.Rendering.SceneGraph { var next = NextDrawAs(); - if (next == null || !next.Equals(clip)) + if (next == null || !next.Item.Equals(clip)) { - Add(new GeometryClipNode(clip)); + Add(RefCountable.Create(new GeometryClipNode(clip))); } else { @@ -295,9 +295,9 @@ namespace Avalonia.Rendering.SceneGraph { var next = NextDrawAs(); - if (next == null || !next.Equals(opacity)) + if (next == null || !next.Item.Equals(opacity)) { - Add(new OpacityNode(opacity)); + Add(RefCountable.Create(new OpacityNode(opacity))); } else { @@ -310,9 +310,9 @@ namespace Avalonia.Rendering.SceneGraph { var next = NextDrawAs(); - if (next == null || !next.Equals(mask, bounds)) + if (next == null || !next.Item.Equals(mask, bounds)) { - Add(new OpacityMaskNode(mask, bounds, CreateChildScene(mask))); + Add(RefCountable.Create(new OpacityMaskNode(mask, bounds, CreateChildScene(mask)))); } else { @@ -342,7 +342,7 @@ namespace Avalonia.Rendering.SceneGraph foreach (var operation in Owner._node.DrawOperations) { - dirty.Add(operation.Bounds); + dirty.Add(operation.Item.Bounds); } Owner._node = Node; @@ -356,7 +356,7 @@ namespace Avalonia.Rendering.SceneGraph public int DrawOperationIndex { get; } } - private void Add(IDrawOperation node) + private void Add(IRef node) { if (_drawOperationindex < _node.DrawOperations.Count) { @@ -370,9 +370,9 @@ namespace Avalonia.Rendering.SceneGraph ++_drawOperationindex; } - private T NextDrawAs() where T : class, IDrawOperation + private IRef NextDrawAs() where T : class, IDrawOperation { - return _drawOperationindex < _node.DrawOperations.Count ? _node.DrawOperations[_drawOperationindex] as T : null; + return _drawOperationindex < _node.DrawOperations.Count ? _node.DrawOperations[_drawOperationindex] as IRef : null; } private IDictionary CreateChildScene(IBrush brush) diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/DrawOperation.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/DrawOperation.cs index 4c6ed189ff..1a5a6fad3f 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/DrawOperation.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/DrawOperation.cs @@ -22,5 +22,9 @@ namespace Avalonia.Rendering.SceneGraph public abstract bool HitTest(Point p); public abstract void Render(IDrawingContextImpl context); + + public virtual void Dispose() + { + } } } diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/GeometryClipNode.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/GeometryClipNode.cs index a11d641151..2c1decf5dc 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/GeometryClipNode.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/GeometryClipNode.cs @@ -60,5 +60,8 @@ namespace Avalonia.Rendering.SceneGraph context.PopGeometryClip(); } } + public void Dispose() + { + } } } diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/IDrawOperation.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/IDrawOperation.cs index 839fd9b0e5..f8d5bf5b7a 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/IDrawOperation.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/IDrawOperation.cs @@ -9,7 +9,7 @@ namespace Avalonia.Rendering.SceneGraph /// /// Represents a node in the low-level scene graph that represents geometry. /// - public interface IDrawOperation + public interface IDrawOperation : IDisposable { /// /// Gets the bounds of the visible content in the node in global coordinates. diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/IVisualNode.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/IVisualNode.cs index 234cadbf31..681f00799b 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/IVisualNode.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/IVisualNode.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using Avalonia.Media; using Avalonia.Platform; +using Avalonia.Utilities; using Avalonia.VisualTree; namespace Avalonia.Rendering.SceneGraph @@ -12,7 +13,7 @@ namespace Avalonia.Rendering.SceneGraph /// /// Represents a node in the low-level scene graph representing an . /// - public interface IVisualNode + public interface IVisualNode : IDisposable { /// /// Gets the visual to which the node relates. @@ -66,7 +67,7 @@ namespace Avalonia.Rendering.SceneGraph /// /// Gets the drawing operations for the visual. /// - IReadOnlyList DrawOperations { get; } + IReadOnlyList> DrawOperations { get; } /// /// Sets up the drawing context for rendering the node's geometry. diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/ImageNode.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/ImageNode.cs index 8a1a5cb481..06fdb3f86c 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/ImageNode.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/ImageNode.cs @@ -10,7 +10,7 @@ namespace Avalonia.Rendering.SceneGraph /// /// A node in the scene graph which represents an image draw. /// - internal class ImageNode : DrawOperation, IDisposable + internal class ImageNode : DrawOperation { /// /// Initializes a new instance of the class. @@ -89,7 +89,7 @@ namespace Avalonia.Rendering.SceneGraph /// public override bool HitTest(Point p) => Bounds.Contains(p); - public void Dispose() + public override void Dispose() { Source?.Dispose(); } diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/OpacityNode.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/OpacityNode.cs index 097bf15828..ef49ac8879 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/OpacityNode.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/OpacityNode.cs @@ -60,5 +60,9 @@ namespace Avalonia.Rendering.SceneGraph context.PopOpacity(); } } + + public void Dispose() + { + } } } diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs index 6bea4d9bd6..ed8a2960dc 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs @@ -3,8 +3,10 @@ using System; using System.Collections.Generic; +using System.Linq; using Avalonia.Media; using Avalonia.Platform; +using Avalonia.Utilities; using Avalonia.VisualTree; namespace Avalonia.Rendering.SceneGraph @@ -15,12 +17,12 @@ namespace Avalonia.Rendering.SceneGraph internal class VisualNode : IVisualNode { private static readonly IReadOnlyList EmptyChildren = new IVisualNode[0]; - private static readonly IReadOnlyList EmptyDrawOperations = new IDrawOperation[0]; + private static readonly IReadOnlyList> EmptyDrawOperations = new IRef[0]; private Rect? _bounds; private double _opacity; private List _children; - private List _drawOperations; + private List> _drawOperations; private bool _drawOperationsCloned; private Matrix transformRestore; @@ -101,7 +103,7 @@ namespace Avalonia.Rendering.SceneGraph public IReadOnlyList Children => _children ?? EmptyChildren; /// - public IReadOnlyList DrawOperations => _drawOperations ?? EmptyDrawOperations; + public IReadOnlyList> DrawOperations => _drawOperations ?? EmptyDrawOperations; /// /// Adds a child to the collection. @@ -117,10 +119,10 @@ namespace Avalonia.Rendering.SceneGraph /// Adds an operation to the collection. /// /// The operation to add. - public void AddDrawOperation(IDrawOperation operation) + public void AddDrawOperation(IRef operation) { EnsureDrawOperationsCreated(); - _drawOperations.Add(operation); + _drawOperations.Add(operation.Clone()); } /// @@ -131,6 +133,7 @@ namespace Avalonia.Rendering.SceneGraph { EnsureChildrenCreated(); _children.Remove(child); + child.Dispose(); } /// @@ -141,7 +144,9 @@ namespace Avalonia.Rendering.SceneGraph public void ReplaceChild(int index, IVisualNode node) { EnsureChildrenCreated(); + var old = _children[index]; _children[index] = node; + old.Dispose(); } /// @@ -149,10 +154,15 @@ namespace Avalonia.Rendering.SceneGraph /// /// The opeation to be replaced. /// The operation to add. - public void ReplaceDrawOperation(int index, IDrawOperation operation) + public void ReplaceDrawOperation(int index, IRef operation) { EnsureDrawOperationsCreated(); - _drawOperations[index] = operation; + var old = _drawOperations[index]; + _drawOperations[index] = operation.Clone(); + if (old is IDisposable disposable) + { + disposable.Dispose(); + } } /// @@ -165,6 +175,10 @@ namespace Avalonia.Rendering.SceneGraph if (first < _children?.Count) { EnsureChildrenCreated(); + for (int i = first; i < _children.Count - first; i++) + { + _children[i].Dispose(); + } _children.RemoveRange(first, _children.Count - first); } } @@ -179,6 +193,10 @@ namespace Avalonia.Rendering.SceneGraph if (first < _drawOperations?.Count) { EnsureDrawOperationsCreated(); + for (int i = first; i < _drawOperations.Count - first; i++) + { + _drawOperations[i].Dispose(); + } _drawOperations.RemoveRange(first, _drawOperations.Count - first); } } @@ -209,7 +227,7 @@ namespace Avalonia.Rendering.SceneGraph { foreach (var operation in DrawOperations) { - if (operation.HitTest(p) == true) + if (operation.Item.HitTest(p) == true) { return true; } @@ -280,7 +298,7 @@ namespace Avalonia.Rendering.SceneGraph foreach (var operation in DrawOperations) { - result = result.Union(operation.Bounds); + result = result.Union(operation.Item.Bounds); } _bounds = result; @@ -299,13 +317,28 @@ namespace Avalonia.Rendering.SceneGraph { if (_drawOperations == null) { - _drawOperations = new List(); + _drawOperations = new List>(); } else if (_drawOperationsCloned) { - _drawOperations = new List(_drawOperations); + _drawOperations = new List>(_drawOperations.Select(op => op.Clone())); _drawOperationsCloned = false; } } + + public void Dispose() + { + foreach (var child in Children) + { + child.Dispose(); + } + if (!_drawOperationsCloned) + { + foreach (var operation in DrawOperations) + { + operation.Dispose(); + } + } + } } } diff --git a/src/Windows/Avalonia.Direct2D1/Media/DrawingContextImpl.cs b/src/Windows/Avalonia.Direct2D1/Media/DrawingContextImpl.cs index a6c3e97a85..ec21741100 100644 --- a/src/Windows/Avalonia.Direct2D1/Media/DrawingContextImpl.cs +++ b/src/Windows/Avalonia.Direct2D1/Media/DrawingContextImpl.cs @@ -399,7 +399,7 @@ namespace Avalonia.Direct2D1.Media return new ImageBrushImpl( imageBrush, _renderTarget, - (BitmapImpl)imageBrush.Source.PlatformImpl, + (BitmapImpl)imageBrush.Source.PlatformImpl.Item, destinationSize); } else if (visualBrush != null) From 952f89883d07b60fb76018ce67d2aa81cd8b7fe4 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Sun, 28 Jan 2018 13:36:34 -0600 Subject: [PATCH 05/12] Fix build errors in SceneGraph and DeferredRenderer tests. --- .../Rendering/SceneGraph/VisualNode.cs | 5 +--- .../Rendering/DeferredRendererTests.cs | 2 +- .../DeferredDrawingContextImplTests.cs | 13 +++++---- .../Rendering/SceneGraph/VisualNodeTests.cs | 28 +++++++++++++++---- 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs index ed8a2960dc..0208511c4c 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs @@ -159,10 +159,7 @@ namespace Avalonia.Rendering.SceneGraph EnsureDrawOperationsCreated(); var old = _drawOperations[index]; _drawOperations[index] = operation.Clone(); - if (old is IDisposable disposable) - { - disposable.Dispose(); - } + old.Dispose(); } /// diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/DeferredRendererTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/DeferredRendererTests.cs index 8fcb54775b..7821f60a51 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/DeferredRendererTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/DeferredRendererTests.cs @@ -359,7 +359,7 @@ namespace Avalonia.Visuals.UnitTests.Rendering private Mock GetLayerContext(DeferredRenderer renderer, IControl layerRoot) { - return Mock.Get(renderer.Layers[layerRoot].Bitmap.CreateDrawingContext(null)); + return Mock.Get(renderer.Layers[layerRoot].Bitmap.Item.CreateDrawingContext(null)); } private void IgnoreFirstFrame(Mock loop, Mock sceneBuilder) diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs index 1906775783..b0c9e28c98 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs @@ -3,6 +3,7 @@ using System.Linq; using Avalonia.Media; using Avalonia.Rendering.SceneGraph; using Avalonia.UnitTests; +using Avalonia.Utilities; using Avalonia.VisualTree; using Moq; using Xunit; @@ -111,7 +112,7 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph public void Should_Not_Replace_Identical_DrawOperation() { var node = new VisualNode(new TestRoot(), null); - var operation = new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 100, 100), 0); + var operation = RefCountable.Create(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 100, 100), 0)); var layers = new SceneLayers(node.Visual); var target = new DeferredDrawingContextImpl(null, layers); @@ -133,7 +134,7 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph public void Should_Replace_Different_DrawOperation() { var node = new VisualNode(new TestRoot(), null); - var operation = new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 100, 100), 0); + var operation = RefCountable.Create(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 100, 100), 0)); var layers = new SceneLayers(node.Visual); var target = new DeferredDrawingContextImpl(null, layers); @@ -175,10 +176,10 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph var node = new VisualNode(new TestRoot(), null); node.LayerRoot = node.Visual; - node.AddDrawOperation(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 10, 100), 0)); - node.AddDrawOperation(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 20, 100), 0)); - node.AddDrawOperation(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 30, 100), 0)); - node.AddDrawOperation(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 40, 100), 0)); + node.AddDrawOperation(RefCountable.Create(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 10, 100), 0))); + node.AddDrawOperation(RefCountable.Create(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 20, 100), 0))); + node.AddDrawOperation(RefCountable.Create(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 30, 100), 0))); + node.AddDrawOperation(RefCountable.Create(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 40, 100), 0))); var layers = new SceneLayers(node.Visual); var target = new DeferredDrawingContextImpl(null, layers); diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/VisualNodeTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/VisualNodeTests.cs index 3d2e780e0b..1101ccacba 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/VisualNodeTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/VisualNodeTests.cs @@ -1,5 +1,6 @@ using System; using Avalonia.Rendering.SceneGraph; +using Avalonia.Utilities; using Avalonia.VisualTree; using Moq; using Xunit; @@ -43,7 +44,7 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph var node = new VisualNode(Mock.Of(), null); var collection = node.DrawOperations; - node.AddDrawOperation(Mock.Of()); + node.AddDrawOperation(RefCountable.Create(Mock.Of())); Assert.NotSame(collection, node.DrawOperations); } @@ -52,7 +53,7 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph public void Cloned_Nodes_Should_Share_DrawOperations_Collection() { var node1 = new VisualNode(Mock.Of(), null); - node1.AddDrawOperation(Mock.Of()); + node1.AddDrawOperation(RefCountable.Create(Mock.Of())); var node2 = node1.Clone(null); @@ -63,18 +64,33 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph public void Adding_DrawOperation_To_Cloned_Node_Should_Create_New_Collection() { var node1 = new VisualNode(Mock.Of(), null); - var operation1 = Mock.Of(); + var operation1 = RefCountable.Create(Mock.Of()); node1.AddDrawOperation(operation1); var node2 = node1.Clone(null); - var operation2 = Mock.Of(); + var operation2 = RefCountable.Create(Mock.Of()); node2.ReplaceDrawOperation(0, operation2); Assert.NotSame(node1.DrawOperations, node2.DrawOperations); Assert.Equal(1, node1.DrawOperations.Count); Assert.Equal(1, node2.DrawOperations.Count); - Assert.Same(operation1, node1.DrawOperations[0]); - Assert.Same(operation2, node2.DrawOperations[0]); + Assert.Same(operation1.Item, node1.DrawOperations[0].Item); + Assert.Same(operation2.Item, node2.DrawOperations[0].Item); + } + + [Fact] + public void DrawOperations_In_Cloned_Node_Are_Cloned() + { + var node1 = new VisualNode(Mock.Of(), null); + var operation1 = RefCountable.Create(Mock.Of()); + node1.AddDrawOperation(operation1); + + var node2 = node1.Clone(null); + var operation2 = RefCountable.Create(Mock.Of()); + node2.AddDrawOperation(operation2); + + Assert.Same(node1.DrawOperations[0].Item, node2.DrawOperations[0].Item); + Assert.NotSame(node1.DrawOperations[0], node2.DrawOperations[0]); } } } From 8b97174735bdf0231ca899daaebbafcbdfc6d07a Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Sun, 28 Jan 2018 14:11:10 -0600 Subject: [PATCH 06/12] Fix tests to correctly access the item of the ref-counted instance. --- .../SceneGraph/DeferredDrawingContextImplTests.cs | 10 +++++----- .../Rendering/SceneGraph/SceneBuilderTests.cs | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs index b0c9e28c98..0fa70315fd 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs @@ -104,8 +104,8 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph } Assert.Equal(2, node.DrawOperations.Count); - Assert.IsType(node.DrawOperations[0]); - Assert.IsType(node.DrawOperations[1]); + Assert.IsType(node.DrawOperations[0].Item); + Assert.IsType(node.DrawOperations[1].Item); } [Fact] @@ -125,9 +125,9 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph } Assert.Equal(1, node.DrawOperations.Count); - Assert.Same(operation, node.DrawOperations.Single()); + Assert.Same(operation.Item, node.DrawOperations.Single().Item); - Assert.IsType(node.DrawOperations[0]); + Assert.IsType(node.DrawOperations[0].Item); } [Fact] @@ -149,7 +149,7 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph Assert.Equal(1, node.DrawOperations.Count); Assert.NotSame(operation, node.DrawOperations.Single()); - Assert.IsType(node.DrawOperations[0]); + Assert.IsType(node.DrawOperations[0].Item); } [Fact] diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs index 44b9ebe8ce..2eb3dfb15c 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs @@ -53,15 +53,15 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph Assert.Equal(1, borderNode.Children.Count); Assert.Equal(1, borderNode.DrawOperations.Count); - var backgroundNode = (RectangleNode)borderNode.DrawOperations[0]; + var backgroundNode = (RectangleNode)borderNode.DrawOperations[0].Item; Assert.Equal(Brushes.Red, backgroundNode.Brush); - var textBlockNode = (VisualNode)borderNode.Children[0]; + var textBlockNode = borderNode.Children[0]; Assert.Same(textBlockNode, result.FindNode(textBlock)); Assert.Same(textBlock, textBlockNode.Visual); Assert.Equal(1, textBlockNode.DrawOperations.Count); - var textNode = (TextNode)textBlockNode.DrawOperations[0]; + var textNode = (TextNode)textBlockNode.DrawOperations[0].Item; Assert.NotNull(textNode.Text); } } @@ -358,15 +358,15 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph var borderNode = (VisualNode)result.Root.Children[0]; Assert.Same(border, borderNode.Visual); - var backgroundNode = (RectangleNode)borderNode.DrawOperations[0]; + var backgroundNode = (RectangleNode)borderNode.DrawOperations[0].Item; Assert.NotSame(initialBackgroundNode, backgroundNode); Assert.Equal(Brushes.Green, backgroundNode.Brush); var textBlockNode = (VisualNode)borderNode.Children[0]; Assert.Same(textBlock, textBlockNode.Visual); - var textNode = (TextNode)textBlockNode.DrawOperations[0]; - Assert.Same(initialTextNode, textNode); + var textNode = (TextNode)textBlockNode.DrawOperations[0].Item; + Assert.Same(initialTextNode.Item, textNode); } } From 7e174a79e0458a052d25c14729bc60eca3c1d9d9 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 30 Jan 2018 18:02:05 -0600 Subject: [PATCH 07/12] Refactored refcounting and added tests for refcount tracking in the DeferredRenderer and friends. --- src/Avalonia.Base/Utilities/Ref.cs | 44 +++++++++++- src/Avalonia.Visuals/Media/Imaging/Bitmap.cs | 2 +- .../Rendering/DeferredRenderer.cs | 13 +++- .../SceneGraph/DeferredDrawingContextImpl.cs | 37 ++++++---- .../Rendering/SceneGraph/Scene.cs | 7 +- .../Rendering/SceneGraph/SceneBuilder.cs | 17 +++-- .../DeferredDrawingContextImplTests.cs | 23 +++++++ .../SceneGraph/DrawOperationTests.cs | 15 ++++ .../Rendering/SceneGraph/SceneBuilderTests.cs | 68 +++++++++++++++++++ 9 files changed, 196 insertions(+), 30 deletions(-) diff --git a/src/Avalonia.Base/Utilities/Ref.cs b/src/Avalonia.Base/Utilities/Ref.cs index f9e8b29b95..c9ebeefebc 100644 --- a/src/Avalonia.Base/Utilities/Ref.cs +++ b/src/Avalonia.Base/Utilities/Ref.cs @@ -4,22 +4,58 @@ using System.Threading; namespace Avalonia.Utilities { + /// + /// A ref-counted wrapper for a disposable object. + /// + /// public interface IRef : IDisposable where T : class { + /// + /// The item that is being ref-counted. + /// T Item { get; } + + /// + /// Create another reference to this object and increment the refcount. + /// + /// A new reference to this object. IRef Clone(); + + /// + /// Create another reference to the same object, but cast the object to a different type. + /// + /// The type of the new reference. + /// A reference to the value as the new type but sharing the refcount. IRef CloneAs() where TResult : class; + + + /// + /// The current refcount of the object tracked in this reference. For debugging/unit test use only. + /// + int RefCount { get; } } public static class RefCountable { + /// + /// Create a reference counted object wrapping the given item. + /// + /// The type of item. + /// The item to refcount. + /// The refcounted reference to the item. public static IRef Create(T item) where T : class, IDisposable { return new Ref(item, new RefCounter(item)); } - + + /// + /// Create an non-owning non-clonable reference to an item. + /// + /// The type of item. + /// The item. + /// A temporary reference that cannot be cloned that doesn't own the element. public static IRef CreateUnownedNotClonable(T item) where T : class => new TempRef(item); @@ -40,6 +76,8 @@ namespace Avalonia.Utilities public IRef CloneAs() where TResult : class => throw new NotSupportedException(); + + public int RefCount => 1; } class RefCounter @@ -90,6 +128,8 @@ namespace Avalonia.Utilities old = current; } } + + internal int RefCount => _refs; } class Ref : CriticalFinalizerObject, IRef where T : class @@ -161,6 +201,8 @@ namespace Avalonia.Utilities throw new ObjectDisposedException("Ref<" + typeof(T) + ">"); } } + + public int RefCount => _counter.RefCount; } } diff --git a/src/Avalonia.Visuals/Media/Imaging/Bitmap.cs b/src/Avalonia.Visuals/Media/Imaging/Bitmap.cs index 892cd935cf..cb98ed9a9c 100644 --- a/src/Avalonia.Visuals/Media/Imaging/Bitmap.cs +++ b/src/Avalonia.Visuals/Media/Imaging/Bitmap.cs @@ -37,7 +37,7 @@ namespace Avalonia.Media.Imaging /// Initializes a new instance of the class. /// /// A platform-specific bitmap implementation. - protected Bitmap(IRef impl) + public Bitmap(IRef impl) { PlatformImpl = impl.Clone(); } diff --git a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs index 989e2eacdf..709ec670ac 100644 --- a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs +++ b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs @@ -112,7 +112,12 @@ namespace Avalonia.Rendering /// /// Disposes of the renderer and detaches from the render loop. /// - public void Dispose() => Stop(); + public void Dispose() + { + var scene = Interlocked.Exchange(ref _scene, null); + scene.Dispose(); + Stop(); + } /// public IEnumerable HitTest(Point p, IVisual root, Func filter) @@ -391,14 +396,16 @@ namespace Avalonia.Rendering } } - Interlocked.Exchange(ref _scene, scene); + var oldScene = Interlocked.Exchange(ref _scene, scene); + oldScene.Dispose(); _dirty.Clear(); (_root as IRenderRoot)?.Invalidate(new Rect(scene.Size)); } else { - Interlocked.Exchange(ref _scene, null); + var oldScene = Interlocked.Exchange(ref _scene, null); + oldScene.Dispose(); } } finally diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs index d6ab4bd5cb..fa57d07e23 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs @@ -81,7 +81,7 @@ namespace Avalonia.Rendering.SceneGraph /// public void Dispose() { - _node?.Dispose(); + // Nothing to do here since we allocate no unmanaged resources. } /// @@ -105,7 +105,7 @@ namespace Avalonia.Rendering.SceneGraph if (next == null || !next.Item.Equals(Transform, brush, pen, geometry)) { - Add(RefCountable.Create(new GeometryNode(Transform, brush, pen, geometry, CreateChildScene(brush)))); + Add(new GeometryNode(Transform, brush, pen, geometry, CreateChildScene(brush))); } else { @@ -120,7 +120,7 @@ namespace Avalonia.Rendering.SceneGraph if (next == null || !next.Item.Equals(Transform, source, opacity, sourceRect, destRect)) { - Add(RefCountable.Create(new ImageNode(Transform, source, opacity, sourceRect, destRect))); + Add(new ImageNode(Transform, source, opacity, sourceRect, destRect)); } else { @@ -142,7 +142,7 @@ namespace Avalonia.Rendering.SceneGraph if (next == null || !next.Item.Equals(Transform, pen, p1, p2)) { - Add(RefCountable.Create(new LineNode(Transform, pen, p1, p2, CreateChildScene(pen.Brush)))); + Add(new LineNode(Transform, pen, p1, p2, CreateChildScene(pen.Brush))); } else { @@ -157,7 +157,7 @@ namespace Avalonia.Rendering.SceneGraph if (next == null || !next.Item.Equals(Transform, null, pen, rect, cornerRadius)) { - Add(RefCountable.Create(new RectangleNode(Transform, null, pen, rect, cornerRadius, CreateChildScene(pen.Brush)))); + Add(new RectangleNode(Transform, null, pen, rect, cornerRadius, CreateChildScene(pen.Brush))); } else { @@ -172,7 +172,7 @@ namespace Avalonia.Rendering.SceneGraph if (next == null || !next.Item.Equals(Transform, foreground, origin, text)) { - Add(RefCountable.Create(new TextNode(Transform, foreground, origin, text, CreateChildScene(foreground)))); + Add(new TextNode(Transform, foreground, origin, text, CreateChildScene(foreground))); } else { @@ -187,7 +187,7 @@ namespace Avalonia.Rendering.SceneGraph if (next == null || !next.Item.Equals(Transform, brush, null, rect, cornerRadius)) { - Add(RefCountable.Create(new RectangleNode(Transform, brush, null, rect, cornerRadius, CreateChildScene(brush)))); + Add(new RectangleNode(Transform, brush, null, rect, cornerRadius, CreateChildScene(brush))); } else { @@ -207,7 +207,7 @@ namespace Avalonia.Rendering.SceneGraph if (next == null || !next.Item.Equals(null)) { - Add(RefCountable.Create(new ClipNode())); + Add(new ClipNode()); } else { @@ -222,7 +222,7 @@ namespace Avalonia.Rendering.SceneGraph if (next == null || !next.Item.Equals(null)) { - Add(RefCountable.Create((new GeometryClipNode()))); + Add((new GeometryClipNode())); } else { @@ -237,7 +237,7 @@ namespace Avalonia.Rendering.SceneGraph if (next == null || !next.Item.Equals(null)) { - Add(RefCountable.Create(new OpacityNode())); + Add(new OpacityNode()); } else { @@ -252,7 +252,7 @@ namespace Avalonia.Rendering.SceneGraph if (next == null || !next.Item.Equals(null, null)) { - Add(RefCountable.Create(new OpacityMaskNode())); + Add(new OpacityMaskNode()); } else { @@ -267,7 +267,7 @@ namespace Avalonia.Rendering.SceneGraph if (next == null || !next.Item.Equals(clip)) { - Add(RefCountable.Create(new ClipNode(clip))); + Add(new ClipNode(clip)); } else { @@ -282,7 +282,7 @@ namespace Avalonia.Rendering.SceneGraph if (next == null || !next.Item.Equals(clip)) { - Add(RefCountable.Create(new GeometryClipNode(clip))); + Add(new GeometryClipNode(clip)); } else { @@ -297,7 +297,7 @@ namespace Avalonia.Rendering.SceneGraph if (next == null || !next.Item.Equals(opacity)) { - Add(RefCountable.Create(new OpacityNode(opacity))); + Add(new OpacityNode(opacity)); } else { @@ -312,7 +312,7 @@ namespace Avalonia.Rendering.SceneGraph if (next == null || !next.Item.Equals(mask, bounds)) { - Add(RefCountable.Create(new OpacityMaskNode(mask, bounds, CreateChildScene(mask)))); + Add(new OpacityMaskNode(mask, bounds, CreateChildScene(mask))); } else { @@ -356,6 +356,13 @@ namespace Avalonia.Rendering.SceneGraph public int DrawOperationIndex { get; } } + private void Add(IDrawOperation node) + { + var refCounted = RefCountable.Create(node); + Add(refCounted); + refCounted.Dispose(); // Dispose our reference + } + private void Add(IRef node) { if (_drawOperationindex < _node.DrawOperations.Count) diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/Scene.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/Scene.cs index 9216bae8ad..f2e4f5fdbd 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/Scene.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/Scene.cs @@ -11,7 +11,7 @@ namespace Avalonia.Rendering.SceneGraph /// /// Represents a scene graph used by the . /// - public class Scene + public class Scene : IDisposable { private Dictionary _index; @@ -96,6 +96,11 @@ namespace Avalonia.Rendering.SceneGraph return result; } + public void Dispose() + { + Root.Dispose(); + } + /// /// Tries to find a node in the scene graph representing the specified visual. /// diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/SceneBuilder.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/SceneBuilder.cs index 41ff802164..b219a74119 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/SceneBuilder.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/SceneBuilder.cs @@ -271,22 +271,21 @@ namespace Avalonia.Rendering.SceneGraph private static void Deindex(Scene scene, VisualNode node) { - scene.Remove(node); - node.SubTreeUpdated = true; - - scene.Layers[node.LayerRoot].Dirty.Add(node.Bounds); - - node.Visual.TransformedBounds = null; - foreach (VisualNode child in node.Children) { - var geometry = child as IDrawOperation; - if (child is VisualNode visual) { Deindex(scene, visual); } } + scene.Remove(node); + + node.SubTreeUpdated = true; + + scene.Layers[node.LayerRoot].Dirty.Add(node.Bounds); + + node.Visual.TransformedBounds = null; + if (node.LayerRoot == node.Visual && node.Visual != scene.Root.Visual) { diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs index 0fa70315fd..6faba372d5 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs @@ -1,6 +1,7 @@ using System; using System.Linq; using Avalonia.Media; +using Avalonia.Platform; using Avalonia.Rendering.SceneGraph; using Avalonia.UnitTests; using Avalonia.Utilities; @@ -192,5 +193,27 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph Assert.Equal(2, node.DrawOperations.Count); } + + [Fact] + public void Trimmed_DrawOperations_Releases_Reference() + { + var node = new VisualNode(new TestRoot(), null); + var operation = RefCountable.Create(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 100, 100), 0)); + var layers = new SceneLayers(node.Visual); + var target = new DeferredDrawingContextImpl(null, layers); + + node.LayerRoot = node.Visual; + node.AddDrawOperation(operation); + Assert.Equal(2, operation.RefCount); + + using (target.BeginUpdate(node)) + { + target.FillRectangle(Brushes.Green, new Rect(0, 0, 100, 100)); + } + + Assert.Equal(1, node.DrawOperations.Count); + Assert.NotSame(operation, node.DrawOperations.Single()); + Assert.Equal(1, operation.RefCount); + } } } diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DrawOperationTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DrawOperationTests.cs index 76fe103c1b..8c905dab2f 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DrawOperationTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DrawOperationTests.cs @@ -2,6 +2,8 @@ using Avalonia.Media; using Avalonia.Platform; using Avalonia.Rendering.SceneGraph; +using Avalonia.Utilities; +using Moq; using Xunit; namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph @@ -40,6 +42,19 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph Assert.Equal(new Rect(expectedX, expectedY, expectedWidth, expectedHeight), target.Bounds); } + [Fact] + public void Image_Node_Releases_Reference_To_Bitmap_On_Dispose() + { + var bitmap = RefCountable.Create(Mock.Of()); + var imageNode = new ImageNode(Matrix.Identity, bitmap, 1, new Rect(1,1,1,1), new Rect(1,1,1,1)); + + Assert.Equal(2, bitmap.RefCount); + + imageNode.Dispose(); + + Assert.Equal(1, bitmap.RefCount); + } + private class TestDrawOperation : DrawOperation { public TestDrawOperation(Rect bounds, Matrix transform, Pen pen) diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs index 2eb3dfb15c..f44be3f82e 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/SceneBuilderTests.cs @@ -11,6 +11,8 @@ using Moq; using Avalonia.Platform; using System.Reactive.Subjects; using Avalonia.Data; +using Avalonia.Utilities; +using Avalonia.Media.Imaging; namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph { @@ -678,6 +680,72 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph } } + [Fact] + public void Disposing_Scene_Releases_DrawOperation_References() + { + using (TestApplication()) + { + var bitmap = RefCountable.Create(Mock.Of()); + Image img; + var tree = new TestRoot + { + Child = img = new Image + { + Source = new Bitmap(bitmap) + } + }; + + Assert.Equal(2, bitmap.RefCount); + IRef operation; + + using (var scene = new Scene(tree)) + { + var sceneBuilder = new SceneBuilder(); + sceneBuilder.UpdateAll(scene); + operation = scene.FindNode(img).DrawOperations[0]; + Assert.Equal(1, operation.RefCount); + + Assert.Equal(3, bitmap.RefCount); + } + Assert.Equal(0, operation.RefCount); + Assert.Equal(2, bitmap.RefCount); + } + } + + [Fact] + public void Replacing_Control_Releases_DrawOperation_Reference() + { + using (TestApplication()) + { + var bitmap = RefCountable.Create(Mock.Of()); + Image img; + var tree = new TestRoot + { + Child = img = new Image + { + Source = new Bitmap(bitmap) + } + }; + + var scene = new Scene(tree); + var sceneBuilder = new SceneBuilder(); + sceneBuilder.UpdateAll(scene); + + var operation = scene.FindNode(img).DrawOperations[0]; + + tree.Child = new Decorator(); + + using (var result = scene.Clone()) + { + sceneBuilder.Update(result, img); + scene.Dispose(); + + Assert.Equal(0, operation.RefCount); + Assert.Equal(2, bitmap.RefCount); + } + } + } + private IDisposable TestApplication() { return UnitTestApplication.Start( From ebfa79ef64d3f927c9931405fb6148f2247370ed Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 30 Jan 2018 18:20:54 -0600 Subject: [PATCH 08/12] Add null check for scene in DeferredRenderer disposal. --- src/Avalonia.Visuals/Rendering/DeferredRenderer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs index 709ec670ac..3d5f6d5a57 100644 --- a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs +++ b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs @@ -115,7 +115,7 @@ namespace Avalonia.Rendering public void Dispose() { var scene = Interlocked.Exchange(ref _scene, null); - scene.Dispose(); + scene?.Dispose(); Stop(); } From d905721c468e1b94f437375cd33121bb22a347de Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 30 Jan 2018 18:27:54 -0600 Subject: [PATCH 09/12] Add more null short-circuiting checks in DeferredRenderer related to scene disposal. --- src/Avalonia.Visuals/Rendering/DeferredRenderer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs index 3d5f6d5a57..3611085ff3 100644 --- a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs +++ b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs @@ -397,7 +397,7 @@ namespace Avalonia.Rendering } var oldScene = Interlocked.Exchange(ref _scene, scene); - oldScene.Dispose(); + oldScene?.Dispose(); _dirty.Clear(); (_root as IRenderRoot)?.Invalidate(new Rect(scene.Size)); @@ -405,7 +405,7 @@ namespace Avalonia.Rendering else { var oldScene = Interlocked.Exchange(ref _scene, null); - oldScene.Dispose(); + oldScene?.Dispose(); } } finally From ae7e7829d43064508cc11183164d0ec576bbbde9 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 30 Jan 2018 18:48:10 -0600 Subject: [PATCH 10/12] Fixed incorrect cast in DeferredRenderer's IVisualBrushRenderer implementation. --- src/Avalonia.Visuals/Rendering/DeferredRenderer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs index 3611085ff3..344f3f8f2b 100644 --- a/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs +++ b/src/Avalonia.Visuals/Rendering/DeferredRenderer.cs @@ -164,13 +164,13 @@ namespace Avalonia.Rendering /// Size IVisualBrushRenderer.GetRenderTargetSize(IVisualBrush brush) { - return (_currentDraw as IRef)?.Item.ChildScenes?[brush.Visual]?.Size ?? Size.Empty; + return (_currentDraw.Item as BrushDrawOperation)?.ChildScenes?[brush.Visual]?.Size ?? Size.Empty; } /// void IVisualBrushRenderer.RenderVisualBrush(IDrawingContextImpl context, IVisualBrush brush) { - var childScene = (_currentDraw as IRef)?.Item.ChildScenes?[brush.Visual]; + var childScene = (_currentDraw.Item as BrushDrawOperation)?.ChildScenes?[brush.Visual]; if (childScene != null) { From 85966d3ecf926325ac2af44b883c760f4d4c6ac5 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 30 Jan 2018 19:30:59 -0600 Subject: [PATCH 11/12] Ref-count the draw operations collection in VisualNode so it correctly keeps the elements alive when only the collection is cloned (aka no changes are made by a cloned VisualNode) and the original VisualNode is disposed. --- .../Rendering/SceneGraph/VisualNode.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs index 0208511c4c..c651fee67f 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Reactive.Disposables; using Avalonia.Media; using Avalonia.Platform; using Avalonia.Utilities; @@ -23,6 +24,7 @@ namespace Avalonia.Rendering.SceneGraph private double _opacity; private List _children; private List> _drawOperations; + private IRef _drawOperationsRefCounter; private bool _drawOperationsCloned; private Matrix transformRestore; @@ -214,6 +216,7 @@ namespace Avalonia.Rendering.SceneGraph _opacity = Opacity, OpacityMask = OpacityMask, _drawOperations = _drawOperations, + _drawOperationsRefCounter = _drawOperationsRefCounter?.Clone(), _drawOperationsCloned = true, LayerRoot= LayerRoot, }; @@ -315,10 +318,14 @@ namespace Avalonia.Rendering.SceneGraph if (_drawOperations == null) { _drawOperations = new List>(); + _drawOperationsRefCounter = RefCountable.Create(Disposable.Create(DisposeDrawOperations)); + _drawOperationsCloned = false; } else if (_drawOperationsCloned) { _drawOperations = new List>(_drawOperations.Select(op => op.Clone())); + _drawOperationsRefCounter.Dispose(); + _drawOperationsRefCounter = RefCountable.Create(Disposable.Create(DisposeDrawOperations)); _drawOperationsCloned = false; } } @@ -329,12 +336,14 @@ namespace Avalonia.Rendering.SceneGraph { child.Dispose(); } - if (!_drawOperationsCloned) + _drawOperationsRefCounter?.Dispose(); + } + + private void DisposeDrawOperations() + { + foreach (var operation in DrawOperations) { - foreach (var operation in DrawOperations) - { - operation.Dispose(); - } + operation.Dispose(); } } } From 74f893027725613e2e538717f7d600bab79f9716 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Sun, 4 Feb 2018 14:11:09 -0600 Subject: [PATCH 12/12] Fixed bug in TrimDrawOperations that wasn't disposing draw operations correctly. --- .../SceneGraph/DeferredDrawingContextImpl.cs | 7 ++++--- .../Rendering/SceneGraph/VisualNode.cs | 2 +- .../DeferredDrawingContextImplTests.cs | 20 ++++++++++++++----- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs index fa57d07e23..dfd45fba9c 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/DeferredDrawingContextImpl.cs @@ -358,9 +358,10 @@ namespace Avalonia.Rendering.SceneGraph private void Add(IDrawOperation node) { - var refCounted = RefCountable.Create(node); - Add(refCounted); - refCounted.Dispose(); // Dispose our reference + using (var refCounted = RefCountable.Create(node)) + { + Add(refCounted); + } } private void Add(IRef node) diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs index c651fee67f..3cb7e53d21 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs @@ -192,7 +192,7 @@ namespace Avalonia.Rendering.SceneGraph if (first < _drawOperations?.Count) { EnsureDrawOperationsCreated(); - for (int i = first; i < _drawOperations.Count - first; i++) + for (int i = first; i < _drawOperations.Count; i++) { _drawOperations[i].Dispose(); } diff --git a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs index 6faba372d5..4d23c57eed 100644 --- a/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/Rendering/SceneGraph/DeferredDrawingContextImplTests.cs @@ -175,13 +175,18 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph public void Should_Trim_DrawOperations() { var node = new VisualNode(new TestRoot(), null); - node.LayerRoot = node.Visual; - node.AddDrawOperation(RefCountable.Create(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 10, 100), 0))); - node.AddDrawOperation(RefCountable.Create(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 20, 100), 0))); - node.AddDrawOperation(RefCountable.Create(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 30, 100), 0))); - node.AddDrawOperation(RefCountable.Create(new RectangleNode(Matrix.Identity, Brushes.Red, null, new Rect(0, 0, 40, 100), 0))); + for (var i = 0; i < 4; ++i) + { + var drawOperation = new Mock(); + using (var r = RefCountable.Create(drawOperation.Object)) + { + node.AddDrawOperation(r); + } + } + + var drawOperations = node.DrawOperations.Select(op => op.Item).ToList(); var layers = new SceneLayers(node.Visual); var target = new DeferredDrawingContextImpl(null, layers); @@ -192,6 +197,11 @@ namespace Avalonia.Visuals.UnitTests.Rendering.SceneGraph } Assert.Equal(2, node.DrawOperations.Count); + + foreach (var i in drawOperations) + { + Mock.Get(i).Verify(x => x.Dispose()); + } } [Fact]