Merge into development-branch : new-display-api : Code : Mir

Status:	Merged
Approved by:	Alexandros Frantzis on 2016-02-26
Approved revision:	3375
Merged at revision:	3343
Proposed branch:	lp:~raof/mir/new-display-api
Merge into:	lp:mir
Diff against target:	2114 lines (+1638/-42) 19 files modified include/client/mir_toolkit/client_types.h (+35/-0) include/client/mir_toolkit/mir_client_library.h (+1/-0) include/client/mir_toolkit/mir_connection.h (+15/-1) include/client/mir_toolkit/mir_display_configuration.h (+360/-0) include/test/mir/test/display_config_matchers.h (+9/-0) include/test/mir/test/doubles/stub_display_configuration.h (+4/-0) src/client/CMakeLists.txt (+2/-0) src/client/display_configuration.cpp (+29/-15) src/client/display_configuration.h (+12/-2) src/client/display_configuration_api.cpp (+295/-0) src/client/mir_connection.cpp (+5/-0) src/client/mir_connection.h (+2/-0) src/client/mir_connection_api.cpp (+14/-0) src/client/symbols.map (+33/-0) tests/acceptance-tests/CMakeLists.txt (+1/-0) tests/acceptance-tests/test_display_configuration.cpp (+15/-15) tests/acceptance-tests/test_new_display_configuration.cpp (+671/-0) tests/mir_test/display_config_matchers.cpp (+97/-0) tests/mir_test_doubles/stub_display_configuration.cpp (+38/-9)
To merge this branch:	bzr merge lp:~raof/mir/new-display-api
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Mir CI Bot	continuous-integration	2016-02-24	Approve on 2016-02-26
Alexandros Frantzis (community)		2016-02-24	Approve on 2016-02-26
Alan Griffiths		2016-02-24	Approve on 2016-02-26
Andreas Pokorny (community)			Approve on 2016-02-25
Kevin DuBois (community)		2016-02-24	Approve on 2016-02-24
Cemil Azizoglu (community)		2016-02-24	Approve on 2016-02-24
PS Jenkins bot	continuous-integration	2016-02-24	Needs Fixing on 2016-02-24
Review via email: mp+286975@code.launchpad.net

Mir CI Bot (mir-ci-bot) wrote on 2016-02-12: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:3309
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~raof/mir/new-display-api/+merge/285832/+edit-commit-message

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/289/rebuild

review: Needs Fixing (continuous-integration)

Mir CI Bot (mir-ci-bot) wrote on 2016-02-12: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/290/rebuild

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-12: Posted in a previous version of this proposal

#

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

review: Needs Fixing (continuous-integration)

Alexandros Frantzis (afrantzis) wrote on 2016-02-12: Posted in a previous version of this proposal

#

A first pass at the API:

> MirDisplayConfig* mir_connection_create_display_configuration()
> MirDisplayConfiguration* mir_connection_create_display_config()

The naming is confusing due to the mismatch of configuration vs config, especially given that other new API functions use mir_display_config (e.g. mir_display_config_release). Not sure what a nice solution would be, though, given our existing API.

> MirOutputConnection mir_output_is_connected(MirOutput const* output);

The object_is_X naming usually implies a boolean return value, but here we get an enum with three values.

> int mir_output_physical_width_mm(MirOutput const* output);
> int mir_output_physical_height_mm(MirOutput const* output);

Should be mir_output_get_physical_*_mm for consistency.

> MirExtent const* mir_output_mode_get_resolution(MirOutputMode const* mode);

Perhaps we should have separate functions to get width and height. It's more consistent with other functions in the new API and also avoids the need for the non-opaque MirExtent.

review: Needs Fixing

Mir CI Bot (mir-ci-bot) wrote on 2016-02-15: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/298/rebuild

review: Approve (continuous-integration)

Chris Halse Rogers (raof) wrote on 2016-02-15: Posted in a previous version of this proposal

#

Fixed the things that can be fixed.

mir_connection_create_display_config()/mir_connection_create_display_configuration() isn't one of those things, unless we go back on the “no #define-time magic” we decided in Austin ☺.

Mir CI Bot (mir-ci-bot) wrote on 2016-02-15: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/299/rebuild

review: Needs Fixing (continuous-integration)

Mir CI Bot (mir-ci-bot) wrote on 2016-02-15: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/300/rebuild

review: Approve (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-15: Posted in a previous version of this proposal

#

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

review: Approve (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-15: Posted in a previous version of this proposal

#

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

review: Approve (continuous-integration)

Mir CI Bot (mir-ci-bot) wrote on 2016-02-16: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/306/rebuild

review: Approve (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-16: Posted in a previous version of this proposal

#

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

review: Approve (continuous-integration)

Mir CI Bot (mir-ci-bot) wrote on 2016-02-16: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/307/rebuild

review: Approve (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-16: Posted in a previous version of this proposal

#

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

review: Approve (continuous-integration)

Kevin DuBois (kdub) wrote on 2016-02-16: Posted in a previous version of this proposal

#

discussion/planning:
Should we remove the MirWaitHandles? (https://code.launchpad.net/~kdub/mir/finish-presentation-chain/+merge/285720/comments/728846)

clarifying comment needed:
+int mir_display_config_get_num_outputs(MirDisplayConfig const* config);
Is this the number of active outputs? (in light of mir_display_config_get_max_simultaneous_outputs) or the the number of enabled outputs?

do we need two functions here:
+MirOutput const* mir_display_config_get_output(MirDisplayConfig const* config, size_t index);
+MirOutput* mir_display_config_get_mutable_output(MirDisplayConfig* config, size_t index);
I guess I'd prefer just having:
+MirOutput* mir_display_config_get_output(MirDisplayConfig const* config, size_t index);
In the name of one less function to support.

clarifying content needed:
After reading for a while, I made the connection that MirOutputMode was formerly the list of resolution modes... perhaps MirOutputResolution would have helped make that mental connection quicker.

clarifying content needed:
void mir_output_set_position(MirOutput* output, int x, int y);
the comment says the top-left corner, but in light of rotation, not sure if the corner rotates or not when orientation is applied.

review: Needs Information

Mir CI Bot (mir-ci-bot) wrote on 2016-02-17: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:3333
https://mir-jenkins.ubuntu.com/job/mir-ci/320/
Executed test runs:
    ABORTED: https://mir-jenkins.ubuntu.com/job/build/109/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/115
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/111
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/111

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/320/rebuild

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-17: Posted in a previous version of this proposal

#

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

review: Approve (continuous-integration)

Mir CI Bot (mir-ci-bot) wrote on 2016-02-18: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/332/rebuild

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-18: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:3335
http://jenkins.qa.ubuntu.com/job/mir-ci/6321/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5967/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4874/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5923/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/645/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/645/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5920/console

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

review: Needs Fixing (continuous-integration)

Mir CI Bot (mir-ci-bot) wrote on 2016-02-18: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/334/rebuild

review: Needs Fixing (continuous-integration)

Mir CI Bot (mir-ci-bot) wrote on 2016-02-18: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/335/rebuild

review: Approve (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-18: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:3336
http://jenkins.qa.ubuntu.com/job/mir-ci/6323/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5969/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4876/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5925/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/647/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/647/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5922/console

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-18: Posted in a previous version of this proposal

#

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

review: Approve (continuous-integration)

Chris Halse Rogers (raof) wrote on 2016-02-19: Posted in a previous version of this proposal

#

I've stripped all the setters out of this; since we're implementing a new API I think it's the right time to look at doing something about https://docs.google.com/document/d/1wpoHVYdPZGthN2WS8oDvhRlpyuUqrizK1C4quE4XwMk , but that's likely to be more contentious and take a bit more work.

So just do the bits that QtMir needs now, and add the setters in a follow up.

Mir CI Bot (mir-ci-bot) wrote on 2016-02-19: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/337/rebuild

review: Approve (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-19: Posted in a previous version of this proposal

#

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

review: Approve (continuous-integration)

Cemil Azizoglu (cemil-azizoglu) wrote on 2016-02-19: Posted in a previous version of this proposal

#

Cursory review:

40 + * \deprecated Use mir_connection_create_display_configuration() instead.
41 +

Why not mark the function with '__attribute__ ((deprecated))'?

-------------------------------------------------------------------

57 +MirDisplayConfig* mir_connection_create_display_configuration(MirConnection* connection);
58

We are not really creating it, are we? How about mir_connection_get_current_display_configuration() in tandem with mir_output_get_current_mode()?

-------------------------------------------------------------------

124 +int mir_display_config_get_max_simultaneous_outputs(MirDisplayConfig const* config);

Why do we need the config arg here? Also, what does knowing this number give you?

-------------------------------------------------------------------

129 + * This returns the total number of outputs the system has.

should read

129 + * This returns the total number of outputs the system has, matching this configuration.

review: Needs Fixing

Chris Halse Rogers (raof) wrote on 2016-02-22: Posted in a previous version of this proposal

#

> Cursory review:
>
> 40 + * \deprecated Use mir_connection_create_display_configuration()
> instead.
> 41 +
>
> Why not mark the function with '__attribute__ ((deprecated))'?

Because that immediately causes all of our current users (including our examples, and the nested server) to fail to build.

I'm following up with changing these in-Mir usages, and I'll update our downstreams, but I didn't immediately force the issue so as to make it easier to break up into reviewable chunks.

>
> -------------------------------------------------------------------
>
> 57 +MirDisplayConfig*
> mir_connection_create_display_configuration(MirConnection* connection);
> 58
>
> We are not really creating it, are we? How about
> mir_connection_get_current_display_configuration() in tandem with
> mir_output_get_current_mode()?
>

This matches the current naming, and - in contrast to mir_output_get_current_mode() - we *are* allocating something that requires cleanup later.

I don't mind renaming, but maybe I'll wait for a second opinion :)

> -------------------------------------------------------------------
>
> 124 +int mir_display_config_get_max_simultaneous_outputs(MirDisplayConfig
> const* config);
>
> Why do we need the config arg here? Also, what does knowing this number give
> you?
>

It's something exposed by our current API, and it can be used by clients to pre-reject obviously invalid setups. For example, many laptops used to have two external output ports and their internal displays, but their GPUs could only drive two outputs at once. Knowing mir_display_get_max_simultaneous_outputs(config) == 2 a configuration client can disallow trying to enable the internal panel *and* both external displays.

We need the config arg here for two reasons: (a) as an implementation detail this is stored in the configuration, and (b) on the conceptual level it can change over time, so it's tied to a particular configuration snapshot.

> -------------------------------------------------------------------
>
> 129 + * This returns the total number of outputs the system has.
>
> should read
>
> 129 + * This returns the total number of outputs the system has, matching
> this configuration.

Hm, that seems confusing to me. What does “matching” mean in this case? We're not doing any filtering here; this *is* the total number of outputs the system had, as of the call to mir_connection_create_display_configuration().

Alexandros Frantzis (afrantzis) wrote on 2016-02-22: Posted in a previous version of this proposal

#

Looks good overall.

A few points for discussion:

+ MirDisplayConfig* mir_connection_create_display_configuration(MirConnection* connection);

DisplayConfig/display_configuration vs DisplayConfiguration/display_config still bugs me. Some day...

+int mir_output_get_num_output_formats(MirOutput const* output)
+ MirPixelFormat mir_output_get_format(MirOutput const* output, size_t index);
...

More consistent as:

mir_output_get_num_pixel_formats()
mir_output_get_pixel_format()
mir_output_set_pixel_format()

+ * \note This may be removed in future, in favour of passing the MirOutput* directly to such APIs.

I don't think this note is useful for a user of the API at the moment.

+ MirDisplayOutputType mir_output_get_type(MirOutput const* output);

We should provide a new type MirOutputType for API consistency.

+typedef enum
+{
+ mir_output_disconnected = 0,
+ mir_output_connected,
+ mir_output_connection_unknown
+} MirOutputConnection;

A more verbose, but also more consistent alternative:

mir_output_connection_state_disconnected
mir_output_connection_state_connected
mir_output_connection_state_unknown
MirOutputConnectionState

review: Needs Information

Alan Griffiths (alan-griffiths) wrote on 2016-02-22: Posted in a previous version of this proposal

#

+ return *reinterpret_cast<std::shared_ptr<mcl::DisplayConfiguration::Config>*>(config);

Ugh!!

Alan Griffiths (alan-griffiths) wrote on 2016-02-22: Posted in a previous version of this proposal

#

Minor edits, consistency fixes and musings:

+ * \note This may be removed in future, in favour of passing the MirOutput* directly to such APIs.

Not useful API documentation. Possibly a TODO?

~~~~

+ * Get the number of pixel formats supported by this output
...
+int mir_output_get_num_output_formats(MirOutput const* output);

s/output_formats/formats/

(to match mir_output_get_format())

~~~~

+int mir_output_get_position_x(MirOutput const* output);
...

I know we aim for opaque types, but I wonder if this would be easier to use:

MirRectangle mir_output_get_position(MirOutput const* output);

Also, it is far from clear that this API is tested (the only use of this function I see is in tests/mir_test/display_config_matchers.cpp).

~~~~

+typedef enum
+{
+ mir_output_disconnected = 0,
+ mir_output_connected,
+ mir_output_connection_unknown
+} MirOutputConnection;

Possibly better in include/client/mir_toolkit/client_types.h?

Also, we have tag names on our other typedef'd enums so, be consistent.

review: Needs Fixing

Cemil Azizoglu (cemil-azizoglu) wrote on 2016-02-22: Posted in a previous version of this proposal

#

LGTM

review: Approve

Chris Halse Rogers (raof) wrote on 2016-02-23: Posted in a previous version of this proposal

#

> + return *reinterpret_cast<std::shared_ptr<mcl::DisplayConfiguration::Config>*
> >(config);
>
> Ugh!!

Yeah. It goes away in the followup MP :).

> +int mir_output_get_position_x(MirOutput const* output);
> ...
>
> I know we aim for opaque types, but I wonder if this would be easier to use:
>
> MirRectangle mir_output_get_position(MirOutput const* output);
>
> Also, it is far from clear that this API is tested (the only use of this
> function I see is in tests/mir_test/display_config_matchers.cpp).

Hah! I originally added a MirExtent struct (MirRectangle isn't quite what we want, as that has width/height) to do this and Alexandros asked to change it to position_x / position_y.

I'll let you fight that out and implement the result ☺.

Mir CI Bot (mir-ci-bot) wrote on 2016-02-23: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/359/rebuild

review: Approve (continuous-integration)

Mir CI Bot (mir-ci-bot) wrote on 2016-02-23: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/360/rebuild

review: Needs Fixing (continuous-integration)

Mir CI Bot (mir-ci-bot) wrote on 2016-02-23: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/361/rebuild

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-23: Posted in a previous version of this proposal

#

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

review: Approve (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-23: Posted in a previous version of this proposal

#

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

review: Needs Fixing (continuous-integration)

Alan Griffiths (alan-griffiths) wrote on 2016-02-23: Posted in a previous version of this proposal

#

> > + return
> *reinterpret_cast<std::shared_ptr<mcl::DisplayConfiguration::Config>*
> > >(config);
> >
> > Ugh!!
>
> Yeah. It goes away in the followup MP :).

Are all the varied casting ugliness bits going away?

> > +int mir_output_get_position_x(MirOutput const* output);
> > ...
> >
> > I know we aim for opaque types, but I wonder if this would be easier to use:
> >
> > MirRectangle mir_output_get_position(MirOutput const* output);
> >
> > Also, it is far from clear that this API is tested (the only use of this
> > function I see is in tests/mir_test/display_config_matchers.cpp).
>
> Hah! I originally added a MirExtent struct (MirRectangle isn't quite what we
> want, as that has width/height) to do this and Alexandros asked to change it
> to position_x / position_y.
>
> I'll let you fight that out and implement the result ☺.

Not sure that's an effective strategy - you could wait a long time for us to generate the interest needed for a fight. ;)

MirRectangle has a set of members that are unlikely to change. MirExtent on the other hand doesn't have quite as fixed a meaning. (From the name I'd assume it was much the same as MirRectangle - what data members did you propose?)

If you don't like MirRectangle how about MirPosition/MirPoint?

review: Needs Information

Alexandros Frantzis (afrantzis) wrote on 2016-02-23: Posted in a previous version of this proposal

#

Pasting my argument for having separate function, from the old MP:

> Perhaps we should have separate functions to get width and height. It's more consistent with other > functions in the new API and also avoids the need for the non-opaque MirExtent.

Cemil Azizoglu (cemil-azizoglu) wrote on 2016-02-23: Posted in a previous version of this proposal

#

> Pasting my argument for having separate function, from the old MP:
>
> > Perhaps we should have separate functions to get width and height. It's more
> consistent with other > functions in the new API and also avoids the need for
> the non-opaque MirExtent.

Yes there are other APIs that use *_x(), *_y(). Although, I like the extents/rectangle/point idea, I vote for consistency at this point.

Kevin DuBois (kdub) wrote on 2016-02-23: Posted in a previous version of this proposal

#

needs fixing:
mir_output_set_orientation is still around in comment and has an implementation, but is not in a header. Should those be removed, or the header define be added for this iteration?
other than that, lgtm

non-blocking trivialities:

Might be helpful to copy this line:
+ * Output orientation, as set by mir_output_set_orientation(), changes the orientation of the output rectangle
+ * in virtual display space, but does not change its top-left corner.
to the get_width/get_height functions.

+ * \pre 0 ≤ index < mir_output_get_num_modes(output)
prefer <= for C api documentation.

+ * This can be used to refer to the output in other APIs, such as
in other parts of the API?

review: Needs Fixing

Mir CI Bot (mir-ci-bot) wrote on 2016-02-24: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/372/rebuild

review: Needs Fixing (continuous-integration)

Mir CI Bot (mir-ci-bot) wrote on 2016-02-24:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/374/rebuild

review: Approve (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-24:

#

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-24: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:3360
http://jenkins.qa.ubuntu.com/job/mir-ci/6367/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/6024/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4931/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5980/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/691/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/691/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5977/console

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

review: Needs Fixing (continuous-integration)

lp:~raof/mir/new-display-api updated on 2016-02-24

3370. By Chris Halse Rogers on 2016-02-24: Appease the almighty Clang harder

Mir CI Bot (mir-ci-bot) wrote on 2016-02-24:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/379/rebuild

review: Approve (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-24:

#

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

review: Needs Fixing (continuous-integration)

Andreas Pokorny (andreas-pokorny) wrote on 2016-02-24:

#

void mir_output_set_current_mode(MirOutput* client_output, MirOutputMode const* mode)
+{
+ auto output = client_to_output(client_output);
+
+ auto internal_mode = reinterpret_cast<MirDisplayMode const*>(mode);
+ ptrdiff_t offset = internal_mode - output->modes;
+ int index = offset / sizeof(MirDisplayMode);
+
+ mir::require(index >= 0);
+ mir::require(index < static_cast<int>(output->num_modes));
+
+ output->current_mode = static_cast<uint32_t>(index);
+}

I believe the offset / sizeof() is wrong and should be / could be:

void mir_output_set_current_mode(MirOutput* client_output, MirOutputMode const* mode)
+{
+ auto output = client_to_output(client_output);
+
+ auto index = std::distance(output->modes, reinterpret_cast<MirDisplayMode const*>(mode));
+
+ mir::require(index >= 0);
+ mir::require(index < static_cast<int>(output->num_modes));
+
+ output->current_mode = static_cast<uint32_t>(index);
+}

nits:
----

There are various longer than 80 columns in the new mirclient display config header and client_to_config is beyond 125 columns

-----

+1198 The new test cases could get an updated year/author info.

review: Needs Fixing

Cemil Azizoglu (cemil-azizoglu) wrote on 2016-02-24:

#

LGTM

review: Approve

Kevin DuBois (kdub) wrote on 2016-02-24:

#

ascii mafia approved ^_^

review: Approve

lp:~raof/mir/new-display-api updated on 2016-02-24

3371. By Chris Halse Rogers on 2016-02-24: Supplicants for the almighty clang must be consistent.
3372. By Chris Halse Rogers on 2016-02-24: Use std::distance instead of open-coding it.
3374. By Chris Halse Rogers on 2016-02-24: Wrap header at 80 cols

Mir CI Bot (mir-ci-bot) wrote on 2016-02-25:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/393/rebuild

review: Approve (continuous-integration)

Andreas Pokorny (andreas-pokorny) wrote on 2016-02-25:

#

looks good

review: Approve

Alan Griffiths (alan-griffiths) wrote on 2016-02-25:

#

StubDisplayConfig::StubDisplayConfig now creates one card (not one card per output). That seems reasonable, but is that part of the new API?

Alan Griffiths (alan-griffiths) wrote on 2016-02-25:

#

The doxygen markup is seriously b. E.g.

+ * \note The MirOutput handle is only valid while \param config is valid.

The \param tag is for documenting a parameter, not referring to it in the note text. The result looks like:

Note
The MirOutput handle is only valid while

Parameters
config is valid.

In addition, for the functions to appear in the "MIR graphics tools API" documentation module the definitions need to be enclosed in:

/**
* \addtogroup mir_toolkit
* @{
*/
...
/**@}*/

Please run $ make doc && firefox doc/html/group__mir__toolkit.html and check the results!

review: Needs Fixing

Chris Halse Rogers (raof) wrote on 2016-02-26:

#

> StubDisplayConfig::StubDisplayConfig now creates one card (not one card per
> output). That seems reasonable, but is that part of the new API?

Kind of; the new API has no notion of cards. This is reasonable because none of our existing backends can handle multiple cards, and once we *do* support multiple cards the existing API we have is ill-suited to handling them (for example, you can have an output connected to more than one card).

Urgh. I did briefly check the documentation, but didn't notice that. I'm clearly misrememdering the doxygen markup to reference a function argument...

Chris Halse Rogers (raof) wrote on 2016-02-26:

#

Ok. That's not formatted *quite* how I want it, but it's at least not borken!

lp:~raof/mir/new-display-api updated on 2016-02-26

3375. By Chris Halse Rogers on 2016-02-26

Fixup doxygen

Thanks, Alan!

Mir CI Bot (mir-ci-bot) wrote on 2016-02-26:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/410/rebuild

review: Approve (continuous-integration)

Alan Griffiths (alan-griffiths) wrote on 2016-02-26:

#

LGTM

review: Approve

Alexandros Frantzis (afrantzis) wrote on 2016-02-26:

#

Looks good.

review: Approve

Mir CI Bot (mir-ci-bot) on 2016-02-26:

#

review: Approve (continuous-integration)

Mir

Merge lp:~raof/mir/new-display-api into lp:mir

Commit Message

Description of the Change

Preview Diff

Subscribers

 === modified file 'include/client/mir_toolkit/client_types.h'
 --- include/client/mir_toolkit/client_types.h	2016-01-29 08:18:22 +0000
 +++ include/client/mir_toolkit/client_types.h	2016-02-26 07:01:49 +0000
@@ -45,6 +45,15 @@
  typedef struct MirBufferStream MirBufferStream;
  typedef struct MirPersistentId MirPersistentId;
  typedef struct MirBlob MirBlob;
++typedef struct MirDisplayConfig MirDisplayConfig;
++
++/**
++ * Descriptor for an output connection.
++ *
++ *  Each MirOutput corresponds to a video output. This may be a physical connection on the system,
++ *  like HDMI or DisplayPort, or may be a virtual output such as a remote display or screencast display.
++ */
++typedef struct MirOutput MirOutput;
  /**
   * Returned by asynchronous functions. Must not be free'd by
@@ -272,6 +281,32 @@
      mir_display_output_type_edp
  } MirDisplayOutputType;
++typedef enum MirOutputType
++{
++    mir_output_type_unknown,
++    mir_output_type_vga,
++    mir_output_type_dvii,
++    mir_output_type_dvid,
++    mir_output_type_dvia,
++    mir_output_type_composite,
++    mir_output_type_svideo,
++    mir_output_type_lvds,
++    mir_output_type_component,
++    mir_output_type_ninepindin,
++    mir_output_type_displayport,
++    mir_output_type_hdmia,
++    mir_output_type_hdmib,
++    mir_output_type_tv,
++    mir_output_type_edp
++} MirOutputType;
++
++typedef enum MirOutputConnectionState
++{
++    mir_output_connection_state_disconnected = 0,
++    mir_output_connection_state_connected,
++    mir_output_connection_state_unknown
++} MirOutputConnectionState;
++
  typedef struct MirDisplayMode
+ {
      uint32_t vertical_resolution;
 === modified file 'include/client/mir_toolkit/mir_client_library.h'
 --- include/client/mir_toolkit/mir_client_library.h	2016-01-22 05:26:25 +0000
 +++ include/client/mir_toolkit/mir_client_library.h	2016-02-26 07:01:49 +0000
@@ -26,5 +26,6 @@
  #include <mir_toolkit/mir_platform_message.h>
  #include <mir_toolkit/cursors.h>
  #include <mir_toolkit/mir_cookie.h>
++#include <mir_toolkit/mir_display_configuration.h>
  #endif /* MIR_CLIENT_LIBRARY_H */
 === modified file 'include/client/mir_toolkit/mir_connection.h'
 --- include/client/mir_toolkit/mir_connection.h	2016-01-29 08:18:22 +0000
 +++ include/client/mir_toolkit/mir_connection.h	2016-02-26 07:01:49 +0000
@@ -145,6 +145,9 @@
  /**
   * Query the display
++ *
++ *   \deprecated  Use mir_connection_create_display_configuration() instead.
++ *
   *   \warning return value must be destroyed via mir_display_config_destroy()
   *   \warning may return null if connection is invalid
   *   \param [in]  connection        The connection
@@ -153,9 +156,20 @@
  MirDisplayConfiguration* mir_connection_create_display_config(MirConnection *connection);
  /**
++ * Query the display
++ *
++ * \pre mir_connection_is_valid(connection) == true
++ * \warning return value must be destroyed via mir_display_config_release()
++ *
++ * \param [in]  connection        The connection
++ * \return                        structure that describes the display configuration
++ */
++MirDisplayConfig* mir_connection_create_display_configuration(MirConnection* connection);
++
++/**
   * Register a callback to be called when the hardware display configuration changes
+  *
-- * Once a change has occurred, you can use mir_connection_create_display_config to see
++ * Once a change has occurred, you can use mir_connection_create_display_configuration to see
   * the new configuration.
+  *
   *   \param [in] connection  The connection
 === added file 'include/client/mir_toolkit/mir_display_configuration.h'
 --- include/client/mir_toolkit/mir_display_configuration.h	1970-01-01 00:00:00 +0000
 +++ include/client/mir_toolkit/mir_display_configuration.h	2016-02-26 07:01:49 +0000
@@ -0,0 +1,360 @@
++/*
++ * Copyright © 2016 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 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 Lesser General Public License for more details.
++ *
++ * You should have received a copy of the GNU Lesser General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#ifndef MIR_TOOLKIT_MIR_DISPLAY_CONFIGURATION_H_
++#define MIR_TOOLKIT_MIR_DISPLAY_CONFIGURATION_H_
++
++#include "client_types.h"
++
++#ifdef __cplusplus
++extern "C" {
++#endif
++
++/**
++ * \addtogroup mir_toolkit
++ * @{
++ */
++
++/**
++ * A descriptor for a display mode.
++ *
++ * A display mode contains all the information necessary to drive a display.  It
++ * includes resolution and refresh rate, but also pixel clock, vsync and hsync
++ * timings, and so on.
++ */
++typedef struct MirOutputMode MirOutputMode;
++
++/**
++ * Release resources associated with a MirDisplayConfig handle.
++ *
++ * \param [in]  config              The handle to release
++ */
++void mir_display_config_release(MirDisplayConfig* config);
++
++/**
++ * Get the maximum possible number of simultaneously active outputs this system
++ * supports.
++ *
++ * \note There may be restrictions on the configuration required to achieve this
++ * many active outputs. Typically the achievable number of simultaneously active
++ * outputs is lower than this number.
++ *
++ * \param [in]  config    The configuration to query
++ * \returns      The maximum number of simultaneously active outputs
++ *               supportable at this time.
++ */
++int mir_display_config_get_max_simultaneous_outputs(
++    MirDisplayConfig const* config);
++
++/**
++ * Get the number of outputs available in this display configuration.
++ *
++ * This returns the total number of outputs the system has. This includes both
++ * enabled and disabled output connections, and is typically larger than the
++ * value returned from mir_display_config_get_max_simultaneous_outputs().
++ *
++ * Typically this will be constant over the lifetime of a client as devices
++ * usually do not gain extra physical ports at runtime. However, hotpluggable
++ * display devices exist and the number of virtual outputs may change at
++ * runtime, so this should always be called to determine the number of outputs
++ * to iterate over.
++ *
++ * \param [in]  config    The configuration to query
++ * \returns      The number of outputs available in this configuration.
++ */
++int mir_display_config_get_num_outputs(MirDisplayConfig const* config);
++
++/**
++ * Get a read-only handle to the index 'th output of this configuration
++ *
++ * \note The MirOutput handle is only valid while config is valid.
++ * \pre 0 <= index < mir_display_config_get_num_outputs(config)
++ * \param [in]  config  The configuration to query
++ * \param [in]  index   The index of the output to get
++ * \returns     A read-only handle to a MirOutput within config which is valid
++ *              until mir_display_config_release(config) is called.
++ */
++MirOutput const* mir_display_config_get_output(MirDisplayConfig const* config,
++    size_t index);
++
++/**
++ * Get the number of modes in the supported mode list of this output.
++ *
++ * The list of supported modes is retrieved from the hardware, possibly modified
++ * by any applicable quirk tables, and may not be exhaustive.
++ *
++ * \param [in]  output  The MirOutput to query
++ * \returns     The number of modes in the supported mode list of output.
++ */
++int mir_output_get_num_modes(MirOutput const* output);
++
++/**
++ * Get a handle for a mode descriptor from the list of supported modes.
++ *
++ * The list of supported modes is retrieved from the hardware, possibly modified
++ * by any applicable quirk tables, and may not be exhaustive.
++ *
++ * \pre 0 <= index < mir_output_get_num_modes(output)
++ * \note    The handle remains valid as long as output is valid.
++ *
++ * \param [in]  output  The MirOutput to query
++ * \param [in]  index   The index of the mode to retrieve.
++ * \returns     A handle for a description of the supported mode. This is valid
++ *              for as long as output is valid. The return value is never null.
++ */
++MirOutputMode const* mir_output_get_mode(MirOutput const* output, size_t index);
++
++/**
++ * Get a handle to the output's preferred mode.
++ *
++ * This is provided by the output itself. For modern displays (LCD, OLED, etc)
++ * it is typically a mode with the native resolution.
++ *
++ * An output may not have a preferred mode, in which case this call will return
++ * NULL.
++ *
++ * \note    The handle remains valid as long as output is valid.
++ *
++ * \param [in]  output  The MirOutput to query
++ * \returns     A handle for a description of the supported mode. This is valid
++ *              for as long as output is valid. If the output does not have a
++ *              preferred mode, it returns NULL.
++ */
++MirOutputMode const* mir_output_get_preferred_mode(MirOutput const* output);
++
++/**
++ * Get a handle to the output's current mode.
++ *
++ * An output may not have a current mode (for example, if it is disabled), in
++ * which case this call will return NULL.
++ *
++ * \note    The handle remains valid as long as output is valid.
++ *
++ * \param [in]  output  The MirOutput to query
++ * \returns     A handle for a description of the supported mode. This is valid
++ *              for as long as output is valid. If the output does not have a
++ *              current mode, it returns NULL.
++ */
++MirOutputMode const* mir_output_get_current_mode(MirOutput const* output);
++
++/**
++ * Get the number of pixel formats supported by this output
++ *
++ * \param [in]  output  The MirOutput to query
++ * \returns     The number of pixel formats for output.
++ */
++int mir_output_get_num_pixel_formats(MirOutput const* output);
++
++/**
++ * Get a pixel format from the list of supported formats
++ *
++ * \pre 0 <= index < mir_output_get_num_pixel_formats(output)
++ *
++ * \param [in] output  The MirOutput to query
++ * \param [in] index   The index of the format to query
++ * \returns    The index 'th supported pixel format.
++ */
++MirPixelFormat mir_output_get_pixel_format(MirOutput const* output,
++    size_t index);
++
++/**
++ * Get the current pixel format.
++ *
++ * \param [in]  output  The MirOutput to query
++ * \returns     The current pixel format. This may be mir_pixel_format_invalid
++ *              (for example, if the output is not currently enabled).
++ */
++MirPixelFormat mir_output_get_current_pixel_format(MirOutput const* output);
++
++/**
++ * Set the output format
++ *
++ * \param [in]  output  The MirOutput to modify
++ * \param [in]  format  The MirPixelFormat to set
++ */
++void mir_output_set_pixel_format(MirOutput* output, MirPixelFormat format);
++
++/**
++ * Get the ID of an output
++ *
++ * This can be used to refer to the output in other parts of the API, such as
++ * mir_surface_spec_set_fullscreen_on_output().
++ *
++ * \param [in]  output  The MirOutput to query.
++ * \returns     The ID of output, which may be used to refer to it in other
++ *              parts of the API.
++ */
++int mir_output_get_id(MirOutput const* output);
++
++/**
++ * Get the physical connection type of an output.
++ *
++ * This is a best-effort determination, and may be incorrect.
++ *
++ * \param [in]  output  The MirOutput to query
++ * \returns     The type of the display connection, or mir_output_type_unknown
++ *              if it cannot be determined.
++ */
++MirOutputType mir_output_get_type(MirOutput const* output);
++
++/**
++ * Get the x coordinate of the top-left point of the output in the virtual
++ * display space.
++ *
++ * Outputs can be thought of as viewports into a virtual display space. They may
++ * freely overlap, coincide, or be disjoint as desired.
++ *
++ * Output orientation changes the orientation of the output rectangle in virtual
++ * display space, but does not change its top-left corner.
++ *
++ * \param [in]  output  The MirOutput to query
++ * \returns     The x coordinate of the top-left point of the output in the
++ *              virtual display space.
++ */
++int mir_output_get_position_x(MirOutput const* output);
++
++/**
++ * Get the y coordinate of the top-left point of the output in the virtual
++ * display space.
++ *
++ * Outputs can be thought of as viewports into a virtual display space. They may
++ * freely overlap, coincide, or be disjoint as desired.
++ *
++ * Output orientation changes the orientation of the output rectangle in virtual
++ * display space, but does not change its top-left corner.
++ *
++ * \param [in]  output  The MirOutput to query
++ * \returns     The y coordinate of the top-left point of the output in the
++ *              virtual display space.
++ */
++int mir_output_get_position_y(MirOutput const* output);
++
++/**
++ * Get whether there is a display physically connected to the output.
++ *
++ * This gives a best-effort determination of whether or not enabling this output
++ * will result in an image being displayed to the user.
++ *
++ * The accuracy of this determination varies with connection type - for example,
++ * for DisplayPort and HDMI connections a return value of
++ * mir_output_connection_state_connected is usually a reliable indicator that
++ * there is a powered-on display connected.
++ *
++ * VGA and DVI connectors can usually determine whether or not there is a
++ * physically connected display, but cannot distinguish between a powered or
++ * unpowered display.
++ *
++ * It is not always possible to determine whether or not there is a display
++ * connected; in such cases mir_output_connection_state_unknown is returned.
++ *
++ * \param [in]  output  The MirOutput to query
++ * \returns     Whether there is a display connected to this output.
++ */
++MirOutputConnectionState mir_output_get_connection_state(
++    MirOutput const* output);
++
++/**
++ * Get whether this output is enabled in the current configuration.
++ *
++ * \param [in]  output  the MirOutput to query
++ * \returns     Whether the output is enabled.
++ */
++bool mir_output_is_enabled(MirOutput const* output);
++
++/**
++ * Get the physical width of the connected display, in millimetres.
++ *
++ * A best-effort report of the physical width of the display connected to this
++ * output. This is retrieved from the display hardware, possibly modified by any
++ * applicable quirk tables.
++ *
++ * Where this information is unavailable or inapplicable (for example,
++ * projectors), 0 is returned.
++ *
++ * \param [in]  output  the MirOutput to query
++ * \returns     Physical width of the connected display, in mm.
++ */
++int mir_output_get_physical_width_mm(MirOutput const* output);
++
++/**
++ * Get the physical height of the connected display, in millimetres.
++ *
++ * A best-effort report of the physical height of the display connected to this
++ * output. This is retrieved from the display hardware, possibly modified by any
++ * applicable quirk tables.
++ *
++ * Where this information is unavailable or inapplicable (for example,
++ * projectors), 0 is returned.
++ *
++ * \param [in]  output  the MirOutput to query
++ * \returns     Physical height of the connected display, in mm.
++ */
++int mir_output_get_physical_height_mm(MirOutput const* output);
++
++/**
++ * Get the power state of a connected display.
++ *
++ * It is undefined which power state is returned for an output which is not
++ * connected.
++ *
++ * \param [in]  output  The MirOutput to query
++ * \returns     The power state of the display connected to output.
++ */
++MirPowerMode mir_output_get_power_mode(MirOutput const* output);
++
++/**
++ * Get the orientation of a display.
++ *
++ * \param [in]  output  The MirOutput to query
++ * \returns     The orientation of output
++ */
++MirOrientation mir_output_get_orientation(MirOutput const* output);
++
++/**
++ * Get the width, in pixels, of a MirOutputMode
++ *
++ * \note    This is unaffected by the orientation of the output
++ *
++ * \param [in] mode    The MirOutputMode to query
++ * \returns     The width, in pixels, of mode.
++ */
++int mir_output_mode_get_width(MirOutputMode const* mode);
++
++/** Get the height, in pixels, of a MirOutputMode
++ *
++ * \note    This is unaffected by the orientation of the output
++ *
++ * \param [in] mode    The MirOutputMode to query
++ * \returns     The height, in pixels, of mode.
++ */
++int mir_output_mode_get_height(MirOutputMode const* mode);
++
++/** Get the refresh rate, in Hz, of a MirOutputMode
++ *
++ * \param [in]  mode    The MirOutputMode to query
++ * \returns     The refresh rate, in Hz, of mode
++ */
++double mir_output_mode_get_refresh_rate(MirOutputMode const* mode);
++
++/**@}*/
++
++#ifdef __cplusplus
++}
++#endif
++
++#endif //MIR_TOOLKIT_MIR_DISPLAY_CONFIGURATION_H_
 === modified file 'include/test/mir/test/display_config_matchers.h'
 --- include/test/mir/test/display_config_matchers.h	2016-01-29 08:18:22 +0000
 +++ include/test/mir/test/display_config_matchers.h	2016-02-26 07:01:49 +0000
@@ -76,6 +76,15 @@
  bool compare_display_configurations(MirDisplayConfiguration const* display_config2,
                                      graphics::DisplayConfiguration const& display_config1);
++bool compare_display_configurations(MirDisplayConfig const* client_config,
++                                   graphics::DisplayConfiguration const& server_config);
++
++bool compare_display_configurations(graphics::DisplayConfiguration const& server_config,
++    MirDisplayConfig const* client_config);
++
++bool compare_display_configurations(MirDisplayConfig const* config1,
++    MirDisplayConfig const* config2);
++
  MATCHER_P(DisplayConfigMatches, config, "")
+ {
      return compare_display_configurations(arg, config);
 === modified file 'include/test/mir/test/doubles/stub_display_configuration.h'
 --- include/test/mir/test/doubles/stub_display_configuration.h	2016-01-29 08:18:22 +0000
 +++ include/test/mir/test/doubles/stub_display_configuration.h	2016-02-26 07:01:49 +0000
@@ -22,6 +22,7 @@
  #include "mir/graphics/display_configuration.h"
  #include <vector>
++#include <mir_toolkit/client_types.h>
  namespace mir
+ {
@@ -37,6 +38,9 @@
      StubDisplayConfigurationOutput(graphics::DisplayConfigurationOutputId id,
          geometry::Size px_size, geometry::Size mm_size, MirPixelFormat format, double vrefresh, bool connected);
++
++    StubDisplayConfigurationOutput(graphics::DisplayConfigurationOutputId id,
++        std::vector<graphics::DisplayConfigurationMode> modes, std::vector<MirPixelFormat> formats);
  };
  class StubDisplayConfig : public graphics::DisplayConfiguration
 === modified file 'src/client/CMakeLists.txt'
 --- src/client/CMakeLists.txt	2016-02-23 20:08:23 +0000
 +++ src/client/CMakeLists.txt	2016-02-26 07:01:49 +0000
@@ -81,8 +81,10 @@
    presentation_chain.cpp
    mir_presentation_chain_api.cpp
    mir_buffer_api.cpp
++  display_configuration_api.cpp
    ${MIR_CLIENT_SOURCES}
    ${CMAKE_SOURCE_DIR}/include/client/mir_toolkit/events/surface_output_event.h
++  ${CMAKE_SOURCE_DIR}/include/client/mir_toolkit/mir_display_configuration.h
+ )
  # Ensure protobuf C++ headers have been produced before
 === modified file 'src/client/display_configuration.cpp'
 --- src/client/display_configuration.cpp	2016-01-29 08:18:22 +0000
 +++ src/client/display_configuration.cpp	2016-02-26 07:01:49 +0000
@@ -53,6 +53,13 @@
      output_formats = new MirPixelFormat[num_formats];
+ }
++mcl::DisplayOutput::DisplayOutput(DisplayOutput&& rhs)
++{
++    std::memcpy(this, &rhs, sizeof(*this));
++    rhs.modes = nullptr;
++    rhs.output_formats = nullptr;
++}
++
  mcl::DisplayOutput::~DisplayOutput()
+ {
      delete[] modes;
@@ -103,7 +110,8 @@
+ }
  mcl::DisplayConfiguration::DisplayConfiguration()
--    : notify_change([]{})
++    : config{std::make_shared<Config>()},
++      notify_change([]{})
+ {
+ }
@@ -113,25 +121,25 @@
  void mcl::DisplayConfiguration::set_configuration(mp::DisplayConfiguration const& msg)
+ {
--    std::lock_guard<std::mutex> lk(guard);
++    auto new_config = std::make_shared<Config>();
--    cards.clear();
      for (auto i = 0; i < msg.display_card_size(); i++)
+     {
          auto const& msg_card = msg.display_card(i);
          MirDisplayCard card;
          fill_display_card(card, msg_card);
--        cards.push_back(card);
++        new_config->cards.push_back(card);
+     }
--    outputs.clear();
      for (auto i = 0; i < msg.display_output_size(); i++)
+     {
          auto const& msg_output = msg.display_output(i);
--        auto output = std::make_shared<mcl::DisplayOutput>(msg_output.mode_size(), msg_output.pixel_format_size());
--        fill_display_output(*output, msg_output);
--        outputs.push_back(output);
++        new_config->outputs.emplace_back(msg_output.mode_size(), msg_output.pixel_format_size());
++        fill_display_output(new_config->outputs.back(), msg_output);
+     }
++
++    std::lock_guard<std::mutex> lk{guard};
++    config = new_config;
+ }
  void mcl::DisplayConfiguration::update_configuration(mp::DisplayConfiguration const& msg)
@@ -144,24 +152,24 @@
  //user is responsible for freeing the returned value
  MirDisplayConfiguration* mcl::DisplayConfiguration::copy_to_client() const
+ {
--    std::lock_guard<std::mutex> lk(guard);
++    auto snapshot = take_snapshot();
      auto new_config = new MirDisplayConfiguration;
      /* Cards */
--    new_config->num_cards = cards.size();
++    new_config->num_cards = snapshot->cards.size();
      new_config->cards = new MirDisplayCard[new_config->num_cards];
--    for (auto i = 0u; i < cards.size(); i++)
--        new_config->cards[i] = cards[i];
++    for (auto i = 0u; i < snapshot->cards.size(); i++)
++        new_config->cards[i] = snapshot->cards[i];
      /* Outputs */
--    new_config->num_outputs = outputs.size();
++    new_config->num_outputs = snapshot->outputs.size();
      new_config->outputs = new MirDisplayOutput[new_config->num_outputs];
--    for (auto i = 0u; i < outputs.size(); i++)
++    for (auto i = 0u; i < snapshot->outputs.size(); i++)
+     {
          auto new_info = &new_config->outputs[i];
--        MirDisplayOutput* output = outputs[i].get();
++        MirDisplayOutput* output = &snapshot->outputs[i];
          std::memcpy(new_info, output, sizeof(MirDisplayOutput));
          new_info->output_formats = new MirPixelFormat[new_info->num_output_formats];
@@ -176,6 +184,12 @@
      return new_config;
+ }
++auto mcl::DisplayConfiguration::take_snapshot() const -> std::shared_ptr<Config>
++{
++    std::lock_guard<std::mutex> lk{guard};
++    return config;
++}
++
  void mcl::DisplayConfiguration::set_display_change_handler(std::function<void()> const& fn)
+ {
      std::lock_guard<std::mutex> lk(guard);
 === modified file 'src/client/display_configuration.h'
 --- src/client/display_configuration.h	2013-08-28 03:41:48 +0000
 +++ src/client/display_configuration.h	2016-02-26 07:01:49 +0000
@@ -35,6 +35,9 @@
+ {
  public:
      DisplayOutput(size_t num_modes_, size_t num_formats);
++
++    DisplayOutput(DisplayOutput const&) = delete;
++    DisplayOutput(DisplayOutput&& rhs);
      ~DisplayOutput();
  };
@@ -44,6 +47,12 @@
  class DisplayConfiguration
+ {
  public:
++    struct Config
++    {
++        std::vector<MirDisplayCard> cards;
++        std::vector<DisplayOutput> outputs;
++    };
++
      DisplayConfiguration();
      ~DisplayConfiguration();
@@ -53,11 +62,12 @@
      //copying to a c POD, so kinda kludgy
      MirDisplayConfiguration* copy_to_client() const;
++    std::shared_ptr<Config> take_snapshot() const;
  private:
      std::mutex mutable guard;
--    std::vector<MirDisplayCard> cards;
--    std::vector<std::shared_ptr<DisplayOutput>> outputs;
++    std::shared_ptr<Config> config;
++
      std::function<void()> notify_change;
  };
 === added file 'src/client/display_configuration_api.cpp'
 --- src/client/display_configuration_api.cpp	1970-01-01 00:00:00 +0000
 +++ src/client/display_configuration_api.cpp	2016-02-26 07:01:49 +0000
@@ -0,0 +1,295 @@
++/*
++ * Copyright © 2016 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 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 Lesser General Public License for more details.
++ *
++ * You should have received a copy of the GNU Lesser General Public License
++ * along with this program. If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#include <mir_toolkit/mir_display_configuration.h>
++#include <mir/require.h>
++
++#include "display_configuration.h"
++
++namespace mcl = mir::client;
++
++namespace
++{
++std::shared_ptr<mcl::DisplayConfiguration::Config>& client_to_config(MirDisplayConfig* config)
++{
++    return *reinterpret_cast<std::shared_ptr<mcl::DisplayConfiguration::Config>*>(config);
++}
++
++std::shared_ptr<mcl::DisplayConfiguration::Config const>& client_to_config(MirDisplayConfig const* config)
++{
++    return *reinterpret_cast<std::shared_ptr<mcl::DisplayConfiguration::Config const>*>(const_cast<MirDisplayConfig*>(config));
++}
++
++MirOutput* output_to_client(MirDisplayOutput* output)
++{
++    return reinterpret_cast<MirOutput*>(output);
++}
++
++MirOutput const* output_to_client(MirDisplayOutput const* output)
++{
++    return reinterpret_cast<MirOutput const*>(output);
++}
++
++MirDisplayOutput* client_to_output(MirOutput* client)
++{
++    return reinterpret_cast<MirDisplayOutput*>(client);
++}
++
++MirDisplayOutput const* client_to_output(MirOutput const* client)
++{
++    return reinterpret_cast<MirDisplayOutput const*>(client);
++}
++}
++
++int mir_display_config_get_num_outputs(MirDisplayConfig const* client)
++{
++    auto config = client_to_config(client);
++    return config->outputs.size();
++}
++
++MirOutput const* mir_display_config_get_output(MirDisplayConfig const* client_config, size_t index)
++{
++    auto config = client_to_config(client_config);
++
++    mir::require(index < config->outputs.size());
++
++    return output_to_client(&config->outputs[index]);
++}
++
++MirOutput* mir_display_config_get_mutable_output(MirDisplayConfig* client_config, size_t index)
++{
++    auto config = client_to_config(client_config);
++
++    mir::require(index < config->outputs.size());
++
++    return output_to_client(&config->outputs[index]);
++}
++
++bool mir_output_is_enabled(MirOutput const* client_output)
++{
++    auto output = client_to_output(client_output);
++
++    return (output->used != 0);
++}
++
++void mir_output_enable(MirOutput* client_output)
++{
++    auto output = client_to_output(client_output);
++
++    output->used = 1;
++}
++
++void mir_output_disable(MirOutput* client_output)
++{
++    auto output = client_to_output(client_output);
++
++    output->used = 0;
++}
++
++int mir_display_config_get_max_simultaneous_outputs(MirDisplayConfig const* client_config)
++{
++    auto config = client_to_config(client_config);
++
++    return config->cards[0].max_simultaneous_outputs;
++}
++
++int mir_output_get_id(MirOutput const* client_output)
++{
++    auto output = client_to_output(client_output);
++
++    return output->output_id;
++}
++
++MirOutputType mir_output_get_type(MirOutput const* client_output)
++{
++    auto output = client_to_output(client_output);
++
++    return static_cast<MirOutputType>(output->type);
++}
++
++int mir_output_get_physical_width_mm(MirOutput const *client_output)
++{
++    auto output = client_to_output(client_output);
++
++    return output->physical_width_mm;
++}
++
++int mir_output_get_num_modes(MirOutput const* client_output)
++{
++    auto output = client_to_output(client_output);
++
++    return output->num_modes;
++}
++
++MirOutputMode const* mir_output_get_preferred_mode(MirOutput const* client_output)
++{
++    auto output = client_to_output(client_output);
++
++    if (output->preferred_mode >= output->num_modes)
++    {
++        return nullptr;
++    }
++    else
++    {
++        return reinterpret_cast<MirOutputMode const*>(&output->modes[output->preferred_mode]);
++    }
++}
++
++MirOutputMode const* mir_output_get_current_mode(MirOutput const* client_output)
++{
++    auto output = client_to_output(client_output);
++
++    if (output->current_mode >= output->num_modes)
++    {
++        return nullptr;
++    }
++    else
++    {
++        return reinterpret_cast<MirOutputMode const*>(&output->modes[output->current_mode]);
++    }
++}
++
++void mir_output_set_current_mode(MirOutput* client_output, MirOutputMode const* mode)
++{
++    auto output = client_to_output(client_output);
++
++    int index = std::distance(
++        const_cast<MirDisplayMode const*>(output->modes),
++        reinterpret_cast<MirDisplayMode const*>(mode));
++
++    mir::require(index >= 0);
++    mir::require(index < static_cast<int>(output->num_modes));
++
++    output->current_mode = static_cast<uint32_t>(index);
++}
++
++MirOutputMode const* mir_output_get_mode(MirOutput const* client_output, size_t index)
++{
++    auto output = client_to_output(client_output);
++
++    return reinterpret_cast<MirOutputMode const*>(&output->modes[index]);
++}
++
++int mir_output_get_num_pixel_formats(MirOutput const* client_output)
++{
++    auto output = client_to_output(client_output);
++
++    return output->num_output_formats;
++}
++
++MirPixelFormat mir_output_get_pixel_format(MirOutput const* client_output, size_t index)
++{
++    auto output = client_to_output(client_output);
++
++    return output->output_formats[index];
++}
++
++MirPixelFormat mir_output_get_current_pixel_format(MirOutput const* client_output)
++{
++    auto output = client_to_output(client_output);
++
++    return output->current_format;
++}
++
++void mir_output_set_pixel_format(MirOutput* client_output, MirPixelFormat format)
++{
++    auto output = client_to_output(client_output);
++
++    // TODO: Maybe check format validity?
++    output->current_format = format;
++}
++
++int mir_output_get_position_x(MirOutput const* client_output)
++{
++    auto output = client_to_output(client_output);
++
++    return output->position_x;
++}
++
++int mir_output_get_position_y(MirOutput const* client_output)
++{
++    auto output = client_to_output(client_output);
++
++    return output->position_y;
++}
++
++void mir_output_set_position(MirOutput* client_output, int x, int y)
++{
++    auto output = client_to_output(client_output);
++
++    output->position_x = x;
++    output->position_y = y;
++}
++
++MirOutputConnectionState mir_output_get_connection_state(MirOutput const *client_output)
++{
++    auto output = client_to_output(client_output);
++
++    // TODO: actually plumb through mir_output_connection_state_unknown.
++    return output->connected == 0 ? mir_output_connection_state_disconnected :
++           mir_output_connection_state_connected;
++}
++
++int mir_output_get_physical_height_mm(MirOutput const *client_output)
++{
++    auto output = client_to_output(client_output);
++
++    return output->physical_height_mm;
++}
++
++MirPowerMode mir_output_get_power_mode(MirOutput const* client_output)
++{
++    auto output = client_to_output(client_output);
++
++    return output->power_mode;
++}
++
++void mir_output_set_power_mode(MirOutput* client_output, MirPowerMode mode)
++{
++    auto output = client_to_output(client_output);
++
++    output->power_mode = mode;
++}
++
++MirOrientation mir_output_get_orientation(MirOutput const* client_output)
++{
++    auto output = client_to_output(client_output);
++
++    return output->orientation;
++}
++
++int mir_output_mode_get_width(MirOutputMode const* mode)
++{
++    auto internal_mode = reinterpret_cast<MirDisplayMode const *>(mode);
++
++    return internal_mode->horizontal_resolution;
++}
++
++int mir_output_mode_get_height(MirOutputMode const* mode)
++{
++    auto internal_mode = reinterpret_cast<MirDisplayMode const *>(mode);
++
++    return internal_mode->vertical_resolution;
++}
++
++double mir_output_mode_get_refresh_rate(MirOutputMode const* mode)
++{
++    auto internal_mode = reinterpret_cast<MirDisplayMode const *>(mode);
++
++    return internal_mode->refresh_rate;
++}
 === modified file 'src/client/mir_connection.cpp'
 --- src/client/mir_connection.cpp	2016-02-23 16:12:59 +0000
 +++ src/client/mir_connection.cpp	2016-02-26 07:01:49 +0000
@@ -1009,6 +1009,11 @@
      surface_map->erase(stream->rpc_id());
+ }
++std::shared_ptr<mcl::DisplayConfiguration::Config> MirConnection::snapshot_display_configuration() const
++{
++    return display_configuration->take_snapshot();
++}
++
  void MirConnection::create_presentation_chain(
      mir_presentation_chain_callback callback,
      void *context)
 === modified file 'src/client/mir_connection.h'
 --- src/client/mir_connection.h	2016-02-23 16:12:59 +0000
 +++ src/client/mir_connection.h	2016-02-26 07:01:49 +0000
@@ -31,6 +31,7 @@
  #include "mir_toolkit/mir_client_library.h"
  #include "mir_toolkit/client_types_nbs.h"
  #include "mir_surface.h"
++#include "display_configuration.h"
  #include <atomic>
  #include <memory>
@@ -132,6 +133,7 @@
      void populate(MirPlatformPackage& platform_package);
      void populate_graphics_module(MirModuleProperties& properties);
      MirDisplayConfiguration* create_copy_of_display_config();
++    std::shared_ptr<mir::client::DisplayConfiguration::Config> snapshot_display_configuration() const;
      void available_surface_formats(MirPixelFormat* formats,
                                     unsigned int formats_size, unsigned int& valid_formats);
 === modified file 'src/client/mir_connection_api.cpp'
 --- src/client/mir_connection_api.cpp	2016-01-29 08:18:22 +0000
 +++ src/client/mir_connection_api.cpp	2016-02-26 07:01:49 +0000
@@ -232,6 +232,20 @@
      return nullptr;
+ }
++MirDisplayConfig* mir_connection_create_display_configuration(
++    MirConnection* connection)
++{
++    mir::require(mir_connection_is_valid(connection));
++
++    return reinterpret_cast<MirDisplayConfig*>(new std::shared_ptr<mcl::DisplayConfiguration::Config>{connection->snapshot_display_configuration()});
++}
++
++void mir_display_config_release(MirDisplayConfig* user_config)
++{
++    auto config = reinterpret_cast<std::shared_ptr<mcl::DisplayConfiguration::Config>*>(user_config);
++    delete config;
++}
++
  void mir_connection_set_display_config_change_callback(
      MirConnection* connection,
      mir_display_config_callback callback,
 === modified file 'src/client/symbols.map'
 --- src/client/symbols.map	2016-02-23 20:08:23 +0000
 +++ src/client/symbols.map	2016-02-26 07:01:49 +0000
@@ -222,6 +222,39 @@
      mir_surface_spec_set_shell_chrome;
  } MIR_CLIENT_9v18;
++MIR_CLIENT_9_unreleased {
++  global:
++    mir_connection_create_display_configuration;
++    mir_display_config_release;
++    mir_display_config_get_num_outputs;
++    mir_connection_apply_display_configuration;
++    mir_connection_set_base_display_configuration;
++    mir_display_config_get_output;
++    mir_output_is_enabled;
++    mir_display_config_get_max_simultaneous_outputs;
++    mir_output_get_id;
++    mir_output_get_type;
++    mir_output_get_preferred_mode;
++    mir_output_get_physical_width_mm;
++    mir_output_get_physical_height_mm;
++    mir_output_get_connection_state;
++    mir_output_get_position_x;
++    mir_output_get_position_y;
++    mir_output_get_current_mode;
++    mir_output_get_current_pixel_format;
++    mir_output_get_power_mode;
++    mir_output_get_orientation;
++    mir_output_get_num_modes;
++    mir_output_get_mode;
++    mir_output_get_num_pixel_formats;
++    mir_output_get_pixel_format;
++    mir_output_mode_get_width;
++    mir_output_mode_get_height;
++    mir_output_mode_get_refresh_rate;
++  local:
++    *;
++} MIR_CLIENT_9v19;
++
  MIR_CLIENT_DETAIL_9 {
    global:
      extern "C++" {
 === modified file 'tests/acceptance-tests/CMakeLists.txt'
 --- tests/acceptance-tests/CMakeLists.txt	2016-01-29 08:18:22 +0000
 +++ tests/acceptance-tests/CMakeLists.txt	2016-02-26 07:01:49 +0000
@@ -53,6 +53,7 @@
    test_session_mediator_report.cpp
    test_surface_raise.cpp
    test_client_cookie.cpp
++  test_new_display_configuration.cpp
+ )
  if (MIR_TEST_PLATFORM STREQUAL "mesa-kms" OR MIR_TEST_PLATFORM STREQUAL "mesa-x11")
 === modified file 'tests/acceptance-tests/test_display_configuration.cpp'
 --- tests/acceptance-tests/test_display_configuration.cpp	2016-01-29 08:18:22 +0000
 +++ tests/acceptance-tests/test_display_configuration.cpp	2016-02-26 07:01:49 +0000
@@ -103,7 +103,7 @@
+ }
+ }
--struct DisplayConfigurationTest : mtf::ConnectedClientWithASurface
++struct LegacyDisplayConfigurationTest : mtf::ConnectedClientWithASurface
+ {
      void SetUp() override
+     {
@@ -116,7 +116,7 @@
      StubAuthorizer stub_authorizer;
  };
--TEST_F(DisplayConfigurationTest, display_configuration_reaches_client)
++TEST_F(LegacyDisplayConfigurationTest, display_configuration_reaches_client)
+ {
      auto configuration = mir_connection_create_display_config(connection);
@@ -141,7 +141,7 @@
+ }
+ }
--TEST_F(DisplayConfigurationTest, hw_display_change_notification_reaches_all_clients)
++TEST_F(LegacyDisplayConfigurationTest, hw_display_change_notification_reaches_all_clients)
+ {
      std::atomic<bool> callback_called{false};
@@ -175,7 +175,7 @@
      mir_connection_release(unsubscribed_connection);
+ }
--TEST_F(DisplayConfigurationTest, display_change_request_for_unauthorized_client_fails)
++TEST_F(LegacyDisplayConfigurationTest, display_change_request_for_unauthorized_client_fails)
+ {
      stub_authorizer.allow_configure_display = false;
@@ -250,7 +250,7 @@
  };
+ }
--TEST_F(DisplayConfigurationTest, changing_config_for_focused_client_configures_display)
++TEST_F(LegacyDisplayConfigurationTest, changing_config_for_focused_client_configures_display)
+ {
      EXPECT_CALL(mock_display, configure(_)).Times(0);
@@ -271,7 +271,7 @@
      display_client.disconnect();
+ }
--TEST_F(DisplayConfigurationTest, focusing_client_with_display_config_configures_display)
++TEST_F(LegacyDisplayConfigurationTest, focusing_client_with_display_config_configures_display)
+ {
      EXPECT_CALL(mock_display, configure(_)).Times(0);
@@ -300,7 +300,7 @@
      testing::Mock::VerifyAndClearExpectations(&mock_display);
+ }
--TEST_F(DisplayConfigurationTest, changing_focus_from_client_with_config_to_client_without_config_configures_display)
++TEST_F(LegacyDisplayConfigurationTest, changing_focus_from_client_with_config_to_client_without_config_configures_display)
+ {
      DisplayClient display_client{new_connection()};
      SimpleClient simple_client{new_connection()};
@@ -331,7 +331,7 @@
      simple_client.disconnect();
+ }
--TEST_F(DisplayConfigurationTest, hw_display_change_doesnt_apply_base_config_if_per_session_config_is_active)
++TEST_F(LegacyDisplayConfigurationTest, hw_display_change_doesnt_apply_base_config_if_per_session_config_is_active)
+ {
      DisplayClient display_client{new_connection()};
@@ -373,7 +373,7 @@
+ }
+ }
--TEST_F(DisplayConfigurationTest, shell_initiated_display_configuration_notifies_clients)
++TEST_F(LegacyDisplayConfigurationTest, shell_initiated_display_configuration_notifies_clients)
+ {
      using namespace testing;
@@ -415,7 +415,7 @@
      client.disconnect();
+ }
--TEST_F(DisplayConfigurationTest,
++TEST_F(LegacyDisplayConfigurationTest,
         client_setting_base_config_configures_display_if_a_session_config_is_not_applied)
+ {
      DisplayClient display_client{new_connection()};
@@ -434,7 +434,7 @@
      display_client.disconnect();
+ }
--TEST_F(DisplayConfigurationTest,
++TEST_F(LegacyDisplayConfigurationTest,
         client_setting_base_config_does_not_configure_display_if_a_session_config_is_applied)
+ {
      DisplayClient display_client{new_connection()};
@@ -468,7 +468,7 @@
+ }
+ }
--TEST_F(DisplayConfigurationTest,
++TEST_F(LegacyDisplayConfigurationTest,
         client_is_notified_of_new_base_config_eventually_after_set_base_configuration)
+ {
      DisplayClient display_client{new_connection()};
@@ -492,7 +492,7 @@
      display_client.disconnect();
+ }
--TEST_F(DisplayConfigurationTest,
++TEST_F(LegacyDisplayConfigurationTest,
         set_base_configuration_for_unauthorized_client_fails)
+ {
      stub_authorizer.allow_set_base_display_configuration = false;
@@ -508,7 +508,7 @@
      mir_connection_release(connection);
+ }
--TEST_F(DisplayConfigurationTest, disconnection_of_client_with_display_config_reconfigures_display)
++TEST_F(LegacyDisplayConfigurationTest, disconnection_of_client_with_display_config_reconfigures_display)
+ {
      EXPECT_CALL(mock_display, configure(_)).Times(0);
@@ -534,7 +534,7 @@
      testing::Mock::VerifyAndClearExpectations(&mock_display);
+ }
--TEST_F(DisplayConfigurationTest,
++TEST_F(LegacyDisplayConfigurationTest,
         disconnection_without_releasing_surfaces_of_client_with_display_config_reconfigures_display)
+ {
      EXPECT_CALL(mock_display, configure(_)).Times(0);
 === added file 'tests/acceptance-tests/test_new_display_configuration.cpp'
 --- tests/acceptance-tests/test_new_display_configuration.cpp	1970-01-01 00:00:00 +0000
 +++ tests/acceptance-tests/test_new_display_configuration.cpp	2016-02-26 07:01:49 +0000
@@ -0,0 +1,671 @@
++/*
++ * Copyright © 2013-2016 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: Alexandros Frantzis <alexandros.frantzis@canonical.com>
++ *              Kevin DuBois <kevin.dubois@canonical.com>
++ *              Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#include "mir/main_loop.h"
++#include "mir/frontend/session_authorizer.h"
++#include "mir/graphics/event_handler_register.h"
++#include "mir/shell/display_configuration_controller.h"
++
++#include "mir_test_framework/connected_client_with_a_surface.h"
++#include "mir/test/doubles/null_platform.h"
++#include "mir/test/doubles/fake_display.h"
++#include "mir/test/doubles/null_display_sync_group.h"
++#include "mir/test/doubles/null_platform.h"
++#include "mir/test/display_config_matchers.h"
++#include "mir/test/doubles/stub_display_configuration.h"
++#include "mir/test/doubles/stub_session_authorizer.h"
++#include "mir/test/fake_shared.h"
++#include "mir/test/pipe.h"
++#include "mir/test/wait_condition.h"
++#include "mir/test/signal.h"
++
++#include "mir_toolkit/mir_client_library.h"
++
++#include <atomic>
++#include <chrono>
++
++#include <gmock/gmock.h>
++#include <gtest/gtest.h>
++
++namespace mg = mir::graphics;
++namespace geom = mir::geometry;
++namespace mf = mir::frontend;
++namespace mtf = mir_test_framework;
++namespace mtd = mir::test::doubles;
++namespace mt = mir::test;
++
++using namespace testing;
++using namespace std::literals::chrono_literals;
++
++namespace
++{
++mtd::StubDisplayConfig stub_display_config;
++
++mtd::StubDisplayConfig changed_stub_display_config{1};
++
++class MockDisplay : public mtd::FakeDisplay
++{
++public:
++    MockDisplay(): mtd::FakeDisplay()
++    {
++        using namespace testing;
++        ON_CALL(*this, configure(_))
++            .WillByDefault(Invoke(
++                [this](mg::DisplayConfiguration const& new_config)
++                {
++                    mtd::FakeDisplay::configure(new_config);
++                }));
++    }
++
++    MOCK_METHOD1(configure, void(mg::DisplayConfiguration const&));
++};
++
++struct StubAuthorizer : mtd::StubSessionAuthorizer
++{
++    bool configure_display_is_allowed(mf::SessionCredentials const&) override
++    {
++        return allow_configure_display;
++    }
++
++    bool set_base_display_configuration_is_allowed(mf::SessionCredentials const&) override
++    {
++        return allow_set_base_display_configuration;
++    }
++
++    std::atomic<bool> allow_configure_display{true};
++    std::atomic<bool> allow_set_base_display_configuration{true};
++};
++
++void wait_for_server_actions_to_finish(mir::ServerActionQueue& server_action_queue)
++{
++    mt::WaitCondition last_action_done;
++    server_action_queue.enqueue(
++        &last_action_done,
++        [&] { last_action_done.wake_up_everyone(); });
++
++    last_action_done.wait_for_at_most_seconds(5);
++}
++}
++
++struct DisplayConfigurationTest : mtf::ConnectedClientWithASurface
++{
++    void SetUp() override
++    {
++        server.override_the_session_authorizer([this] { return mt::fake_shared(stub_authorizer); });
++        preset_display(mt::fake_shared(mock_display));
++        mtf::ConnectedClientWithASurface::SetUp();
++    }
++
++    testing::NiceMock<MockDisplay> mock_display;
++    StubAuthorizer stub_authorizer;
++};
++
++TEST_F(DisplayConfigurationTest, display_configuration_reaches_client)
++{
++    auto configuration = mir_connection_create_display_configuration(connection);
++
++    EXPECT_THAT(configuration,
++        mt::DisplayConfigMatches(std::cref(stub_display_config)));
++
++    mir_display_config_release(configuration);
++}
++
++namespace
++{
++void display_change_handler(MirConnection* connection, void* context)
++{
++    auto configuration = mir_connection_create_display_configuration(connection);
++
++    EXPECT_THAT(configuration,
++                mt::DisplayConfigMatches(std::cref(changed_stub_display_config)));
++    mir_display_config_release(configuration);
++
++    auto callback_called = static_cast<mt::Signal*>(context);
++    callback_called->raise();
++}
++}
++
++TEST_F(DisplayConfigurationTest, hw_display_change_notification_reaches_all_clients)
++{
++    mt::Signal callback_called;
++
++    mir_connection_set_display_config_change_callback(connection, &display_change_handler, &callback_called);
++
++    MirConnection* unsubscribed_connection = mir_connect_sync(new_connection().c_str(), "notifier");
++
++    mock_display.emit_configuration_change_event(
++        mt::fake_shared(changed_stub_display_config));
++
++    EXPECT_TRUE(callback_called.wait_for(std::chrono::seconds{10}));
++
++    // At this point, the message has gone out on the wire. since with unsubscribed_connection
++    // we're emulating a client that is passively subscribed, we will just wait for the display
++    // configuration to change and then will check the new config.
++
++    auto config = mir_connection_create_display_configuration(unsubscribed_connection);
++    while((unsigned)mir_display_config_get_num_outputs(config) != changed_stub_display_config.outputs.size())
++    {
++        mir_display_config_release(config);
++        std::this_thread::sleep_for(std::chrono::microseconds(500));
++        config = mir_connection_create_display_configuration(unsubscribed_connection);
++    }
++
++    EXPECT_THAT(config,
++        mt::DisplayConfigMatches(std::cref(changed_stub_display_config)));
++
++    mir_display_config_release(config);
++
++    mir_connection_release(unsubscribed_connection);
++}
++
++namespace
++{
++struct SimpleClient
++{
++    SimpleClient(std::string const& mir_test_socket) :
++        mir_test_socket{mir_test_socket} {}
++
++    void connect()
++    {
++        connection = mir_connect_sync(mir_test_socket.c_str(), __PRETTY_FUNCTION__);
++
++        auto const spec = mir_connection_create_spec_for_normal_surface(connection, 100, 100, mir_pixel_format_abgr_8888);
++        surface = mir_surface_create_sync(spec);
++        mir_surface_spec_release(spec);
++        mir_buffer_stream_swap_buffers_sync(mir_surface_get_buffer_stream(surface));
++    }
++
++    void disconnect()
++    {
++        mir_surface_release_sync(surface);
++        mir_connection_release(connection);
++    }
++
++    void disconnect_without_releasing_surface()
++    {
++        mir_connection_release(connection);
++    }
++
++    std::string mir_test_socket;
++    MirConnection* connection{nullptr};
++    MirSurface* surface{nullptr};
++};
++
++struct DisplayClient : SimpleClient
++{
++    using SimpleClient::SimpleClient;
++
++    std::unique_ptr<MirDisplayConfig,void(*)(MirDisplayConfig*)> get_base_config()
++    {
++        return {mir_connection_create_display_configuration(connection),
++            &mir_display_config_release};
++    }
++};
++}
++
++namespace
++{
++struct DisplayConfigMatchingContext
++{
++    std::function<void(MirDisplayConfig*)> matcher;
++    mt::Signal done;
++};
++
++void new_display_config_matches(MirConnection* connection, void* ctx)
++{
++    auto context = reinterpret_cast<DisplayConfigMatchingContext*>(ctx);
++
++    auto config = mir_connection_create_display_configuration(connection);
++    context->matcher(config);
++    mir_display_config_release(config);
++    context->done.raise();
++}
++}
++
++TEST_F(DisplayConfigurationTest, shell_initiated_display_configuration_notifies_clients)
++{
++    using namespace testing;
++
++    // Create a new client for explicit lifetime handling.
++    SimpleClient client{new_connection()};
++
++    client.connect();
++
++    std::shared_ptr<mg::DisplayConfiguration> new_conf;
++    new_conf = server.the_display()->configuration();
++
++    new_conf->for_each_output([](mg::UserDisplayConfigurationOutput& output)
++        {
++            if (output.connected)
++            {
++                output.used = !output.used;
++            }
++        });
++
++    DisplayConfigMatchingContext context;
++    context.matcher = [new_conf](MirDisplayConfig* conf)
++    {
++        EXPECT_THAT(conf, mt::DisplayConfigMatches(std::cref(*new_conf)));
++    };
++
++    mir_connection_set_display_config_change_callback(
++        client.connection,
++        &new_display_config_matches,
++        &context);
++
++    server.the_display_configuration_controller()->set_base_configuration(new_conf);
++
++    EXPECT_TRUE(context.done.wait_for(std::chrono::seconds{10}));
++
++    EXPECT_THAT(
++        *server.the_display()->configuration(),
++        mt::DisplayConfigMatches(std::cref(*new_conf)));
++
++    client.disconnect();
++}
++
++struct DisplayPowerSetting : public DisplayConfigurationTest, public ::testing::WithParamInterface<MirPowerMode> {};
++struct DisplayOrientationSetting : public DisplayConfigurationTest, public ::testing::WithParamInterface<MirOrientation> {};
++struct DisplayFormatSetting : public DisplayConfigurationTest, public ::testing::WithParamInterface<MirPixelFormat> {};
++
++TEST_P(DisplayPowerSetting, can_get_power_mode)
++{
++    using namespace testing;
++
++    auto mode = GetParam();
++
++    std::shared_ptr<mg::DisplayConfiguration> server_config = server.the_display()->configuration();
++
++    server_config->for_each_output(
++        [mode](mg::UserDisplayConfigurationOutput& output)
++        {
++            output.power_mode = mode;
++        });
++
++    server.the_display_configuration_controller()->set_base_configuration(server_config);
++    wait_for_server_actions_to_finish(*server.the_main_loop());
++
++    DisplayClient client{new_connection()};
++
++    client.connect();
++
++    auto client_config = client.get_base_config();
++
++    for (int i = 0; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
++    {
++        auto output = mir_display_config_get_output(client_config.get(), i);
++
++        EXPECT_THAT(mir_output_get_power_mode(output), Eq(mode));
++    }
++
++    client.disconnect();
++}
++
++TEST_P(DisplayOrientationSetting, can_get_orientation)
++{
++    using namespace testing;
++
++    auto orientation = GetParam();
++
++    std::shared_ptr<mg::DisplayConfiguration> server_config = server.the_display()->configuration();
++
++    server_config->for_each_output(
++        [orientation](mg::UserDisplayConfigurationOutput& output)
++        {
++            output.orientation = orientation;
++        });
++
++    mock_display.emit_configuration_change_event(server_config);
++    mock_display.wait_for_configuration_change_handler();
++
++    DisplayClient client{new_connection()};
++
++    client.connect();
++
++    auto client_config = client.get_base_config();
++
++    for (int i = 0; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
++    {
++        auto output = mir_display_config_get_output(client_config.get(), i);
++
++        EXPECT_THAT(mir_output_get_orientation(output), Eq(orientation));
++    }
++
++    client.disconnect();
++}
++
++namespace
++{
++std::vector<MirPixelFormat> const formats{
++    mir_pixel_format_abgr_8888,
++    mir_pixel_format_xbgr_8888,
++    mir_pixel_format_argb_8888,
++    mir_pixel_format_xrgb_8888,
++    mir_pixel_format_bgr_888,
++    mir_pixel_format_rgb_888,
++    mir_pixel_format_rgb_565,
++    mir_pixel_format_rgba_5551,
++    mir_pixel_format_rgba_4444,
++};
++}
++
++TEST_P(DisplayFormatSetting, can_get_all_output_format)
++{
++    using namespace testing;
++
++    auto format = GetParam();
++    mtd::StubDisplayConfig single_format_config(1, {format});
++
++    mock_display.emit_configuration_change_event(mt::fake_shared(single_format_config));
++    mock_display.wait_for_configuration_change_handler();
++
++    DisplayClient client{new_connection()};
++
++    client.connect();
++
++    auto client_config = client.get_base_config();
++
++    for (int i = 0; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
++    {
++        auto output = mir_display_config_get_output(client_config.get(), i);
++
++        EXPECT_THAT(mir_output_get_current_pixel_format(output), Eq(format));
++    }
++
++    client.disconnect();
++}
++
++INSTANTIATE_TEST_CASE_P(DisplayConfiguration, DisplayPowerSetting,
++    Values(mir_power_mode_on, mir_power_mode_standby, mir_power_mode_suspend, mir_power_mode_off));
++
++INSTANTIATE_TEST_CASE_P(DisplayConfiguration, DisplayFormatSetting,
++    ValuesIn(formats));
++
++TEST_F(DisplayConfigurationTest, client_received_configuration_matches_server_config)
++{
++    mg::DisplayConfigurationMode hd{{1280, 720}, 60.0};
++    mg::DisplayConfigurationMode fhd{{1920, 1080}, 60.0};
++    mg::DisplayConfigurationMode proper_size{{1920, 1200}, 60.0};
++    mg::DisplayConfigurationMode retina{{3210, 2800}, 60.0};
++
++    mtd::StubDisplayConfigurationOutput tv{
++        mg::DisplayConfigurationOutputId{1},
++        {hd, fhd},
++        {mir_pixel_format_abgr_8888}};
++    mtd::StubDisplayConfigurationOutput monitor{
++        mg::DisplayConfigurationOutputId{2},
++        {hd, fhd, proper_size, retina},
++        {mir_pixel_format_abgr_8888}};
++
++    std::vector<mg::DisplayConfigurationOutput> outputs{tv, monitor};
++
++    auto config = std::make_shared<mtd::StubDisplayConfig>(outputs);
++
++    mock_display.emit_configuration_change_event(config);
++    mock_display.wait_for_configuration_change_handler();
++
++    DisplayClient client{new_connection()};
++
++    client.connect();
++
++    auto client_config = client.get_base_config();
++    auto server_config = server.the_display()->configuration();
++
++    EXPECT_THAT(client_config.get(), mt::DisplayConfigMatches(std::cref(*server_config)));
++
++    client.disconnect();
++}
++
++TEST_F(DisplayConfigurationTest, client_receives_correct_mode_information)
++{
++    mg::DisplayConfigurationMode hd{{1280, 720}, 60.0};
++    mg::DisplayConfigurationMode fhd{{1920, 1080}, 60.0};
++    mg::DisplayConfigurationMode proper_size{{1920, 1200}, 60.0};
++    mg::DisplayConfigurationMode retina{{3210, 2800}, 60.0};
++
++    mg::DisplayConfigurationOutputId const id{2};
++
++    std::vector<mg::DisplayConfigurationMode> modes{hd, fhd, proper_size, retina};
++
++    mtd::StubDisplayConfigurationOutput monitor{
++        id,
++        modes,
++        {mir_pixel_format_abgr_8888}};
++
++    std::vector<mg::DisplayConfigurationOutput> outputs{monitor};
++
++    mock_display.emit_configuration_change_event(std::make_shared<mtd::StubDisplayConfig>(outputs));
++    mock_display.wait_for_configuration_change_handler();
++
++    DisplayClient client{new_connection()};
++
++    client.connect();
++
++    auto config = client.get_base_config();
++
++    ASSERT_THAT(mir_display_config_get_num_outputs(config.get()), Eq(1));
++
++    std::vector<mg::DisplayConfigurationMode> received_modes;
++
++    auto output = mir_display_config_get_output(config.get(), 0);
++
++    for (auto i = 0; i < mir_output_get_num_modes(output) ; ++i)
++    {
++        auto mode = mir_output_get_mode(output, i);
++        auto width = mir_output_mode_get_width(mode);
++        auto height = mir_output_mode_get_height(mode);
++        auto refresh = mir_output_mode_get_refresh_rate(mode);
++
++        received_modes.push_back(mg::DisplayConfigurationMode{{width, height}, refresh});
++    }
++
++    EXPECT_THAT(received_modes, ContainerEq(modes));
++
++    client.disconnect();
++}
++
++TEST_F(DisplayConfigurationTest, mode_width_and_height_are_independent_of_orientation)
++{
++    mg::DisplayConfigurationMode hd{{1280, 720}, 60.0};
++    mg::DisplayConfigurationMode fhd{{1920, 1080}, 60.0};
++    mg::DisplayConfigurationMode proper_size{{1920, 1200}, 60.0};
++    mg::DisplayConfigurationMode retina{{3210, 2800}, 60.0};
++
++    mg::DisplayConfigurationOutputId const id{2};
++
++    std::vector<mg::DisplayConfigurationMode> modes{hd, fhd, proper_size, retina};
++
++    mtd::StubDisplayConfigurationOutput monitor{
++        id,
++        modes,
++        {mir_pixel_format_abgr_8888}};
++
++    std::vector<mg::DisplayConfigurationOutput> outputs{monitor};
++
++    std::shared_ptr<mg::DisplayConfiguration> server_config;
++    server_config = std::make_shared<mtd::StubDisplayConfig>(outputs);
++    mock_display.emit_configuration_change_event(server_config);
++    mock_display.wait_for_configuration_change_handler();
++
++    DisplayClient client{new_connection()};
++    client.connect();
++
++
++    DisplayConfigMatchingContext context;
++    context.matcher = [&server_config](MirDisplayConfig* conf)
++        {
++            EXPECT_THAT(conf, mt::DisplayConfigMatches(std::cref(*server_config)));
++        };
++
++    mir_connection_set_display_config_change_callback(
++        client.connection,
++        &new_display_config_matches,
++        &context);
++
++    for (auto const orientation :
++        {mir_orientation_normal, mir_orientation_left, mir_orientation_inverted, mir_orientation_right})
++    {
++        server_config = server.the_display()->configuration();
++        server_config->for_each_output(
++            [orientation](mg::UserDisplayConfigurationOutput& output)
++            {
++                output.orientation = orientation;
++            });
++        server.the_display_configuration_controller()->set_base_configuration(server_config);
++
++        EXPECT_TRUE(context.done.wait_for(std::chrono::seconds{10}));
++        context.done.reset();
++
++        auto config = client.get_base_config();
++        ASSERT_THAT(mir_display_config_get_num_outputs(config.get()), Eq(1));
++
++        std::vector<mg::DisplayConfigurationMode> received_modes;
++
++        auto output = mir_display_config_get_output(config.get(), 0);
++
++        for (auto i = 0; i < mir_output_get_num_modes(output) ; ++i)
++        {
++            auto mode = mir_output_get_mode(output, i);
++            auto width = mir_output_mode_get_width(mode);
++            auto height = mir_output_mode_get_height(mode);
++            auto refresh = mir_output_mode_get_refresh_rate(mode);
++
++            received_modes.push_back(mg::DisplayConfigurationMode{{width, height}, refresh});
++        }
++
++        EXPECT_THAT(received_modes, ContainerEq(modes));
++    }
++
++    client.disconnect();
++}
++
++TEST_F(DisplayConfigurationTest, output_position_is_independent_of_orientation)
++{
++    std::array<mir::geometry::Point, 3> const positions = {{
++        mir::geometry::Point{-100, 10},
++        mir::geometry::Point{100, 10000},
++        mir::geometry::Point{-100, 10}
++    }};
++
++    std::shared_ptr<mg::DisplayConfiguration> server_config = server.the_display()->configuration();
++    server_config->for_each_output(
++        [position = positions.begin()](mg::UserDisplayConfigurationOutput& output) mutable
++        {
++            output.top_left = *position;
++            ++position;
++        });
++    server.the_display_configuration_controller()->set_base_configuration(server_config);
++
++    DisplayClient client{new_connection()};
++
++    client.connect();
++
++    DisplayConfigMatchingContext context;
++    context.matcher = [&server_config](MirDisplayConfig* conf)
++        {
++            EXPECT_THAT(conf, mt::DisplayConfigMatches(std::cref(*server_config)));
++        };
++
++    mir_connection_set_display_config_change_callback(
++        client.connection,
++        &new_display_config_matches,
++        &context);
++
++    for (auto const orientation :
++        {mir_orientation_normal, mir_orientation_left, mir_orientation_inverted, mir_orientation_right})
++    {
++        server_config = server.the_display()->configuration();
++        server_config->for_each_output(
++            [orientation](mg::UserDisplayConfigurationOutput& output)
++                {
++                    output.orientation = orientation;
++                });
++        server.the_display_configuration_controller()->set_base_configuration(server_config);
++
++        EXPECT_TRUE(context.done.wait_for(std::chrono::seconds{10}));
++        context.done.reset();
++
++        auto config = client.get_base_config();
++
++        auto position = positions.begin();
++        for (auto i = 0; i < mir_display_config_get_num_outputs(config.get()); ++i)
++        {
++            auto output = mir_display_config_get_output(config.get(), i);
++
++            EXPECT_THAT(mir_output_get_position_x(output), Eq(position->x.as_int()));
++            EXPECT_THAT(mir_output_get_position_y(output), Eq(position->y.as_int()));
++
++            ++position;
++        }
++    }
++
++    client.disconnect();
++}
++
++TEST_F(DisplayConfigurationTest, client_receives_correct_output_positions)
++{
++    std::array<mir::geometry::Point, 3> const positions = {{
++        mir::geometry::Point{-100, 10},
++        mir::geometry::Point{100, 10000},
++        mir::geometry::Point{-100, 10}
++    }};
++
++    std::shared_ptr<mg::DisplayConfiguration> server_config = server.the_display()->configuration();
++    server_config->for_each_output(
++    [position = positions.begin()](mg::UserDisplayConfigurationOutput& output) mutable
++    {
++        output.top_left = *position;
++        ++position;
++    });
++
++    DisplayClient client{new_connection()};
++
++    client.connect();
++
++    DisplayConfigMatchingContext context;
++    context.matcher = [server_config](MirDisplayConfig* conf)
++        {
++            EXPECT_THAT(conf, mt::DisplayConfigMatches(std::cref(*server_config)));
++        };
++
++    mir_connection_set_display_config_change_callback(
++        client.connection,
++        &new_display_config_matches,
++        &context);
++
++    server.the_display_configuration_controller()->set_base_configuration(server_config);
++
++    EXPECT_TRUE(context.done.wait_for(std::chrono::seconds{10}));
++
++    auto config = client.get_base_config();
++
++    auto position = positions.begin();
++    for (auto i = 0; i < mir_display_config_get_num_outputs(config.get()); ++i)
++    {
++        auto output = mir_display_config_get_output(config.get(), i);
++
++        EXPECT_THAT(mir_output_get_position_x(output), Eq(position->x.as_int()));
++        EXPECT_THAT(mir_output_get_position_y(output), Eq(position->y.as_int()));
++
++        ++position;
++    }
++
++    client.disconnect();
++}
 === modified file 'tests/mir_test/display_config_matchers.cpp'
 --- tests/mir_test/display_config_matchers.cpp	2016-01-29 08:18:22 +0000
 +++ tests/mir_test/display_config_matchers.cpp	2016-02-26 07:01:49 +0000
@@ -20,6 +20,7 @@
  #include "mir/graphics/display_configuration.h"
  #include "mir_protobuf.pb.h"
  #include "mir_toolkit/client_types.h"
++#include "mir_toolkit/mir_display_configuration.h"
  #include <gtest/gtest.h>
  namespace mg = mir::graphics;
@@ -30,6 +31,18 @@
  namespace
+ {
++size_t find_mode_index(MirOutput const* output, MirOutputMode const* mode)
++{
++    for (int i = 0; i < mir_output_get_num_modes(output); ++i)
++    {
++        if (mir_output_get_mode(output, i) == mode)
++        {
++            return i;
++        }
++    }
++    return static_cast<size_t>(-1);
++}
++
  class TestDisplayConfiguration : public mg::DisplayConfiguration
+ {
  public:
@@ -168,6 +181,68 @@
+         }
+     }
++    TestDisplayConfiguration(MirDisplayConfig const* config)
++    {
++        /* Cards; fake it, 'cause we only ever support 1 card at the moment */
++        cards.push_back(
++            mg::DisplayConfigurationCard{
++                mg::DisplayConfigurationCardId{1},
++                static_cast<size_t>(mir_display_config_get_max_simultaneous_outputs(config))
++            });
++
++        /* Outputs */
++        for (int i = 0; i < mir_display_config_get_num_outputs(config); i++)
++        {
++            auto const client_output = mir_display_config_get_output(config, i);
++            mg::DisplayConfigurationOutput display_output
++                {
++                    mg::DisplayConfigurationOutputId(mir_output_get_id(client_output)),
++                    mg::DisplayConfigurationCardId(1),
++                    static_cast<mg::DisplayConfigurationOutputType>(mir_output_get_type(client_output)),
++                    {},
++                    {},
++                    static_cast<uint32_t>(find_mode_index(client_output, mir_output_get_preferred_mode(client_output))),
++                    geom::Size{mir_output_get_physical_width_mm(client_output),
++                               mir_output_get_physical_height_mm(client_output)},
++                    mir_output_get_connection_state(client_output) == mir_output_connection_state_connected,
++                    mir_output_is_enabled(client_output),
++                    geom::Point{mir_output_get_position_x(client_output),
++                        mir_output_get_position_y(client_output)},
++                    static_cast<uint32_t>(find_mode_index(client_output, (mir_output_get_current_mode(client_output)))),
++                    mir_output_get_current_pixel_format(client_output),
++                    mir_output_get_power_mode(client_output),
++                    mir_output_get_orientation(client_output),
++                    1.0f,
++                    mir_form_factor_monitor
++                };
++
++            /* Modes */
++            std::vector<mg::DisplayConfigurationMode> modes;
++            for (int n = 0; n < mir_output_get_num_modes(client_output); n++)
++            {
++                auto const client_mode = mir_output_get_mode(client_output, n);
++                modes.push_back(
++                    {
++                        geom::Size{
++                            mir_output_mode_get_width(client_mode),
++                            mir_output_mode_get_height(client_mode)},
++                        mir_output_mode_get_refresh_rate(client_mode)
++                    });
++            }
++            display_output.modes = modes;
++
++            /* Pixel formats */
++            std::vector<MirPixelFormat> pixel_formats;
++            for (int n = 0; n < mir_output_get_num_pixel_formats(client_output); n++)
++            {
++                pixel_formats.push_back(mir_output_get_pixel_format(client_output, n));
++            }
++            display_output.pixel_formats = pixel_formats;
++
++            outputs.push_back(display_output);
++        }
++    }
++
      TestDisplayConfiguration(TestDisplayConfiguration const& other)
          : mg::DisplayConfiguration(),
            cards{other.cards},
@@ -323,3 +398,25 @@
      TestDisplayConfiguration config2{*display_config2};
      return compare_display_configurations(display_config1, config2);
+ }
++
++bool mt::compare_display_configurations(MirDisplayConfig const* client_config,
++    mg::DisplayConfiguration const& server_config)
++{
++    TestDisplayConfiguration translated_config{client_config};
++    return compare_display_configurations(server_config, translated_config);
++}
++
++bool mt::compare_display_configurations(mg::DisplayConfiguration const& server_config,
++    MirDisplayConfig const* client_config)
++{
++    TestDisplayConfiguration translated_config{client_config};
++    return compare_display_configurations(server_config, translated_config);
++}
++
++bool mt::compare_display_configurations(MirDisplayConfig const* config1,
++    MirDisplayConfig const* config2)
++{
++    TestDisplayConfiguration translated_config_one{config1};
++    TestDisplayConfiguration translated_config_two{config2};
++    return compare_display_configurations(translated_config_one, translated_config_two);
++}
 === modified file 'tests/mir_test_doubles/stub_display_configuration.cpp'
 --- tests/mir_test_doubles/stub_display_configuration.cpp	2015-12-03 17:22:33 +0000
 +++ tests/mir_test_doubles/stub_display_configuration.cpp	2016-02-26 07:01:49 +0000
@@ -19,6 +19,7 @@
  #include "mir/test/doubles/stub_display_configuration.h"
  #include <limits>
++#include <boost/throw_exception.hpp>
  namespace mtd = mir::test::doubles;
@@ -52,6 +53,35 @@
+ {
+ }
++mtd::StubDisplayConfigurationOutput::StubDisplayConfigurationOutput(
++    graphics::DisplayConfigurationOutputId id,
++    std::vector<graphics::DisplayConfigurationMode> modes,
++    std::vector<MirPixelFormat> formats)
++    : DisplayConfigurationOutput{
++        id,
++        graphics::DisplayConfigurationCardId{0},
++        graphics::DisplayConfigurationOutputType::edp,
++        formats,
++        modes,
++        static_cast<uint32_t>(modes.size() - 1),
++        {200, 200},
++        true,
++        true,
++        {0, 0},
++        0,
++        formats[0],
++        mir_power_mode_on,
++        mir_orientation_normal,
++        1.0f,
++        mir_form_factor_monitor
++    }
++{
++    if (modes.empty())
++    {
++        BOOST_THROW_EXCEPTION(std::logic_error{"Attempted to create a stub output with no modes"});
++    }
++}
++
  mtd::StubDisplayConfig::StubDisplayConfig() :
      StubDisplayConfig(3)
+ {
@@ -95,7 +125,7 @@
  mtd::StubDisplayConfig::StubDisplayConfig(unsigned int num_displays, std::vector<MirPixelFormat> const& pfs)
+ {
      /* construct a non-trivial dummy display config to send */
--    int mode_index = 0;
++    int mode_index = 1;
      for (auto i = 0u; i < num_displays; i++)
+     {
          std::vector<graphics::DisplayConfigurationMode> modes;
@@ -131,7 +161,7 @@
          geometry::Point top_left{};
          graphics::DisplayConfigurationOutput output{
              graphics::DisplayConfigurationOutputId{static_cast<int>(i + 1)},
--            graphics::DisplayConfigurationCardId{static_cast<int>(i)},
++            graphics::DisplayConfigurationCardId{1},
              graphics::DisplayConfigurationOutputType::vga,
              pfs,
              connected(i) ? modes : std::vector<graphics::DisplayConfigurationMode>{},
@@ -148,14 +178,13 @@
          };
          outputs.push_back(output);
--
--        graphics::DisplayConfigurationCard card{
--            graphics::DisplayConfigurationCardId{static_cast<int>(i)},
--            i + 1
--        };
--
--        cards.push_back(card);
+     }
++    graphics::DisplayConfigurationCard card{
++        graphics::DisplayConfigurationCardId{1},
++        5
++    };
++
++    cards.push_back(card);
+ }
  mtd::StubDisplayConfig::StubDisplayConfig(std::vector<geometry::Rectangle> const& rects)