dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
7.03k stars 1.17k forks source link

Window freezes during resize when updating WriteableBitmap from worker thread #5816

Open andrekoehler opened 2 years ago

andrekoehler commented 2 years ago

3) Write changes to the back buffer. Other threads may write changes to the back buffer when the WriteableBitmap is locked.

Minimal repro: Create a new Wpf App, then copy the following code over MainWindow.xaml.cs:

using System;
using System.Runtime.InteropServices;
using System.Threading;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Threading;

namespace WpfApp1
{
    public partial class MainWindow : Window
    {
        readonly byte[] buffer = new byte[1920 * 1080 * 3];

        readonly WriteableBitmap wb = new WriteableBitmap(
            1920, 1080, 96, 96, PixelFormats.Bgr24, null);

        readonly Dispatcher dispatcher;
        Thread thread;

        public MainWindow()
        {
            InitializeComponent();

            // set all pixels to blue
            for (int y = 0; y < wb.PixelHeight; ++y)
            {
                for (int x = 0; x < wb.PixelWidth; ++x)
                {
                    int offset = 3 * x + (y * wb.PixelWidth * 3);
                    buffer[offset + 0] = 0xFF;
                }
            }

            Image image = new Image() { Source = wb };
            this.Content = image;

            dispatcher = Dispatcher;
            Loaded += MainWindow_Loaded;
        }

        void MainWindow_Loaded(object sender, RoutedEventArgs e)
        {
            thread = new Thread(ThreadFunc);
            thread.Start();
        }

        void ThreadFunc()
        {
            for (; ; )
            {
                IntPtr backBuffer = IntPtr.Zero;

                // lock on UI thread
                dispatcher.Invoke(() =>
                {
                    wb.Lock();

                    // we have to copy this because it must only be accessed from the UI thread
                    backBuffer = wb.BackBuffer;

                    // assume there is no padding to make this example short
                    System.Diagnostics.Debug.Assert(wb.BackBufferStride == wb.PixelWidth * wb.Format.BitsPerPixel / 8);
                });
                System.Diagnostics.Debug.Assert(backBuffer != IntPtr.Zero);

                // update on this thread
                Marshal.Copy(buffer, 0, backBuffer, buffer.Length);

                // unlock on UI thread
                dispatcher.Invoke(() =>
                {
                    wb.AddDirtyRect(new Int32Rect(0, 0, wb.PixelWidth, wb.PixelHeight));
                    wb.Unlock();
                });
            }
        }
    }
}
lindexi commented 2 years ago

@andrekoehler See https://github.com/dotnet/wpf/issues/4396

And I fixed it in https://github.com/dotnet/wpf/pull/4425

And,

I publish the CustomWPF nuget in https://www.nuget.org/packages/Lindexi.Src.CustomWPF/1.0.3

And I write a demo code with your demo repo, see https://github.com/lindexi/lindexi_gd/tree/c81be0e0fc06b3df9504c7ff44d7377ac5b26b55/BitmapSourceTest

You should publish the demo with x86 and run the application in BitmapSourceTest\bin\Debug\net6.0-windows\win-x86\publish\BitmapSourceTest.exe

dotnet publish -r win-x86 --self-contained

Could you test the demo code?

miloush commented 2 years ago

Well the example on the same page you cited does not really suggest it is safe to allow other dispatcher operations during the lock. You might want to prepare your frame in another buffer and then perform just a memory copy inside the lock on UI thread.

Alternatively, in case tearing was not an issue for your application, you might not need to keep the bitmap locked, only lock it for obtaining the BackBuffer (which contractually never changes so you can even do that only once) and then for calling AddDirtyRectangle (which in most cases does not even have to be blocking, but obviously don't stall the dispatcher).

ghost commented 1 year ago

I'm using a very similar design to this example (although using a timer rather than a loop), I sometimes experience the same issue. Another issue with WriteableBitmap is freezes when hosted in a TabControl and the selected tab is changed. The stack trace is similar to the resize issue, seems to get stuck waiting a window message.

Is there an alternative control we should be using?

@andrekoehler Did you by chance find a solution?

andrekoehler commented 1 year ago

@aidan-g I followed @miloush's advice and only performed the memory copy on the UI thread using the WriteableBitmap.WritePixels method.

ghost commented 1 year ago

@andrekoehler I actually did some tests last night, moved all rendering code to the UI thread (Diapatcher.Invoked the whole thing) and I still get freezes. I need to do more testing, it might be specific to low core count or an older framework version as it's only happening on an old laptop now though without getting to the bottom of it I can't be sure it won't happen on modern machines too. Resizing a grid splitter also triggers the issue.

ghost commented 1 year ago

I changed my ui structure to be more like the example here, using an Image instead of Rectangle/ImageBrush:

   <Windows:Visualization>
-       <Rectangle>
-           <Rectangle.Resources>
-               <LocalWindows:SpectrogramRenderer x:Key="Renderer"
-                   Width="{Binding ActualWidth, Mode=OneWay, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type LocalWindows:Spectrogram}}}" 
-                   Height="{Binding ActualHeight, Mode=OneWay, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type LocalWindows:Spectrogram}}}" 
-                   Color="{Binding Foreground, Converter={StaticResource BrushConverter}, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type LocalWindows:Spectrogram}}}"></LocalWindows:SpectrogramRenderer>
-           </Rectangle.Resources>
-           <Rectangle.Fill>
-               <ImageBrush ImageSource="{Binding Bitmap, Source={StaticResource Renderer}}" Viewbox="{Binding Viewbox, Source={StaticResource Renderer}}" ViewboxUnits="Absolute"></ImageBrush>
-           </Rectangle.Fill>
-       </Rectangle>
+       <Border>
+           <Image>
+               <Image.Resources>
+                   <LocalWindows:SpectrogramRenderer x:Key="Renderer"
+                       Width="{Binding ActualWidth, Mode=OneWay, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type Border}}}" 
+                       Height="{Binding ActualHeight, Mode=OneWay, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type Border}}}" 
+                       Color="{Binding Foreground, Converter={StaticResource BrushConverter}, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type LocalWindows:Spectrogram}}}"></LocalWindows:SpectrogramRenderer>
+               </Image.Resources>
+               <Image.Source>
+                   <Binding Source="{StaticResource Renderer}" Path="Bitmap"></Binding>
+               </Image.Source>
+           </Image>
+       </Border>
    </Windows:Visualization>

This, combined with doing all rendering on the UI thread fixed the issue. Window and Grid resize no longer freezes. Additionally my TabControl issue was also fixed (I suspected it was the same cause).

Unfortunately the performance is really bad, from 0.5% CPU to basically maxing out a core.

lindexi commented 1 year ago

@aidan-g I worry about your problem for a different reason than this issues

macromaniac commented 1 year ago

I had a similar problem, was able to get past it by locking the bitmap only to grab data or to update the dirty rectangle.

This goes against the workflow written here: https://learn.microsoft.com/en-us/dotnet/api/system.windows.media.imaging.writeablebitmap?view=windowsdesktop-7.0 ; But it works without tearing (afaict), doesn't lock the UI thread, and cpu usage is at 2% for 4k 30 fps video.

               Application.Current.Dispatcher.Invoke(() =>
                {
                    bmp.Lock();
                    backBuffer = bmp.BackBuffer;
                    backBufferStride = bmp.BackBufferStride;
                    bmp.Unlock();
                });

                //Do stuff here using backBuffer and backBufferStride

                Application.Current.Dispatcher.Invoke(() =>
                {
                    bmp.Lock();
                    // Specify the area of the bitmap that changed.
                    bmp.AddDirtyRect(new Int32Rect(0, 0, width, height));
                    bmp.Unlock();
                });
macromaniac commented 1 year ago

Copying the data on the UI thread is not a good idea IMO. For 4k video the mem copy alone takes 15 milliseconds, so that means 45% of the time your UI thread would be locked. UI thread should be locked 0% of the time.

Not to mention, what is the point of a "Lock" feature that only works on one thread. Maybe for WPF threads? Idk, just seems weird to me.

andrekoehler commented 1 year ago

I had a similar problem, was able to get past it by locking the bitmap only to grab data or to update the dirty rectangle.

This goes against the workflow written here: https://learn.microsoft.com/en-us/dotnet/api/system.windows.media.imaging.writeablebitmap?view=windowsdesktop-7.0 ; But it works without tearing (afaict), doesn't lock the UI thread, and cpu usage is at 2% for 4k 30 fps video.

               Application.Current.Dispatcher.Invoke(() =>
                {
                    bmp.Lock();
                    backBuffer = bmp.BackBuffer;
                    backBufferStride = bmp.BackBufferStride;
                    bmp.Unlock();
                });

                //Do stuff here using backBuffer and backBufferStride

                Application.Current.Dispatcher.Invoke(() =>
                {
                    bmp.Lock();
                    // Specify the area of the bitmap that changed.
                    bmp.AddDirtyRect(new Int32Rect(0, 0, width, height));
                    bmp.Unlock();
                });

Its great that this worked for you, but I'm a bit concerned because this is explicitly forbidden here: "Update the back buffer only between calls to the Lock and Unlock methods. If you do not follow the Lock/Unlock workflow described in the WriteableBitmap class remarks, undefined behaviors, such as tearing, can occur." source

andrekoehler commented 1 year ago

Copying the data on the UI thread is not a good idea IMO. For 4k video the mem copy alone takes 15 milliseconds, so that means 45% of the time your UI thread would be locked. UI thread should be locked 0% of the time.

Not to mention, what is the point of a "Lock" feature that only works on one thread. Maybe for WPF threads? Idk, just seems weird to me.

I absolutely agree with you. The only improvement I found up to now is to make the copy (on the UI thread) as fast as possible by pinvoking the MFCopyImage function from the Media Foundation DLL: Doc IIRC this was faster than several other copy-methods I tried.