Merge lp:~vanvugt/mir/fix-1655804 into lp:mir

[Daniel van Vugt]

Fix distorted mir_demo_client_target on start-up (LP: #1655804)
by adding a workaround for the Unity8 bug that triggers it (LP: #1542029)

[Alan Griffiths (alan-griffiths)]

How is a client ignoring the size allocated by the server a Unity8 bug? There are all sorts of reasons that a server may allocate a size that isn't requested (e.g. a tiling shell).

review: Needs Information

[lp:~vanvugt/mir/fix-1655804]

3924. By Daniel van Vugt on 2017-01-12

Add a mutex too (because LP: #1194384)

[Daniel van Vugt (vanvugt)]

Resizing a window is perfectly valid, expected and supported. Resizing a window immediately on creation before the user has even seen it (and before the app has set a callback) is a bit unusual. No other windowing system (not tiling) I've ever seen does that. But indeed it's allowed if there are no resize constraints set.

So it's allowed but unexpected. Continue discussion in LP: #1542029...

[Alan Griffiths (alan-griffiths)]

> Resizing a window is perfectly valid, expected and supported. Resizing a
> window immediately on creation before the user has even seen it (and before
> the app has set a callback) is a bit unusual.

The app ought to set the callback on creation (that's why it is a property of the spec). And, if this fix is working, then it is clearly being called.

> No other windowing system (not
> tiling) I've ever seen does that. But indeed it's allowed if there are no
> resize constraints set.

It is allowed even if resize constraints have been set. (And required for some cases by the Mir & Unity8 design doc.)

> So it's allowed but unexpected. Continue discussion in LP: #1542029...

This is simply fixing a client bug, not working around a server issue.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

If anyone can explain why Unity8 picks a random window size over the application specified size, then please explain. Keep in mind that scaling is not an excuse, because many legacy apps and non-toolkit apps simply don't care about scaling, only pixels.

And yes I agree using the newer APIs to fix this is more appropriate. Obviously they did not exist when this code was written.

[Daniel d'Andrada (dandrader)]

> If anyone can explain why Unity8 picks a random window size over the
> application specified size, then please explain. Keep in mind that scaling is
> not an excuse, because many legacy apps and non-toolkit apps simply don't care
> about scaling, only pixels.

1 - in phone and tablet mode, window size must be exactly what unity8 wants. application doesn't have a say on that

2 - in desktop mode unity8 remembers the size the window had when it was last closed. so when you launch it again it will give that size. But the first time an application is ever launched that size is a generic default unity8 guesses indeed. That needs more thought and work.

Unmerged revisions

3924. By Daniel van Vugt on 2017-01-12

Add a mutex too (because LP: #1194384)

3923. By Daniel van Vugt on 2017-01-12

Forward-port from 0.25 experiment

=== modified file 'examples/eglapp.c'
--- examples/eglapp.c	2017-01-11 18:13:25 +0000
+++ examples/eglapp.c	2017-01-12 10:50:44 +0000
@@ -25,6 +25,7 @@
 #include <string.h>
 #include <EGL/egl.h>
 #include <GLES2/gl2.h>
+#include <pthread.h>
 
 #include <xkbcommon/xkbcommon-keysyms.h>
 
@@ -37,7 +38,10 @@
 static EGLDisplay egldisplay;
 static EGLSurface eglsurface;
 static volatile sig_atomic_t running = 0;
+
+static pthread_mutex_t event_mutex = PTHREAD_MUTEX_INITIALIZER;
 static double refresh_rate = 0.0;
+static int resized_width = -1, resized_height = -1;
 
 #define CHECK(_cond, _err) \
     if (!(_cond)) \
@@ -136,7 +140,9 @@
     unsigned ff = mir_window_output_event_get_form_factor(out);
     char const* form_factor = (ff < 6) ? form_factor_name[ff] : "out-of-range";
 
+    pthread_mutex_lock(&event_mutex);
     refresh_rate = mir_window_output_event_get_refresh_rate(out);
+    pthread_mutex_unlock(&event_mutex);
 
     printf("Window is on output %u: %d DPI, scale %.1fx, %s form factor, %.2fHz\n",
            mir_window_output_event_get_output_id(out),
@@ -148,7 +154,10 @@
 
 double mir_eglapp_display_hz(void)
 {
-    return refresh_rate;
+    pthread_mutex_lock(&event_mutex);
+    double rate = refresh_rate;
+    pthread_mutex_unlock(&event_mutex);
+    return rate;
 }
 
 void mir_eglapp_handle_event(MirWindow* window, MirEvent const* ev, void* unused)
@@ -177,9 +186,11 @@
          */
         {
             MirResizeEvent const* resize = mir_event_get_resize_event(ev);
-            printf("Resized to %dx%d\n",
-                   mir_resize_event_get_width(resize),
-                   mir_resize_event_get_height(resize));
+            pthread_mutex_lock(&event_mutex);
+            resized_width = mir_resize_event_get_width(resize);
+            resized_height = mir_resize_event_get_height(resize);
+            printf("Resized to %dx%d\n", resized_width, resized_height);
+            pthread_mutex_unlock(&event_mutex);
         }
         break;
     case mir_event_type_close_window:
@@ -518,6 +529,18 @@
 
     running = 1;
 
+    /*
+     * Work around Unity8 behaving badly (LP: #1542029), which then caused
+     * LP: #1655804 ...
+     */
+    pthread_mutex_lock(&event_mutex);
+    if (resized_width >= 0 || resized_height >= 0)
+    {
+        *width = resized_width;
+        *height = resized_height;
+    }
+    pthread_mutex_unlock(&event_mutex);
+
     return 1;
 }