Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cache pages to avoid system memory leak #400

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 4 additions & 22 deletions Screenbox.Core/ViewModels/AlbumsPageViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;
using Windows.System;
using Windows.UI.Xaml.Controls;

namespace Screenbox.Core.ViewModels
Expand All @@ -20,28 +19,16 @@ public sealed class AlbumsPageViewModel : BaseMusicContentViewModel

private readonly ILibraryService _libraryService;
private readonly IFilesService _filesService;
private readonly DispatcherQueue _dispatcherQueue;
private readonly DispatcherQueueTimer _refreshTimer;

public AlbumsPageViewModel(ILibraryService libraryService, IFilesService filesService)
public AlbumsPageViewModel(ILibraryService libraryService, IFilesService filesService) : base(libraryService)
{
_libraryService = libraryService;
_filesService = filesService;
_dispatcherQueue = DispatcherQueue.GetForCurrentThread();
_refreshTimer = _dispatcherQueue.CreateTimer();
GroupedAlbums = new ObservableGroupedCollection<string, AlbumViewModel>();

libraryService.MusicLibraryContentChanged += OnMusicLibraryContentChanged;
PropertyChanged += OnPropertyChanged;
}

public void OnNavigatedFrom()
{
_libraryService.MusicLibraryContentChanged -= OnMusicLibraryContentChanged;
_refreshTimer.Stop();
}

public void FetchAlbums()
public override void FetchContent()
{
// No need to run fetch async. HomePageViewModel should already called the method.
MusicLibraryFetchResult musicLibrary = _libraryService.GetMusicFetchResult();
Expand All @@ -67,11 +54,11 @@ public void FetchAlbums()
// Progressively update when it's still loading
if (_libraryService.IsLoadingMusic)
{
_refreshTimer.Debounce(FetchAlbums, TimeSpan.FromSeconds(5));
RefreshTimer.Debounce(FetchContent, TimeSpan.FromSeconds(5));
}
else
{
_refreshTimer.Stop();
RefreshTimer.Stop();
}
}

Expand Down Expand Up @@ -153,10 +140,5 @@ private void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
}
}
}

private void OnMusicLibraryContentChanged(ILibraryService sender, object args)
{
_dispatcherQueue.TryEnqueue(FetchAlbums);
}
}
}
26 changes: 4 additions & 22 deletions Screenbox.Core/ViewModels/ArtistsPageViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using Screenbox.Core.Services;
using System;
using System.Linq;
using Windows.System;

namespace Screenbox.Core.ViewModels
{
Expand All @@ -14,27 +13,15 @@ public sealed class ArtistsPageViewModel : BaseMusicContentViewModel
public ObservableGroupedCollection<string, ArtistViewModel> GroupedArtists { get; }

private readonly ILibraryService _libraryService;
private readonly DispatcherQueue _dispatcherQueue;
private readonly DispatcherQueueTimer _refreshTimer;

public ArtistsPageViewModel(ILibraryService libraryService)
public ArtistsPageViewModel(ILibraryService libraryService) : base(libraryService)
{
_libraryService = libraryService;
_dispatcherQueue = DispatcherQueue.GetForCurrentThread();
_refreshTimer = _dispatcherQueue.CreateTimer();
GroupedArtists = new ObservableGroupedCollection<string, ArtistViewModel>();
PopulateGroups();

libraryService.MusicLibraryContentChanged += OnMusicLibraryContentChanged;
}

public void OnNavigatedFrom()
{
_libraryService.MusicLibraryContentChanged -= OnMusicLibraryContentChanged;
_refreshTimer.Stop();
}

public void FetchArtists()
public override void FetchContent()
{
// No need to run fetch async. HomePageViewModel should already called the method.
MusicLibraryFetchResult musicLibrary = _libraryService.GetMusicFetchResult();
Expand All @@ -51,11 +38,11 @@ public void FetchArtists()
// Progressively update when it's still loading
if (_libraryService.IsLoadingMusic)
{
_refreshTimer.Debounce(FetchArtists, TimeSpan.FromSeconds(5));
RefreshTimer.Debounce(FetchContent, TimeSpan.FromSeconds(5));
}
else
{
_refreshTimer.Stop();
RefreshTimer.Stop();
}
}

Expand All @@ -66,10 +53,5 @@ private void PopulateGroups()
GroupedArtists.AddGroup(key);
}
}

private void OnMusicLibraryContentChanged(ILibraryService sender, object args)
{
_dispatcherQueue.TryEnqueue(FetchArtists);
}
}
}
33 changes: 33 additions & 0 deletions Screenbox.Core/ViewModels/BaseMusicContentViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,23 @@
using CommunityToolkit.Mvvm.Input;
using CommunityToolkit.Mvvm.Messaging;
using Screenbox.Core.Messages;
using Screenbox.Core.Services;
using System;
using System.Collections.Generic;
using System.Linq;
using Windows.System;

namespace Screenbox.Core.ViewModels;
public abstract partial class BaseMusicContentViewModel : ObservableRecipient
{
private bool HasSongs => Songs.Count > 0;

private DispatcherQueue DispatcherQueue { get; }

protected DispatcherQueueTimer RefreshTimer { get; }

protected ILibraryService LibraryService { get; }

[ObservableProperty]
[NotifyCanExecuteChangedFor(nameof(ShuffleAndPlayCommand))]
private IReadOnlyList<MediaViewModel> _songs = Array.Empty<MediaViewModel>();
Expand All @@ -22,6 +30,31 @@ public abstract partial class BaseMusicContentViewModel : ObservableRecipient

[ObservableProperty] private bool _isLoading;

protected BaseMusicContentViewModel(ILibraryService libraryService)
{
LibraryService = libraryService;
DispatcherQueue = DispatcherQueue.GetForCurrentThread();
RefreshTimer = DispatcherQueue.CreateTimer();
}

public abstract void FetchContent();

public void OnNavigatedTo()
{
LibraryService.MusicLibraryContentChanged += OnMusicLibraryContentChanged;
}

public void OnNavigatedFrom()
{
LibraryService.MusicLibraryContentChanged -= OnMusicLibraryContentChanged;
RefreshTimer.Stop();
}

private void OnMusicLibraryContentChanged(ILibraryService sender, object args)
{
DispatcherQueue.TryEnqueue(FetchContent);
}

[RelayCommand(CanExecute = nameof(HasSongs))]
private void ShuffleAndPlay()
{
Expand Down
11 changes: 10 additions & 1 deletion Screenbox.Core/ViewModels/MusicPageViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,20 @@ public MusicPageViewModel(ILibraryService libraryService, IResourceService resou
{
_libraryService = libraryService;
_resourceService = resourceService;
_libraryService.MusicLibraryContentChanged += OnMusicLibraryContentChanged;
_dispatcherQueue = DispatcherQueue.GetForCurrentThread();
_hasContent = true;
}

public void OnNavigatedTo()
{
_libraryService.MusicLibraryContentChanged += OnMusicLibraryContentChanged;
}

public void OnNavigatedFrom()
{
_libraryService.MusicLibraryContentChanged -= OnMusicLibraryContentChanged;
}

public void UpdateSongs()
{
MusicLibraryFetchResult musicLibrary = _libraryService.GetMusicFetchResult();
Expand Down
37 changes: 8 additions & 29 deletions Screenbox.Core/ViewModels/SongsPageViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,40 +10,24 @@
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;
using Windows.System;

namespace Screenbox.Core.ViewModels
{
public sealed partial class SongsPageViewModel : BaseMusicContentViewModel
{
public ObservableGroupedCollection<string, MediaViewModel> GroupedSongs { get; }

private readonly ILibraryService _libraryService;
private readonly DispatcherQueue _dispatcherQueue;
private readonly DispatcherQueueTimer _refreshTimer;

public SongsPageViewModel(ILibraryService libraryService)
public SongsPageViewModel(ILibraryService libraryService) : base(libraryService)
{
_libraryService = libraryService;
_dispatcherQueue = DispatcherQueue.GetForCurrentThread();
_refreshTimer = _dispatcherQueue.CreateTimer();
GroupedSongs = new ObservableGroupedCollection<string, MediaViewModel>();

libraryService.MusicLibraryContentChanged += OnMusicLibraryContentChanged;
PropertyChanged += OnPropertyChanged;
}

public void OnNavigatedFrom()
{
_libraryService.MusicLibraryContentChanged -= OnMusicLibraryContentChanged;
_refreshTimer.Stop();
}

public void FetchSongs()
public override void FetchContent()
{
// No need to run fetch async. HomePageViewModel should already called the method.
MusicLibraryFetchResult musicLibrary = _libraryService.GetMusicFetchResult();
IsLoading = _libraryService.IsLoadingMusic;
MusicLibraryFetchResult musicLibrary = LibraryService.GetMusicFetchResult();
IsLoading = LibraryService.IsLoadingMusic;
Songs = musicLibrary.Songs;

// Populate song groups with fetched result
Expand All @@ -64,13 +48,13 @@ public void FetchSongs()
}

// Progressively update when it's still loading
if (_libraryService.IsLoadingMusic)
if (LibraryService.IsLoadingMusic)
{
_refreshTimer.Debounce(FetchSongs, TimeSpan.FromSeconds(5));
RefreshTimer.Debounce(FetchContent, TimeSpan.FromSeconds(5));
}
else
{
_refreshTimer.Stop();
RefreshTimer.Stop();
}
}

Expand Down Expand Up @@ -146,16 +130,11 @@ private List<IGrouping<string, MediaViewModel>> GetCurrentGrouping(MusicLibraryF
};
}

private void OnMusicLibraryContentChanged(ILibraryService sender, object args)
{
_dispatcherQueue.TryEnqueue(FetchSongs);
}

private void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
{
if (e.PropertyName == nameof(SortBy))
{
var groups = GetCurrentGrouping(_libraryService.GetMusicFetchResult());
var groups = GetCurrentGrouping(LibraryService.GetMusicFetchResult());
GroupedSongs.Clear();
foreach (IGrouping<string, MediaViewModel> group in groups)
{
Expand Down
3 changes: 3 additions & 0 deletions Screenbox/App.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@
<Thickness x:Key="ContentPageBottomMargin">0,0,0,106</Thickness>
<x:Double x:Key="ContentPageBottomPaddingHeight">106</x:Double>

<x:String x:Key="EntranceTranslateAnimationOffset">0,40,0</x:String>
<x:String x:Key="EntranceTranslateAnimationDuration">0:0:0.4</x:String>

<CornerRadius x:Key="CircularCornerRadius">99</CornerRadius>

<Style TargetType="FontIcon">
Expand Down
3 changes: 1 addition & 2 deletions Screenbox/Controls/MediaListViewItem.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
xmlns:muxc="using:Microsoft.UI.Xaml.Controls"
xmlns:strings="using:Screenbox.Strings"
xmlns:toolkitConverters="using:CommunityToolkit.WinUI.Converters"
xmlns:viewModels="using:Screenbox.Core.ViewModels"
d:DataContext="{d:DesignInstance Type=viewModels:MediaViewModel}"
d:DesignHeight="300"
Expand Down Expand Up @@ -228,7 +227,7 @@
</VisualState>
</VisualStateGroup>

<VisualStateGroup x:Name="PlayingStates">
<VisualStateGroup x:Name="PlayingStates" CurrentStateChanged="PlayingStatesOnCurrentStateChanged">
<VisualState x:Name="Playing">
<VisualState.Setters>
<Setter Target="PlayingIndicator.PlaybackRate" Value="1" />
Expand Down
1 change: 0 additions & 1 deletion Screenbox/Controls/MediaListViewItem.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ public MediaListViewItem()
{
this.InitializeComponent();
Common = Ioc.Default.GetRequiredService<CommonViewModel>();
PlayingStates.CurrentStateChanged += PlayingStatesOnCurrentStateChanged;
}

private GridLength BoolToGridLength(bool visibility) =>
Expand Down
22 changes: 19 additions & 3 deletions Screenbox/Pages/AlbumsPage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
x:Class="Screenbox.Pages.AlbumsPage"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:controls="using:Screenbox.Controls"
xmlns:animations="using:CommunityToolkit.WinUI.Animations"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:interactions="using:Screenbox.Controls.Interactions"
xmlns:interactivity="using:Microsoft.Xaml.Interactivity"
Expand All @@ -11,7 +11,7 @@
xmlns:strings="using:Screenbox.Strings"
xmlns:triggers="using:CommunityToolkit.WinUI"
xmlns:ui="using:CommunityToolkit.WinUI"
xmlns:viewModels="using:Screenbox.Core.ViewModels"
NavigationCacheMode="Enabled"
mc:Ignorable="d">

<Page.Resources>
Expand Down Expand Up @@ -112,7 +112,10 @@
IsIndeterminate="True"
Visibility="{x:Bind ViewModel.IsLoading, Mode=OneWay}" />

<SemanticZoom Grid.Row="1" Margin="{StaticResource TopLargeMargin}">
<SemanticZoom
x:Name="SemanticZoom"
Grid.Row="1"
Margin="{StaticResource TopLargeMargin}">
<SemanticZoom.ZoomedInView>
<GridView
x:Name="AlbumGridView"
Expand Down Expand Up @@ -167,6 +170,19 @@
</interactivity:Interaction.Behaviors>
</GridView>
</SemanticZoom.ZoomedOutView>

<animations:Implicit.ShowAnimations>
<animations:TranslationAnimation
EasingMode="EaseOut"
EasingType="Default"
From="{StaticResource EntranceTranslateAnimationOffset}"
To="0"
Duration="{StaticResource EntranceTranslateAnimationDuration}" />
<animations:OpacityAnimation
From="0"
To="1"
Duration="{StaticResource EntranceTranslateAnimationDuration}" />
</animations:Implicit.ShowAnimations>
</SemanticZoom>

<VisualStateManager.VisualStateGroups>
Expand Down
5 changes: 3 additions & 2 deletions Screenbox/Pages/AlbumsPage.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ private void ViewModelOnPropertyChanged(object sender, PropertyChangedEventArgs
protected override void OnNavigatedTo(NavigationEventArgs e)
{
base.OnNavigatedTo(e);
ViewModel.OnNavigatedTo();
if (e.NavigationMode == NavigationMode.Back
&& Common.TryGetPageState(nameof(AlbumsPage), Frame.BackStackDepth, out var state)
&& state is KeyValuePair<string, double> pair)
Expand All @@ -62,8 +63,8 @@ protected override void OnNavigatedTo(NavigationEventArgs e)
_contentVerticalOffset = pair.Value;
}

if (!_dispatcherQueue.TryEnqueue(ViewModel.FetchAlbums))
ViewModel.FetchAlbums();
if (!_dispatcherQueue.TryEnqueue(ViewModel.FetchContent))
ViewModel.FetchContent();
}

protected override void OnNavigatedFrom(NavigationEventArgs e)
Expand Down
Loading
Loading