Provide an attempted optimization when damage reports appear wrong.

With compositing window managers like mutter, when used with the
spice-video-dummy, you can get damage reports that are the whole screen.

Essentially, you just get an indication that the screen has changed,
but no sense of what has changed.

This change detects that behavior and stops trusting those damage
reports, and instead increases the scan frequency to compensate.

Acked-by: Frediano Ziglio <fziglio@redhat.com>

7 files changed, 55 insertions, 11 deletions

diff --git a/src/display.c b/src/display.c
index ae8002e..ca43e0a 100644
--- a/src/display.c
+++ b/src/display.c
@@ -114,13 +114,6 @@ static void handle_damage_notify(display_t *display, xcb_damage_notify_event_t *
     int i, n;
     pixman_box16_t *p;
 
-#if defined(DEBUG_DISPLAY_EVENTS)
-    g_debug("Damage Notify [seq %d|level %d|more %d|area (%dx%d)@%dx%d|geo (%dx%d)@%dx%d",
-            dev->sequence, dev->level, dev->level & 0x80,
-            dev->area.width, dev->area.height, dev->area.x, dev->area.y,
-            dev->geometry.width, dev->geometry.height, dev->geometry.x, dev->geometry.y);
-#endif
-
     pixman_region_union_rect(damage_region, damage_region,
                              dev->area.x, dev->area.y, dev->area.width, dev->area.height);
 
@@ -134,9 +127,31 @@ static void handle_damage_notify(display_t *display, xcb_damage_notify_event_t *
 
     p = pixman_region_rectangles(damage_region, &n);
 
-    for (i = 0; i < n; i++)
-        scanner_push(&display->session->scanner, DAMAGE_SCAN_REPORT,
-                     p[i].x1, p[i].y1, p[i].x2 - p[i].x1, p[i].y2 - p[i].y1);
+    /* Compositing window managers such as mutter have a bad habit of sending
+       whole screen updates, which ends up being harmful to user experience.
+       In that case, we want to stop trusting those damage reports */
+    if (dev->area.width == display->width && dev->area.height == display->height) {
+        display->fullscreen_damage_count++;
+    } else {
+        display->fullscreen_damage_count = 0;
+    }
+
+#if defined(DEBUG_DISPLAY_EVENTS)
+    g_debug("Damage Notify [seq %d|level %d|more %d|area (%dx%d)@%dx%d|geo (%dx%d)@%dx%d%s",
+            dev->sequence, dev->level, dev->level & 0x80,
+            dev->area.width, dev->area.height, dev->area.x, dev->area.y,
+            dev->geometry.width, dev->geometry.height, dev->geometry.x, dev->geometry.y,
+            display_trust_damage(display) ? "" : " SKIPPED");
+#endif
+
+    if (display_trust_damage(display)) {
+        for (i = 0; i < n; i++)
+            scanner_push(&display->session->scanner, DAMAGE_SCAN_REPORT,
+                         p[i].x1, p[i].y1, p[i].x2 - p[i].x1, p[i].y2 - p[i].y1);
+    } else {
+        scanner_push(&display->session->scanner, PERIODIC_SCAN_REQUEST,
+                         0, 0, 0, 0);
+    }
 
     pixman_region_clear(damage_region);
 }
@@ -670,3 +685,8 @@ void display_close(display_t *d)
     display_destroy_screen_images(d);
     xcb_disconnect(d->c);
 }
+
+int display_trust_damage(display_t *d)
+{
+    return d->session->options.always_trust_damage || d->fullscreen_damage_count <= 2;
+}
diff --git a/src/display.h b/src/display.h
index 0aea348..ecf0655 100644
--- a/src/display.h
+++ b/src/display.h
@@ -55,6 +55,7 @@ typedef struct {
 
     const xcb_query_extension_reply_t *damage_ext;
     xcb_damage_damage_t damage;
+    unsigned int fullscreen_damage_count;
 
     const xcb_query_extension_reply_t *shm_ext;
 
@@ -91,4 +92,6 @@ shm_image_t *create_shm_image(display_t *d, unsigned int w, unsigned int h);
 int read_shm_image(display_t *d, shm_image_t *shmi, int x, int y);
 void destroy_shm_image(display_t *d, shm_image_t *shmi);
 
+int display_trust_damage(display_t *d);
+
 #endif
diff --git a/src/options.c b/src/options.c
index a6c7b9e..f18c5d5 100644
--- a/src/options.c
+++ b/src/options.c
@@ -381,6 +381,7 @@ static void options_from_config(options_t *options)
     string_option(&options->on_disconnect, userkey, systemkey, "spice", "on-disconnect");
     options->audit = bool_option(userkey, systemkey, "spice", "audit");
     options->audit_message_type = int_option(userkey, systemkey, "spice", "audit-message-type");
+    options->always_trust_damage = bool_option(userkey, systemkey, "spice", "always-trust-damage");
 
 #if defined(HAVE_LIBAUDIT_H)
     /* Pick an arbitrary default in the user range.  CodeWeavers was founed in 1996, so 1196 it is... */
diff --git a/src/options.h b/src/options.h
index 94341e8..42e43b4 100644
--- a/src/options.h
+++ b/src/options.h
@@ -62,6 +62,7 @@ typedef struct {
     char *on_disconnect;
     int audit;
     int audit_message_type;
+    int always_trust_damage;
 
     /* file names of config files */
     char *user_config_file;
diff --git a/src/scan.c b/src/scan.c
index 6ee399c..aa57427 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -365,6 +365,12 @@ static void *scanner_run(void *opaque)
         }
         scan_update_fps(scanner, 1);
 
+        if (r->type == PERIODIC_SCAN_REQUEST) {
+            free_queue_item(r);
+            scanner_periodic(scanner);
+            continue;
+        }
+
         if (r->type == EXIT_SCAN_REPORT) {
             free_queue_item(r);
             break;
diff --git a/src/scan.h b/src/scan.h
index 51bd06d..69991fe 100644
--- a/src/scan.h
+++ b/src/scan.h
@@ -26,7 +26,12 @@
 /*----------------------------------------------------------------------------
 **  Definitions and simple types
 **--------------------------------------------------------------------------*/
-typedef enum { DAMAGE_SCAN_REPORT, SCANLINE_SCAN_REPORT, EXIT_SCAN_REPORT } scan_type_t;
+typedef enum {
+    DAMAGE_SCAN_REPORT,
+    SCANLINE_SCAN_REPORT,
+    EXIT_SCAN_REPORT,
+    PERIODIC_SCAN_REQUEST,
+} scan_type_t;
 
 struct session_struct;
 /*----------------------------------------------------------------------------
diff --git a/src/xdg/x11spice/x11spice.conf b/src/xdg/x11spice/x11spice.conf
index 8f74091..f510d44 100644
--- a/src/xdg/x11spice/x11spice.conf
+++ b/src/xdg/x11spice/x11spice.conf
@@ -86,6 +86,14 @@
 #audit-message-type=1196
 
 #-----------------------------------------------------------------------------
+# always-trust-damage
+#           By default, x11spice will distrust repeated fullscreen
+#           damage reports.  If this is on, we will always trust the reports.
+#           Default false.
+#-----------------------------------------------------------------------------
+#always-trust-damage=false
+
+#-----------------------------------------------------------------------------
 # minimize  Starts the x11spice gui minimized.  Default false.
 #-----------------------------------------------------------------------------
 #minimize=false