xdg-shell: Rework the state system
authorJasper St. Pierre <jstpierre@mecheye.net>
Thu, 10 Apr 2014 17:41:46 +0000 (10:41 -0700)
committerKristian Høgsberg <krh@bitplanet.net>
Tue, 13 May 2014 06:33:59 +0000 (23:33 -0700)
The states system, so far, has been a complicated mix of weird APIs
that solved a real race condition, but have been particularly ugly
for both compositors and clients to implement.

clients/simple-egl.c
clients/simple-shm.c
clients/window.c
desktop-shell/shell.c
protocol/xdg-shell.xml

index 0d4673b..165ce10 100644 (file)
@@ -268,40 +268,39 @@ init_gl(struct window *window)
 
 static void
 handle_surface_configure(void *data, struct xdg_surface *surface,
-                        int32_t width, int32_t height)
+                        int32_t width, int32_t height,
+                        struct wl_array *states, uint32_t serial)
 {
        struct window *window = data;
+       uint32_t *p;
+
+       window->fullscreen = 0;
+       wl_array_for_each(p, states) {
+               uint32_t state = *p;
+               switch (state) {
+               case XDG_SURFACE_STATE_FULLSCREEN:
+                       window->fullscreen = 1;
+                       break;
+               }
+       }
 
-       if (window->native)
-               wl_egl_window_resize(window->native, width, height, 0, 0);
-
-       window->geometry.width = width;
-       window->geometry.height = height;
-
-       if (!window->fullscreen)
-               window->window_size = window->geometry;
-}
-
-static void
-handle_surface_change_state(void *data, struct xdg_surface *xdg_surface,
-                           uint32_t state,
-                           uint32_t value,
-                           uint32_t serial)
-{
-       struct window *window = data;
-
-       switch (state) {
-       case XDG_SURFACE_STATE_FULLSCREEN:
-               window->fullscreen = value;
-
-               if (!value)
-                       handle_surface_configure(window, window->xdg_surface,
-                                                window->window_size.width,
-                                                window->window_size.height);
-               break;
+       if (width > 0 && height > 0) {
+               if (!window->fullscreen) {
+                       window->window_size.width = width;
+                       window->window_size.height = height;
+               }
+               window->geometry.width = width;
+               window->geometry.height = height;
+       } else if (!window->fullscreen) {
+               window->geometry = window->window_size;
        }
 
-       xdg_surface_ack_change_state(xdg_surface, state, value, serial);
+       if (window->native)
+               wl_egl_window_resize(window->native,
+                                    window->geometry.width,
+                                    window->geometry.height, 0, 0);
+
+       xdg_surface_ack_configure(surface, serial);
 }
 
 static void
@@ -322,7 +321,6 @@ handle_surface_delete(void *data, struct xdg_surface *xdg_surface)
 
 static const struct xdg_surface_listener xdg_surface_listener = {
        handle_surface_configure,
-       handle_surface_change_state,
        handle_surface_activated,
        handle_surface_deactivated,
        handle_surface_delete,
@@ -343,8 +341,8 @@ create_surface(struct window *window)
 
        window->native =
                wl_egl_window_create(window->surface,
-                                    window->window_size.width,
-                                    window->window_size.height);
+                                    window->geometry.width,
+                                    window->geometry.height);
        window->egl_surface =
                eglCreateWindowSurface(display->egl.dpy,
                                       display->egl.conf,
@@ -359,9 +357,8 @@ create_surface(struct window *window)
        if (!window->frame_sync)
                eglSwapInterval(display->egl.dpy, 0);
 
-       xdg_surface_request_change_state(window->xdg_surface,
-                                        XDG_SURFACE_STATE_FULLSCREEN,
-                                        window->fullscreen, 0);
+       if (window->fullscreen)
+               xdg_surface_set_fullscreen(window->xdg_surface, NULL);
 }
 
 static void
@@ -620,11 +617,12 @@ keyboard_handle_key(void *data, struct wl_keyboard *keyboard,
 {
        struct display *d = data;
 
-       if (key == KEY_F11 && state)
-               xdg_surface_request_change_state(d->window->xdg_surface,
-                                                XDG_SURFACE_STATE_FULLSCREEN,
-                                                !d->window->fullscreen, 0);
-       else if (key == KEY_ESC && state)
+       if (key == KEY_F11 && state) {
+               if (d->window->fullscreen)
+                       xdg_surface_unset_fullscreen(d->window->xdg_surface);
+               else
+                       xdg_surface_set_fullscreen(d->window->xdg_surface, NULL);
+       } else if (key == KEY_ESC && state)
                running = 0;
 }
 
@@ -772,8 +770,9 @@ main(int argc, char **argv)
 
        window.display = &display;
        display.window = &window;
-       window.window_size.width  = 250;
-       window.window_size.height = 250;
+       window.geometry.width  = 250;
+       window.geometry.height = 250;
+       window.window_size = window.geometry;
        window.buffer_size = 32;
        window.frame_sync = 1;
 
index 2087a0e..d0cd7e3 100644 (file)
@@ -117,16 +117,10 @@ create_shm_buffer(struct display *display, struct buffer *buffer,
 
 static void
 handle_configure(void *data, struct xdg_surface *surface,
-                int32_t width, int32_t height)
-{
-}
-
-static void
-handle_change_state(void *data, struct xdg_surface *xdg_surface,
-                   uint32_t state,
-                   uint32_t value,
-                   uint32_t serial)
+                int32_t width, int32_t height,
+                struct wl_array *states, uint32_t serial)
 {
+       xdg_surface_ack_configure(surface, serial);
 }
 
 static void
@@ -147,7 +141,6 @@ handle_delete(void *data, struct xdg_surface *xdg_surface)
 
 static const struct xdg_surface_listener xdg_surface_listener = {
        handle_configure,
-       handle_change_state,
        handle_activated,
        handle_deactivated,
        handle_delete,
index 9fe6cd7..9ec12c7 100644 (file)
@@ -230,6 +230,8 @@ struct window {
        int fullscreen;
        int maximized;
 
+       int next_attach_serial;
+
        enum preferred_format preferred_format;
 
        window_key_handler_t key_handler;
@@ -1338,6 +1340,12 @@ surface_flush(struct surface *surface)
                surface->input_region = NULL;
        }
 
+       if (surface->window->next_attach_serial > 0) {
+               xdg_surface_ack_configure(surface->window->xdg_surface,
+                                         surface->window->next_attach_serial);
+               surface->window->next_attach_serial = 0;
+       }
+
        surface->toysurface->swap(surface->toysurface,
                                  surface->buffer_transform, surface->buffer_scale,
                                  &surface->server_allocation);
@@ -3849,37 +3857,39 @@ widget_schedule_resize(struct widget *widget, int32_t width, int32_t height)
 
 static void
 handle_surface_configure(void *data, struct xdg_surface *xdg_surface,
-                        int32_t width, int32_t height)
-{
-       struct window *window = data;
-
-       window_schedule_resize(window, width, height);
-}
-
-static void
-handle_surface_change_state(void *data, struct xdg_surface *xdg_surface,
-                           uint32_t state,
-                           uint32_t value,
-                           uint32_t serial)
+                        int32_t width, int32_t height,
+                        struct wl_array *states, uint32_t serial)
 {
        struct window *window = data;
+       uint32_t *p;
 
-       switch (state) {
-       case XDG_SURFACE_STATE_MAXIMIZED:
-               window->maximized = value;
-               break;
-       case XDG_SURFACE_STATE_FULLSCREEN:
-               window->fullscreen = value;
-               break;
-       }
-
-       if (!window->fullscreen && !window->maximized)
+       if (width > 0 && height > 0) {
+               window_schedule_resize(window, width, height);
+       } else {
                window_schedule_resize(window,
                                       window->saved_allocation.width,
                                       window->saved_allocation.height);
+       }
 
-       xdg_surface_ack_change_state(xdg_surface, state, value, serial);
-       window_schedule_redraw(window);
+       window->maximized = 0;
+       window->fullscreen = 0;
+
+       wl_array_for_each(p, states) {
+               uint32_t state = *p;
+               switch (state) {
+               case XDG_SURFACE_STATE_MAXIMIZED:
+                       window->maximized = 1;
+                       break;
+               case XDG_SURFACE_STATE_FULLSCREEN:
+                       window->fullscreen = 1;
+                       break;
+               default:
+                       /* Unknown state */
+                       break;
+               }
+       }
+
+       window->next_attach_serial = serial;
 }
 
 static void
@@ -3905,7 +3915,6 @@ handle_surface_delete(void *data, struct xdg_surface *xdg_surface)
 
 static const struct xdg_surface_listener xdg_surface_listener = {
        handle_surface_configure,
-       handle_surface_change_state,
        handle_surface_activated,
        handle_surface_deactivated,
        handle_surface_delete,
@@ -4145,10 +4154,10 @@ window_set_fullscreen(struct window *window, int fullscreen)
        if (window->fullscreen == fullscreen)
                return;
 
-       xdg_surface_request_change_state(window->xdg_surface,
-                                        XDG_SURFACE_STATE_FULLSCREEN,
-                                        fullscreen ? 1 : 0,
-                                        0);
+       if (fullscreen)
+               xdg_surface_set_fullscreen(window->xdg_surface, NULL);
+       else
+               xdg_surface_unset_fullscreen(window->xdg_surface);
 }
 
 int
@@ -4166,10 +4175,10 @@ window_set_maximized(struct window *window, int maximized)
        if (window->maximized == maximized)
                return;
 
-       xdg_surface_request_change_state(window->xdg_surface,
-                                        XDG_SURFACE_STATE_MAXIMIZED,
-                                        maximized ? 1 : 0,
-                                        0);
+       if (maximized)
+               xdg_surface_set_maximized(window->xdg_surface);
+       else
+               xdg_surface_unset_maximized(window->xdg_surface);
 }
 
 void
index ea0e049..574681f 100644 (file)
@@ -3418,86 +3418,73 @@ xdg_surface_resize(struct wl_client *client, struct wl_resource *resource,
 }
 
 static void
-xdg_surface_set_output(struct wl_client *client,
-                      struct wl_resource *resource,
-                      struct wl_resource *output_resource)
+xdg_surface_ack_configure(struct wl_client *client,
+                         struct wl_resource *resource,
+                         uint32_t serial)
 {
        struct shell_surface *shsurf = wl_resource_get_user_data(resource);
-       struct weston_output *output;
-
-       if (output_resource)
-               output = wl_resource_get_user_data(output_resource);
-       else
-               output = NULL;
 
-       shsurf->recommended_output = output;
+       if (shsurf->state_requested) {
+               shsurf->next_state = shsurf->requested_state;
+               shsurf->state_changed = true;
+               shsurf->state_requested = false;
+       }
 }
 
 static void
-xdg_surface_change_state(struct shell_surface *shsurf,
-                        uint32_t state, uint32_t value, uint32_t serial)
+xdg_surface_set_maximized(struct wl_client *client,
+                         struct wl_resource *resource)
 {
-       xdg_surface_send_change_state(shsurf->resource, state, value, serial);
+       struct shell_surface *shsurf = wl_resource_get_user_data(resource);
+
        shsurf->state_requested = true;
+       shsurf->requested_state.maximized = true;
+       set_maximized(shsurf, NULL);
+}
 
-       switch (state) {
-       case XDG_SURFACE_STATE_MAXIMIZED:
-               shsurf->requested_state.maximized = value;
-               if (value)
-                       set_maximized(shsurf, NULL);
-               break;
-       case XDG_SURFACE_STATE_FULLSCREEN:
-               shsurf->requested_state.fullscreen = value;
+static void
+xdg_surface_unset_maximized(struct wl_client *client,
+                           struct wl_resource *resource)
+{
+       struct shell_surface *shsurf = wl_resource_get_user_data(resource);
 
-               if (value)
-                       set_fullscreen(shsurf,
-                                      WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT,
-                                      0, shsurf->recommended_output);
-               break;
-       }
+       shsurf->state_requested = true;
+       shsurf->requested_state.maximized = false;
+       shsurf->client->send_configure(shsurf->surface, 0, 0);
 }
 
 static void
-xdg_surface_request_change_state(struct wl_client *client,
-                                struct wl_resource *resource,
-                                uint32_t state,
-                                uint32_t value,
-                                uint32_t serial)
+xdg_surface_set_fullscreen(struct wl_client *client,
+                          struct wl_resource *resource,
+                          struct wl_resource *output_resource)
 {
        struct shell_surface *shsurf = wl_resource_get_user_data(resource);
+       struct weston_output *output;
 
-       /* The client can't know what the current state is, so we need
-          to always send a state change in response. */
+       shsurf->state_requested = true;
+       shsurf->requested_state.fullscreen = true;
 
-       if (shsurf->type != SHELL_SURFACE_TOPLEVEL)
-               return;
+       if (output_resource)
+               output = wl_resource_get_user_data(output_resource);
+       else
+               output = NULL;
 
-       switch (state) {
-       case XDG_SURFACE_STATE_MAXIMIZED:
-       case XDG_SURFACE_STATE_FULLSCREEN:
-               break;
-       default:
-               /* send error? ignore? send change state with value 0? */
-               return;
-       }
+       shsurf->recommended_output = output;
 
-       xdg_surface_change_state(shsurf, state, value, serial);
+       set_fullscreen(shsurf,
+                      WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT,
+                      0, shsurf->recommended_output);
 }
 
 static void
-xdg_surface_ack_change_state(struct wl_client *client,
-                            struct wl_resource *resource,
-                            uint32_t state,
-                            uint32_t value,
-                            uint32_t serial)
+xdg_surface_unset_fullscreen(struct wl_client *client,
+                            struct wl_resource *resource)
 {
        struct shell_surface *shsurf = wl_resource_get_user_data(resource);
 
-       if (shsurf->state_requested) {
-               shsurf->next_state = shsurf->requested_state;
-               shsurf->state_changed = true;
-               shsurf->state_requested = false;
-       }
+       shsurf->state_requested = true;
+       shsurf->requested_state.fullscreen = false;
+       shsurf->client->send_configure(shsurf->surface, 0, 0);
 }
 
 static void
@@ -3521,10 +3508,12 @@ static const struct xdg_surface_interface xdg_surface_implementation = {
        xdg_surface_set_app_id,
        xdg_surface_move,
        xdg_surface_resize,
-       xdg_surface_set_output,
-       xdg_surface_request_change_state,
-       xdg_surface_ack_change_state,
-       xdg_surface_set_minimized
+       xdg_surface_ack_configure,
+       xdg_surface_set_maximized,
+       xdg_surface_unset_maximized,
+       xdg_surface_set_fullscreen,
+       xdg_surface_unset_fullscreen,
+       xdg_surface_set_minimized,
 };
 
 static void
@@ -3532,11 +3521,28 @@ xdg_send_configure(struct weston_surface *surface,
                   int32_t width, int32_t height)
 {
        struct shell_surface *shsurf = get_shell_surface(surface);
+       uint32_t *s;
+       struct wl_array states;
+       uint32_t serial;
 
        assert(shsurf);
 
-       if (shsurf->resource)
-               xdg_surface_send_configure(shsurf->resource, width, height);
+       if (!shsurf->resource)
+               return;
+
+       wl_array_init(&states);
+       if (shsurf->requested_state.fullscreen) {
+               s = wl_array_add(&states, sizeof *s);
+               *s = XDG_SURFACE_STATE_FULLSCREEN;
+       } else if (shsurf->requested_state.maximized) {
+               s = wl_array_add(&states, sizeof *s);
+               *s = XDG_SURFACE_STATE_MAXIMIZED;
+       }
+
+       serial = wl_display_next_serial(shsurf->surface->compositor->wl_display);
+       xdg_surface_send_configure(shsurf->resource, width, height, &states, serial);
+
+       wl_array_release(&states);
 }
 
 static const struct weston_shell_client xdg_client = {
@@ -4107,7 +4113,6 @@ maximize_binding(struct weston_seat *seat, uint32_t time, uint32_t button, void
        struct weston_surface *focus = seat->keyboard->focus;
        struct weston_surface *surface;
        struct shell_surface *shsurf;
-       uint32_t serial;
 
        surface = weston_surface_get_main_surface(focus);
        if (surface == NULL)
@@ -4120,9 +4125,10 @@ maximize_binding(struct weston_seat *seat, uint32_t time, uint32_t button, void
        if (!shell_surface_is_xdg_surface(shsurf))
                return;
 
-       serial = wl_display_next_serial(seat->compositor->wl_display);
-       xdg_surface_change_state(shsurf, XDG_SURFACE_STATE_MAXIMIZED,
-                                !shsurf->state.maximized, serial);
+       shsurf->state_requested = true;
+       shsurf->requested_state.maximized = !shsurf->state.maximized;
+       if (shsurf->requested_state.maximized)
+               set_maximized(shsurf, NULL);
 }
 
 static void
@@ -4131,7 +4137,6 @@ fullscreen_binding(struct weston_seat *seat, uint32_t time, uint32_t button, voi
        struct weston_surface *focus = seat->keyboard->focus;
        struct weston_surface *surface;
        struct shell_surface *shsurf;
-       uint32_t serial;
 
        surface = weston_surface_get_main_surface(focus);
        if (surface == NULL)
@@ -4144,9 +4149,12 @@ fullscreen_binding(struct weston_seat *seat, uint32_t time, uint32_t button, voi
        if (!shell_surface_is_xdg_surface(shsurf))
                return;
 
-       serial = wl_display_next_serial(seat->compositor->wl_display);
-       xdg_surface_change_state(shsurf, XDG_SURFACE_STATE_FULLSCREEN,
-                                !shsurf->state.fullscreen, serial);
+       shsurf->state_requested = true;
+       shsurf->requested_state.fullscreen = !shsurf->state.fullscreen;
+       if (shsurf->requested_state.fullscreen)
+               set_fullscreen(shsurf,
+                              WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT,
+                              0, shsurf->recommended_output);
 }
 
 static void
index 7882693..d3d6a37 100644 (file)
       <arg name="edges" type="uint" summary="which edge or corner is being dragged"/>
     </request>
 
-    <event name="configure">
-      <description summary="suggest resize">
-       The configure event asks the client to resize its surface.
-
-       The size is a hint, in the sense that the client is free to
-       ignore it if it doesn't resize, pick a smaller size (to
-       satisfy aspect ratio or resize in steps of NxM pixels).
-
-       The client is free to dismiss all but the last configure
-       event it received.
-
-       The width and height arguments specify the size of the window
-       in surface local coordinates.
-      </description>
-
-      <arg name="width" type="int"/>
-      <arg name="height" type="int"/>
-    </event>
-
-    <request name="set_output">
-      <description summary="set the default output used by this surface">
-       Set the default output used by this surface when it is first mapped.
-
-       If this value is NULL (default), it's up to the compositor to choose
-       which display will be used to map this surface.
-
-       When fullscreen or maximized state are set on this surface, and it
-       wasn't mapped yet, the output set with this method will be used.
-       Otherwise, the output where the surface is currently mapped will be
-       used.
-      </description>
-      <arg name="output" type="object" interface="wl_output" allow-null="true"/>
-    </request>
-
     <enum name="state">
       <description summary="types of state on the surface">
         The different state values used on the surface. This is designed for
         0x1000 - 0x1FFF: GNOME
       </description>
       <entry name="maximized" value="1" summary="the surface is maximized">
-        A non-zero value indicates the surface is maximized. Otherwise,
-        the surface is unmaximized.
+        The surface is maximized. The window geometry specified in the configure
+        event must be obeyed by the client.
       </entry>
       <entry name="fullscreen" value="2" summary="the surface is fullscreen">
-        A non-zero value indicates the surface is fullscreen. Otherwise,
-        the surface is not fullscreen.
+        The surface is fullscreen. The window geometry specified in the configure
+        event must be obeyed by the client.
       </entry>
     </enum>
 
-    <request name="request_change_state">
-      <description summary="client requests to change a surface's state">
-        This asks the compositor to change the state. If the compositor wants
-        to change the state, it will send a change_state event with the same
-        state_type, value, and serial, and the event flow continues as if it
-        it was initiated by the compositor.
+    <event name="configure">
+      <description summary="suggest a surface chnage">
+       The configure event asks the client to resize its surface.
 
-        If the compositor does not want to change the state, it will send a
-        change_state to the client with the old value of the state.
-      </description>
-      <arg name="state_type" type="uint" summary="the state to set"/>
-      <arg name="value" type="uint" summary="the value to change the state to"/>
-      <arg name="serial" type="uint" summary="an event serial">
-        This serial is so the client can know which change_state event corresponds
-        to which request_change_state request it sent out.
-      </arg>
-    </request>
+       The width and height arguments specify a hint to the window
+        about how its surface should be resized in surface local
+        coordinates. The states listed in the event specify how the
+        width/height arguments should be interpreted.
+
+        A client should arrange a new surface, and then send a
+        ack_configure request with the serial sent in this configure
+        event before attaching a new surface.
 
-    <event name="change_state">
-      <description summary="compositor wants to change a surface's state">
-        This event tells the client to change a surface's state. The client
-        should respond with an ack_change_state request to the compositor to
-        guarantee that the compositor knows that the client has seen it.
+       If the client receives multiple configure events before it
+        can respond to one, it is free to discard all but the last
+        event it received.
       </description>
 
-      <arg name="state_type" type="uint" summary="the state to set"/>
-      <arg name="value" type="uint" summary="the value to change the state to"/>
-      <arg name="serial" type="uint" summary="a serial for the compositor's own tracking"/>
+      <arg name="width" type="int"/>
+      <arg name="height" type="int"/>
+      <arg name="states" type="array"/>
+      <arg name="serial" type="uint"/>
     </event>
 
-    <request name="ack_change_state">
-      <description summary="ack a change_state event">
-        When a change_state event is received, a client should then ack it
-        using the ack_change_state request to ensure that the compositor
+    <request name="ack_configure">
+      <description summary="ack a configure event">
+        When a configure event is received, a client should then ack it
+        using the ack_configure request to ensure that the compositor
         knows the client has seen the event.
 
         By this point, the state is confirmed, and the next attach should
-        contain the buffer drawn for the new state value.
-
-        The values here need to be the same as the values in the cooresponding
-        change_state event.
+        contain the buffer drawn for the configure event you are acking.
       </description>
-      <arg name="state_type" type="uint" summary="the state to set"/>
-      <arg name="value" type="uint" summary="the value to change the state to"/>
-      <arg name="serial" type="uint" summary="a serial to pass to change_state"/>
+      <arg name="serial" type="uint" summary="a serial to configure for"/>
     </request>
 
-    <request name="set_minimized">
-      <description summary="minimize the surface">
-        Minimize the surface.
+    <request name="set_maximized" />
+    <request name="unset_maximized" />
+
+    <request name="set_fullscreen">
+      <description summary="set the window as fullscreen on a monitor">
+       Make the surface fullscreen.
+
+        You can specify an output that you would prefer to be fullscreen.
+       If this value is NULL, it's up to the compositor to choose which
+        display will be used to map this surface.
       </description>
+      <arg name="output" type="object" interface="wl_output" allow-null="true"/>
     </request>
+    <request name="unset_fullscreen" />
+
+    <request name="set_minimized" />
 
     <event name="activated">
       <description summary="surface was activated">