Merge pull request #65 from AvaloniaUI/fixes/gtk-offscreen-buffer-pixel-format

GTK: Offscreen pixel buffer fixes and several marshalling issues

Author: maxkatz6

src/Avalonia.Controls.WebView.Core/Gtk/GdkEvent.cs

@@ -94,7 +94,7 @@ internal struct GdkEventScroll
     public Double x_root, y_root;
     public Double delta_x;
     public Double delta_y;
-    public bool is_stop;//public guint is_stop : 1;
+    public UInt32 is_stop;
 }
 
 [StructLayout(LayoutKind.Sequential)]
@@ -110,7 +110,7 @@ internal struct GdkEventCrossing
     public Double x_root, y_root;
     public Int32 mode;
     public Int32 detail;
-    public bool focus;
+    public Int32 focus;
     public UInt32 state;
 }
 
@@ -154,5 +154,5 @@ internal unsafe struct GdkEventKey
     public Byte *_string;
     public UInt16 hardware_keycode;
     public Byte group;
-    public bool is_modifier;//public guint is_modifier : 1;
+    public UInt32 is_modifier;
 }

src/Avalonia.Controls.WebView.Core/Gtk/GtkInterop.cs

@@ -85,7 +85,7 @@ internal static partial IntPtr webkit_website_data_manager_new(
     [DllImport(LibWebKit)]
     internal static extern IntPtr webkit_web_view_get_uri(IntPtr webView);
 
-    [DllImport(LibGio)]
+    [DllImport(LibGLib)]
     internal static extern void g_free(IntPtr ptr);
 
     [DllImport(LibGio)]
@@ -95,7 +95,7 @@ internal static partial IntPtr webkit_website_data_manager_new(
     internal static extern IntPtr g_main_context_default();
 
     [LibraryImport(LibGLib)]
-    [return: MarshalAs(UnmanagedType.I1)]
+    [return: MarshalAs(UnmanagedType.Bool)]
     internal static partial bool g_main_context_is_owner(IntPtr context);
 
     [DllImport(LibWebKit)]
@@ -105,7 +105,7 @@ internal static partial IntPtr webkit_website_data_manager_new(
     internal static extern void webkit_user_content_manager_add_script(IntPtr manager, IntPtr userScript);
 
     [LibraryImport(LibWebKit, StringMarshalling = StringMarshalling.Utf8)]
-    [return: MarshalAs(UnmanagedType.I1)]
+    [return: MarshalAs(UnmanagedType.Bool)]
     internal static partial bool webkit_user_content_manager_register_script_message_handler(IntPtr manager, string messageHandler);
 
     [LibraryImport(LibWebKit, StringMarshalling = StringMarshalling.Utf8)]
@@ -174,10 +174,10 @@ internal static partial IntPtr webkit_website_data_manager_new(
     [DllImport(LibWebKit)]
     public static extern void webkit_print_operation_set_page_setup(IntPtr operation, IntPtr pageSetup);
 
-    [DllImport(LibWebKit)]
+    [DllImport(LibGtk)]
     public static extern IntPtr gtk_print_settings_new();
 
-    [LibraryImport(LibWebKit, StringMarshalling = StringMarshalling.Utf8)]
+    [LibraryImport(LibGtk, StringMarshalling = StringMarshalling.Utf8)]
     internal static partial void gtk_print_settings_set(IntPtr settings, string key, string value);
 
     [DllImport(LibGtk)]
@@ -218,8 +218,8 @@ internal static partial IntPtr webkit_website_data_manager_new(
     [DllImport(LibGtk)]
     internal static extern void gtk_widget_destroy(IntPtr widget);
 
-    [DllImport(LibGObject)]
-    internal static extern ulong g_error_free(GError* error);
+    [DllImport(LibGLib)]
+    internal static extern void g_error_free(GError* error);
 
     [DllImport(LibGObject)]
     internal static extern IntPtr g_object_ref(IntPtr handle);
@@ -230,8 +230,8 @@ internal static partial IntPtr webkit_website_data_manager_new(
     [DllImport (LibGObject)]
     internal static extern void g_object_unref(IntPtr handle);
 
-    [LibraryImport(LibGObject, StringMarshalling = StringMarshalling.Utf8)]
-    internal static partial void g_variant_get(IntPtr variant, string formatString, out string? result);
+    [LibraryImport(LibGLib, StringMarshalling = StringMarshalling.Utf8)]
+    internal static partial void g_variant_get(IntPtr variant, string formatString, out IntPtr result);
 
     [DllImport(LibGtk)]
     internal static extern IntPtr gtk_window_new(int type);
@@ -312,7 +312,7 @@ internal static partial IntPtr webkit_website_data_manager_new(
     internal static extern IntPtr gdk_keymap_get_for_display(IntPtr display);
 
     [LibraryImport (LibGdk)]
-    [return: MarshalAs(UnmanagedType.I1)]
+    [return: MarshalAs(UnmanagedType.Bool)]
     internal static partial bool gdk_keymap_translate_keyboard_state(IntPtr keymap, uint hardware_keycode, GdkModifierType state, int group, out uint keyval, out int effective_group, out int level, out int consumed_modifiers);
 
     [DllImport(LibGdk)]
@@ -351,10 +351,10 @@ internal static partial IntPtr webkit_website_data_manager_new(
     [DllImport(LibGdk)]
     public static extern IntPtr gdk_event_new(GdkEventType type);
 
-    [DllImport(LibGdk)]
+    [DllImport(LibGtk)]
     public static extern bool gtk_widget_event(IntPtr widget, IntPtr gdkEvent);
 
-    [DllImport(LibGdk)]
+    [DllImport(LibGtk)]
     public static extern void gtk_main_do_event(IntPtr gdkEvent);
 
     [DllImport(LibGdk)]
@@ -453,12 +453,13 @@ internal static partial IntPtr webkit_website_data_manager_new(
     public static extern void soup_message_headers_foreach(
         IntPtr headers,
         delegate* unmanaged[Cdecl]<IntPtr, IntPtr, IntPtr, void> func,
-        GCHandle user_data);
+        IntPtr user_data);
 
+    // Don't return string for SOUP headers, we don't want to auto-release them (as it's done by LibraryImport)
     [LibraryImport(LibSoup, StringMarshalling = StringMarshalling.Utf8)]
-    public static partial string? soup_message_headers_get_list(IntPtr headers, string name);
+    public static partial IntPtr soup_message_headers_get_list(IntPtr headers, string name);
     [LibraryImport(LibSoup, StringMarshalling = StringMarshalling.Utf8)]
-    public static partial string? soup_message_headers_get_one(IntPtr headers, string name);
+    public static partial IntPtr soup_message_headers_get_one(IntPtr headers, string name);
     [LibraryImport(LibSoup, StringMarshalling = StringMarshalling.Utf8)]
     public static partial void soup_message_headers_replace(IntPtr headers, string name, string value);
     [LibraryImport(LibSoup, StringMarshalling = StringMarshalling.Utf8)]
@@ -555,9 +556,11 @@ public struct GdkRectangle
         public int width, height;
     }
 
+    [Flags]
     public enum GConnectFlags : int
     {
-        AFTER,
-        SWAPPED
+        NONE = 0,
+        AFTER = 1 << 0,
+        SWAPPED = 1 << 1
     }
 }

src/Avalonia.Controls.WebView.Core/Gtk/GtkOffscreenAvaloniaWebViewAdapter.cs

@@ -31,15 +31,19 @@ private GtkOffscreenAvaloniaWebViewAdapter(GtkWebViewEnvironmentRequestedEventAr
 
     public Control? Parent { get; private set; }
 
-    public static async Task<WebViewAdapter.OffscreenWebViewAdapterBuilder> CreateBuilder(
+    public static Task<WebViewAdapter.OffscreenWebViewAdapterBuilder> CreateBuilder(
         GtkWebViewEnvironmentRequestedEventArgs environmentArgs)
     {
-        var adapter = await RunOnGlibThreadAsync(() => new GtkOffscreenAvaloniaWebViewAdapter(environmentArgs));
-        return (parent) =>
+        // A fresh adapter per attachment: the host disposes the previous one when the control is
+        // detached, so handing out a cached instance leaves a dead web view after a re-attach.
+        WebViewAdapter.OffscreenWebViewAdapterBuilder builder = async parent =>
         {
+            var adapter = await RunOnGlibThreadAsync(() => new GtkOffscreenAvaloniaWebViewAdapter(environmentArgs));
             adapter.Parent = parent;
-            return Task.FromResult<IWebViewAdapterWithOffscreenBuffer>(adapter);
+            return adapter;
         };
+
+        return Task.FromResult(builder);
     }
 
     protected override void DisposeSafe(bool disposing)

src/Avalonia.Controls.WebView.Core/Gtk/GtkOffscreenWebViewAdapter.cs

@@ -38,7 +38,10 @@ protected GtkOffscreenWebViewAdapter(GtkWebViewEnvironmentRequestedEventArgs arg
     }
 
     public event Action? DrawRequested;
-    
+
+    public PixelFormat BufferPixelFormat => PixelFormats.Rgba8888;
+    public AlphaFormat BufferAlphaFormat => AlphaFormat.Unpremul;
+
     public Task UpdateWriteableBitmap(PixelSize _, FrameChainBase<WriteableBitmap, PixelSize>.IProducer producer)
     {
         if (_windowHandle == IntPtr.Zero)
@@ -65,11 +68,6 @@ public Task UpdateWriteableBitmap(PixelSize _, FrameChainBase<WriteableBitmap, P
                 int wHeight = gtk_widget_get_allocated_height(_windowHandle);
 
                 pixbuf = gdk_pixbuf_get_from_window(gdkWindow, 0, 0, wWidth, wHeight);
-                if (pixbuf != IntPtr.Zero && gdk_pixbuf_get_n_channels(pixbuf) == 3)
-                {
-                    var pixbufRgba = gdk_pixbuf_add_alpha(pixbuf, false, 0, 0, 0);
-                    pixbuf = pixbufRgba;
-                }
             }
 
             if (pixbuf == IntPtr.Zero)
@@ -79,46 +77,73 @@ public Task UpdateWriteableBitmap(PixelSize _, FrameChainBase<WriteableBitmap, P
 
             try
             {
+                // Windows without an alpha channel produce a three channel pixbuf.
+                if (gdk_pixbuf_get_n_channels(pixbuf) == 3)
+                {
+                    var pixbufRgba = gdk_pixbuf_add_alpha(pixbuf, false, 0, 0, 0);
+                    g_object_unref(pixbuf);
+                    pixbuf = pixbufRgba;
+
+                    if (pixbuf == IntPtr.Zero)
+                    {
+                        return;
+                    }
+                }
+
                 var width = gdk_pixbuf_get_width(pixbuf);
                 var height = gdk_pixbuf_get_height(pixbuf);
                 var stride = gdk_pixbuf_get_rowstride(pixbuf);
                 var channels = gdk_pixbuf_get_n_channels(pixbuf);
                 var pixelsPtr = gdk_pixbuf_get_pixels(pixbuf);
 
+                if (width <= 0 || height <= 0 || channels != 4 || pixelsPtr == IntPtr.Zero)
+                {
+                    return;
+                }
+
                 var size = new PixelSize(width, height);
 
-                if (channels == 4)
+                using (producer.GetNextFrame(size, out var frame))
                 {
-                    using (producer.GetNextFrame(size, out var frame))
+                    using var buf = frame.Lock();
+                    var dstStride = buf.RowBytes;
+
+                    if (stride == dstStride)
                     {
-                        using var buf = frame.Lock();
-                        var bytesPerRow = Math.Min(stride, buf.RowBytes);
-                        var totalBytes = bytesPerRow * height;
-
-                        Buffer.MemoryCopy(
-                            source: (void*)pixelsPtr,
-                            destination: (void*)buf.Address,
-                            destinationSizeInBytes: buf.RowBytes * height,
-                            sourceBytesToCopy: totalBytes
-                        );
+                        Buffer.MemoryCopy((void*)pixelsPtr, (void*)buf.Address,
+                            (long)height * dstStride, (long)height * stride);
+                    }
+                    else
+                    {
+                        var copyBytes = Math.Min(stride, dstStride);
+                        for (var y = 0; y < height; y++)
+                        {
+                            Buffer.MemoryCopy(
+                                (byte*)pixelsPtr + (long)y * stride,
+                                (byte*)buf.Address + (long)y * dstStride,
+                                dstStride, copyBytes);
+                        }
                     }
                 }
             }
             finally
             {
-                g_object_unref(pixbuf);
+                if (pixbuf != IntPtr.Zero)
+                {
+                    g_object_unref(pixbuf);
+                }
             }
         });
     }
 
     public override void SizeChanged(PixelSize containerSize)
     {
-        if (_windowHandle == IntPtr.Zero)
-            return;
-
         _sizeRequest = containerSize;
         RunOnGlibThreadAsync(() =>
         {
+            if (_windowHandle == IntPtr.Zero)
+                return;
+
             if (_experimentalOffscreen)
                 gtk_window_set_default_size(_windowHandle, _sizeRequest.Width, _sizeRequest.Height);
             else
@@ -300,8 +325,10 @@ protected override void DisposeSafe(bool disposing)
 
         if (window != IntPtr.Zero)
         {
-            g_object_unref(window);
+            // Destroy before releasing our reference: unreffing first can drop the last reference,
+            // and gtk_widget_destroy would then run against freed memory.
             gtk_widget_destroy(window);
+            g_object_unref(window);
         }
     }