MaterialDesignInXAML / MaterialDesignInXamlToolkit

Google's Material Design in XAML & WPF, for C# & VB.Net.
http://materialdesigninxaml.net
MIT License
15.09k stars 3.42k forks source link

Button styles: MaterialDesignRaisedButton used with MahApps.Metro causes serious memory leak #3065

Open shushu789 opened 1 year ago

shushu789 commented 1 year ago

Bug explanation

I found a problem, maybe a compatibility issue or a design bug, but I think it can be fixed. My WPF program uses MahApps, (<mah:MetroWindow...), in which I use a button control of MaterialDesignInXamlToolkit, the style of this button control is: MaterialDesignRaisedButton. When I run the program, everything seems to be fine, but immediately after I minimize the form, the memory increases at a rate of 1MB/s, and it won't stop, unless I restore this form that was just minimized, after restoring the memory immediately drop and return to normal levels. I know my question is about coming from two different places, (MahApps and MaterialDesignInXamlToolkit), but if I don't use MahApps(mah:MetroWindow), then, the above problem doesn't exist, everything works fine, also I tried several other Different styles of a similar button, only this style is found so far: MaterialDesignRaisedButton has this problem, the condition that triggers this problem is when the form is minimized.

xaml code:** `<mah:MetroWindow x:Class="WpfApp1.MainWindow" xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:d="http://schemas.microsoft.com/expression/blend/2008" xmlns:mah="clr-namespace:MahApps.Metro.Controls;assembly=MahApps.Metro" xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" xmlns:materialDesign="http://materialdesigninxaml.net/winfx/xaml/themes" xmlns:local="clr-namespace:WpfApp1" mc:Ignorable="d" WindowStartupLocation="CenterScreen" Title="MainWindow" Height="450" Width="800">

shushu789 commented 1 year ago

Sorry, I'll just add to this question. This button is a progress process effect, which uses the following two attributes: materialDesign:ButtonProgressAssist.IsIndeterminate="True" materialDesign:ButtonProgressAssist.IsIndicatorVisible="True" The values of these two properties are both True to trigger when minimized, if one of them is false, it will not, but if it is False, the process animation effect will be lost

nicolaihenriksen commented 1 year ago

@shushu789 Could you please write up a small sample and attach it? Based on your description above, I have added a small WPF app which uses a MahApps MetroWindow including a Button using the MaterialDesignRaisedButton style like your XAML does. I do not see the memory leak you're describing. My XAML is nearly identical to yours, apart from not binding the IsEnabled property. I am also curious to see what type your DataContext is and how it is implemented. You can get some memory leaks with WPF bindings if you don't set them up right.

The snapshot list in the image below is not that long and could indicate a small increase every time, but when I keep taking snapshots, eventually GC kicks in and the Heap Size (Diff) falls back to "normal" again; thus no apparent memory leak.

image

image

image

shushu789 commented 1 year ago

thank you for your reply,I uploaded a demo project for this problem: https://github.com/shushu789/DotWPFDemo/tree/master

shushu789 commented 1 year ago

The problem looks like this: wpf301

nicolaihenriksen commented 1 year ago

@shushu789 Thanks for the sample.

There are possibly other things affecting this, because even when I run the sample you provided, I do not see a memory leak. The Visual Studio diagnostics tool does not show any signs of a leak, and I also used Redgate ANTS Memory Profiler which similarly does not show a leak.

Have you tried using a different memory profiler (i.e. something other than the Diagnostics Tools built into Visual Studio)?

I notice that you're on an OS with Chinese language (I presume, apologies if I am mistaken). Not really sure how that could affect this though... But can you reproduce the issue on another developer machine?

NoLeak

shushu789 commented 1 year ago

I just used 3 different machines to test this WPFDemo, the problem still exists (one of the machines is win7), I also sent this Demo sample to other users, the same problem. Also I changed my windows system's language and region to both English and US (I thought it would have something to do with this), but it still does, so I don't think the problem has much to do with the machine. I am not very familiar with the use of memory analysis tools, but I just used the dotMemory tool for analysis. I reproduced the problem and took a snapshot. I am not sure what caused the memory explosion. I intercepted some of the analysis pictures in dotMemory, I don't know if it will help this problem. If you have time, can you help me to see where is the problem shown in the picture below? If I need to provide other information, I can provide it at any time, thank you

snap01 sna02cpmpare snap2callTree snap4 snap5
Keboo commented 1 year ago

I am also not able to replicate this issue. Could you provide a little more information on your system. Which versions of Windows are you running? Which version of dotnet is installed (you can check by running: dotnet --list-runtimes)?

shushu789 commented 1 year ago

Here is my windows system info: Edition Windows 10 Pro Version 21H2 Installed on ‎2022/‎5/‎2 OS build 19044.2364 Experience Windows Feature Experience Pack 120.2212.4190.0

dotnet --list-runtimes

Microsoft.AspNetCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 7.0.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 7.0.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 6.0.13 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 7.0.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

gitjsdr26 commented 1 year ago

I faced also memory leaks on some computers with GPU NVidia cards. I'm also using Mahapps coupled with Material Design. Memory was increasing until 5 Gbytes whereas the application normally uses 250 Mbytes. After long researches and tests, I found a way to avoid this problem by disabling the WPF GPU rendering. See the below link Microsoft Explanation : https://learn.microsoft.com/en-us/dotnet/api/system.windows.media.renderoptions.processrendermode?view=windowsdesktop-7.0

I don't know if it is the same problem than yours, but you can try adding at application startup in App.xaml.cs this part of code :

/// <summary>
/// OnStartup event
/// </summary>
/// <param name="e"></param>
protected override void OnStartup(StartupEventArgs e)
{
    base.OnStartup(e);

    // Option diable hardware GPU acceleration
    RenderOptions.ProcessRenderMode = System.Windows.Interop.RenderMode.SoftwareOnly;
...
}
BlackFeatherOfficial commented 1 year ago

I tried this way, but it doesn't work

BlackFeatherOfficial commented 1 year ago

@nicolaihenriksen I'm having a similar issue, but I'm not using the MahApps.Metro library. There are also differences in the triggering method, see for details #3282

AndrewKeepCoding commented 1 year ago

I was able to reproduce this issue on a test app.

<mah:MetroWindow x:Class="Issue3065ReproApp.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        xmlns:local="clr-namespace:Issue3065ReproApp"
        mc:Ignorable="d"
        Title="MainWindow" Height="450" Width="800"
        xmlns:mah="clr-namespace:MahApps.Metro.Controls;assembly=MahApps.Metro"
        xmlns:materialDesign="http://materialdesigninxaml.net/winfx/xaml/themes">
    <Grid>
      <Button
        materialDesign:ButtonProgressAssist.IsIndeterminate="True"
        materialDesign:ButtonProgressAssist.IsIndicatorVisible="True" />
    </Grid>
</mah:MetroWindow>

These are the facts that I've found so far:

I need to find some time to dig deeper so might take another few days. Since I'm not familiar with MaterialDesign, any tips are appreciated.

Keboo commented 1 year ago

Great diagnosis. Since you noted that it does not occur with just a regular ProgressBar without a button, I would test with a ProgressBar that uses the MaterialDesignLinearProgressBar style. This is all that is functionally being done inside of the Button template.

I find it interesting that MetroWindow makes a difference on the ability to reproduce. From hunting both MahApps and our MDIX MahApps integration library I don't see anything that immediately would link those things together. It may be worth dropping various storyboards to see if we can track down which one appears to be holding the references.

AndrewKeepCoding commented 1 year ago

Sorry for the late response.

New facts:

nicolaihenriksen commented 1 year ago

@AndrewKeepCoding Thank you so much for the detailed analysis! Unfortunately, even using your sample, the issue does not reproduce on my machine.

However, having chatted with @keboo about this while trying to reproduce, and afterwards some time to think about it all, I think I have found something which could be the culprit. I was, like you, curious as to why the elevation had an effect on the issue.

Looking at the code, we nearly always use the ShadowConverter when turning an Elevation value into the corresponding DropShadowEffect. I found this open issue in WPF regarding ´DropShadowEffects` in particular: https://github.com/dotnet/wpf/issues/6782

and funnily enough, our ShadowConverter does not freeze the DropShadowEffect it returns.

Could you possibly try to extract the effect into a variable, eg. effect, and then call effect.Freeze(); before returning it? Just to see if that solves it for you.

Another thing is that I am a little unsure why we even clone the effect in the first place, since the "source effect" pulled from the ResourceDictionary is already frozen, and I believe that should allow it to be reused. So you may even be able to remove the Clone() method completely.

Another funny thing I stumbled upon, was this WPF memory leak: https://github.com/dotnet/wpf/issues/6898

Here the author is experiencing a memory leak, and the posted screenshots reveals that he is using a non-western character set. And just like me, the dev trying to reproduce this does not see a leak. I don't know what the missing link is here, but this is a surprisingly similar situation as what we're facing here.

AndrewKeepCoding commented 1 year ago

@nicolaihenriksen Thanks for the tips!

Unfortunately, neither of these changes stops the memory leak when the app is minimized:

Another thing is that I am a little unsure why we even clone the effect in the first place, since the "source effect" pulled from the ResourceDictionary is already frozen, and I believe that should allow it to be reused.

I guess this is because a "not-frozen" effect is required. This callback is called after the minimized app is restored and it throws InvalidOperationException if the effect is frozen.

ShadowAssists.cs

private static void DarkenPropertyChangedCallback(DependencyObject dependencyObject, DependencyPropertyChangedEventArgs dependencyPropertyChangedEventArgs)
{
    var uiElement = dependencyObject as UIElement;
    var dropShadowEffect = uiElement?.Effect as DropShadowEffect;

    if (dropShadowEffect is null) return;

    double? toOpacity;
    if ((bool)dependencyPropertyChangedEventArgs.NewValue)
    {
        // System.InvalidOperationException: 
        // 'Cannot animate the 'Opacity' property on 'System.Windows.Media.Effects.DropShadowEffect' 
        // because the object is sealed or frozen.'
        dropShadowEffect.BeginAnimation(DropShadowEffect.OpacityProperty, null);

        SetLocalInfo(dependencyObject, new ShadowLocalInfo(dropShadowEffect.Opacity));

        toOpacity = 1;
    }
    else

Not related to this bug but at least it might be better to check if the effect is frozen or not:

if (dropShadowEffect is null || dropShadowEffect.IsFrozen is true)
    return;

Here the author is experiencing a memory leak, and the posted screenshots reveals that he is using a non-western character set. And just like me, the dev trying to reproduce this does not see a leak. I don't know what the missing link is here, but this is a surprisingly similar situation as what we're facing here.

I'm still not sure if this is language related. As I mentioned before, I was able to reproduce this right after I did a clean install on my laptop and installed the English version of Windows 11 (the keyboard is also in US-layout). But the fact that you and @Keboo cannot reproduce this says otherwise though...

AndrewKeepCoding commented 1 year ago

I totally missed @gitjsdr26 comment on ProcessRenderMode. It actually prevents the memory increasing.

RenderOptions.ProcessRenderMode = System.Windows.Interop.RenderMode.SoftwareOnly;
AndrewKeepCoding commented 1 year ago

I also tried this on another computer (Win7 x64 English with Japanese IME installed) but didn't reproduce the memory increase issue.

I think that this dotnet/wpf#7704 is somehow related to the issue. The computer that doesn't reproduce the issue, has a NVIDIA GeForce GPU, whereas my laptop that does reproduce the issue, has an Intel Iris Xe Graphics GPU.

This might be the reason that you can't reproduce the memory leak.

nicolaihenriksen commented 1 year ago

This might be the reason that you can't reproduce the memory leak.

After spending quite a bit of time trying to reproduce this the other day, I was very close to concluding that this must be a hardware-dependent (or at least system-dependent) issue. I think the issue you link to here is very interesting, and quite possible the issue we're seeing. Could you, @shushu789, provide information about which graphics card you're using. I am on NVIDIA GeForce which is likely why I can't reproduce.

shushu789 commented 1 year ago

@nicolaihenriksen First of all, thank you for your attention to this issue. I'm not sure if it's related to the system or the graphics card. I just tried disabling my dedicated graphics card driver and encountered the same problem again, but it still wasn't resolved. On the system side, I attempted to change all the relevant settings such as region and language to the United States, but the issue still persists.

Below are screenshots of system information and graphics card information respectively:

image

image

AndrewKeepCoding commented 1 year ago

@shushu789 Does the RenderOptions.ProcessRenderMode workaround work for you?

shushu789 commented 1 year ago

@AndrewKeepCoding @gitjsdr26 Yes, this method does work: RenderOptions.ProcessRenderMode = System.Windows.Interop.RenderMode.SoftwareOnly; However, from what I understand, setting the rendering mode to software rendering can potentially lead to a performance drop in terms of graphics, especially when dealing with complex graphics or animations. Sacrificing more for the sake of resolving a memory leak issue caused by a specific button animation might not align with the expectations of most projects. It would be ideal if the issue could be resolved while still allowing GPU acceleration.

AndrewKeepCoding commented 1 year ago

Hi @shushu789! I'm not saying that you should disable GPU support. I just wanted to add here another case that this might be related to GPU support.

It would be ideal if the issue could be resolved while still allowing GPU acceleration.

One workaround until this issue gets fixed could be by setting the elevation to Dp0 (The default is Dp2). At least this works on my environment.

<mah:MetroWindow
    x:Class="WpfApp1.MainWindow"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:local="clr-namespace:WpfApp1"
    xmlns:mah="clr-namespace:MahApps.Metro.Controls;assembly=MahApps.Metro"
    xmlns:materialDesign="http://materialdesigninxaml.net/winfx/xaml/themes"
    xmlns:materialDesignWpf="clr-namespace:MaterialDesignThemes.Wpf;assembly=MaterialDesignThemes.Wpf"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    Title="MainWindow"
    Width="800"
    Height="450"
    mc:Ignorable="d">
    <Grid>
        <Button
            materialDesign:ButtonProgressAssist.IsIndeterminate="True"
            materialDesign:ButtonProgressAssist.IsIndicatorVisible="True"
            materialDesignWpf:ElevationAssist.Elevation="Dp0" />
    </Grid>
</mah:MetroWindow>
shushu789 commented 1 year ago

@AndrewKeepCoding Yes, I just tested your solution, setting the shadow effect to DP0, and it indeed solves the issue. materialDesignWpf:ElevationAssist.Elevation="Dp0"

HappyJying commented 10 months ago

This problem happens again after v4.8.1, it performs well before v4.8.0.