Go Back   Cockos Incorporated Forums > REAPER Forums > REAPER Bug Reports

Reply
 
Thread Tools Display Modes
Old 11-05-2019, 05:01 AM   #1
azslow3
Human being with feelings
 
Join Date: Nov 2017
Location: Heidelberg, Germany
Posts: 797
Default Linux/Swell: XBridgeWindow (used in REAPER FX window) [solved]

It is purely Linux SWELL/GDK related, but since it is affecting REAPER I write here.

It all has started with GDK warning: https://forums.cockos.com/showthread...90#post2198890

I have compiled SWELL from GitHub and tested with reaper v5.984 on Ubuntu 18.04 x64. SWELL is compiled/working with gdk-3.22.30

It seems like mentioned GDK warning in fact uncover incorrect use of XWindows embedding in SWELL.

I attach the patch I was using to produce the following. Changing lines in ~bridgeState. The patch as is does not change anything, related changed are commented. In REAPER, I just open FX window with one VST3 loaded (VST3 is an example from SDK) and then close it.

1. current SWELL code:
Code:
Created 0x25997e0, XWindow 79692736 exist
Bridge dtor 0x25997e0, XWindow 79692736 exist
Gdk-WARNING **: 11:58:50.977: gdk_window_set_user_time called on non-toplevel
So, GdkWindow never finalize, not even when FX window is closed. It seems like the same for XWindow,
but I am not sure (Gdk tries to get it out of dialog hierarchy, so it has good chance to survive
even after parent dialog XWindow is destroyed).
Note that gdk_window_destroy does not change the reference counter in this case (that is not in patch,
but I have checked that).

2. after adding unref
Code:
Created 0x17927e0, XWindow 79692470 exist
Bridge dtor 0x17927e0, XWindow 79692470 exist
Gdk-WARNING **: 12:10:48.495: gdk_window_set_user_time called on non-toplevel
Finalized 0x17927e0, XWindow 79692470 exist
So, the same as before, but GdkWindow is finalized. No future "object does not exist" warnings/errors.

3. after adding XDestroyWindow
Code:
Created 0x22397e0, XWindow 81789541 exist
Bridge dtor 0x22397e0, XWindow 81789541 exist
Gdk-WARNING **: 12:20:50.950: gdk_window_set_user_time called on non-toplevel
Finalized 0x22397e0, XWindow 81789541 not there
GLib-GObject-CRITICAL **: 12:20:50.955: g_object_unref: assertion 'G_IS_OBJECT (object)' failed
So, this time the X Window is destroyed and GdkWindow is finalized, but somewhere still referencing something (I have not checked what and where)

4. after commenting gdk_window_destroy, keeping unref and XDestroyWindow.
Code:
Created 0x2ea77e0, XWindow 81789536 exist
Bridge dtor 0x2ea77e0, XWindow 81789536 exist
Finalized 0x2ea77e0, XWindow 81789536 not there
So, looks like how it should work. GdkWindow is destroyed. XWindow is destroyed. No warning. No error.

---------------------
I do not pretend (4) is proper way to go. But current SWELL code is not correct.

Somehow related discussion:
https://mail.gnome.org/archives/gtk-.../msg00007.html
In GTK itself, the functionality is used in GtkPlug and DnD.

---------------------

In short (IMHO, I am not sure). The topic is almost undocumented.

gdk_x11_window_foreign_new_for_display is for making outside world X Windows known to GDK.

Original GDK Warning comes from that. GDK "think" X Window is foreign, so it (force) sends WM_DELETE_WINDOW in hope the owner is notified we no longer need it. But (GDK bug?), GDK still has the reference to this X Window as GdkWindow. So it receives the message (it has sent previously), detect there is GdkWindow and think the message is for it. Such message is not expected (WM for child window), so GDK prints warning.

Reference counter is not the same as for own GDK windows.

X Window itself is not managed by GDK, f.e. not destroyed.

Many functions are working completely different way when called for foreign windows, f.e. gdk_window_destroy does not try to destroy Gdk nor X window. If GDK think the window is a child, It "unparent" Gdk Window and send WM hint to X Window. It it think the window is not a child, it does nothing....

----------------------

I want to mention that again, all my investigations was for GTK 3.22.30. F.e. current GTK3 master has significant changes in related code.
Attached Files
File Type: txt bridge_test_patch.txt (2.8 KB, 100 views)

Last edited by azslow3; 11-13-2019 at 02:02 PM.
azslow3 is offline   Reply With Quote
Old 11-07-2019, 09:08 PM   #2
Justin
Administrator
 
Justin's Avatar
 
Join Date: Jan 2005
Location: NYC
Posts: 15,721
Default

Thanks! So this is the suggested change you are suggesting?

Code:
+static void bridgeFinalizeNotify(gpointer data, GObject *where_the_object_was)
+{
+}
+
+
 static WDL_PtrList<bridgeState> filter_windows;
 bridgeState::~bridgeState() 
 { 
   filter_windows.DeletePtr(this); 
+  if (w) g_object_weak_ref(G_OBJECT(w), bridgeFinalizeNotify, (gpointer)native_w); 
   if (w) gdk_window_destroy(w);
+  if (w) g_object_unref(G_OBJECT(w));
+  if (w) XDestroyWindow(native_disp, native_w);
 }
 bridgeState::bridgeState(bool needrep, GdkWindow *_w, Window _nw, Display *_disp)
 {
(I will test, but that looks pretty reasonable to me!)
Justin is offline   Reply With Quote
Old 11-08-2019, 12:46 AM   #3
azslow3
Human being with feelings
 
Join Date: Nov 2017
Location: Heidelberg, Germany
Posts: 797
Default

In my case gdk_window_destroy is what produce GDK warning and then assertion (test 3). So I simply commented it (test 4).

So (without debugging hook):
Code:
 static WDL_PtrList<bridgeState> filter_windows;
 bridgeState::~bridgeState() 
 { 
   filter_windows.DeletePtr(this); 
-  if (w) gdk_window_destroy(w);
+  if (w) g_object_unref(G_OBJECT(w));
+  if (w) XDestroyWindow(native_disp, native_w);
 }
 bridgeState::bridgeState(bool needrep, GdkWindow *_w, Window _nw, Display *_disp)
g_object_unref(): GDK window creation from X11 window can return existing GDK window, with increased counter, in case it already exist. So according to the documentation unref is right way to inform GDK we do not need it.

XDestroyWindow(): GDK does not destroy external windows, so it should be destroyed manually.

gdk_window_destroy(): should be used when X Window is a top window of some external application, so not for this case.
That is a bit misleading, it does not do what it does for normal window.
azslow3 is offline   Reply With Quote
Old 11-08-2019, 08:23 AM   #4
Justin
Administrator
 
Justin's Avatar
 
Join Date: Jan 2005
Location: NYC
Posts: 15,721
Default

Thanks! This will be in the next reaper build, and is in the public WDL repository now.
Justin is offline   Reply With Quote
Old 11-13-2019, 02:05 PM   #5
azslow3
Human being with feelings
 
Join Date: Nov 2017
Location: Heidelberg, Germany
Posts: 797
Default

Thanks!

One puzzle less in my iPlug2 Linux porting journey
azslow3 is offline   Reply With Quote
Reply

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump


All times are GMT -7. The time now is 05:16 AM.


Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2024, vBulletin Solutions Inc.