I am using WPF as UI framework to develop a game. The game engine doesn't mutate it self, it only got changed when a call from the UI application telling it to update to next frame.
This is the key part of my codes:
public class GameWorldViewModel : ViewModel {
public GameWorldViewModel(GameWorld gameWorld) {
this.gameWorld = gameWorld;
GameBodyCollectionViewModel = new GameBodyCollectionViewModel(gameWorld);
CopyData();
Proceed();
}
public GameBodyCollectionViewModel GameBodyCollectionViewModel { get; init; }
private readonly GameWorld gameWorld;
private readonly Stopwatch stopwatch = new Stopwatch();
private bool isProceed = false;
public void Pause() {
if (!isProceed) throw new InvalidOperationException("The GameWorld is already paused.");
isProceed = false;
stopwatch.Reset();
}
public void Proceed() {
if (isProceed) throw new InvalidOperationException("The GameWorld is already proceeding.");
isProceed = true;
Action action = () => DispatherLoopCallback(Task.CompletedTask);
stopwatch.Start();
Application.Current.Dispatcher.BeginInvoke(action, DispatcherPriority.Background);
}
private void DispatherLoopCallback(Task task) {
if (!isProceed) return;
if (task.IsCompleted) {//Check if the backgroud update has completed
CopyData();//Copy the data but not update UI
double deltaTime = stopwatch.Elapsed.TotalSeconds;
stopwatch.Restart();
task = gameWorld.BeginUpdate(deltaTime);//Let backgroud game engine calculate next frame of the game.
NotifyChange();//Update UI, runing concurrently with backgroud game engine thread. After this line is removed, the memory leak doesn't occur any more.
}
Task task_forLambda = task;
Action action = () => DispatherLoopCallback(task_forLambda);
Application.Current.Dispatcher.BeginInvoke(action, DispatcherPriority.Background);//Send next call of this method to the dispatcher, leave space for other WPF process.
}
private void CopyData() {
GameBodyCollectionViewModel.CopyData();
}
private void NotifyChange() {
GameBodyCollectionViewModel.NotifyChange();
}
}
However, when the game is running, even there are nothing, the memory usage keep increasing. And this increasing stop when the game pause. So I am sure there is a memory leak. After investigation, I found the problem come from NotifyChange(). But I cannot figure out how are these ViewModel classes causing problem.
public class GameBodyCollectionViewModel : CollectionViewModel<GameBodyViewModel> {
public GameBodyCollectionViewModel(GameWorld gameWorld) {
this.gameWorld = gameWorld;
}
private readonly GameWorld gameWorld;
public override IEnumerator<GameBodyViewModel> GetEnumerator() => copiedData.GetEnumerator();
internal void CopyData() {
copiedData.Clear();
copiedData.AddRange(from gb in gameWorld.GetGameBodies() select new GameBodyViewModel(gb));
}
private readonly List<GameBodyViewModel> copiedData = new List<GameBodyViewModel>();
internal void NotifyChange() {
NotifyCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));//After this line is removed, the memory leak doesn't happen any more.
}
}
public class GameBodyViewModel : ViewModel {
public GameBodyViewModel(GameBody gameBody) {
AABBLowerX = gameBody.AABB.LowerBound.X;
AABBLowerY = gameBody.AABB.LowerBound.Y;
AABBWidth = gameBody.AABB.Width;
AABBHeight = gameBody.AABB.Height;
}
public double AABBLowerX { get; }
public double AABBLowerY { get; }
public double AABBWidth { get; }
public double AABBHeight { get; }
}
public abstract class ViewModel : INotifyPropertyChanged {
public event PropertyChangedEventHandler PropertyChanged;
protected void NotifyPropertyChanged([CallerMemberName] String propertyName = null) {
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
}
}
public abstract class CollectionViewModel<T> : ViewModel, INotifyCollectionChanged, IEnumerable<T> {
public abstract IEnumerator<T> GetEnumerator();
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
public event NotifyCollectionChangedEventHandler CollectionChanged;
protected void NotifyCollectionChanged(NotifyCollectionChangedEventArgs e) {
CollectionChanged?.Invoke(this, e);
}
}
The xaml codes of views are unlikely to be relevant to the context so I didn't write them here. If any more detail is required please tell me.
Update0
It was a fault to neglect the xaml codes. I found its actually relevant.
<v:View x:TypeArguments="local:GameBodyCollectionViewModel" x:Name="view"
x:Class="Enigma.GameWPF.Visual.Game.GameBodyCollectionView"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:v ="clr-namespace:Enigma.GameWPF.Visual"
xmlns:local="clr-namespace:Enigma.GameWPF.Visual.Game"
mc:Ignorable="d"
d:DesignHeight="450" d:DesignWidth="800">
<ItemsControl ItemsSource="{Binding ViewModel,ElementName=view}">
<ItemsControl.ItemsPanel>
<ItemsPanelTemplate>
<Canvas></Canvas>
</ItemsPanelTemplate>
</ItemsControl.ItemsPanel>
<ItemsControl.ItemContainerStyle>
<Style>
<Setter Property="Canvas.Left" Value="{Binding AABBLowerX}"/>
<Setter Property="Canvas.Bottom" Value="{Binding AABBLowerY}"/>
</Style>
</ItemsControl.ItemContainerStyle>
<ItemsControl.ItemTemplate>
<DataTemplate>
<local:GameBodyView ViewModel="{Binding}" Width="{Binding AABBWidth}" Height="{Binding AABBHeight}"></local:GameBodyView>
</DataTemplate>
</ItemsControl.ItemTemplate>
</ItemsControl>
</v:View>
After removing the whole ItemsControl, the memory leak was not happenning.
Update1
Based on what I observed, I made a demo project inorder for audiences to have better information and able to reproduce the bug. As StackOverFlow dosen't support file attach and the same thing is posted on github as well, I am posting the link of that post here. You may goto that link to download the demo. https://github.com/dotnet/wpf/issues/5739
Update2
Based on whats done already, I tried to used .NET provided ObservableCollection instead of my own implementation of INotifyCollectionChanged. DemoCollectionViewModel was changed to :
public class DemoCollectionViewModel : ViewModel {
public DemoCollectionViewModel() {
DemoItemViewModels = new ObservableCollection<DemoItemViewModel>();
}
public ObservableCollection<DemoItemViewModel> DemoItemViewModels { get; }
private readonly List<DemoItemViewModel> copiedData = new List<DemoItemViewModel>();
internal void CopyData() {
copiedData.Clear();
copiedData.AddRange(from SimulatedModelItem smi in SimulatedModel.GetItems select new DemoItemViewModel(smi));
}
internal void NotifyChange() {
DemoItemViewModels.Clear();
foreach (DemoItemViewModel vm in copiedData) {
DemoItemViewModels.Add(vm);
}
}
}
And in the view, the ItemsControl's ItemsSource is binded to the ObservableCollection instead. However the problem persists
CodePudding user response：
After a lot of attempts, I eventually found the answer by myself. Instead of keep creating new instances, I modified it to be only adding new item when the collection retrieved from model has more item count. Each time copying data from model is just updating existing instances if the source has less count.
Here's the final code:
public class DemoCollectionViewModel : CollectionViewModel<DemoItemViewModel> {
public DemoCollectionViewModel() {
}
public override IEnumerator<DemoItemViewModel> GetEnumerator() => copiedData.GetEnumerator();
private readonly List<DemoItemViewModel> copiedData = new List<DemoItemViewModel>();
private int includedCount = 0;
internal void CopyData() {
int i = 0;
SimulatedModelItem[] simulatedModelItems = SimulatedModel.Items.ToArray();
foreach (SimulatedModelItem smi in simulatedModelItems) {
if (copiedData.Count == i) {
DemoItemViewModel demoItemViewModel = new DemoItemViewModel();
copiedData.Add(demoItemViewModel);
addRecord.Add(demoItemViewModel);
}
copiedData[i].CopyData(smi);
i ;
}
for (int j = i; j < includedCount; j ) {
copiedData[j].IsEmpty = true;
}
includedCount = i;
}
private readonly List<DemoItemViewModel> addRecord = new List<DemoItemViewModel>();
private int lastNotifyIncludedCount = 0;
internal void NotifyChange() {
foreach (DemoItemViewModel demoItemViewModel in addRecord) {
NotifyCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, demoItemViewModel));
}
addRecord.Clear();
int greaterInt = Math.Max(includedCount, lastNotifyIncludedCount);
for (int i = 0; i < greaterInt; i ) {
copiedData[i].NotifyChange();
}
lastNotifyIncludedCount = includedCount;
}
}
public class DemoViewModel : ViewModel {
public DemoViewModel() {
DemoCollectionViewModel = new DemoCollectionViewModel();
Proceed();
}
public DemoCollectionViewModel DemoCollectionViewModel { get; }
private bool isProceed = false;
public void Pause() {
if (!isProceed) throw new InvalidOperationException("The GameWorld is already paused.");
isProceed = false;
}
public void Proceed() {
if (isProceed) throw new InvalidOperationException("The GameWorld is already proceeding.");
isProceed = true;
Action action = () => DispatherLoopCallback();
Application.Current.Dispatcher.BeginInvoke(action, DispatcherPriority.Background);
}
private void DispatherLoopCallback() {
if (!isProceed) return;
CopyData();
NotifyChange();
Application.Current.Dispatcher.BeginInvoke(DispatherLoopCallback, DispatcherPriority.Background);//Send next call of this method to the dispatcher, leave space for other WPF process.
}
private void CopyData() {
DemoCollectionViewModel.CopyData();
}
private void NotifyChange() {
DemoCollectionViewModel.NotifyChange();
}
}
The final version of demo, with the memory leak bug fixed is also available in the github post https://github.com/dotnet/wpf/issues/5739
CodePudding user response：
As stated in INotifyCollectionChanged is not updating the UI INotifyCollectionChanged should be implemented by a collection and not by a view model and here it would be better to just use ObservableCollection which automatically handles updating the UI when the collection changes and get rid of the GameBodyCollectionViewModel.NotifyChange().
Update
I apologize forI haven't noticed the IEnumerable before. The leak is most likely because you're using "NotifyCollectionChangedAction.Reset" every time the loop is called which would trigger a UI update for all the items in the collection regardless of whether there were any changes.