Merge lp:~mir-team/mir/allow-lock-surf-orientation into lp:mir

[Alberto Aguirre]

Add client API to set the preferred orientation of a surface.

This is now basically a big setter/getter MP.

The shell can reject the preference request through SurfaceConfigurator->select_attribute_value.

Mir does not do anything with this value currently. It's up to each shell implementation to deal with device orientation changes; such implementations may query the preferred orientation for each surface and decide to ignore or honour the preference, calling surface->set_orientation(...) accordingly.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2098
http://jenkins.qa.ubuntu.com/job/mir-ci/2274/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/367
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/367
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/341
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/263
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/263/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/341
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/341/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3584
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/16293

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2274/rebuild

review: Approve (continuous-integration)

[Andreas Pokorny (andreas-pokorny)]

lgtm

review: Approve

[Alexandros Frantzis (afrantzis)]

Too bad MirOrientation values are not bit flags. Would it be preferable to change the MirOrientation values instead of introducing a new enum?

Needs discussion

review: Needs Information

[Gerry Boland (gerboland)]

I think it should be up to the shell to decide whether to honour an client orientation restrictions. There are many cases in our converged world where the client's request cannot be honoured - one example is tablet in landscape, so it has a landscape main stage application open, and a sidestage app. If that sidestage app wants to be landscape too - shell cannot let it, it must remain portrait.

I'm a little worried about adding this feature to the mir client API if it's not something we were planning to support as a platform. E.g. we're adding orientation support to shell where it sets the preferred orientations in the desktop file, and these cannot be altered at runtime - design and the UITK guys were happy enough with this.

Sure I can see advantages to runtime alterations, but could we check with design & uitk to see if they really want this?

review: Needs Information

[Alan Griffiths (alan-griffiths)]

I think that we ought to either change the MirOrientation values to bitflags or abstract out the necessary conversions.

*Needs Discussion*

I agree with Gerry that setting the "allowed orientations" is a request, not a requirement and that the shell could refuse.

I also wonder if this is something that should be set on the surface spec and applied at creation (or mutation) time rather than being an attribute on the surface that can be set at arbitrary times.

review: Needs Information

[Gerry Boland (gerboland)]

I'm just out of a meeting with UITK folk, and they could find use-cases to justify runtime changing of preferred orientations. A Netflix would be a big user of this feature - on phone list of video would ideally be in portrait, but playback would be best in landscape. So yeah this is good.

Still needs to be a request shell can deny if necessary though.

[Alberto Aguirre (albaguirre)]

I introduced the new enum to avoid breaking client ABI. If the team is ok breaking ABI for this, then no problem changing it.

The shell/server is given a change to reject the setting with SurfaceConfigurator->select_attribute_value, just return the appropriate value for the mir_surface_attrib_permitted_orientations attribute.

[Robert Carr (robertcarr)]

As discussed in the stand-up I think I am ok with breaking ABI.

If we don't break ABI, maybe lets think of a better name? I think "permit" misphrases the balance
of power here, rather the client declares it's "Supported" orientations.

Diff looks fine though and name is reasonable enough so +1 if you end up deciding to land it this way.

Nit:

40 + * set the orientations a surface is allowed to change into.
41 + * The server may reject the request so you may have to wait on the function
42 + * and check the result using mir_surface_get_permitted_orientations.

This reads weird imo, I think the object is unclear and the first sentence, and the second one becomes run on due to repetition of a clause

"Request to set the orientations which a surface supports. This request may be rejected by the server, so it is necessary to wait on the result and check the applied value using mir_surface_get_permitted_orientations"

Rest of diff looks fine

review: Approve

[Chris Halse Rogers (raof)]

+1 for mir_surface_{get,set}_supported_orientations.

Nit:
178 +MirPermittedOrientations mir_surface_get_permitted_orientations(MirSurface *surf)
...
184 + if (surf)
185 + {

Incorrect precondition check. If you're going to do a precondition check it should be mir_surface_is_valid(surf), and it should abort() if the precondition fails.

[Chris Halse Rogers (raof)]

Also - what is the expectation if you call mir_surface_set_supported_orientations() and don't include the *current* orientation in the set of supported orientations?

It's technically possible to change MirOrientation to a bitfield without changing ABI by the magic of symbol versioning - since mir_surface_get_orientation is the only client-visible user of MirOrientation you'd just need to provide a translation shim for that one function.

I guess I'm suggesting that MirPermittedOrientations shouldn't exist :)

[Alan Griffiths (alan-griffiths)]

Do we have the right "orientation" here?

I would guess (with insufficient data) that most apps that restrict supported orientations care more about landscape/portrait than they do about the angle from the "natural" display orientation. Forcing them to discover screen metrics before requesting orientation restrictions seems perverse.

[Gerry Boland (gerboland)]

> Also - what is the expectation if you call
> mir_surface_set_supported_orientations() and don't include the *current*
> orientation in the set of supported orientations?

That's the apps way to ask for shell to change orientation to suit it. Usecase: youtube app, list of videos portrait, but playing video should be landscape. So app would change its preferred orientations from [portrait] to [landscape].

Of course shell may not respect that immediately (e.g. user interacting with Indicators at the time), or at all (tablet side stage)

> It's technically possible to change MirOrientation to a bitfield without
> changing ABI by the magic of symbol versioning - since
> mir_surface_get_orientation is the only client-visible user of MirOrientation
> you'd just need to provide a translation shim for that one function.
>
> I guess I'm suggesting that MirPermittedOrientations shouldn't exist :)

/me didn't follow

[Alberto Aguirre (albaguirre)]

> Also - what is the expectation if you call
> mir_surface_set_supported_orientations() and don't include the *current*
> orientation in the set of supported orientations?
>
> It's technically possible to change MirOrientation to a bitfield without
> changing ABI by the magic of symbol versioning - since
> mir_surface_get_orientation is the only client-visible user of MirOrientation
> you'd just need to provide a translation shim for that one function.

One also needs to translate the orientation values for the event handler during orientation events.

>
> I guess I'm suggesting that MirPermittedOrientations shouldn't exist :)

Right the options were break the ABI and change the existing MirOrientation enum, introduce a new enum and avoid symbol magic or use symbol magic and maintain backwards compatibility.

I've done the latter locally (symbol magic) but the results in oddness in testing and because of the event handler, MirSurface then needs to know whether to translate the values or not depending on the client symbol version that setup the handler.

[Alberto Aguirre (albaguirre)]

> Do we have the right "orientation" here?
>
> I would guess (with insufficient data) that most apps that restrict supported
> orientations care more about landscape/portrait than they do about the angle
> from the "natural" display orientation. Forcing them to discover screen
> metrics before requesting orientation restrictions seems perverse.

One use case I was thinking about was a client may want to keep a surface in a specific orientation relative to a sensor location - say for example a flute app that requires you to blow into the mic, but perhaps if an application already has a list of sensor locations per phone model, then they should know the "natural" orientation of the screen - so portrait/landscape make sense.

So perhaps they SHOULD be separate - leave MirOrientation as is and introduce:

MirOrientationMode
{
    mir_orientation_mode_portrait = 1 << 1,
    mir_orientation_mode_landscape = 1 << 2,
    mir_orientation_mode_inverted_portrait = 1 << 3,
    mir_orientation_mode_inverted_landscape = 1 << 4,
    mir_orientation_mode_portrait_any = mir_orientation_mode_portrait | mir_orientation_mode_inverted_portrait,
    mir_orientation_mode_landscape_any = mir_orientation_mode_landscape | mir_orientation_mode_inverted_landscape,
    mir_orientation_mode_any = mir_orientation_mode_portrait_any | mir_orientation_mode_landscape_any
}

MirWaitHandle* mir_surface_set_preferred_orientation(MirSurface *surface, MirOrientationMode orientation_mode)

MirOrientationMode mir_surface_get_preferred_orientation(MirSurface *surface)

[Cemil Azizoglu (cemil-azizoglu)]

Awaiting final version for review. For now I have the following nits :

It'd make sense to base this MP on ~vanvugt/mir/named-attrib-values
---------------------------
82 + mir_permitted_orientation_all = mir_permitted_orientation_normal |
83 + mir_permitted_orientation_left |
84 + mir_permitted_orientation_right |
85 + mir_permitted_orientation_inverted,
Would make more sense if mir_permitted_orientation_all were at the end.
---------------------------
180 + MirPermittedOrientations perm_orientations = mir_permitted_orientation_all;
Would it not make more sense to restrict the client to something like mir_permitted_orientation_normal if this function were to throw?

[Alberto Aguirre (albaguirre)]

> +1 for mir_surface_{get,set}_supported_orientations.
>
> Nit:
> 178 +MirPermittedOrientations
> mir_surface_get_permitted_orientations(MirSurface *surf)
> ...
> 184 + if (surf)
> 185 + {
>
> Incorrect precondition check. If you're going to do a precondition check it
> should be mir_surface_is_valid(surf), and it should abort() if the
> precondition fails.

OK, I guess we ought to fix all the other function preconditions checks as well as this was copy/pasta.

[Alexandros Frantzis (afrantzis)]

Looks good.

67 + MirOrientationMode mode = mir_orientation_mode_any;

I'd slightly prefer we had a mir_orientation_mode_unknown value and return that on error. Having said that, if surf->attrib() throws then something is very wrong, and it probably won't matter that we may have misinformed the client.

review: Approve

[Alan Griffiths (alan-griffiths)]

Trunk has changed

<<<<<<< TREE
=======
void ms::BasicSurface::initialize_attributes()
{
    std::lock_guard<std::mutex> lg(guard);

    attrib_values[mir_surface_attrib_type] = mir_surface_type_normal;
    attrib_values[mir_surface_attrib_state] = mir_surface_state_restored;
    attrib_values[mir_surface_attrib_swapinterval] = 1;
    attrib_values[mir_surface_attrib_focus] = mir_surface_unfocused;
    attrib_values[mir_surface_attrib_dpi] = 0;
    attrib_values[mir_surface_attrib_visibility] = mir_surface_visibility_exposed;
    attrib_values[mir_surface_attrib_preferred_orientation] = mir_orientation_mode_any;
}

>>>>>>> MERGE-SOURCE

review: Needs Fixing

[Kevin DuBois (kdub)]

lgtm, apart from the needed update.

review: Approve

[Gerry Boland (gerboland)]

review: Approve

[Alan Griffiths (alan-griffiths)]

OK

review: Approve

[Laurie Bradshaw (lauriebradshaw)]

I've been trying to integrate with the system rotation instead of doing it myself for neverball, neverputt and tuxracer.

mir_surface_set_preferred_orientation doesn't seem to do anything on my bq aquaris, with or without X-Ubuntu-Supported-Orientations in the desktop file, is this due to missing shell support?

=== modified file 'client-ABI-sha1sums'
--- client-ABI-sha1sums	2014-12-05 03:26:03 +0000
+++ client-ABI-sha1sums	2014-12-05 22:17:20 +0000
@@ -5,11 +5,11 @@
 1ef8f51a3e3f8d1559266c5af58fbfde7cfabf0a  include/client/mir_toolkit/mir_cursor_configuration.h
 9d50df5a141ca03ee8a79f7e844ed4b8b3b7d5d3  include/client/mir_toolkit/mir_prompt_session.h
 21d07e655e85eeec8a3523e1c6f9c2252176ec01  include/client/mir_toolkit/mir_screencast.h
-b9a11709737d6178c4bf30d067922ca823957045  include/client/mir_toolkit/mir_surface.h
+42bcad3a72b47f4d904e6b7033b0001b89cb9779  include/client/mir_toolkit/mir_surface.h
 b141c4d79802ad626d969249c0004744e5c2a525  include/client/mir_toolkit/mir_wait.h
 c367f5c9285936820fbd50961827d6ce58fb7ff4  include/client/mir_toolkit/version.h
 1863a7fe3a382061f2752c0236cc971e7e68f93f  include/common/mir_toolkit/client_types.h
-2100c0674d9d882c1845550847357f6a5de5af66  include/common/mir_toolkit/common.h
+3e85c6d9dd1dca9767ea8bdf13ea6bbd1cb11c30  include/common/mir_toolkit/common.h
 fce4c1a9e0d037244f7e9e96ea2d8eaab4fc404c  include/common/mir_toolkit/cursors.h
 bccde9cdbc2f0d07e186c2950ef88d3a7d5f0d3b  include/common/mir_toolkit/event.h
 4975998aa1056ed0d39dcc538127453e516ad8e9  include/common/mir_toolkit/mesa/native_display.h
 
=== modified file 'common-ABI-sha1sums'
--- common-ABI-sha1sums	2014-12-05 03:26:03 +0000
+++ common-ABI-sha1sums	2014-12-05 22:17:20 +0000
@@ -15,7 +15,7 @@
 2fd7274766a92f22e8784cb64af91947ba03d0de  include/common/mir/logging/logger.h
 9ae8473df05dd9e048a73797f01a2f34f7447554  include/common/mir/time/types.h
 1863a7fe3a382061f2752c0236cc971e7e68f93f  include/common/mir_toolkit/client_types.h
-2100c0674d9d882c1845550847357f6a5de5af66  include/common/mir_toolkit/common.h
+3e85c6d9dd1dca9767ea8bdf13ea6bbd1cb11c30  include/common/mir_toolkit/common.h
 fce4c1a9e0d037244f7e9e96ea2d8eaab4fc404c  include/common/mir_toolkit/cursors.h
 bccde9cdbc2f0d07e186c2950ef88d3a7d5f0d3b  include/common/mir_toolkit/event.h
 4975998aa1056ed0d39dcc538127453e516ad8e9  include/common/mir_toolkit/mesa/native_display.h
 
=== modified file 'include/client/mir_toolkit/mir_surface.h'
--- include/client/mir_toolkit/mir_surface.h	2014-12-05 03:26:03 +0000
+++ include/client/mir_toolkit/mir_surface.h	2014-12-05 22:17:20 +0000
@@ -408,6 +408,23 @@
  */
 MirOrientation mir_surface_get_orientation(MirSurface *surface);
 
+/**
+ * Request to set the preferred orientations of a surface.
+ * The request may be rejected by the server; to check wait on the
+ * result and check the applied value using mir_surface_get_preferred_orientation
+ *   \param [in] surface     The surface to operate on
+ *   \param [in] orientation The preferred orientation modes
+ *   \return                 A wait handle that can be passed to mir_wait_for
+ */
+MirWaitHandle* mir_surface_set_preferred_orientation(MirSurface *surface, MirOrientationMode orientation);
+
+/**
+ * Get the preferred orientation modes of a surface.
+ *   \param [in] surface  The surface to query
+ *   \return              The preferred orientation modes
+ */
+MirOrientationMode mir_surface_get_preferred_orientation(MirSurface *surface);
+
 #ifdef __cplusplus
 }
 /**@}*/
 
=== modified file 'include/common/mir_toolkit/common.h'
--- include/common/mir_toolkit/common.h	2014-12-05 03:26:03 +0000
+++ include/common/mir_toolkit/common.h	2014-12-05 22:17:20 +0000
@@ -42,6 +42,7 @@
     mir_surface_attrib_focus,
     mir_surface_attrib_dpi,
     mir_surface_attrib_visibility,
+    mir_surface_attrib_preferred_orientation,
     /* Must be last */
     mir_surface_attribs
 } MirSurfaceAttrib;
@@ -143,6 +144,20 @@
     mir_orientation_right = 270
 } MirOrientation;
 
+typedef enum MirOrientationMode
+{
+    mir_orientation_mode_portrait = 1 << 0,
+    mir_orientation_mode_landscape = 1 << 1,
+    mir_orientation_mode_portrait_inverted = 1 << 2,
+    mir_orientation_mode_landscape_inverted = 1 << 3,
+    mir_orientation_mode_portrait_any = mir_orientation_mode_portrait |
+                                        mir_orientation_mode_portrait_inverted,
+    mir_orientation_mode_landscape_any = mir_orientation_mode_landscape |
+                                         mir_orientation_mode_landscape_inverted,
+    mir_orientation_mode_any = mir_orientation_mode_portrait_any |
+                               mir_orientation_mode_landscape_any
+} MirOrientationMode;
+
 /**@}*/
 
 #endif
 
=== modified file 'platform-ABI-sha1sums'
--- platform-ABI-sha1sums	2014-12-05 03:26:03 +0000
+++ platform-ABI-sha1sums	2014-12-05 22:17:20 +0000
@@ -15,7 +15,7 @@
 2fd7274766a92f22e8784cb64af91947ba03d0de  include/common/mir/logging/logger.h
 9ae8473df05dd9e048a73797f01a2f34f7447554  include/common/mir/time/types.h
 1863a7fe3a382061f2752c0236cc971e7e68f93f  include/common/mir_toolkit/client_types.h
-2100c0674d9d882c1845550847357f6a5de5af66  include/common/mir_toolkit/common.h
+3e85c6d9dd1dca9767ea8bdf13ea6bbd1cb11c30  include/common/mir_toolkit/common.h
 fce4c1a9e0d037244f7e9e96ea2d8eaab4fc404c  include/common/mir_toolkit/cursors.h
 bccde9cdbc2f0d07e186c2950ef88d3a7d5f0d3b  include/common/mir_toolkit/event.h
 4975998aa1056ed0d39dcc538127453e516ad8e9  include/common/mir_toolkit/mesa/native_display.h
 
=== modified file 'server-ABI-sha1sums'
--- server-ABI-sha1sums	2014-12-05 03:26:03 +0000
+++ server-ABI-sha1sums	2014-12-05 22:17:20 +0000
@@ -15,7 +15,7 @@
 2fd7274766a92f22e8784cb64af91947ba03d0de  include/common/mir/logging/logger.h
 9ae8473df05dd9e048a73797f01a2f34f7447554  include/common/mir/time/types.h
 1863a7fe3a382061f2752c0236cc971e7e68f93f  include/common/mir_toolkit/client_types.h
-2100c0674d9d882c1845550847357f6a5de5af66  include/common/mir_toolkit/common.h
+3e85c6d9dd1dca9767ea8bdf13ea6bbd1cb11c30  include/common/mir_toolkit/common.h
 fce4c1a9e0d037244f7e9e96ea2d8eaab4fc404c  include/common/mir_toolkit/cursors.h
 bccde9cdbc2f0d07e186c2950ef88d3a7d5f0d3b  include/common/mir_toolkit/event.h
 4975998aa1056ed0d39dcc538127453e516ad8e9  include/common/mir_toolkit/mesa/native_display.h
 
=== modified file 'src/client/mir_surface.cpp'
--- src/client/mir_surface.cpp	2014-12-05 03:26:03 +0000
+++ src/client/mir_surface.cpp	2014-12-05 22:17:20 +0000
@@ -501,6 +501,7 @@
         case mir_surface_attrib_focus:
         case mir_surface_attrib_swapinterval:
         case mir_surface_attrib_dpi:
+        case mir_surface_attrib_preferred_orientation:
             if (configure_result.has_ivalue())
                 attrib_cache[a] = configure_result.ivalue();
             else
@@ -607,3 +608,9 @@
 
     return orientation;
 }
+
+MirWaitHandle* MirSurface::set_preferred_orientation(MirOrientationMode mode)
+{
+    return configure(mir_surface_attrib_preferred_orientation, mode);
+}
+
 
=== modified file 'src/client/mir_surface.h'
--- src/client/mir_surface.h	2014-12-05 03:26:03 +0000
+++ src/client/mir_surface.h	2014-12-05 22:17:20 +0000
@@ -121,7 +121,8 @@
     int attrib(MirSurfaceAttrib a) const;
 
     MirOrientation get_orientation() const;
-    
+    MirWaitHandle* set_preferred_orientation(MirOrientationMode mode);
+
     MirWaitHandle* configure_cursor(MirCursorConfiguration const* cursor);
 
     void set_event_handler(MirEventDelegate const* delegate);
 
=== modified file 'src/client/mir_surface_api.cpp'
--- src/client/mir_surface_api.cpp	2014-12-05 03:26:03 +0000
+++ src/client/mir_surface_api.cpp	2014-12-05 22:17:20 +0000
@@ -435,3 +435,36 @@
 
     return result;
 }
+
+MirOrientationMode mir_surface_get_preferred_orientation(MirSurface *surf)
+{
+    if (!mir_surface_is_valid(surf)) abort();
+
+    MirOrientationMode mode = mir_orientation_mode_any;
+
+    try
+    {
+        mode = static_cast<MirOrientationMode>(surf->attrib(mir_surface_attrib_preferred_orientation));
+    }
+    catch (...)
+    {
+    }
+
+    return mode;
+}
+
+MirWaitHandle* mir_surface_set_preferred_orientation(MirSurface *surf, MirOrientationMode mode)
+{
+    if (!mir_surface_is_valid(surf)) abort();
+
+    MirWaitHandle *result{nullptr};
+    try
+    {
+        result = surf->set_preferred_orientation(mode);
+    }
+    catch (...)
+    {
+    }
+
+    return result;
+}
 
=== modified file 'src/server/scene/basic_surface.cpp'
--- src/server/scene/basic_surface.cpp	2014-12-05 03:26:03 +0000
+++ src/server/scene/basic_surface.cpp	2014-12-05 22:17:20 +0000
@@ -465,6 +465,25 @@
     return new_state;
 }
 
+MirOrientationMode ms::BasicSurface::set_preferred_orientation(MirOrientationMode new_orientation_mode)
+{
+    if ((new_orientation_mode & mir_orientation_mode_any) == 0)
+    {
+        BOOST_THROW_EXCEPTION(std::logic_error("Invalid orientation mode"));
+    }
+
+    std::unique_lock<std::mutex> lg(guard);
+    if (pref_orientation_mode != new_orientation_mode)
+    {
+        pref_orientation_mode = new_orientation_mode;
+        lg.unlock();
+
+        observers.attrib_changed(mir_surface_attrib_preferred_orientation, new_orientation_mode);
+    }
+
+    return new_orientation_mode;
+}
+
 void ms::BasicSurface::take_input_focus(std::shared_ptr<msh::InputTargeter> const& targeter)
 {
     targeter->focus_changed(input_channel());
@@ -493,6 +512,9 @@
     case mir_surface_attrib_visibility:
         result = set_visibility(static_cast<MirSurfaceVisibility>(result));
         break;
+    case mir_surface_attrib_preferred_orientation:
+        result = set_preferred_orientation(static_cast<MirOrientationMode>(result));
+        break;
     default:
         BOOST_THROW_EXCEPTION(std::logic_error("Invalid surface "
                                                "attribute."));
@@ -515,6 +537,7 @@
         case mir_surface_attrib_focus: return focus_;
         case mir_surface_attrib_dpi: return dpi_;
         case mir_surface_attrib_visibility: return visibility_;
+        case mir_surface_attrib_preferred_orientation: return pref_orientation_mode;
         default: BOOST_THROW_EXCEPTION(std::logic_error("Invalid surface "
                                                         "attribute."));
     }
 
=== modified file 'src/server/scene/basic_surface.h'
--- src/server/scene/basic_surface.h	2014-12-05 03:26:03 +0000
+++ src/server/scene/basic_surface.h	2014-12-05 22:17:20 +0000
@@ -158,6 +158,7 @@
     MirSurfaceVisibility set_visibility(MirSurfaceVisibility v);
     int set_swap_interval(int);
     MirSurfaceFocusState set_focus_state(MirSurfaceFocusState f);
+    MirOrientationMode set_preferred_orientation(MirOrientationMode mode);
 
     SurfaceObservers observers;
     std::mutex mutable guard;
@@ -184,6 +185,7 @@
     MirSurfaceFocusState focus_ = mir_surface_unfocused;
     int dpi_ = 0;
     MirSurfaceVisibility visibility_ = mir_surface_visibility_exposed;
+    MirOrientationMode pref_orientation_mode = mir_orientation_mode_any;
 };
 
 }
 
=== modified file 'tests/acceptance-tests/test_client_surface_events.cpp'
--- tests/acceptance-tests/test_client_surface_events.cpp	2014-12-05 03:26:03 +0000
+++ tests/acceptance-tests/test_client_surface_events.cpp	2014-12-05 22:17:20 +0000
@@ -270,3 +270,19 @@
     EXPECT_THAT(last_event_surface, Eq(surface));
     EXPECT_THAT(last_event.type, Eq(mir_event_type_close_surface));
 }
+
+TEST_F(ClientSurfaceEvents, client_can_query_preferred_orientation)
+{
+
+    for (auto const mode:
+        {mir_orientation_mode_portrait, mir_orientation_mode_portrait_inverted,
+         mir_orientation_mode_landscape, mir_orientation_mode_landscape_inverted,
+         mir_orientation_mode_portrait_any, mir_orientation_mode_landscape_any,
+         mir_orientation_mode_any})
+    {
+        reset_last_event();
+
+        mir_wait_for(mir_surface_set_preferred_orientation(surface, mode));
+        EXPECT_THAT(mir_surface_get_preferred_orientation(surface), Eq(mode));
+    }
+}
 
=== modified file 'tests/unit-tests/client/test_client_mir_surface.cpp'
--- tests/unit-tests/client/test_client_mir_surface.cpp	2014-12-05 03:26:03 +0000
+++ tests/unit-tests/client/test_client_mir_surface.cpp	2014-12-05 22:17:20 +0000
@@ -188,7 +188,8 @@
     { mir_surface_attrib_swapinterval, 1 },
     { mir_surface_attrib_focus, mir_surface_focused },
     { mir_surface_attrib_dpi, 19 },
-    { mir_surface_attrib_visibility, mir_surface_visibility_exposed }
+    { mir_surface_attrib_visibility, mir_surface_visibility_exposed },
+    { mir_surface_attrib_preferred_orientation, mir_orientation_mode_any }
 };
 
 struct MockBuffer : public mcl::ClientBuffer
 
=== modified file 'tests/unit-tests/scene/test_surface_impl.cpp'
--- tests/unit-tests/scene/test_surface_impl.cpp	2014-12-05 03:26:03 +0000
+++ tests/unit-tests/scene/test_surface_impl.cpp	2014-12-05 22:17:20 +0000
@@ -460,3 +460,21 @@
 
     surf.request_client_surface_close();
 }
+
+TEST_F(Surface, preferred_orientation_mode_defaults_to_any)
+{
+    using namespace testing;
+
+    ms::BasicSurface surf(
+        std::string("stub"),
+        geom::Rectangle{{},{}},
+        false,
+        buffer_stream,
+        std::shared_ptr<mi::InputChannel>(),
+        stub_input_sender,
+        null_configurator,
+        std::shared_ptr<mg::CursorImage>(),
+        report);
+
+    EXPECT_EQ(mir_orientation_mode_any, surf.query(mir_surface_attrib_preferred_orientation));
+}