diff --git a/osu.Game.Tests/Rulesets/TestSceneDrawableRulesetDependencies.cs b/osu.Game.Tests/Rulesets/TestSceneDrawableRulesetDependencies.cs new file mode 100644 index 0000000000..89e3b48aa3 --- /dev/null +++ b/osu.Game.Tests/Rulesets/TestSceneDrawableRulesetDependencies.cs @@ -0,0 +1,134 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading.Tasks; +using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Audio; +using osu.Framework.Audio.Sample; +using osu.Framework.Audio.Track; +using osu.Framework.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.OpenGL.Textures; +using osu.Framework.Graphics.Shapes; +using osu.Framework.Graphics.Textures; +using osu.Game.Rulesets.Osu; +using osu.Game.Rulesets.UI; +using osu.Game.Tests.Visual; + +namespace osu.Game.Tests.Rulesets +{ + public class TestSceneDrawableRulesetDependencies : OsuTestScene + { + [Test] + public void TestDisposalDoesNotDisposeParentStores() + { + DrawableWithDependencies drawable = null; + TestTextureStore textureStore = null; + TestSampleStore sampleStore = null; + + AddStep("add dependencies", () => + { + Child = drawable = new DrawableWithDependencies(); + textureStore = drawable.ParentTextureStore; + sampleStore = drawable.ParentSampleStore; + }); + + AddStep("clear children", Clear); + AddUntilStep("wait for disposal", () => drawable.IsDisposed); + + AddStep("GC", () => + { + drawable = null; + + GC.Collect(); + GC.WaitForPendingFinalizers(); + }); + + AddAssert("parent texture store not disposed", () => !textureStore.IsDisposed); + AddAssert("parent sample store not disposed", () => !sampleStore.IsDisposed); + } + + private class DrawableWithDependencies : CompositeDrawable + { + public TestTextureStore ParentTextureStore { get; private set; } + public TestSampleStore ParentSampleStore { get; private set; } + + public DrawableWithDependencies() + { + InternalChild = new Box { RelativeSizeAxes = Axes.Both }; + } + + protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) + { + var dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); + + dependencies.CacheAs(ParentTextureStore = new TestTextureStore()); + dependencies.CacheAs(ParentSampleStore = new TestSampleStore()); + + return new DrawableRulesetDependencies(new OsuRuleset(), dependencies); + } + + public new bool IsDisposed { get; private set; } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + IsDisposed = true; + } + } + + private class TestTextureStore : TextureStore + { + public override Texture Get(string name, WrapMode wrapModeS, WrapMode wrapModeT) => null; + + public bool IsDisposed { get; private set; } + + protected override void Dispose(bool disposing) + { + base.Dispose(disposing); + IsDisposed = true; + } + } + + private class TestSampleStore : ISampleStore + { + public bool IsDisposed { get; private set; } + + public void Dispose() + { + IsDisposed = true; + } + + public SampleChannel Get(string name) => null; + + public Task GetAsync(string name) => null; + + public Stream GetStream(string name) => null; + + public IEnumerable GetAvailableResources() => throw new NotImplementedException(); + + public BindableNumber Volume => throw new NotImplementedException(); + public BindableNumber Balance => throw new NotImplementedException(); + public BindableNumber Frequency => throw new NotImplementedException(); + public BindableNumber Tempo => throw new NotImplementedException(); + + public void AddAdjustment(AdjustableProperty type, BindableNumber adjustBindable) => throw new NotImplementedException(); + + public void RemoveAdjustment(AdjustableProperty type, BindableNumber adjustBindable) => throw new NotImplementedException(); + + public void RemoveAllAdjustments(AdjustableProperty type) => throw new NotImplementedException(); + + public IBindable AggregateVolume => throw new NotImplementedException(); + public IBindable AggregateBalance => throw new NotImplementedException(); + public IBindable AggregateFrequency => throw new NotImplementedException(); + public IBindable AggregateTempo => throw new NotImplementedException(); + + public int PlaybackConcurrency { get; set; } + } + } +} diff --git a/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs b/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs index 83a1077d70..a9b2a15b35 100644 --- a/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs +++ b/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs @@ -10,6 +10,7 @@ using osu.Framework.Audio; using osu.Framework.Audio.Sample; using osu.Framework.Audio.Track; using osu.Framework.Bindables; +using osu.Framework.Graphics.OpenGL.Textures; using osu.Framework.Graphics.Textures; using osu.Framework.IO.Stores; using osu.Game.Rulesets.Configuration; @@ -46,12 +47,11 @@ namespace osu.Game.Rulesets.UI if (resources != null) { TextureStore = new TextureStore(new TextureLoaderStore(new NamespacedResourceStore(resources, @"Textures"))); - TextureStore.AddStore(parent.Get()); - Cache(TextureStore); + CacheAs(TextureStore = new FallbackTextureStore(TextureStore, parent.Get())); SampleStore = parent.Get().GetSampleStore(new NamespacedResourceStore(resources, @"Samples")); SampleStore.PlaybackConcurrency = OsuGameBase.SAMPLE_CONCURRENCY; - CacheAs(new FallbackSampleStore(SampleStore, parent.Get())); + CacheAs(SampleStore = new FallbackSampleStore(SampleStore, parent.Get())); } RulesetConfigManager = parent.Get().GetConfigFor(ruleset); @@ -82,69 +82,92 @@ namespace osu.Game.Rulesets.UI isDisposed = true; SampleStore?.Dispose(); + TextureStore?.Dispose(); RulesetConfigManager = null; } #endregion - } - /// - /// A sample store which adds a fallback source. - /// - /// - /// This is a temporary implementation to workaround ISampleStore limitations. - /// - public class FallbackSampleStore : ISampleStore - { - private readonly ISampleStore primary; - private readonly ISampleStore secondary; - - public FallbackSampleStore(ISampleStore primary, ISampleStore secondary) + /// + /// A sample store which adds a fallback source and prevents disposal of the fallback source. + /// + private class FallbackSampleStore : ISampleStore { - this.primary = primary; - this.secondary = secondary; + private readonly ISampleStore primary; + private readonly ISampleStore fallback; + + public FallbackSampleStore(ISampleStore primary, ISampleStore fallback) + { + this.primary = primary; + this.fallback = fallback; + } + + public SampleChannel Get(string name) => primary.Get(name) ?? fallback.Get(name); + + public Task GetAsync(string name) => primary.GetAsync(name) ?? fallback.GetAsync(name); + + public Stream GetStream(string name) => primary.GetStream(name) ?? fallback.GetStream(name); + + public IEnumerable GetAvailableResources() => throw new NotSupportedException(); + + public void AddAdjustment(AdjustableProperty type, BindableNumber adjustBindable) => throw new NotSupportedException(); + + public void RemoveAdjustment(AdjustableProperty type, BindableNumber adjustBindable) => throw new NotSupportedException(); + + public void RemoveAllAdjustments(AdjustableProperty type) => throw new NotSupportedException(); + + public BindableNumber Volume => throw new NotSupportedException(); + + public BindableNumber Balance => throw new NotSupportedException(); + + public BindableNumber Frequency => throw new NotSupportedException(); + + public BindableNumber Tempo => throw new NotSupportedException(); + + public IBindable GetAggregate(AdjustableProperty type) => throw new NotSupportedException(); + + public IBindable AggregateVolume => throw new NotSupportedException(); + + public IBindable AggregateBalance => throw new NotSupportedException(); + + public IBindable AggregateFrequency => throw new NotSupportedException(); + + public IBindable AggregateTempo => throw new NotSupportedException(); + + public int PlaybackConcurrency + { + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); + } + + public void Dispose() + { + primary?.Dispose(); + } } - public SampleChannel Get(string name) => primary.Get(name) ?? secondary.Get(name); - - public Task GetAsync(string name) => primary.GetAsync(name) ?? secondary.GetAsync(name); - - public Stream GetStream(string name) => primary.GetStream(name) ?? secondary.GetStream(name); - - public IEnumerable GetAvailableResources() => throw new NotSupportedException(); - - public void AddAdjustment(AdjustableProperty type, BindableNumber adjustBindable) => throw new NotSupportedException(); - - public void RemoveAdjustment(AdjustableProperty type, BindableNumber adjustBindable) => throw new NotSupportedException(); - - public void RemoveAllAdjustments(AdjustableProperty type) => throw new NotSupportedException(); - - public BindableNumber Volume => throw new NotSupportedException(); - - public BindableNumber Balance => throw new NotSupportedException(); - - public BindableNumber Frequency => throw new NotSupportedException(); - - public BindableNumber Tempo => throw new NotSupportedException(); - - public IBindable GetAggregate(AdjustableProperty type) => throw new NotSupportedException(); - - public IBindable AggregateVolume => throw new NotSupportedException(); - - public IBindable AggregateBalance => throw new NotSupportedException(); - - public IBindable AggregateFrequency => throw new NotSupportedException(); - - public IBindable AggregateTempo => throw new NotSupportedException(); - - public int PlaybackConcurrency + /// + /// A texture store which adds a fallback source and prevents disposal of the fallback source. + /// + private class FallbackTextureStore : TextureStore { - get => throw new NotSupportedException(); - set => throw new NotSupportedException(); - } + private readonly TextureStore primary; + private readonly TextureStore fallback; - public void Dispose() - { + public FallbackTextureStore(TextureStore primary, TextureStore fallback) + { + this.primary = primary; + this.fallback = fallback; + } + + public override Texture Get(string name, WrapMode wrapModeS, WrapMode wrapModeT) + => primary.Get(name, wrapModeS, wrapModeT) ?? fallback.Get(name, wrapModeS, wrapModeT); + + protected override void Dispose(bool disposing) + { + base.Dispose(disposing); + primary?.Dispose(); + } } } }