FreeSync doesn't work with awesomewm (unless --no-argb)

[clapbr]

Output of awesome --version: awesome v4.2-321-gf4301592 (Human after all) • Compiled against Lua 5.3.5 (running with Lua 5.3) • D-Bus support: ✔ • execinfo support: ✔ • xcb-randr version: 1.6 • LGI version: 0.9.2

VRR/FreeSync works fine in other WM's under same conditions - no compositor, fullscreen window covers the screen.

Getting a clue from #2444, running awesome with --no-argb also worked around it for me and FreeSync enables properly then, so maybe merge these two issues.

[psychon]

so maybe merge these two issues.

Uhm... okay. So I should close this as a duplicate of #2444?

[clapbr]

so maybe merge these two issues.

Uhm... okay. So I should close this as a duplicate of #2444?

Sure. Just a question: is there a way to script --no-argb at runtime so I only lose transparencies while I need Freesync?

[psychon]

Not currently, no. You cannot change --no-argb at runtime. It would be possible to implement/add, but it needs AwesomeWM to destroy and re-create all its windows (with a different bit depth).

[clapbr]

Not currently, no. You cannot change --no-argb at runtime. It would be possible to implement/add, but it needs AwesomeWM to destroy and re-create all its windows (with a different bit depth).

Any way I could hack around this ? Something like echo 'awesome.exec("awesome -r --no-argb")' | awesome-client just kills my X session, and awesome.restart() takes no parameters so not an option.

Also, is this a bug in awesome or should I report it to xf86-video-amdgpu (which handles the Freesync X11 logic) ?

[Elv13]

Also, is this a bug in awesome or should I report it to xf86-video-amdgpu (which handles the Freesync X11 logic) ?

Everything that can be solved with --no-argb is a driver bug. This flag exists to detect and temporarily mitigate those bugs until the driver get fixed. As you know, since AwesomeWM is a framework and let the user manipulate the surfaces directly, there is no way to know ahead of time if the alpha channel will be used or not. It is also not possible to know if a compositing manager will be added (or removed). This means the "always on" approach is the only sane way to handle this. Unfortunately, because "always on" isn't a very common default for X11 window managers, it keeps hitting bugs in the various video drivers. Usually when they are reported, they are solved. In most case, it happens because nobody tried this configuration before pushing code in production.

Adding arguments to awesome.restart() could be done. I am think neither me or @psychon have much time currently to work on this kind of stuff. Otherwise, patches for this are welcome and should be rather trivial. execvp already has the argument logic. It only needs to have upgradable arguments. This means reading a lua table (list) provided to restart(), adding "awesome_argv[0]" in front of an array then copy the strings and swap the argument pointer passed to execvp.

[clapbr]

Thanks a lot! I will report this to xf86-video-amdgpu then. I've settled this temporarily with some ugly wrappers that handle the switch context on .xinitrc - probably a stupid idea but it will do until I learn more awesome and Lua. I've just started reading through the docs after a few days messing with other people's code so I've got to be a bit more familiar with the codebase before messing with PR's myself.

I'll close this as there isn´t much to be done here for now.

[psychon]

Just some details on what --no-argb actually does: It changes "bit depth 24" to "bit depth 32" in some requests that AwesomeWM sends to the X11 server. Cairo is also told about this difference. This is (basically(*)) it. Thus, my claim always is: Any bug that is fixed with --no-argb is a bug elsewhere (**).

In this specific case, I guess that the problem with FreeSync is basically: Normally in X11, windows do not get their own memory for window content. Instead, everything is just drawn to the screen directly. However, when windows of different bit depth are involved, the X11 server allocates some dedicated storage and copies window contents around where needed. (This is basically always the problem that --no-argb mitigates; most rendering bugs are an instance of "the copying contents around has a bug", I think.) I do not know how FreeSync works in the X11 server, but my random guess would be that it is just disabled when some "copying around" needs to be done.

(*): We also need to allocate a dummy colormap if we are not using the default visual and... stuff. But really, nothing interesting happens when you specify --no-argb. (**): Well, okay. AwesomeWM also has to pick a visual with bit depth 32. Supposedly, this should not make much of a difference either.

[clapbr]

@psychon

Contacted the driver developer in IRC:

[04:36] <MrCooper> diwr: it's not a driver bug but an issue between awesome and the X11/Xorg design: Windows with a different depth than the root window's are automatically redirected and composited, so page flipping cannot be used, so FreeSync cannot be enabled
[04:51] <MrCooper> diwr: the easiest solution would probably be for awesome not to reparent fullscreen windows to its own ones with depth 32

Is it feasible to do what he suggests?

[psychon]

Everything that can be solved with --no-argb is a driver bug.

Just so that we are a little less part of the problem: The above should read "Everything that can be solved with --no-argb is not a bug in AwesomeWM. (MrCooper pointed out that in this specific case it's not actually a driver bug, but rather something in the X11 server itself.)

@clapbr Well... yes. However, it makes some things slightly complicated and becomes really complicated with titlebars and RandR hotplug. Basically, we can do this whenever no part of the titlebar is visible (which can change during RandR hotplug).

Oh and this will flicker on screen, I guess. And bring back the old problems with Java assuming a reparenting WM (actually, I guess lots of code out there assumes that a WM is either reparenting or not, but not "it depends"...?).

Here is a patch which takes the lazy approach of just un-reparenting fullscreen windows. I tested this for like 5 seconds and it did not immediately explode.

diff --git a/awesomerc.lua b/awesomerc.lua
index 5d2bd2c10..8a43bcc0c 100644
--- a/awesomerc.lua
+++ b/awesomerc.lua
@@ -586,3 +586,11 @@ end)
 client.connect_signal("focus", function(c) c.border_color = beautiful.border_focus end)
 client.connect_signal("unfocus", function(c) c.border_color = beautiful.border_normal end)
 -- }}}
+
+gears.timer.start_new(2, function()
+    for _, c in pairs(client.get()) do
+        print(c, c.fullscreen, not c.fullscreen)
+        c.fullscreen = not c.fullscreen
+    end
+    return true
+end)
diff --git a/objects/client.c b/objects/client.c
index f38e1626b..03aa8b1ca 100644
--- a/objects/client.c
+++ b/objects/client.c
@@ -1358,6 +1358,7 @@ client_geometry_refresh(void)
     foreach(_c, globalconf.clients)
     {
         client_t *c = *_c;
+        bool should_be_reparented = !c->fullscreen;

         /* Compute the client window's and frame window's geometry */
         area_t geometry = c->geometry;
@@ -1396,6 +1397,54 @@ client_geometry_refresh(void)
             real_geometry.y = 0;
         }

+        if ((c->frame_window != XCB_NONE) != should_be_reparented) {
+            uint32_t no_event[] = { 0 };
+            xcb_grab_server(globalconf.connection);
+
+            xcb_change_window_attributes(globalconf.connection,
+                                         globalconf.screen->root,
+                                         XCB_CW_EVENT_MASK,
+                                         no_event);
+            xcb_change_window_attributes(globalconf.connection,
+                                         c->window,
+                                         XCB_CW_EVENT_MASK,
+                                         no_event);
+            if (should_be_reparented) {
+                assert(c->frame_window == XCB_NONE);
+
+                c->frame_window = xcb_generate_id(globalconf.connection);
+                xcb_create_window(globalconf.connection, globalconf.default_depth, c->frame_window, globalconf.screen->root,
+                                  real_geometry.x, real_geometry.y, real_geometry.width, real_geometry.height,
+                                  c->border_width, XCB_COPY_FROM_PARENT, globalconf.visual->visual_id,
+                                  XCB_CW_BORDER_PIXEL | XCB_CW_BIT_GRAVITY | XCB_CW_WIN_GRAVITY
+                                  | XCB_CW_OVERRIDE_REDIRECT | XCB_CW_EVENT_MASK | XCB_CW_COLORMAP,
+                                  (const uint32_t [])
+                                  {
+                                      globalconf.screen->black_pixel,
+                                      XCB_GRAVITY_NORTH_WEST,
+                                      XCB_GRAVITY_NORTH_WEST,
+                                      1,
+                                      FRAME_SELECT_INPUT_EVENT_MASK,
+                                      globalconf.default_cmap
+                                  });
+                xcb_reparent_window(globalconf.connection, c->window, c->frame_window, real_geometry.x, real_geometry.y);
+                xcb_map_window(globalconf.connection, c->frame_window);
+                stack_windows();
+            } else {
+                assert(c->frame_window != XCB_NONE);
+
+                xcb_reparent_window(globalconf.connection, c->window, globalconf.screen->root, geometry.x, geometry.y);
+                xcb_destroy_window(globalconf.connection, c->frame_window);
+                c->frame_window = XCB_NONE;
+            }
+            xcb_change_window_attributes(globalconf.connection,
+                                         globalconf.screen->root,
+                                         XCB_CW_EVENT_MASK,
+                                         ROOT_WINDOW_EVENT_MASK);
+            xcb_change_window_attributes(globalconf.connection, c->window, XCB_CW_EVENT_MASK, (uint32_t[]) { CLIENT_SELECT_INPUT_EVENT_MASK });
+            xutil_ungrab_server(globalconf.connection);
+        }
+
         /* Is there anything to do? */
         if (AREA_EQUAL(geometry, c->x11_frame_geometry)
                 && AREA_EQUAL(real_geometry, c->x11_client_geometry)) {
@@ -1412,12 +1461,18 @@ client_geometry_refresh(void)
             ignored_enterleave = true;
         }

-        xcb_configure_window(globalconf.connection, c->frame_window,
-                XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
-                (uint32_t[]) { geometry.x, geometry.y, geometry.width, geometry.height });
-        xcb_configure_window(globalconf.connection, c->window,
-                XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
-                (uint32_t[]) { real_geometry.x, real_geometry.y, real_geometry.width, real_geometry.height });
+        if (c->frame_window != XCB_NONE) {
+            xcb_configure_window(globalconf.connection, c->frame_window,
+                    XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
+                    (uint32_t[]) { geometry.x, geometry.y, geometry.width, geometry.height });
+            xcb_configure_window(globalconf.connection, c->window,
+                    XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
+                    (uint32_t[]) { real_geometry.x, real_geometry.y, real_geometry.width, real_geometry.height });
+        } else {
+            xcb_configure_window(globalconf.connection, c->window,
+                    XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
+                    (uint32_t[]) { real_geometry.x, real_geometry.y, real_geometry.width, real_geometry.height });
+        }

         c->x11_frame_geometry = geometry;
         c->x11_client_geometry = real_geometry;
@@ -2839,6 +2894,8 @@ client_refresh_titlebar_partial(client_t *c, client_titlebar_t bar, int16_t x, i
             || c->titlebar[bar].drawable->pixmap == XCB_NONE
             || !c->titlebar[bar].drawable->refreshed)
         return;
+    if(c->frame_window == XCB_NONE)
+        return;

     /* Is the titlebar part of the area that should get redrawn? */
     area_t area = titlebar_get_area(c, bar);

[psychon]

It just occur to me that the above patch brings back an old problem. With this patch, fullscreen clients that are not on any selected tag and that do not follow ICCCM correctly cannot close themselves anymore. Actually... switching to another tag will not hide the client anyway, I think, because it will just try to hide the frame window, but there is none. (And for the original issue I outlined above: When a window is unmapped already, clients have to send a synthetic unmap event to the root window. Some will just try to unmap their window, which the X11 server will just ignore, because the window is already unmapped.)

[psychon]

Here is a follow-up patch that fixes input handling: If the window had the input focus before, it will get the focus again afterwards:

diff --git a/objects/client.c b/objects/client.c
index 03aa8b1ca..7cda8d33b 100644
--- a/objects/client.c
+++ b/objects/client.c
@@ -1437,6 +1437,17 @@ client_geometry_refresh(void)
                 xcb_destroy_window(globalconf.connection, c->frame_window);
                 c->frame_window = XCB_NONE;
             }
+            if (globalconf.focus.client == c) {
+                xcb_window_t win = globalconf.focus.window_no_focus;
+                if (client_on_selected_tags(c)) {
+                    if(!c->nofocus)
+                        win = c->window;
+                    else
+                        win = client_get_nofocus_window(c);
+                }
+                xcb_set_input_focus(globalconf.connection, XCB_INPUT_FOCUS_PARENT,
+                                    win, globalconf.timestamp);
+            }
             xcb_change_window_attributes(globalconf.connection,
                                          globalconf.screen->root,
                                          XCB_CW_EVENT_MASK,

[clapbr]

Here is a follow-up patch that fixes input handling: If the window had the input focus before, it will get the focus again afterwards:

diff --git a/objects/client.c b/objects/client.c
index 03aa8b1ca..7cda8d33b 100644
--- a/objects/client.c
+++ b/objects/client.c
@@ -1437,6 +1437,17 @@ client_geometry_refresh(void)
                 xcb_destroy_window(globalconf.connection, c->frame_window);
                 c->frame_window = XCB_NONE;
             }
+            if (globalconf.focus.client == c) {
+                xcb_window_t win = globalconf.focus.window_no_focus;
+                if (client_on_selected_tags(c)) {
+                    if(!c->nofocus)
+                        win = c->window;
+                    else
+                        win = client_get_nofocus_window(c);
+                }
+                xcb_set_input_focus(globalconf.connection, XCB_INPUT_FOCUS_PARENT,
+                                    win, globalconf.timestamp);
+            }
             xcb_change_window_attributes(globalconf.connection,
                                          globalconf.screen->root,
                                          XCB_CW_EVENT_MASK,

Following up IRC discussion, the only quirk I found with these patches is having to unfullscreen before I can switch workspaces (very easy to deal with via scripting so not a problem for me).

@yshui also might be interested in this as it makes compton unredirection work.

Thank you very much again for the quick take at this.

[psychon]

the only quirk I found with these patches is having to unfullscreen before I can switch workspaces (very easy to deal with via scripting so not a problem for me).

Meaning that clientkeys still work, but globalkeys do not work in this situation? After a quick look at the code: No idea what is going on here. I'm too lazy to test right now (sorry, heading to bed), but since you talk about FreeSync, I suppose this is about some game. Could it be that the game just grabs all input and so the key presses do not "bubble up" to the root window? Put differently: Does this (also not) work with a fullscreened xterm?

[clapbr]

the only quirk I found with these patches is having to unfullscreen before I can switch workspaces (very easy to deal with via scripting so not a problem for me).

Meaning that clientkeys still work, but globalkeys do not work in this situation? After a quick look at the code: No idea what is going on here. I'm too lazy to test right now (sorry, heading to bed), but since you talk about FreeSync, I suppose this is about some game. Could it be that the game just grabs all input and so the key presses do not "bubble up" to the root window? Put differently: Does this (also not) work with a fullscreened xterm?

What ACTUALLY happens is it switches workspaces but they stay under the fullscreened window so just something that can be dealt via config easily so not a real problem. I just mentioned it cause afaik this is not default behavior, so a harmless sympton of the patch - quick demo https://www.youtube.com/watch?v=4_S1AnoLCp4

edit: Read everything you wrote again, brainfart let this slip

Actually... switching to another tag will not hide the client anyway, I think, because it will just try to hide the frame window, but there is none.

[clapbr]

Another remark to the previous patches

diff --git a/awesomerc.lua b/awesomerc.lua
index 5d2bd2c10..8a43bcc0c 100644
--- a/awesomerc.lua
+++ b/awesomerc.lua
@@ -586,3 +586,11 @@ end)
 client.connect_signal("focus", function(c) c.border_color = beautiful.border_focus end)
 client.connect_signal("unfocus", function(c) c.border_color = beautiful.border_normal end)
 -- }}}
+
+gears.timer.start_new(2, function()
+    for _, c in pairs(client.get()) do
+        print(c, c.fullscreen, not c.fullscreen)
+        c.fullscreen = not c.fullscreen
+    end
+    return true
+end)

Not sure what this was supposed to do but this just keeps fullscreening and unfullscreening a window every two seconds, had to remove this part of patch otherwise when I mess my config and it falls back to default desktop became unusable.

[psychon]

Not sure what this was supposed

That's just the code I used for testing this. Sorry that I kept it in the diff.

[dragonnn]

@psychon I tested that path, it works fine when using one monitor, but when having 3 and runnig the fullscreen app on the middle screen the window got rended in the middle betwen monitor 1 and 2.. But changing the line 88 from real_geometry.x, real_geometry.y to geometry.x, geometry.y, fixed that issue. Here is my current patch:

diff --git a/objects/client.c b/objects/client.c
index f38e1626b..03aa8b1ca 100644
--- a/objects/client.c
+++ b/objects/client.c
@@ -1358,6 +1358,7 @@ client_geometry_refresh(void)
     foreach(_c, globalconf.clients)
     {
         client_t *c = *_c;
+        bool should_be_reparented = !c->fullscreen;

         /* Compute the client window's and frame window's geometry */
         area_t geometry = c->geometry;
@@ -1396,6 +1397,54 @@ client_geometry_refresh(void)
             real_geometry.y = 0;
         }

+        if ((c->frame_window != XCB_NONE) != should_be_reparented) {
+            uint32_t no_event[] = { 0 };
+            xcb_grab_server(globalconf.connection);
+
+            xcb_change_window_attributes(globalconf.connection,
+                                         globalconf.screen->root,
+                                         XCB_CW_EVENT_MASK,
+                                         no_event);
+            xcb_change_window_attributes(globalconf.connection,
+                                         c->window,
+                                         XCB_CW_EVENT_MASK,
+                                         no_event);
+            if (should_be_reparented) {
+                assert(c->frame_window == XCB_NONE);
+
+                c->frame_window = xcb_generate_id(globalconf.connection);
+                xcb_create_window(globalconf.connection, globalconf.default_depth, c->frame_window, globalconf.screen->root,
+                                  real_geometry.x, real_geometry.y, real_geometry.width, real_geometry.height,
+                                  c->border_width, XCB_COPY_FROM_PARENT, globalconf.visual->visual_id,
+                                  XCB_CW_BORDER_PIXEL | XCB_CW_BIT_GRAVITY | XCB_CW_WIN_GRAVITY
+                                  | XCB_CW_OVERRIDE_REDIRECT | XCB_CW_EVENT_MASK | XCB_CW_COLORMAP,
+                                  (const uint32_t [])
+                                  {
+                                      globalconf.screen->black_pixel,
+                                      XCB_GRAVITY_NORTH_WEST,
+                                      XCB_GRAVITY_NORTH_WEST,
+                                      1,
+                                      FRAME_SELECT_INPUT_EVENT_MASK,
+                                      globalconf.default_cmap
+                                  });
+                xcb_reparent_window(globalconf.connection, c->window, c->frame_window, real_geometry.x, real_geometry.y);
+                xcb_map_window(globalconf.connection, c->frame_window);
+                stack_windows();
+            } else {
+                assert(c->frame_window != XCB_NONE);
+
+                xcb_reparent_window(globalconf.connection, c->window, globalconf.screen->root, geometry.x, geometry.y);
+                xcb_destroy_window(globalconf.connection, c->frame_window);
+                c->frame_window = XCB_NONE;
+            }
+            xcb_change_window_attributes(globalconf.connection,
+                                         globalconf.screen->root,
+                                         XCB_CW_EVENT_MASK,
+                                         ROOT_WINDOW_EVENT_MASK);
+            xcb_change_window_attributes(globalconf.connection, c->window, XCB_CW_EVENT_MASK, (uint32_t[]) { CLIENT_SELECT_INPUT_EVENT_MASK });
+            xutil_ungrab_server(globalconf.connection);
+        }
+
         /* Is there anything to do? */
         if (AREA_EQUAL(geometry, c->x11_frame_geometry)
                 && AREA_EQUAL(real_geometry, c->x11_client_geometry)) {
@@ -1412,12 +1461,18 @@ client_geometry_refresh(void)
             ignored_enterleave = true;
         }

-        xcb_configure_window(globalconf.connection, c->frame_window,
-                XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
-                (uint32_t[]) { geometry.x, geometry.y, geometry.width, geometry.height });
-        xcb_configure_window(globalconf.connection, c->window,
-                XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
-                (uint32_t[]) { real_geometry.x, real_geometry.y, real_geometry.width, real_geometry.height });
+        if (c->frame_window != XCB_NONE) {
+            xcb_configure_window(globalconf.connection, c->frame_window,
+                    XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
+                    (uint32_t[]) { geometry.x, geometry.y, geometry.width, geometry.height });
+            xcb_configure_window(globalconf.connection, c->window,
+                    XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
+                    (uint32_t[]) { real_geometry.x, real_geometry.y, real_geometry.width, real_geometry.height });
+        } else {
+            xcb_configure_window(globalconf.connection, c->window,
+                    XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
+                    (uint32_t[]) { geometry.x, geometry.y, real_geometry.width, real_geometry.height });
+        }

         c->x11_frame_geometry = geometry;
         c->x11_client_geometry = real_geometry;
@@ -2839,6 +2894,8 @@ client_refresh_titlebar_partial(client_t *c, client_titlebar_t bar, int16_t x, i
             || c->titlebar[bar].drawable->pixmap == XCB_NONE
             || !c->titlebar[bar].drawable->refreshed)
         return;
+    if(c->frame_window == XCB_NONE)
+        return;

     /* Is the titlebar part of the area that should get redrawn? */
     area_t area = titlebar_get_area(c, bar);

diff --git a/objects/client.c b/objects/client.c
index 03aa8b1ca..7cda8d33b 100644
--- a/objects/client.c
+++ b/objects/client.c
@@ -1437,6 +1437,17 @@ client_geometry_refresh(void)
                 xcb_destroy_window(globalconf.connection, c->frame_window);
                 c->frame_window = XCB_NONE;
             }
+            if (globalconf.focus.client == c) {
+                xcb_window_t win = globalconf.focus.window_no_focus;
+                if (client_on_selected_tags(c)) {
+                    if(!c->nofocus)
+                        win = c->window;
+                    else
+                        win = client_get_nofocus_window(c);
+                }
+                xcb_set_input_focus(globalconf.connection, XCB_INPUT_FOCUS_PARENT,
+                                    win, globalconf.timestamp);
+            }
             xcb_change_window_attributes(globalconf.connection,
                                          globalconf.screen->root,
                                          XCB_CW_EVENT_MASK,

Anyway of course Xorg doesn't support FreeSync on a multimonitor setup at all... but at least I have unreder dir.

[actionless]

shouldn't it be reopened then?

[dragonnn]

I am not sure, since it was called a driver bug? Sounds to me like the issue is "don't fix" and that patch is only a workaround. In my opinion it should be included since it works, it could be hide under some flags --fullscreen-unredir-workaround. Using --no-argb isn't really a fix since you lose option to run any compositor and have any transparency? At least that happen when I tried it. I wanted to comment to leave a trace if some searches for this and has a multi-monitor setup.