Merge lp:~alan-griffiths/mir/orientation into lp:mir

[Alan Griffiths]

orientation API: support for the first orientation use-case (The shell can inform a surface of its orientation).

A second use case follows as Daniel d'Andrada asked about a client API to query the orientation.

orientation API: support for the first orientation use-case (The shell can inform a surface of its orientation).

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1709
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1934/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/616
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/622
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/616
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/455
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/455/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/452
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/452/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1829
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1829/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1809
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/8640

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1934/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

224 +// An orientation change alone is not enough to trigger recomposition.

couldn't some orientation changes be immediately recomposited?

seems okay otherwise

review: Approve

[Alexandros Frantzis (afrantzis)]

Looks good. I wonder if we could come up with a guideline for when to use the surface attribute mechanism vs dedicated events.

review: Approve

[Alan Griffiths (alan-griffiths)]

> Looks good. I wonder if we could come up with a guideline for when to use the
> surface attribute mechanism vs dedicated events.

I wondered (and still wonder) about this. But couldn't find a good argument for using attributes rather than an explicit event. (And an explicit event is marginally easier to wait for in the tests.)

=== modified file 'include/server/mir/scene/null_surface_observer.h'
--- include/server/mir/scene/null_surface_observer.h	2014-06-12 15:35:08 +0000
+++ include/server/mir/scene/null_surface_observer.h	2014-06-19 14:21:51 +0000
@@ -37,6 +37,7 @@
     void hidden_set_to(bool hide);
     void frame_posted(int frames_available);
     void alpha_set_to(float alpha);
+    void orientation_set_to(MirOrientation orientation) override;
     void transformation_set_to(glm::mat4 const& t);
     void cursor_image_set_to(graphics::CursorImage const& image);
     void reception_mode_set_to(input::InputReceptionMode mode);

=== modified file 'include/server/mir/scene/surface.h'
--- include/server/mir/scene/surface.h	2014-06-05 14:38:50 +0000
+++ include/server/mir/scene/surface.h	2014-06-19 14:21:51 +0000
@@ -70,6 +70,7 @@
     virtual void resize(geometry::Size const& size) = 0;
     virtual void set_transformation(glm::mat4 const& t) = 0;
     virtual void set_alpha(float alpha) = 0;
+    virtual void set_orientation(MirOrientation orientation) = 0;
     virtual void force_requests_to_complete() = 0;
     
     virtual void set_cursor_image(std::shared_ptr<graphics::CursorImage> const& image) = 0;

=== modified file 'include/server/mir/scene/surface_event_source.h'
--- include/server/mir/scene/surface_event_source.h	2014-06-02 17:07:02 +0000
+++ include/server/mir/scene/surface_event_source.h	2014-06-19 14:21:51 +0000
@@ -38,6 +38,7 @@
 
     void attrib_changed(MirSurfaceAttrib attrib, int value) override;
     void resized_to(geometry::Size const& size) override;
+    void orientation_set_to(MirOrientation orientation) override;
 
 private:
     frontend::SurfaceId const id;

=== modified file 'include/server/mir/scene/surface_observer.h'
--- include/server/mir/scene/surface_observer.h	2014-06-12 15:35:08 +0000
+++ include/server/mir/scene/surface_observer.h	2014-06-19 14:21:51 +0000
@@ -48,6 +48,7 @@
     virtual void hidden_set_to(bool hide) = 0;
     virtual void frame_posted(int frames_available) = 0;
     virtual void alpha_set_to(float alpha) = 0;
+    virtual void orientation_set_to(MirOrientation orientation) = 0;
     virtual void transformation_set_to(glm::mat4 const& t) = 0;
     virtual void reception_mode_set_to(input::InputReceptionMode mode) = 0;
     virtual void cursor_image_set_to(graphics::CursorImage const& image) = 0;

=== modified file 'include/shared/mir_toolkit/common.h'
--- include/shared/mir_toolkit/common.h	2014-06-12 15:35:08 +0000
+++ include/shared/mir_toolkit/common.h	2014-06-19 14:21:51 +0000
@@ -1,7 +1,7 @@
 /*
  * Simple definitions common to client and server.
  *
- * Copyright © 2013 Canonical Ltd.
+ * Copyright © 2013-2014 Canonical Ltd.
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU Lesser General Public License version 3 as
@@ -117,6 +117,7 @@
 /* This could be improved... https://bugs.launchpad.net/mir/+bug/1236254 */
 #define MIR_BYTES_PER_PIXEL(f) (((f) == mir_pixel_format_bgr_888) ? 3 : 4)
 
+/** Direction relative to the "natural" orientation of the display */
 typedef enum MirOrientation
 {
     mir_orientation_normal = 0,

=== modified file 'include/shared/mir_toolkit/event.h'
--- include/shared/mir_toolkit/event.h	2014-06-11 16:11:32 +0000
+++ include/shared/mir_toolkit/event.h	2014-06-19 14:21:51 +0000
@@ -41,7 +41,8 @@
     mir_event_type_motion,
     mir_event_type_surface,
     mir_event_type_resize,
-    mir_event_type_prompt_session_state_change
+    mir_event_type_prompt_session_state_change,
+    mir_event_type_orientation
 } MirEventType;
 
 typedef enum {
@@ -212,6 +213,14 @@
     MirPromptSessionState new_state;
 } MirPromptSessionEvent;
 
+typedef struct MirOrientationEvent
+{
+    MirEventType type;
+
+    int surface_id;
+    MirOrientation direction;
+} MirOrientationEvent;
+
 typedef union
 {
     MirEventType    type;
@@ -220,6 +229,7 @@
     MirSurfaceEvent surface;
     MirResizeEvent  resize;
     MirPromptSessionEvent  prompt_session;
+    MirOrientationEvent orientation;
 } MirEvent;
 
 #ifdef __cplusplus

=== modified file 'include/test/mir_test_doubles/stub_scene_surface.h'
--- include/test/mir_test_doubles/stub_scene_surface.h	2014-06-05 14:38:50 +0000
+++ include/test/mir_test_doubles/stub_scene_surface.h	2014-06-19 14:21:51 +0000
@@ -77,6 +77,7 @@
     void resize(geometry::Size const&) override {}
     void set_transformation(glm::mat4 const&) override {}
     void set_alpha(float) override {}
+    void set_orientation(MirOrientation) {}
     void force_requests_to_complete() override {}
 
     void add_observer(std::shared_ptr<scene::SurfaceObserver> const&) override {}

=== modified file 'src/client/rpc/mir_socket_rpc_channel.cpp'
--- src/client/rpc/mir_socket_rpc_channel.cpp	2014-06-12 15:35:08 +0000
+++ src/client/rpc/mir_socket_rpc_channel.cpp	2014-06-19 14:21:51 +0000
@@ -418,24 +418,25 @@
 
                 rpc_report->event_parsing_succeeded(e);
 
-                event_sink->handle_event(e);
-
-                // todo - surfaces should register with the event handler register.
-                if (e.type == mir_event_type_surface)
-                {
-                    surface_map->with_surface_do(e.surface.id,
-                        [&e](MirSurface* surface)
-                        {
-                            surface->handle_event(e);
-                        });
-                }
-                else if (e.type == mir_event_type_resize)
-                {
-                    surface_map->with_surface_do(e.resize.surface_id,
-                        [&e](MirSurface* surface)
-                        {
-                            surface->handle_event(e);
-                        });
+                auto const send_e = [&e](MirSurface* surface)
+                    { surface->handle_event(e); };
+
+                switch (e.type)
+                {
+                case mir_event_type_surface:
+                    surface_map->with_surface_do(e.surface.id, send_e);
+                    break;
+
+                case mir_event_type_resize:
+                    surface_map->with_surface_do(e.resize.surface_id, send_e);
+                    break;
+
+                case mir_event_type_orientation:
+                    surface_map->with_surface_do(e.orientation.surface_id, send_e);
+                    break;
+
+                default:
+                    event_sink->handle_event(e);
                 }
             }
             else

=== modified file 'src/server/scene/basic_surface.cpp'
--- src/server/scene/basic_surface.cpp	2014-06-12 15:35:08 +0000
+++ src/server/scene/basic_surface.cpp	2014-06-19 14:21:51 +0000
@@ -88,6 +88,14 @@
         p->alpha_set_to(alpha);
 }
 
+void ms::SurfaceObservers::orientation_set_to(MirOrientation orientation)
+{
+    std::unique_lock<decltype(mutex)> lock(mutex);
+    // TBD Maybe we should copy observers so we can release the lock?
+    for (auto const& p : observers)
+        p->orientation_set_to(orientation);
+}
+
 void ms::SurfaceObservers::transformation_set_to(glm::mat4 const& t)
 {
     std::unique_lock<decltype(mutex)> lock(mutex);
@@ -340,6 +348,10 @@
     observers.alpha_set_to(alpha);
 }
 
+void ms::BasicSurface::set_orientation(MirOrientation orientation)
+{
+    observers.orientation_set_to(orientation);
+}
 
 void ms::BasicSurface::set_transformation(glm::mat4 const& t)
 {

=== modified file 'src/server/scene/basic_surface.h'
--- src/server/scene/basic_surface.h	2014-06-12 15:35:08 +0000
+++ src/server/scene/basic_surface.h	2014-06-19 14:21:51 +0000
@@ -64,6 +64,7 @@
     void hidden_set_to(bool hide) override;
     void frame_posted(int frames_available) override;
     void alpha_set_to(float alpha) override;
+    void orientation_set_to(MirOrientation orientation) override;
     void transformation_set_to(glm::mat4 const& t) override;
     void reception_mode_set_to(input::InputReceptionMode mode) override;
     void cursor_image_set_to(graphics::CursorImage const& image) override;
@@ -121,6 +122,7 @@
     geometry::Rectangle input_bounds() const override;
     bool input_area_contains(geometry::Point const& point) const override;
     void set_alpha(float alpha) override;
+    void set_orientation(MirOrientation orientation) override;
     void set_transformation(glm::mat4 const&) override;
 
     bool visible() const;

=== modified file 'src/server/scene/legacy_surface_change_notification.cpp'
--- src/server/scene/legacy_surface_change_notification.cpp	2014-06-12 15:35:08 +0000
+++ src/server/scene/legacy_surface_change_notification.cpp	2014-06-19 14:21:51 +0000
@@ -56,6 +56,11 @@
     notify_scene_change();
 }
 
+// An orientation change alone is not enough to trigger recomposition.
+void ms::LegacySurfaceChangeNotification::orientation_set_to(MirOrientation /*orientation*/)
+{
+}
+
 void ms::LegacySurfaceChangeNotification::transformation_set_to(glm::mat4 const& /*t*/)
 {
     notify_scene_change();

=== modified file 'src/server/scene/legacy_surface_change_notification.h'
--- src/server/scene/legacy_surface_change_notification.h	2014-06-12 15:35:08 +0000
+++ src/server/scene/legacy_surface_change_notification.h	2014-06-19 14:21:51 +0000
@@ -42,6 +42,7 @@
     void hidden_set_to(bool /*hide*/) override;
     void frame_posted(int frames_available) override;
     void alpha_set_to(float /*alpha*/) override;
+    void orientation_set_to(MirOrientation orientation) override;
     void transformation_set_to(glm::mat4 const& /*t*/) override;
     void attrib_changed(MirSurfaceAttrib, int) override;
     void reception_mode_set_to(input::InputReceptionMode mode) override;

=== modified file 'src/server/scene/null_surface_observer.cpp'
--- src/server/scene/null_surface_observer.cpp	2014-06-12 15:35:08 +0000
+++ src/server/scene/null_surface_observer.cpp	2014-06-19 14:21:51 +0000
@@ -27,6 +27,7 @@
 void ms::NullSurfaceObserver::hidden_set_to(bool /*hide*/) {}
 void ms::NullSurfaceObserver::frame_posted(int /*frames_available*/) {}
 void ms::NullSurfaceObserver::alpha_set_to(float /*alpha*/) {}
+void ms::NullSurfaceObserver::orientation_set_to(MirOrientation /*orientation*/) {}
 void ms::NullSurfaceObserver::transformation_set_to(glm::mat4 const& /*t*/) {}
 void ms::NullSurfaceObserver::reception_mode_set_to(input::InputReceptionMode /*mode*/) {}
 void ms::NullSurfaceObserver::cursor_image_set_to(mg::CursorImage const& /*image*/) {}

=== modified file 'src/server/scene/surface_event_source.cpp'
--- src/server/scene/surface_event_source.cpp	2014-04-15 05:31:19 +0000
+++ src/server/scene/surface_event_source.cpp	2014-06-19 14:21:51 +0000
@@ -62,3 +62,15 @@
 
     event_sink->handle_event(e);
 }
+
+void ms::SurfaceEventSource::orientation_set_to(MirOrientation orientation)
+{
+    MirEvent e;
+    memset(&e, 0, sizeof e);
+
+    e.type = mir_event_type_orientation;
+    e.orientation.surface_id = id.as_value();
+    e.orientation.direction = orientation;
+
+    event_sink->handle_event(e);
+}

=== modified file 'tests/acceptance-tests/CMakeLists.txt'
--- tests/acceptance-tests/CMakeLists.txt	2014-06-14 19:52:52 +0000
+++ tests/acceptance-tests/CMakeLists.txt	2014-06-19 14:21:51 +0000
@@ -17,6 +17,7 @@
   clients.cpp
   test_client_library.cpp
   test_client_library_old.cpp
+  test_client_surface_events.cpp
   test_custom_input_dispatcher.cpp
   test_client_surfaces.cpp
   test_test_framework.cpp

=== modified file 'tests/acceptance-tests/test_client_library.cpp'
--- tests/acceptance-tests/test_client_library.cpp	2014-06-03 10:26:58 +0000
+++ tests/acceptance-tests/test_client_library.cpp	2014-06-19 14:21:51 +0000
@@ -313,73 +313,6 @@
     mir_connection_release(connection);
 }
 
-TEST_F(ClientLibrary, receives_surface_state_events)
-{
-    connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
-
-    MirSurfaceParameters const request_params =
-    {
-        __PRETTY_FUNCTION__,
-        640, 480,
-        mir_pixel_format_abgr_8888,
-        mir_buffer_usage_hardware,
-        mir_display_output_id_invalid
-    };
-
-    MirEventDelegate delegate{&event_callback, this};
-    MirSurface* other_surface = mir_connection_create_surface_sync(connection, &request_params);
-    ASSERT_TRUE(mir_surface_is_valid(other_surface));
-
-    mir_surface_set_event_handler(other_surface, nullptr);
-
-    surface = mir_connection_create_surface_sync(connection, &request_params);
-    ASSERT_TRUE(mir_surface_is_valid(surface));
-
-    mir_surface_set_event_handler(surface, &delegate);
-
-    int surface_id = mir_debug_surface_id(surface);
-
-    mir_wait_for(mir_surface_set_state(surface, mir_surface_state_fullscreen));
-    mir_wait_for(mir_surface_set_state(other_surface, mir_surface_state_minimized));
-    EXPECT_THAT(last_event_surface, Eq(surface));
-    EXPECT_THAT(last_event.type, Eq(mir_event_type_surface));
-    EXPECT_THAT(last_event.surface.id, Eq(surface_id));
-    EXPECT_THAT(last_event.surface.attrib, Eq(mir_surface_attrib_state));
-    EXPECT_THAT(last_event.surface.value, Eq(mir_surface_state_fullscreen));
-
-    mir_wait_for(mir_surface_set_state(surface, static_cast<MirSurfaceState>(999)));
-    EXPECT_THAT(last_event_surface, Eq(surface));
-    EXPECT_THAT(last_event.type, Eq(mir_event_type_surface));
-    EXPECT_THAT(last_event.surface.id, Eq(surface_id));
-    EXPECT_THAT(last_event.surface.attrib, Eq(mir_surface_attrib_state));
-    EXPECT_THAT(last_event.surface.value, Eq(mir_surface_state_fullscreen));
-
-    memset(&last_event, 0, sizeof last_event);
-    last_event_surface = nullptr;
-
-    mir_wait_for(mir_surface_set_state(surface, mir_surface_state_minimized));
-    EXPECT_THAT(last_event_surface, Eq(surface));
-    EXPECT_THAT(last_event.type, Eq(mir_event_type_surface));
-    EXPECT_THAT(last_event.surface.id, Eq(surface_id));
-    EXPECT_THAT(last_event.surface.attrib, Eq(mir_surface_attrib_state));
-    EXPECT_THAT(last_event.surface.value, Eq(mir_surface_state_minimized));
-
-    memset(&last_event, 0, sizeof last_event);
-    last_event_surface = nullptr;
-
-    mir_wait_for(mir_surface_set_state(surface, static_cast<MirSurfaceState>(777)));
-    mir_wait_for(mir_surface_set_state(other_surface, mir_surface_state_maximized));
-    EXPECT_THAT(last_event_surface, IsNull());
-    EXPECT_THAT(last_event.type, Eq(0));
-    EXPECT_THAT(last_event.surface.id, Eq(0));
-    EXPECT_THAT(last_event.surface.attrib, Eq(0));
-    EXPECT_THAT(last_event.surface.value, Eq(0));
-
-    mir_surface_release_sync(surface);
-    mir_surface_release_sync(other_surface);
-    mir_connection_release(connection);
-}
-
 TEST_F(ClientLibrary, receives_surface_dpi_value)
 {
     connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);

=== added file 'tests/acceptance-tests/test_client_surface_events.cpp'
--- tests/acceptance-tests/test_client_surface_events.cpp	1970-01-01 00:00:00 +0000
+++ tests/acceptance-tests/test_client_surface_events.cpp	2014-06-19 14:21:51 +0000
@@ -0,0 +1,236 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Nick Dedekind <[e-mail]>
+ */
+
+#include "mir_toolkit/mir_client_library.h"
+#include "mir_toolkit/mir_client_library_debug.h"
+
+#include "mir/shell/surface_coordinator_wrapper.h"
+
+#include "mir/scene/surface.h"
+#include "mir/scene/surface_creation_parameters.h"
+
+#include "mir_test_framework/stubbed_server_configuration.h"
+#include "mir_test_framework/basic_client_server_fixture.h"
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include <condition_variable>
+#include <chrono>
+#include <mutex>
+
+namespace mtf = mir_test_framework;
+namespace ms = mir::scene;
+namespace msh = mir::shell;
+
+using namespace testing;
+
+namespace
+{
+struct MockSurfaceCoordinator : msh::SurfaceCoordinatorWrapper
+{
+    MockSurfaceCoordinator(std::shared_ptr<ms::SurfaceCoordinator> const& wrapped) :
+        msh::SurfaceCoordinatorWrapper(wrapped)
+    {
+    }
+
+    std::shared_ptr<ms::Surface> add_surface(
+        ms::SurfaceCreationParameters const& params,
+        ms::Session* session) override
+    {
+        latest_surface = wrapped->add_surface(params, session);
+        return latest_surface;
+    }
+
+    std::shared_ptr<ms::Surface> latest_surface;
+};
+
+struct MyConfig : mtf::StubbedServerConfiguration
+{
+    std::shared_ptr<ms::SurfaceCoordinator> wrap_surface_coordinator(
+        std::shared_ptr<ms::SurfaceCoordinator> const& wrapped) override
+    {
+        auto const msc = std::make_shared<MockSurfaceCoordinator>(wrapped);
+        mock_surface_coordinator = msc;
+        return msc;
+    }
+
+    std::shared_ptr<MockSurfaceCoordinator> the_mock_surface_coordinator() const
+    {
+        return mock_surface_coordinator.lock();
+    }
+
+    std::shared_ptr<ms::Surface> the_latest_surface() const
+    {
+        return the_mock_surface_coordinator()->latest_surface;
+    }
+
+    std::weak_ptr<MockSurfaceCoordinator> mock_surface_coordinator;
+};
+
+using BasicClientServerFixture = mtf::BasicClientServerFixture<MyConfig>;
+
+struct ClientSurfaceEvents : BasicClientServerFixture
+{
+    MirSurfaceParameters const request_params
+    {
+        __FILE__,
+        640, 480,
+        mir_pixel_format_abgr_8888,
+        mir_buffer_usage_hardware,
+        mir_display_output_id_invalid
+    };
+
+    MirSurface* surface{nullptr};
+    MirSurface* other_surface;
+
+    std::mutex last_event_mutex;
+    std::condition_variable last_event_cv;
+    MirEvent last_event{};
+    MirSurface* last_event_surface = nullptr;
+    MirEventDelegate delegate{&event_callback, this};
+
+    std::shared_ptr<ms::Surface> scene_surface;
+
+    static void event_callback(MirSurface* surface, MirEvent const* event, void* ctx)
+    {
+        ClientSurfaceEvents* self = static_cast<ClientSurfaceEvents*>(ctx);
+        std::lock_guard<decltype(self->last_event_mutex)> last_event_lock{self->last_event_mutex};
+        self->last_event = *event;
+        self->last_event_surface = surface;
+        self->last_event_cv.notify_one();
+    }
+
+    bool wait_for_event(MirEventType type, std::chrono::milliseconds delay)
+    {
+        std::unique_lock<decltype(last_event_mutex)> last_event_lock{last_event_mutex};
+        return last_event_cv.wait_for(last_event_lock, delay,
+            [&] { return !!last_event_surface && last_event.type == type; });
+    }
+
+    void reset_last_event()
+    {
+        std::lock_guard<decltype(last_event_mutex)> last_event_lock{last_event_mutex};
+        memset(&last_event, 0, sizeof last_event);
+        last_event_surface = nullptr;
+    }
+
+    void SetUp() override
+    {
+        BasicClientServerFixture::SetUp();
+
+        surface = mir_connection_create_surface_sync(connection, &request_params);
+        mir_surface_set_event_handler(surface, &delegate);
+
+        scene_surface = server_configuration.the_latest_surface();
+
+        other_surface = mir_connection_create_surface_sync(connection, &request_params);
+        mir_surface_set_event_handler(other_surface, nullptr);
+
+        reset_last_event();
+    }
+
+    void TearDown() override
+    {
+        mir_surface_release_sync(other_surface);
+        mir_surface_release_sync(surface);
+
+        BasicClientServerFixture::TearDown();
+    }
+};
+}
+
+TEST_F(ClientSurfaceEvents, surface_receives_state_events)
+{
+    int surface_id = mir_debug_surface_id(surface);
+
+    {
+        mir_wait_for(mir_surface_set_state(surface, mir_surface_state_fullscreen));
+        mir_wait_for(mir_surface_set_state(other_surface, mir_surface_state_minimized));
+
+        std::lock_guard<decltype(last_event_mutex)> last_event_lock{last_event_mutex};
+
+        EXPECT_THAT(last_event_surface, Eq(surface));
+        EXPECT_THAT(last_event.type, Eq(mir_event_type_surface));
+        EXPECT_THAT(last_event.surface.id, Eq(surface_id));
+        EXPECT_THAT(last_event.surface.attrib, Eq(mir_surface_attrib_state));
+        EXPECT_THAT(last_event.surface.value, Eq(mir_surface_state_fullscreen));
+    }
+
+    {
+        mir_wait_for(mir_surface_set_state(surface, static_cast<MirSurfaceState>(999)));
+
+        std::lock_guard<decltype(last_event_mutex)> last_event_lock{last_event_mutex};
+
+        EXPECT_THAT(last_event_surface, Eq(surface));
+        EXPECT_THAT(last_event.type, Eq(mir_event_type_surface));
+        EXPECT_THAT(last_event.surface.id, Eq(surface_id));
+        EXPECT_THAT(last_event.surface.attrib, Eq(mir_surface_attrib_state));
+        EXPECT_THAT(last_event.surface.value, Eq(mir_surface_state_fullscreen));
+    }
+
+    reset_last_event();
+
+    {
+        mir_wait_for(mir_surface_set_state(surface, mir_surface_state_minimized));
+
+        std::lock_guard<decltype(last_event_mutex)> last_event_lock{last_event_mutex};
+
+        EXPECT_THAT(last_event_surface, Eq(surface));
+        EXPECT_THAT(last_event.type, Eq(mir_event_type_surface));
+        EXPECT_THAT(last_event.surface.id, Eq(surface_id));
+        EXPECT_THAT(last_event.surface.attrib, Eq(mir_surface_attrib_state));
+        EXPECT_THAT(last_event.surface.value, Eq(mir_surface_state_minimized));
+    }
+
+    reset_last_event();
+
+    {
+        mir_wait_for(mir_surface_set_state(surface, static_cast<MirSurfaceState>(777)));
+        mir_wait_for(mir_surface_set_state(other_surface, mir_surface_state_maximized));
+
+        std::lock_guard<decltype(last_event_mutex)> last_event_lock{last_event_mutex};
+
+        EXPECT_THAT(last_event_surface, IsNull());
+        EXPECT_THAT(last_event.type, Eq(0));
+        EXPECT_THAT(last_event.surface.id, Eq(0));
+        EXPECT_THAT(last_event.surface.attrib, Eq(0));
+        EXPECT_THAT(last_event.surface.value, Eq(0));
+    }
+}
+
+struct OrientationEvents : ClientSurfaceEvents, ::testing::WithParamInterface<MirOrientation> {};
+
+TEST_P(OrientationEvents, surface_receives_orientation_events)
+{
+    auto const direction = GetParam();
+
+    scene_surface->set_orientation(direction);
+
+    EXPECT_TRUE(wait_for_event(mir_event_type_orientation, std::chrono::seconds(1)));
+
+    std::lock_guard<decltype(last_event_mutex)> last_event_lock{last_event_mutex};
+
+    EXPECT_THAT(last_event_surface, Eq(surface));
+    EXPECT_THAT(last_event.type, Eq(mir_event_type_orientation));
+    EXPECT_THAT(last_event.orientation.direction, Eq(direction));
+}
+
+INSTANTIATE_TEST_CASE_P(ClientSurfaceEvents,
+    OrientationEvents,
+    Values(mir_orientation_normal, mir_orientation_left, mir_orientation_inverted, mir_orientation_right));