vm_tools: sommelier: migrate black_screen_fix to quirk

We are migrating black screen fix as quirk, so that it can easily be enabled/disabled based on the window properties. BUG=b:328703154 TEST=Alt-Tab HADES with the quirk on and off Change-Id: I9197165cdb627a237294625e01be3b9d40aab4ea Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/5368776 Tested-by: Max Lee <[email protected]> Commit-Queue: Max Lee <[email protected]> Reviewed-by: Chloe Pelling <[email protected]>
WhatAmISupposedToPutHere · Mar 15, 2024 · e0466e8 · e0466e8
1 parent b042bd2
commit e0466e8
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 53 deletions.
diff --git a/meson.build b/meson.build
@@ -172,10 +172,6 @@ if get_option('commit_loop_fix')
   cpp_args += '-DCOMMIT_LOOP_FIX'
 endif
 
-if get_option('black_screen_fix')
-  cpp_args += '-DBLACK_SCREEN_FIX'
-endif
-
 sommelier_defines = [
     '-D_GNU_SOURCE',
     '-DWL_HIDE_DEPRECATED',

diff --git a/meson_options.txt b/meson_options.txt
@@ -46,12 +46,6 @@ option('commit_loop_fix',
   description: 'enable a fix to the commit-cycle, which is known to break some apps'
 )
 
-option('black_screen_fix',
-  type: 'boolean',
-  value: false,
-  description: 'enable a fix to fix some apps blackscreening when losing focus'
-)
-
 option('quirks',
   type: 'boolean',
   value: false,

diff --git a/quirks/quirks.proto b/quirks/quirks.proto
@@ -43,4 +43,5 @@ message SommelierCondition {
 enum Feature {
   FEATURE_UNSPECIFIED = 0;
   FEATURE_X11_MOVE_WINDOWS = 1;
+  FEATURE_BLACK_SCREEN_FIX = 2;
 }
diff --git a/sommelier-window-test.cc b/sommelier-window-test.cc
@@ -16,6 +16,10 @@
 #include <xcb/xproto.h>
 #include <cstdint>
 
+#ifdef QUIRKS_SUPPORT
+#include "quirks/sommelier-quirks.h"
+#endif
+
 namespace vm_tools {
 namespace sommelier {
 
@@ -537,13 +541,20 @@ TEST_F(X11Test, NonExistentWindowDoesNotCrash) {
   sl_handle_reparent_notify(&ctx, &reparent_event);
 }
 
-#ifdef BLACK_SCREEN_FIX
+#ifdef QUIRKS_SUPPORT
 TEST_F(X11Test, IconifySuppressesFullscreen) {
   // Arrange: Create an xdg_toplevel surface. Initially it's not iconified.
   sl_window* window = CreateToplevelWindow();
   uint32_t xdg_toplevel_id = XdgToplevelId(window);
   EXPECT_EQ(window->iconified, 0);
 
+  window->steam_game_id = 123;
+  ctx.quirks.Load(
+      "sommelier { \n"
+      "  condition { steam_game_id: 123 }\n"
+      "  enable: FEATURE_BLACK_SCREEN_FIX\n"
+      "}");
+
   // Act: Pretend an X11 client owns the surface, and requests to iconify it.
   xcb_client_message_event_t event;
   event.response_type = XCB_CLIENT_MESSAGE;
@@ -596,6 +607,13 @@ TEST_F(X11Test, IconifySuppressesUnmaximize) {
   uint32_t xdg_toplevel_id = XdgToplevelId(window);
   EXPECT_EQ(window->iconified, 0);
 
+  window->steam_game_id = 123;
+  ctx.quirks.Load(
+      "sommelier { \n"
+      "  condition { steam_game_id: 123 }\n"
+      "  enable: FEATURE_BLACK_SCREEN_FIX\n"
+      "}");
+
   // Arrange: Maximize it.
   xcb_client_message_event_t event;
   event.response_type = XCB_CLIENT_MESSAGE;
@@ -651,7 +669,7 @@ TEST_F(X11Test, IconifySuppressesUnmaximize) {
       .Times(1);
   Pump();
 }
-#endif  // BLACK_SCREEN_FIX
+#endif  // QUIRKS_SUPPORT
 
 // Matcher for the value_list argument of an X11 ConfigureWindow request,
 // which is a const void* pointing to an int array whose size is implied by

diff --git a/sommelier.cc b/sommelier.cc
@@ -67,6 +67,10 @@
 #include "xdg-shell-client-protocol.h"  // NOLINT(build/include_directory)
 #include "xdg-shell-shim.h"             // NOLINT(build/include_directory)
 
+#ifdef QUIRKS_SUPPORT
+#include "quirks/sommelier-quirks.h"
+#endif
+
 // Check that required macro definitions exist.
 #ifndef XWAYLAND_PATH
 #error XWAYLAND_PATH must be defined
@@ -1870,48 +1874,52 @@ void sl_handle_client_message(struct sl_context* ctx,
                 "window->name", window ? window->name : "<unknown>");
     if (window && window->xdg_toplevel) {
       xdg_toplevel_set_minimized(window->xdg_toplevel);
-#ifdef BLACK_SCREEN_FIX
-      // Workaround for some borealis apps showing a black screen after losing
-      // focus from fullscreen.
-      // When a window is iconified, it should be unmapped. To return it back
-      // to a visible state, it should be remapped. However sommelier does not
-      // do this. Therefore we are sending a synthetic unmap then map notify
-      // so that the app is rendered again.
-      xcb_unmap_notify_event_t unmap_event = {.response_type = XCB_UNMAP_NOTIFY,
-                                              .pad0 = 0,
-                                              .event = window->id,
-                                              .window = window->id,
-                                              .from_configure = 0};
-
-      xcb_send_event(ctx->connection, 0, window->id,
-                     XCB_EVENT_MASK_STRUCTURE_NOTIFY,
-                     reinterpret_cast<char*>(&unmap_event));
-      sl_send_configure_notify(window);
-
-      sl_window_set_wm_state(window, WM_STATE_ICONIC);
-      sl_send_configure_notify(window);
-
-      xcb_map_notify_event_t map_event = {.response_type = XCB_MAP_NOTIFY,
-                                          .pad0 = 0,
-                                          .event = window->id,
-                                          .window = window->id,
-                                          .override_redirect = 0};
-
-      xcb_send_event(ctx->connection, 0, window->id,
-                     XCB_EVENT_MASK_STRUCTURE_NOTIFY,
-                     reinterpret_cast<char*>(&map_event));
-      sl_send_configure_notify(window);
-
-      sl_window_set_wm_state(window, WM_STATE_NORMAL);
-      sl_send_configure_notify(window);
-
-      sl_set_input_focus(ctx, nullptr);
-      xcb_flush(ctx->connection);
+#ifdef QUIRKS_SUPPORT
+      if (window->ctx->quirks.IsEnabled(window,
+                                        quirks::FEATURE_BLACK_SCREEN_FIX)) {
+        // Workaround for some borealis apps showing a black screen after losing
+        // focus from fullscreen.
+        // When a window is iconified, it should be unmapped. To return it back
+        // to a visible state, it should be remapped. However sommelier does not
+        // do this. Therefore we are sending a synthetic unmap then map notify
+        // so that the app is rendered again.
+        xcb_unmap_notify_event_t unmap_event = {
+            .response_type = XCB_UNMAP_NOTIFY,
+            .pad0 = 0,
+            .event = window->id,
+            .window = window->id,
+            .from_configure = 0};
+
+        xcb_send_event(ctx->connection, 0, window->id,
+                       XCB_EVENT_MASK_STRUCTURE_NOTIFY,
+                       reinterpret_cast<char*>(&unmap_event));
+        sl_send_configure_notify(window);
 
-      // When we are iconified we want to suppress any calls that deiconify
-      // the window as it should in theory be unmapped.
-      window->iconified = 1;
-#endif
+        sl_window_set_wm_state(window, WM_STATE_ICONIC);
+        sl_send_configure_notify(window);
+
+        xcb_map_notify_event_t map_event = {.response_type = XCB_MAP_NOTIFY,
+                                            .pad0 = 0,
+                                            .event = window->id,
+                                            .window = window->id,
+                                            .override_redirect = 0};
+
+        xcb_send_event(ctx->connection, 0, window->id,
+                       XCB_EVENT_MASK_STRUCTURE_NOTIFY,
+                       reinterpret_cast<char*>(&map_event));
+        sl_send_configure_notify(window);
+
+        sl_window_set_wm_state(window, WM_STATE_NORMAL);
+        sl_send_configure_notify(window);
+
+        sl_set_input_focus(ctx, nullptr);
+        xcb_flush(ctx->connection);
+
+        // When we are iconified we want to suppress any calls that deiconify
+        // the window as it should in theory be unmapped.
+        window->iconified = 1;
+      }
+#endif  // QUIRKS_SUPPORT
     }
   }
 }