Mir

Merge lp:~raof/mir/the-least-dirty-thing into lp:mir

the-least-dirty-thing
Merge into development-branch

Proposed by Chris Halse Rogers on 2014-08-21

Status:

Merged

Approved by:

Alberto Aguirre on 2014-10-27

Approved revision:

no longer in the source branch.

Merged at revision:

2005

Proposed branch:

lp:~raof/mir/the-least-dirty-thing

Merge into:

lp:mir

Diff against target:

1865 lines (+1026/-46)

53 files modified

client-ABI-sha1sums (+1/-1)
debian/control (+46/-0)
debian/libmirclient-debug-extension-dev.install (+3/-0)
debian/libmirclient-debug-extension1.install (+1/-0)
debian/libmirclient-dev.install (+1/-1)
include/client/mir_toolkit/debug/surface.h (+20/-0)
include/platform/mir/options/configuration.h (+1/-0)
include/server/mir/default_server_configuration.h (+3/-0)
include/server/mir/scene/coordinate_translator.h (+66/-0)
platform-ABI-sha1sums (+1/-1)
server-ABI-sha1sums (+3/-2)
src/client/CMakeLists.txt (+52/-2)
src/client/mir_connection.cpp (+3/-1)
src/client/mir_connection.h (+1/-0)
src/client/mir_debug_api.cpp (+39/-0)
src/client/mir_surface.cpp (+63/-0)
src/client/mir_surface.h (+16/-1)
src/client/mir_surface_api.cpp (+2/-3)
src/client/mirclient-debug-extension.pc.in (+11/-0)
src/client/mirclient.pc.in (+5/-3)
src/client/symbols-debug.map (+5/-0)
src/common/protobuf/mir_protobuf.proto (+17/-0)
src/common/symbols.map (+54/-0)
src/include/server/mir/frontend/unsupported_feature_exception.h (+41/-0)
src/platform/options/default_configuration.cpp (+4/-1)
src/platform/symbols.map (+1/-0)
src/server/frontend/CMakeLists.txt (+1/-0)
src/server/frontend/default_configuration.cpp (+21/-8)
src/server/frontend/default_ipc_factory.cpp (+6/-3)
src/server/frontend/default_ipc_factory.h (+8/-1)
src/server/frontend/protobuf_message_processor.cpp (+37/-0)
src/server/frontend/session_mediator.cpp (+30/-2)
src/server/frontend/session_mediator.h (+16/-2)
src/server/frontend/unsupported_coordinate_translator.cpp (+32/-0)
src/server/frontend/unsupported_coordinate_translator.h (+37/-0)
src/server/scene/CMakeLists.txt (+1/-0)
src/server/scene/default_configuration.cpp (+11/-0)
src/server/scene/default_coordinate_translator.cpp (+32/-0)
src/server/scene/default_coordinate_translator.h (+37/-0)
src/server/symbols.map (+6/-0)
tests/acceptance-tests/CMakeLists.txt (+2/-0)
tests/acceptance-tests/test_client_library.cpp (+0/-1)
tests/acceptance-tests/test_client_surface_events.cpp (+1/-1)
tests/acceptance-tests/test_client_surfaces.cpp (+1/-1)
tests/acceptance-tests/test_custom_input_dispatcher.cpp (+2/-2)
tests/acceptance-tests/test_debug_api.cpp (+231/-0)
tests/include/mir_test_doubles/mock_coordinate_translator.h (+42/-0)
tests/include/mir_test_doubles/mock_surface.h (+2/-1)
tests/integration-tests/CMakeLists.txt (+1/-0)
tests/integration-tests/test_stale_frames.cpp (+1/-1)
tests/unit-tests/frontend/test_session_mediator.cpp (+5/-5)
tests/unit-tests/frontend/test_session_mediator_android.cpp (+1/-1)
tests/unit-tests/frontend/test_session_mediator_mesa.cpp (+2/-1)

To merge this branch:

bzr merge lp:~raof/mir/the-least-dirty-thing

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration		Approve on 2014-10-27
Alberto Aguirre (community)			Approve on 2014-10-27
Alan Griffiths			Approve on 2014-10-14
Daniel van Vugt		2014-08-21	Abstain on 2014-10-14
Andreas Pokorny (community)			Approve on 2014-09-24
Alexandros Frantzis (community)			Approve on 2014-09-19
Kevin DuBois (community)			Abstain on 2014-09-08
Review via email: mp+231679@code.launchpad.net

Commit message

Add a debug interface to translate from surface to screen coordinates.

This is the Mir part of the infrastructure for Autopilot to determine the screen location of widgets, for full-stack testing.

This is hidden behind a --debug server option, to make it absolutely clear that applications cannot depend on this functionality outside of a constrained environment.

Fixes: https://bugs.launchpad.net/mir/+bug/1346633

Description of the change

Add a translate_surface_to_screen debug interface, hidden behind running the server with --debug

This can be somewhat cleaned up when we've got real support for extensions, but should do for now.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-08-21:

FAILED: Continuous integration, rev:1848
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2455/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1407/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1413/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1390/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/977
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/977/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/327/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-08-21:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-08-22:

Minor observations so far:

(1) 18 +Package: libmirclient-debug
is potentially confusing with the existing convention: packagename-dbg
so I think the dash needs removing. Something like "libmirclienttesting", since it's really not about "debugging" Mir at all.

(2) It seems we're going backwards in code bloat faster than I can keep up with fixing it. Can we avoid this part?
176 +
177 +# TODO: Extension apparatus so that mirclient-debug can be a MODULE and not
178 +# duplicate (almost) the entirity of mirclient
179 +add_library(
180 + mirclient-debug SHARED
181 +
182 + mir_debug_api.cpp
183 + $<TARGET_OBJECTS:mirclientobjects>
184 +)

(3) The amount of coupling we have is a bit excessive. Or is that an existing issue imposed by the existing class design?; "make_mediator()"

review: Needs Fixing

Revision history for this message

Kevin DuBois (kdub) wrote on 2014-08-22:

36 + Interfaces will come and go; end-user applications SHOULD NOT be linked
37 + against this library.

"interfaces come and go" is true of all the public interfaces... We should say something stronger like:
"Not all mir servers run with the capability to service this api; end-user applications SHOULD NOT depend on the functionality of this api."

review: Needs Fixing

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-08-25:

> Minor observations so far:
>
> (1) 18 +Package: libmirclient-debug
> is potentially confusing with the existing convention: packagename-dbg
> so I think the dash needs removing. Something like "libmirclienttesting",
> since it's really not about "debugging" Mir at all.

It actually is a bit about debugging Mir; at least, I've used the symbols in there to debug Mir and low-level client bits like XMir.

>
> (2) It seems we're going backwards in code bloat faster than I can keep up
> with fixing it. Can we avoid this part?

Not without either (a) exposing all the libmirclient internal symbols, or (b) adding in a real extension mechanism.

Since this library isn't expected to be installed on anything but development machines I wouldn't care if it was 100MB. I don't think we should spend any significant effort caring about its size.

We'll want an actual extension mechanism at some point in the future. At that point this can be switched to using that and libmirclient-debug-extension.so won't be almost exactly the same size as libmirclient.so

>
> (3) The amount of coupling we have is a bit excessive. Or is that an existing
> issue imposed by the existing class design?; "make_mediator()"

Yeah, that's the existing class design.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-08-25:

PASSED: Continuous integration, rev:1852
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2488/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1441
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1447
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1426
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1010
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1010/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/363
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/363/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2507
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12020

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

review: Approve (continuous-integration)

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-08-26:

> "Not all mir servers run with the capability to service this api; end-user applications SHOULD NOT depend on the functionality of this api."

I agree with Kevin. I don't think it's worth having a different debugging version of the client library. Until we get an extension mechanism, I would prefer just exposing the debug APIs in our normal client library builds (like we have been doing up to now).

review: Needs Fixing

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-08-26:

From the client perspective it *is* an extension; libmirclient-debug-extension.so exposes only the three symbols we currently support; it's not a debugging version of the client library.

I can, of course, fold those symbols back into libmirclient. I think it's useful to have the non-core things split out a bit, though.

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-08-28:

OK, I still have the same concerns as mentioned above. But nothing strictly blocking. That's not to say this is a proper review.

review: Abstain

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-09-08:

So, I'm not sure what needs to be done here in order to make this acceptable.

Do I need to fold the three debugging APIs back into libmirclient rather than their own DSO? Do I need to reduce the on-disc size of libmirclient-debug-extension by introducing infrastructure for extension loading in libmirclient?

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-09-08:

> Do I need to fold the three debugging APIs back into libmirclient rather than their own DSO?
> Do I need to reduce the on-disc size of libmirclient-debug-extension by introducing
> infrastructure for extension loading in libmirclient?

I am fine with folding the debugging APIs back into libmirclient until we get a proper extension mechanism.

Revision history for this message

Kevin DuBois (kdub) wrote on 2014-09-08:

So, the stronger wording seems better, so that 'needs fixing' is resolved. As for having two libraries vs the one, I'm on the fence, so I won't object. I somewhat like that there's less temptation to roll these functions into the public api.

As for the DebuggingSessionMediator, it seems to me that the SessionMediator should depend on (via a constructor parameter) some object that is capable of looking up the coordinates. A debug implementation of this object would lookup the coordinates, the normal implementation would throw when the lookup is attempted.
OTOH, SessionMediator already has a pretty bloated constructor... but I'd rather have one bloated constructor on SessionMediator than a bloated constructor on both SessionMediator and DebuggingSessionMediator.

review: Abstain

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-09-10:

Yeah, I could make an extra interface for SessionMediator to depend on. I'll ask the list first about the general “how many DSOs should this be” question and then see what that version of SessionMediator would look like.

A bit ugly, because we're adding even more special-purpose code to a general interface, but until it gains an extension mechanism... :)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-09-10:

In src/server/frontend/debugging_session_mediator.cpp
743 + response->set_x(request->x() + surface->top_left().x.as_int());
744 + response->set_y(request->y() + surface->top_left().y.as_int());

You have to transform x and y according to the surface orientation here (and in theory surface transformation but i doubt we want to be super correct - makes me wonder whether unity8 overrides that information stored at the surface? ). Furthermore in the nested case you have to forward that request to the system compositor.

So dl-load the debug extension in mirserver, and fail unlikely if not available.

Regarding the library vs internal topic. I prefer the separate library approach already implemented in this MP

review: Needs Fixing

Revision history for this message

kevin gunn (kgunn72) wrote on 2014-09-10:

As long as it's a dbug module that's not going to be part of a release image & you have to install when you want. Then I prefer the bloat-on-debug hit to keep it separated from regular ol' libmirclient.

however, is there a plan or mechanism to help resynch this with libmirclient where the code is duplicated ?
iirc, we're adding primarily for AP testing... so we'd want to keep the the functionality the same in the debug client, as to ensure the testing/config is valid and we trust the results.

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-09-10:

Clients wanting to use the debug extensions link against libmirclient as normal, and *also* libmirclient-debug-extension. There's no duplication of source code, just duplication of object code (as libmirclient-debug-extension needs access to the MirSurface implementation).

Andreas: Urgh, yeah, I guess so. I _think_ we have enough information in the nested case to do the translation without bouncing up to the host server, but it'll be safer to do so, yeah.

Revision history for this message

Kevin DuBois (kdub) wrote on 2014-09-15:

Its looking like we're coalescing around 2 DSO's then, everyone? (a second debug library is good by me)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-09-17:

Andreas: Actually, I don't think we *do* need to forward to the system compositor - the nested compositor ensures it has a logical coordinate space that's identical to the system compositor's by means of the display configuration.

And we don't actually use the surface orientation anywhere, so I don't think we need to transform according to that either?

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-18:

FAILED: Continuous integration, rev:1855
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2758/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1808/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1815
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1782/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1280
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1280/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/719/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-19:

FAILED: Continuous integration, rev:1857
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2766/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1818/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1825/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1791/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1288/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/728/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-19:

PASSED: Continuous integration, rev:1858
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2770/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1822
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1829
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1795
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1292
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1292/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/732
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/732/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2827
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/13449

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

review: Approve (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-09-19:

> Andreas: Actually, I don't think we *do* need to forward to the system
> compositor - the nested compositor ensures it has a logical coordinate space
> that's identical to the system compositor's by means of the display
> configuration.
>
> And we don't actually use the surface orientation anywhere, so I don't think
> we need to transform according to that either

I agree for a 'we' as in u-s-c + unity8 - since usc does not change the properties of unity8 surface. hmm.. it could still decide that unity8 and thus the client surface is not visible. But that is probably not a relevant result of the transformation? So I agree if you count demo shell out.

> And we don't actually use the surface orientation anywhere, so I don't think
> we need to transform according to that either

I thought for that calculation coordinates are given as client surface coordinates. Rotating the device turns a client surface of 400x900 pixels into a 900x400 grid. Whatever is now on 800x10, must be touched by the user at 390x800 or 10x100...

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-09-19:

OK. Hopefully we will get a proper extension mechanism in the near/medium term.

509 +TEST_F(TestDebugAPI, ApiIsUnavaliableWhenServerNotStartedWithDebug)

"Unavailable"

> I thought for that calculation coordinates are given as client surface coordinates. Rotating the
> device turns a client surface of 400x900 pixels into a 900x400 grid. Whatever is now on 800x10,
> must be touched by the user at 390x800 or 10x100...

Not sure about this aspect of the code, so a weak approval for me pending further information on this.

review: Approve

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-09-22:

As far as I can see qtmir/unity8 is not using mirs orientation values(, yet?). But it will need a way to influence the calculation instead.

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-09-22:

Yup. CoordinateTranslator is now available for overload by shells.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-22:

FAILED: Continuous integration, rev:1859
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2779/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1834
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1841/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1807/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1301/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/744/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-23:

PASSED: Continuous integration, rev:1860
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2785/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1840
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1847
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1813
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1307
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1307/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/750
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/750/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2844
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/13570

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

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-23:

FAILED: Continuous integration, rev:1861
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2786/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1842
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1849/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1815/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1308/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/752/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-23:

FAILED: Continuous integration, rev:1862
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2787/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1844
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1851
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1817/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1309
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1309/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/754
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/754/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2847/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/13573

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-09-24:

lgtm.

one little detail .. why int32_t in the protobuf messages/int in the client api - but unsigned in the CoordinateTranslator interface and the default implementation does .as_unit32_t(). Looking at the code I believe negative values here are properly transported and returned.. and also in the case where the surface position is promoted to an unsigned int (which could be potentially negative) in 1280, there is only an addition and the result is converted back to int. A bit dirty but it should still work.

(negative positions could be possible too, if decorations are in negative coordinate space of the surface)

review: Approve

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-09-24:

Probably best to use int32_t everywhere. Good catch.

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-09-25:

We do indeed have negative coordinates. So make sure your x's and y's are signed.

Although I recommend against int32_t for a few reasons. "int" is better because:
  * On any modern platform "int" is already >= 32 bits
  * On some 64-bit platforms "int" is 64-bit and it's more efficient to use that native word size than forcing 32-bits.
  * int32_t increases coupling to yet another header.

review: Needs Fixing

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-09-30:

> Although I recommend against int32_t for a few reasons. "int" is better
> because:
> * On any modern platform "int" is already >= 32 bits
> * On some 64-bit platforms "int" is 64-bit and it's more efficient to use
> that native word size than forcing 32-bits.
> * int32_t increases coupling to yet another header.

No since it involves protobuf (https://developers.google.com/protocol-buffers/docs/proto#scalar)
The type should match the type assigned in protobuf or in the very least specifically converted from int32_t to whatever internal type you use. The last thing we need is a subtle type conversion issue.

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-10-01:

Alright, still needs conversion to signed ints though. Like in:
232 + virtual geometry::Point surface_to_screen(std::shared_ptr<frontend::Surface> surface,
233 + uint32_t x, uint32_t y) = 0;

Even local coordinates can go negative -- Consider what happens when you drag something like a scroll bar or image. The scrolling keeps responding even when the pointer itself is out of the window (so could be in negative coordinates).

review: Needs Fixing

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-10-08:

Actually I still disagree with int32_t. If you make the decision to use that type just because Protobuf does, then you leak that information throughout your APIs in a very ugly way. "int" should be used instead, so that we at least hide the protocol implementation details from our client API design.

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-08:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-10-09:

Hah. I think we've found another area in which our stylistic preferences disagree :)

The way I see it, we currently hide the fact from our client API that Mir's coordinates system has exactly 32bits of significant data. “int” isn't hiding irrelevant implementation details, it's hiding a potentially relevant aspect of our API.

Indeed, I think we should be explicitly sizing *all* our types where they correspond to coordinates or other data types that are a part of our API that happen to be integral.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-10:

FAILED: Continuous integration, rev:1866
http://jenkins.qa.ubuntu.com/job/mir-ci/1774/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/2074/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/2081/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/2011/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-ci/112/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/948/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-10-13:

Remember we only support client and server on the same host, and only plan on supporting that. As such the client and server can safely use the same type with an unspecified width, like "int".

Only if you're supporting a networked environment with heterogeneous word sizes is it necessary to specify the size. But we have no plans to ever make Mir a network protocol.

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-10-13:

I run armhf code on my amd64 machine all the time. Local machine only does *not* imply same-word-size, or even same-endianness.

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-10-13:

And, obviously, i386/amd64 is an even more common usecase.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-13:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-10-13:

> And, obviously, i386/amd64 is an even more common usecase.

hm maybe we could add something to the ci script that takes the x86 build and tries to run it with an amd64 server

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2014-10-14:

You have an armhf client/server talking to an amd64 server/client? That sounds interesting. QEMU?

Also interesting is the idea of mixing and matching amd64 and i386 on the same host. That does sound like a plausible albeit uncommon scenario.

Still, even if there are reasonable use cases for mixing and matching I think it's much cleaner design to choose the client API and then modify the protocol to fit rather than vice-versa.

review: Abstain

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-10-14:

On Tue, Oct 14, 2014 at 5:06 PM, Daniel van Vugt
<email address hidden> wrote:
> Review: Abstain
>
> You have an armhf client/server talking to an amd64 server/client?
> That sounds interesting. QEMU?

Indeed. With dpkg multiarch the only limitation on the architecture
combinations are the qemu system hosts.

>
> Also interesting is the idea of mixing and matching amd64 and i386 on
> the same host. That does sound like a plausible albeit uncommon
> scenario.

It's actually *super* common; does Steam ship an amd64 binary, for
example? The majority of games are still i386 binaries.

If Intel has its way we'll end up with lots of x32 binaries on amd64
systems as the norm :).

>
> Still, even if there are reasonable use cases for mixing and matching
> I think it's much cleaner design to choose the client API and then
> modify the protocol to fit rather than vice-versa.

Indeed, but the client API should guarantee appropriately sized data
types :)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-10-14:

I'd rather we didn't have to do this

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-14:

FAILED: Continuous integration, rev:1868
http://jenkins.qa.ubuntu.com/job/mir-ci/1821/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/2133
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/2140
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/2069/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-ci/161/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1006/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-20:

FAILED: Continuous integration, rev:1869
http://jenkins.qa.ubuntu.com/job/mir-ci/1858/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/2184
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/2191
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/2119/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-ci/198/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1056/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-10-20:

Dear protobuf: Your behaviour in registering an init thing that aborts on double-load is really annoying.

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-10-20:

On 20/10/14 22:04, Chris Halse Rogers wrote:
> Dear protobuf: Your behaviour in registering an init thing that aborts on double-load is really annoying.

Yes, maybe it was a mistake to roll libmirprotobuf into libmircommon and
we should split it out again.

--
Alan Griffiths +44 (0)798 9938 758
Octopull Ltd http://www.octopull.co.uk/

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-23:

FAILED: Continuous integration, rev:1869
http://jenkins.qa.ubuntu.com/job/mir-ci/1894/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/2229
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/2236
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/2164/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-ci/234/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1101/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-23:

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

review: Approve (continuous-integration)

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-10-27:

Does what it claims to do

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-27:

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/788/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/2261/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/2268
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/2196
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-autolanding/58
        deb: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-autolanding/58/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1133
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/1133/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3177
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15113

review: Needs Fixing (continuous-integration)

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-10-27:

Looks like a CI hiccup unrelated to MP let's try again...

Revision history for this message

PS Jenkins bot (ps-jenkins) on 2014-10-27:

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Chris Halse Rogers

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file 'client-ABI-sha1sums'
 --- client-ABI-sha1sums	2014-10-21 16:21:14 +0000
 +++ client-ABI-sha1sums	2014-10-23 13:30:17 +0000
@@ -1,4 +1,4 @@
--cbe1e3ccfea6ab5865510f4f598ec11571fcc852  include/client/mir_toolkit/mir_client_library_debug.h
++3a60fb3b2b250ca67f54ade346ef658c9259c9a7  include/client/mir_toolkit/debug/surface.h
  b53736bcb22ddc09aab8275cfdd05c109fdd0f12  include/client/mir_toolkit/mir_client_library_drm.h
 a5fcdb01c0d9a990364539c4f1255853eef7  include/client/mir_toolkit/mir_client_library.h
  e6185b680533c5fdcf1294a4797d5c3dd01d433f  include/client/mir_toolkit/mir_connection.h
 === modified file 'debian/control'
 --- debian/control	2014-10-21 17:22:26 +0000
 +++ debian/control	2014-10-23 13:30:17 +0000
@@ -165,6 +165,52 @@
+  .
   Contains header files required to develop against Mir.
++Package: libmirclient-debug-extension1
++Section: libs
++Architecture: i386 amd64 armhf arm64
++Multi-Arch: same
++Pre-Depends: ${misc:Pre-Depends}
++Depends: ${misc:Depends},
++         ${shlibs:Depends},
++Description: Display server for Ubuntu - optional client debug interfaces
++ Mir is a display server running on linux systems, with a focus on efficiency,
++ robust operation and a well-defined driver model.
++ .
++ This package contains both a shared library containing the client side of
++ various optional debugging interfaces and the development headers required
++ to use them.
++ .
++ This is primarily useful for developing Mir servers, low-level client
++ toolkits, or for automated testing in constrained environments.
++ .
++ Not all Mir servers have the capability to service these APIs; end-user
++ applications SHOULD NOT depend on this library.
++
++Package: libmirclient-debug-extension-dev
++Section: libdevel
++Architecture: i386 amd64 armhf arm64
++Multi-Arch: same
++Pre-Depends: ${misc:Pre-Depends}
++Depends: libmirclient-debug-extension1 (= ${binary:Version})
++	 ${misc:Depends},
++         ${shlibs:Depends},
++Description: Display server for Ubuntu - client debug interface headers
++ Mir is a display server running on linux systems, with a focus on efficiency,
++ robust operation and a well-defined driver model.
++ .
++ This package contains both a shared library containing the client side of
++ various optional debugging interfaces and the development headers required
++ to use them.
++ .
++ This is primarily useful for developing Mir servers, low-level client
++ toolkits, or for automated testing in constrained environments.
++ .
++ Not all Mir servers have the capability to service these APIs; end-user
++ applications SHOULD NOT depend on this library.
++ .
++ This package contains the development headers required to develop against
++ the debug extensions.
++
  Package: mir-demos
  Architecture: i386 amd64 armhf arm64
  Depends: ${misc:Depends},
 === added file 'debian/libmirclient-debug-extension-dev.install'
 --- debian/libmirclient-debug-extension-dev.install	1970-01-01 00:00:00 +0000
 +++ debian/libmirclient-debug-extension-dev.install	2014-10-23 13:30:17 +0000
@@ -0,0 +1,3 @@
++usr/include/mirclient/mir_toolkit/debug
++usr/lib/*/pkgconfig/mirclient-debug-extension.pc
++usr/lib/*/libmirclient-debug-extension.so
 === added file 'debian/libmirclient-debug-extension1.install'
 --- debian/libmirclient-debug-extension1.install	1970-01-01 00:00:00 +0000
 +++ debian/libmirclient-debug-extension1.install	2014-10-23 13:30:17 +0000
@@ -0,0 +1,1 @@
++usr/lib/*/libmirclient-debug-extension.so.*
 === modified file 'debian/libmirclient-dev.install'
 --- debian/libmirclient-dev.install	2013-06-24 12:15:28 +0000
 +++ debian/libmirclient-dev.install	2014-10-23 13:30:17 +0000
@@ -1,3 +1,3 @@
--usr/include/mirclient
++usr/include/mirclient/mir_toolkit/*.h
  usr/lib/*/pkgconfig/mirclient.pc
  usr/lib/*/libmirclient.so
 === added directory 'include/client/mir_toolkit/debug'
 === renamed file 'include/client/mir_toolkit/mir_client_library_debug.h' => 'include/client/mir_toolkit/debug/surface.h'
 --- include/client/mir_toolkit/mir_client_library_debug.h	2014-10-01 06:25:56 +0000
 +++ include/client/mir_toolkit/debug/surface.h	2014-10-23 13:30:17 +0000
@@ -28,6 +28,7 @@
  /**
   * Return the ID of a surface (only useful for debug output).
++ *   \pre                 The surface is valid
   *   \param [in] surface  The surface
   *   \return              An internal ID that identifies the surface
   */
@@ -43,6 +44,25 @@
   */
  uint32_t mir_debug_surface_current_buffer_id(MirSurface *surface);
++/**
++ * Get the screen coordinates corresponding to a pair of surface coordinates
++ * \pre                               The surface is valid
++ * \param   [in] surface              The surface
++ * \param   [in] x, y                 Surface coordinates to map to screen coordinates
++ * \param   [out] screen_x, screen_y  The screen coordinates corresponding to x, y.
++ * \return                            True if screen_x and screen_y contain values
++ * \note    There are many cases where such a mapping does not exist or would be expensive
++ *          to calculate. Only Mir servers started with the --debug option will ever return
++ *          values for this call, and even when --debug is enabled servers are free to
++ *          return nothing.
++ *
++ *          This call will only be interesting for automated testing, where both the client
++ *          and shell state is known and constrained.
++ */
++MirBool mir_debug_surface_coords_to_screen(MirSurface* surface,
++                                           int x, int y,
++                                           int* screen_x, int* screen_y);
++
  #ifdef __cplusplus
+ }
  #endif
 === modified file 'include/platform/mir/options/configuration.h'
 --- include/platform/mir/options/configuration.h	2014-10-01 06:25:56 +0000
 +++ include/platform/mir/options/configuration.h	2014-10-23 13:30:17 +0000
@@ -43,6 +43,7 @@
  extern char const* const frontend_threads_opt;
  extern char const* const touchspots_opt;
  extern char const* const fatal_abort_opt;
++extern char const* const debug_opt;
  extern char const* const name_opt;
  extern char const* const offscreen_opt;
 === modified file 'include/server/mir/default_server_configuration.h'
 --- include/server/mir/default_server_configuration.h	2014-10-22 18:55:40 +0000
 +++ include/server/mir/default_server_configuration.h	2014-10-23 13:30:17 +0000
@@ -101,6 +101,7 @@
  class SceneReport;
  class PromptSessionListener;
  class PromptSessionManager;
++class CoordinateTranslator;
+ }
  namespace graphics
+ {
@@ -286,6 +287,7 @@
       *  @{ */
      // To customize this behaviour it is recommended you override wrap_session_coordinator().
      virtual std::shared_ptr<scene::SessionCoordinator>  the_session_coordinator();
++    virtual std::shared_ptr<scene::CoordinateTranslator> the_coordinate_translator();
      /** @} */
@@ -421,6 +423,7 @@
      CachedPtr<scene::PromptSessionListener> prompt_session_listener;
      CachedPtr<scene::PromptSessionManager> prompt_session_manager;
      CachedPtr<scene::SessionCoordinator> session_coordinator;
++    CachedPtr<scene::CoordinateTranslator> coordinate_translator;
      CachedPtr<EmergencyCleanup> emergency_cleanup;
      CachedPtr<shell::HostLifecycleEventListener> host_lifecycle_event_listener;
 === added file 'include/server/mir/scene/coordinate_translator.h'
 --- include/server/mir/scene/coordinate_translator.h	1970-01-01 00:00:00 +0000
 +++ include/server/mir/scene/coordinate_translator.h	2014-10-23 13:30:17 +0000
@@ -0,0 +1,66 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#ifndef MIR_SCENE_COORDINATE_TRANSLATOR_H_
++#define MIR_SCENE_COORDINATE_TRANSLATOR_H_
++
++#include "mir/geometry/point.h"
++#include <memory>
++
++namespace mir
++{
++namespace frontend
++{
++class Surface;
++}
++
++namespace scene
++{
++
++/**
++ * Support for the debug "surface to screen" coordinate translation interface.
++ * \note For shells which do surface transformations the default implementation
++ *       will return incorrect results.
++ */
++class CoordinateTranslator
++{
++public:
++    virtual ~CoordinateTranslator() = default;
++
++    /**
++     * \brief Translate a surface coordinate into the screen coordinate space
++     * \param [in] surface  A frontend::Surface. This will need to be dynamic_cast into
++     *                      the scene::Surface relevant for the shell.
++     * \param [in] x, y     Coordinates to translate from the surface coordinate space
++     * \return              The coordinates in the screen coordinate space.
++     * \throws              A std::runtime_error if the translation cannot be performed
++     *                      for any reason.
++     *
++     * \note It is acceptable for this call to unconditionally throw a std::runtime_error.
++     *       It is not required for normal functioning of the server or clients; clients which
++     *       use the debug extension will receive an appropriate failure notice.
++     */
++    virtual geometry::Point surface_to_screen(std::shared_ptr<frontend::Surface> surface,
++                                              int32_t x, int32_t y) = 0;
++};
++
++
++}
++}
++
++#endif // MIR_SCENE_COORDINATE_TRANSLATOR_H_
 === modified file 'platform-ABI-sha1sums'
 --- platform-ABI-sha1sums	2014-10-21 16:21:14 +0000
 +++ platform-ABI-sha1sums	2014-10-23 13:30:17 +0000
@@ -40,7 +40,7 @@
 fbe488b4581486fd01e2f03e504f31b72c09040  include/platform/mir/graphics/platform_ipc_operations.h
 c7712af84b6fde1970ff802ad2ea78f892c7b7e  include/platform/mir/graphics/platform_ipc_package.h
 c063346b3bd51b4624d9f940008d4c3f8be066  include/platform/mir/graphics/renderable.h
--f5746dab3336266cfd410ce4e4d01333df6e5b99  include/platform/mir/options/configuration.h
++b19329d35a55f640edd72c0a953908995211d97e  include/platform/mir/options/configuration.h
 c783c174f8e94d332c4b13c6b01358b48fb  include/platform/mir/options/default_configuration.h
  b45f14082c4f8b29efaa1b13de795dcb29deb738  include/platform/mir/options/option.h
 c37cc31e8b290b89c311d82f02e07d342766451  include/platform/mir/options/program_option.h
 === modified file 'server-ABI-sha1sums'
 --- server-ABI-sha1sums	2014-10-22 18:55:40 +0000
 +++ server-ABI-sha1sums	2014-10-23 13:30:17 +0000
@@ -40,7 +40,7 @@
 fbe488b4581486fd01e2f03e504f31b72c09040  include/platform/mir/graphics/platform_ipc_operations.h
 c7712af84b6fde1970ff802ad2ea78f892c7b7e  include/platform/mir/graphics/platform_ipc_package.h
 c063346b3bd51b4624d9f940008d4c3f8be066  include/platform/mir/graphics/renderable.h
--f5746dab3336266cfd410ce4e4d01333df6e5b99  include/platform/mir/options/configuration.h
++b19329d35a55f640edd72c0a953908995211d97e  include/platform/mir/options/configuration.h
 c783c174f8e94d332c4b13c6b01358b48fb  include/platform/mir/options/default_configuration.h
  b45f14082c4f8b29efaa1b13de795dcb29deb738  include/platform/mir/options/option.h
 c37cc31e8b290b89c311d82f02e07d342766451  include/platform/mir/options/program_option.h
@@ -48,7 +48,7 @@
  a9f284ba4b05d58fd3eeb628d1f56fe4ac188526  include/server/mir/compositor/compositor_id.h
 ab5c4007d16dac213a3eb0105421d8ffad206  include/server/mir/compositor/scene_element.h
 fcf34e424128b87ddc76733594e32e09ebbd486  include/server/mir/compositor/scene.h
--7e47baf423fc61960d8e4ea92eae93caf71c93e2  include/server/mir/default_server_configuration.h
++c14448c8f882f44b05fa56ced7c8db51083a9bd3  include/server/mir/default_server_configuration.h
  af1ff0714be973ac76d56006a2e5991f68cd1dec  include/server/mir/display_server.h
  a35c5495d8fd28fc0e375b17495fc5caab51b329  include/server/mir/emergency_cleanup.h
 de641cb0e01e1098b007b39b151a7dfe4adc1  include/server/mir/frontend/display_changer.h
@@ -75,6 +75,7 @@
 b241f1e8211386bb8735ee0f752b666949  include/server/mir/main_loop.h
  edd5a0a6be0be57ef64c5af89fc5a65dc558648c  include/server/mir/report_exception.h
  8192727cf5fa0b2b5798b2211c2c8a4ad65c9f60  include/server/mir/run_mir.h
++c9de8b851fb22092a9a486d71cd998b4cb8392e1  include/server/mir/scene/coordinate_translator.h
 d9082681aa39d652c88ad3b54665695b521d24  include/server/mir/scene/depth_id.h
 e4e563ab429a13ddca7eead16f2b6cefb8edff6  include/server/mir/scene/null_surface_observer.h
  d5273f3363b0b776427d5e3f1af99f0f2bcf3fa6  include/server/mir/scene/observer.h
 === modified file 'src/client/CMakeLists.txt'
 --- src/client/CMakeLists.txt	2014-10-21 16:21:14 +0000
 +++ src/client/CMakeLists.txt	2014-10-23 13:30:17 +0000
@@ -8,12 +8,18 @@
  set(PREFIX "${CMAKE_INSTALL_PREFIX}")
  set(EXEC_PREFIX "${CMAKE_INSTALL_PREFIX}")
  set(LIBDIR "${CMAKE_INSTALL_FULL_LIBDIR}")
--set(INCLUDEDIR "${CMAKE_INSTALL_PREFIX}/include/mirclient")
--set(COMMON_INCLUDEDIR "${CMAKE_INSTALL_PREFIX}/include/mircommon")
++set(INCLUDEDIR "${CMAKE_INSTALL_PREFIX}/include")
  configure_file(
    ${CMAKE_CURRENT_SOURCE_DIR}/mirclient.pc.in
    ${CMAKE_CURRENT_BINARY_DIR}/mirclient.pc
++  @ONLY
++)
++
++configure_file(
++  ${CMAKE_CURRENT_SOURCE_DIR}/mirclient-debug-extension.pc.in
++  ${CMAKE_CURRENT_BINARY_DIR}/mirclient-debug-extension.pc
++  @ONLY
+ )
  include_directories(
@@ -113,3 +119,47 @@
    FILES ${CMAKE_CURRENT_BINARY_DIR}/mirclient.pc
    DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig
+ )
++
++# TODO: Extension apparatus so that mirclient-debug-extension can be a MODULE and not
++#       duplicate (almost) the entirity of mirclient
++
++set(MIRCLIENT_DEBUG_EXTENSION_ABI 1)
++
++add_library(
++  mirclient-debug-extension SHARED
++
++  mir_debug_api.cpp
++  $<TARGET_OBJECTS:mirclientobjects>
++)
++
++target_link_libraries(
++  mirclient-debug-extension
++
++  mirclient
++)
++
++set(debug_symbol_map ${CMAKE_CURRENT_SOURCE_DIR}/symbols-debug.map)
++
++set_target_properties(
++  mirclient-debug-extension
++
++  PROPERTIES
++  SOVERSION ${MIRCLIENT_DEBUG_EXTENSION_ABI}
++  LINK_FLAGS "-Wl,--version-script,${debug_symbol_map}"
++)
++
++install(
++  DIRECTORY ${CMAKE_SOURCE_DIR}/include/client/mir_toolkit/debug
++  DESTINATION "include/mirclient/mir_toolkit/debug"
++)
++
++install(
++  TARGETS mirclient-debug-extension
++  LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
++  ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})
++
++install(
++  FILES ${CMAKE_CURRENT_BINARY_DIR}/mirclient-debug-extension.pc
++  DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig
++)
++
 === modified file 'src/client/mir_connection.cpp'
 --- src/client/mir_connection.cpp	2014-10-01 06:25:56 +0000
 +++ src/client/mir_connection.cpp	2014-10-23 13:30:17 +0000
@@ -87,6 +87,7 @@
      deregisterer{this},
      channel(),
      server(0),
++    debug(0),
      error_message(error_message)
+ {
+ }
@@ -97,6 +98,7 @@
          platform_library{conf.the_platform_library()},
          channel(conf.the_rpc_channel()),
          server(channel.get(), ::google::protobuf::Service::STUB_DOESNT_OWN_CHANNEL),
++        debug(channel.get(), ::google::protobuf::Service::STUB_DOESNT_OWN_CHANNEL),
          logger(conf.the_logger()),
          client_platform_factory(conf.the_client_platform_factory()),
          input_platform(conf.the_input_platform()),
@@ -133,7 +135,7 @@
      mir_surface_callback callback,
      void * context)
+ {
--    auto surface = new MirSurface(this, server, platform->create_buffer_factory(), input_platform, params, callback, context);
++    auto surface = new MirSurface(this, server, &debug, platform->create_buffer_factory(), input_platform, params, callback, context);
      return surface->get_create_wait_handle();
+ }
 === modified file 'src/client/mir_connection.h'
 --- src/client/mir_connection.h	2014-10-01 06:25:56 +0000
 +++ src/client/mir_connection.h	2014-10-23 13:30:17 +0000
@@ -149,6 +149,7 @@
      std::shared_ptr<google::protobuf::RpcChannel> const channel;
      mir::protobuf::DisplayServer::Stub server;
++    mir::protobuf::Debug::Stub debug;
      std::shared_ptr<mir::logging::Logger> const logger;
      mir::protobuf::Void void_response;
      mir::protobuf::Connection connect_result;
 === added file 'src/client/mir_debug_api.cpp'
 --- src/client/mir_debug_api.cpp	1970-01-01 00:00:00 +0000
 +++ src/client/mir_debug_api.cpp	2014-10-23 13:30:17 +0000
@@ -0,0 +1,39 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU Lesser General Public License version 3,
++ * as 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/debug/surface.h"
++
++#include "mir_surface.h"
++
++uint32_t mir_debug_surface_current_buffer_id(MirSurface* surface)
++{
++    return surface->get_current_buffer_id();
++}
++
++int mir_debug_surface_id(MirSurface* surface)
++{
++    return surface->id();
++}
++
++MirBool mir_debug_surface_coords_to_screen(MirSurface* surface,
++                                           int x, int y,
++                                           int* screen_x, int* screen_y)
++{
++    return surface->translate_to_screen_coordinates(x, y, screen_x, screen_y) ?
++           mir_true : mir_false ;
++}
 === modified file 'src/client/mir_surface.cpp'
 --- src/client/mir_surface.cpp	2014-10-21 16:21:14 +0000
 +++ src/client/mir_surface.cpp	2014-10-23 13:30:17 +0000
@@ -51,7 +51,27 @@
      std::shared_ptr<mircv::InputPlatform> const& input_platform,
      MirSurfaceParameters const & params,
      mir_surface_callback callback, void * context)
++    : MirSurface(allocating_connection,
++                 server,
++                 nullptr,
++                 factory,
++                 input_platform,
++                 params,
++                 callback, context)
++{
++}
++
++
++MirSurface::MirSurface(
++    MirConnection *allocating_connection,
++    mp::DisplayServer::Stub & server,
++    mp::Debug::Stub* debug,
++    std::shared_ptr<mcl::ClientBufferFactory> const& factory,
++    std::shared_ptr<mircv::InputPlatform> const& input_platform,
++    MirSurfaceParameters const & params,
++    mir_surface_callback callback, void * context)
      : server(server),
++      debug{debug},
        connection(allocating_connection),
        buffer_depository(std::make_shared<mcl::ClientBufferDepository>(factory, mir::frontend::client_buffer_cache_size)),
        input_platform(input_platform)
@@ -373,6 +393,49 @@
      return &configure_wait_handle;
+ }
++namespace
++{
++void signal_response_received(MirWaitHandle* handle)
++{
++    handle->result_received();
++}
++}
++
++bool MirSurface::translate_to_screen_coordinates(int x, int y,
++                                                 int *screen_x, int *screen_y)
++{
++    if (!debug)
++    {
++        return false;
++    }
++
++    mp::CoordinateTranslationRequest request;
++
++    request.set_x(x);
++    request.set_y(y);
++    *request.mutable_surfaceid() = surface.id();
++    mp::CoordinateTranslationResponse response;
++
++    MirWaitHandle signal;
++    signal.expect_result();
++
++    {
++        std::lock_guard<decltype(mutex)> lock(mutex);
++
++        debug->translate_surface_to_screen(
++            nullptr,
++            &request,
++            &response,
++            google::protobuf::NewCallback(&signal_response_received, &signal));
++    }
++
++    signal.wait_for_one();
++
++    *screen_x = response.x();
++    *screen_y = response.y();
++    return !response.has_error();
++}
++
  void MirSurface::on_configured()
+ {
      std::lock_guard<decltype(mutex)> lock(mutex);
 === modified file 'src/client/mir_surface.h'
 --- src/client/mir_surface.h	2014-10-21 16:21:14 +0000
 +++ src/client/mir_surface.h	2014-10-23 13:30:17 +0000
@@ -62,6 +62,15 @@
      MirSurface(
          MirConnection *allocating_connection,
          mir::protobuf::DisplayServer::Stub & server,
++        mir::protobuf::Debug::Stub* debug,
++        std::shared_ptr<mir::client::ClientBufferFactory> const& buffer_factory,
++        std::shared_ptr<mir::input::receiver::InputPlatform> const& input_platform,
++        MirSurfaceParameters const& params,
++        mir_surface_callback callback, void * context);
++
++    MirSurface(
++        MirConnection *allocating_connection,
++        mir::protobuf::DisplayServer::Stub & server,
          std::shared_ptr<mir::client::ClientBufferFactory> const& buffer_factory,
          std::shared_ptr<mir::input::receiver::InputPlatform> const& input_platform,
          MirSurfaceParameters const& params,
@@ -87,6 +96,11 @@
      EGLNativeWindowType generate_native_window();
      MirWaitHandle* configure(MirSurfaceAttrib a, int value);
++
++    // TODO: Some sort of extension mechanism so that this can be moved
++    //       out into a separate class in the libmirclient-debug DSO.
++    bool translate_to_screen_coordinates(int x, int y,
++                                         int* screen_x, int* screen_y);
      // Non-blocking
      int attrib(MirSurfaceAttrib a) const;
@@ -115,7 +129,8 @@
      MirPixelFormat convert_ipc_pf_to_geometry(google::protobuf::int32 pf);
      void release_cpu_region();
--    mir::protobuf::DisplayServer::Stub & server;
++    mir::protobuf::DisplayServer::Stub& server;
++    mir::protobuf::Debug::Stub* debug;
      mir::protobuf::Surface surface;
      mir::protobuf::BufferRequest buffer_request;
      std::string error_message;
 === modified file 'src/client/mir_surface_api.cpp'
 --- src/client/mir_surface_api.cpp	2014-10-01 06:25:56 +0000
 +++ src/client/mir_surface_api.cpp	2014-10-23 13:30:17 +0000
@@ -18,7 +18,6 @@
  #include "mir_toolkit/mir_surface.h"
  #include "mir_toolkit/mir_wait.h"
--#include "mir_toolkit/mir_client_library_debug.h"
  #include "mir_connection.h"
  #include "mir_surface.h"
@@ -152,9 +151,9 @@
          nullptr));
+ }
--int mir_surface_get_id(MirSurface* surface)
++int mir_surface_get_id(MirSurface* /*surface*/)
+ {
--    return mir_debug_surface_id(surface);
++    return 0;
+ }
  MirWaitHandle* mir_surface_set_type(MirSurface* surf,
 === added file 'src/client/mirclient-debug-extension.pc.in'
 --- src/client/mirclient-debug-extension.pc.in	1970-01-01 00:00:00 +0000
 +++ src/client/mirclient-debug-extension.pc.in	2014-10-23 13:30:17 +0000
@@ -0,0 +1,11 @@
++prefix=@PREFIX@
++exec_prefix=@EXEC_PREFIX@
++libdir=@LIBDIR@
++includedir=@INCLUDEDIR@/mirclient-debug
++common_includedir=@INCLUDEDIR@/mircommon
++
++Name: mirclient-debug
++Description: Mir client debug library
++Version: @MIR_VERSION@
++Libs: -L${libdir} -lmirclient-debug
++Cflags: -I${includedir} -I${common_includedir}
 === modified file 'src/client/mirclient.pc.in'
 --- src/client/mirclient.pc.in	2014-10-01 06:25:56 +0000
 +++ src/client/mirclient.pc.in	2014-10-23 13:30:17 +0000
@@ -1,11 +1,13 @@
  prefix=@PREFIX@
  exec_prefix=@EXEC_PREFIX@
  libdir=@LIBDIR@
--includedir=@INCLUDEDIR@
++includedir=@INCLUDEDIR@/mirclient
++common_includedir=@INCLUDEDIR@/mircommon
++
  Name: mirclient
  Description: Mir client library
  Version: @MIR_VERSION@
  Requires.private: protobuf >= 2.4.1
--Libs: -L@LIBDIR@ -lmirclient
--Cflags: -I@INCLUDEDIR@ -I@COMMON_INCLUDEDIR@
++Libs: -L${libdir} -lmirclient
++Cflags: -I${includedir} -I${common_includedir}
 === added file 'src/client/symbols-debug.map'
 --- src/client/symbols-debug.map	1970-01-01 00:00:00 +0000
 +++ src/client/symbols-debug.map	2014-10-23 13:30:17 +0000
@@ -0,0 +1,5 @@
++{
++ global:
++   mir_debug_*;
++ local: *;
++};
 === modified file 'src/common/protobuf/mir_protobuf.proto'
 --- src/common/protobuf/mir_protobuf.proto	2014-10-21 16:21:14 +0000
 +++ src/common/protobuf/mir_protobuf.proto	2014-10-23 13:30:17 +0000
@@ -228,3 +228,20 @@
    rpc stop_prompt_session(Void) returns (Void);
    rpc exchange_buffer(BufferRequest) returns (Buffer);
+ }
++
++message CoordinateTranslationRequest {
++  required SurfaceId surfaceid = 1;
++  required int32 x = 2;
++  required int32 y = 3;
++}
++
++message CoordinateTranslationResponse {
++  optional int32 x = 1;
++  optional int32 y = 2;
++
++  optional string error = 127;
++}
++
++service Debug {
++  rpc translate_surface_to_screen(CoordinateTranslationRequest) returns (CoordinateTranslationResponse);
++}
 === modified file 'src/common/symbols.map'
 --- src/common/symbols.map	2014-10-21 16:21:14 +0000
 +++ src/common/symbols.map	2014-10-23 13:30:17 +0000
@@ -143,6 +143,42 @@
      mir::protobuf::ConnectParameters::SerializeWithCachedSizes*;
      mir::protobuf::ConnectParameters::SerializeWithCachedSizesToArray*;
      mir::protobuf::ConnectParameters::Swap*;
++    mir::protobuf::CoordinateTranslationRequest::ByteSize*;
++    mir::protobuf::CoordinateTranslationRequest::Clear*;
++    mir::protobuf::CoordinateTranslationRequest::?CoordinateTranslationRequest*;
++    mir::protobuf::CoordinateTranslationRequest::CoordinateTranslationRequest*;
++    mir::protobuf::CoordinateTranslationRequest::CopyFrom*;
++    mir::protobuf::CoordinateTranslationRequest::default_instance*;
++    mir::protobuf::CoordinateTranslationRequest::descriptor*;
++    mir::protobuf::CoordinateTranslationRequest::GetMetadata*;
++    mir::protobuf::CoordinateTranslationRequest::IsInitialized*;
++    mir::protobuf::CoordinateTranslationRequest::kSurfaceidFieldNumber*;
++    mir::protobuf::CoordinateTranslationRequest::kXFieldNumber*;
++    mir::protobuf::CoordinateTranslationRequest::kYFieldNumber*;
++    mir::protobuf::CoordinateTranslationRequest::MergeFrom*;
++    mir::protobuf::CoordinateTranslationRequest::MergePartialFromCodedStream*;
++    mir::protobuf::CoordinateTranslationRequest::New*;
++    mir::protobuf::CoordinateTranslationRequest::SerializeWithCachedSizes*;
++    mir::protobuf::CoordinateTranslationRequest::SerializeWithCachedSizesToArray*;
++    mir::protobuf::CoordinateTranslationRequest::Swap*;
++    mir::protobuf::CoordinateTranslationResponse::ByteSize*;
++    mir::protobuf::CoordinateTranslationResponse::Clear*;
++    mir::protobuf::CoordinateTranslationResponse::?CoordinateTranslationResponse*;
++    mir::protobuf::CoordinateTranslationResponse::CoordinateTranslationResponse*;
++    mir::protobuf::CoordinateTranslationResponse::CopyFrom*;
++    mir::protobuf::CoordinateTranslationResponse::default_instance*;
++    mir::protobuf::CoordinateTranslationResponse::descriptor*;
++    mir::protobuf::CoordinateTranslationResponse::GetMetadata*;
++    mir::protobuf::CoordinateTranslationResponse::IsInitialized*;
++    mir::protobuf::CoordinateTranslationResponse::kErrorFieldNumber*;
++    mir::protobuf::CoordinateTranslationResponse::kXFieldNumber*;
++    mir::protobuf::CoordinateTranslationResponse::kYFieldNumber*;
++    mir::protobuf::CoordinateTranslationResponse::MergeFrom*;
++    mir::protobuf::CoordinateTranslationResponse::MergePartialFromCodedStream*;
++    mir::protobuf::CoordinateTranslationResponse::New*;
++    mir::protobuf::CoordinateTranslationResponse::SerializeWithCachedSizes*;
++    mir::protobuf::CoordinateTranslationResponse::SerializeWithCachedSizesToArray*;
++    mir::protobuf::CoordinateTranslationResponse::Swap*;
      mir::protobuf::CursorSetting::ByteSize*;
      mir::protobuf::CursorSetting::Clear*;
      mir::protobuf::CursorSetting::CopyFrom*;
@@ -160,6 +196,16 @@
      mir::protobuf::CursorSetting::SerializeWithCachedSizes*;
      mir::protobuf::CursorSetting::SerializeWithCachedSizesToArray*;
      mir::protobuf::CursorSetting::Swap*;
++    mir::protobuf::Debug::CallMethod*;
++    mir::protobuf::Debug::?Debug*;
++    mir::protobuf::Debug::descriptor*;
++    mir::protobuf::Debug::GetDescriptor*;
++    mir::protobuf::Debug::GetRequestPrototype*;
++    mir::protobuf::Debug::GetResponsePrototype*;
++    mir::protobuf::Debug_Stub::?Debug_Stub*;
++    mir::protobuf::Debug_Stub::Debug_Stub*;
++    mir::protobuf::Debug_Stub::translate_surface_to_screen*;
++    mir::protobuf::Debug::translate_surface_to_screen*;
      mir::protobuf::DisplayCard::ByteSize*;
      mir::protobuf::DisplayCard::Clear*;
      mir::protobuf::DisplayCard::CopyFrom*;
@@ -780,7 +826,11 @@
      typeinfo?for?mir::protobuf::BufferRequest;
      typeinfo?for?mir::protobuf::Connection;
      typeinfo?for?mir::protobuf::ConnectParameters;
++    typeinfo?for?mir::protobuf::CoordinateTranslationRequest;
++    typeinfo?for?mir::protobuf::CoordinateTranslationResponse;
      typeinfo?for?mir::protobuf::CursorSetting;
++    typeinfo?for?mir::protobuf::Debug;
++    typeinfo?for?mir::protobuf::Debug_Stub;
      typeinfo?for?mir::protobuf::DisplayCard;
      typeinfo?for?mir::protobuf::DisplayConfiguration;
      typeinfo?for?mir::protobuf::DisplayInfo;
@@ -847,7 +897,11 @@
      vtable?for?mir::protobuf::BufferRequest;
      vtable?for?mir::protobuf::Connection;
      vtable?for?mir::protobuf::ConnectParameters;
++    vtable?for?mir::protobuf::CoordinateTranslationRequest;
++    vtable?for?mir::protobuf::CoordinateTranslationResponse;
      vtable?for?mir::protobuf::CursorSetting;
++    vtable?for?mir::protobuf::Debug;
++    vtable?for?mir::protobuf::Debug_Stub;
      vtable?for?mir::protobuf::DisplayCard;
      vtable?for?mir::protobuf::DisplayConfiguration;
      vtable?for?mir::protobuf::DisplayInfo;
 === added file 'src/include/server/mir/frontend/unsupported_feature_exception.h'
 --- src/include/server/mir/frontend/unsupported_feature_exception.h	1970-01-01 00:00:00 +0000
 +++ src/include/server/mir/frontend/unsupported_feature_exception.h	2014-10-23 13:30:17 +0000
@@ -0,0 +1,41 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++
++#ifndef MIR_FRONTEND_UNSUPPORTED_FEATURE_EXCEPTION_H_
++#define MIR_FRONTEND_UNSUPPORTED_FEATURE_EXCEPTION_H_
++
++#include <stdexcept>
++
++namespace mir
++{
++namespace frontend
++{
++class unsupported_feature : public std::runtime_error
++{
++public:
++    unsupported_feature()
++        : std::runtime_error{"Unsupported feature requested"}
++    {
++    }
++};
++}
++}
++
++
++#endif /* MIR_FRONTEND_UNSUPPORTED_FEATURE_EXCEPTION_H_ */
 === modified file 'src/platform/options/default_configuration.cpp'
 --- src/platform/options/default_configuration.cpp	2014-10-01 06:25:56 +0000
 +++ src/platform/options/default_configuration.cpp	2014-10-23 13:30:17 +0000
@@ -45,6 +45,7 @@
  char const* const mo::offscreen_opt               = "offscreen";
  char const* const mo::touchspots_opt               = "enable-touchspots";
  char const* const mo::fatal_abort_opt             = "on-fatal-error-abort";
++char const* const mo::debug_opt                   = "debug";
  char const* const mo::glog                 = "glog";
  char const* const mo::glog_stderrthreshold = "glog-stderrthreshold";
@@ -164,7 +165,9 @@
          (touchspots_opt,
              "Display visualization of touchspots (e.g. for screencasting).")
          (fatal_abort_opt, "On \"fatal error\" conditions [e.g. drivers behaving "
--            "in unexpected ways] abort (to get a core dump)");
++            "in unexpected ways] abort (to get a core dump)")
++        (debug_opt, "Enable extra development debugging. "
++            "This is only interesting for people doing Mir server or client development.");
          add_platform_options();
+ }
 === modified file 'src/platform/symbols.map'
 --- src/platform/symbols.map	2014-10-21 16:21:14 +0000
 +++ src/platform/symbols.map	2014-10-23 13:30:17 +0000
@@ -146,6 +146,7 @@
      mir::options::DefaultConfiguration::parse_environment*;
      mir::options::DefaultConfiguration::the_options*;
      mir::options::display_report_opt*;
++    mir::options::debug_opt*;
      mir::options::enable_input_opt*;
      mir::options::fatal_abort_opt*;
      mir::options::frontend_threads_opt*;
 === modified file 'src/server/frontend/CMakeLists.txt'
 --- src/server/frontend/CMakeLists.txt	2014-10-01 06:25:56 +0000
 +++ src/server/frontend/CMakeLists.txt	2014-10-23 13:30:17 +0000
@@ -22,6 +22,7 @@
    session_credentials.cpp
    default_configuration.cpp
    default_ipc_factory.cpp
++  unsupported_coordinate_translator.cpp
+ )
  add_library(
 === modified file 'src/server/frontend/default_configuration.cpp'
 --- src/server/frontend/default_configuration.cpp	2014-10-01 06:25:56 +0000
 +++ src/server/frontend/default_configuration.cpp	2014-10-23 13:30:17 +0000
@@ -22,6 +22,8 @@
  #include "default_ipc_factory.h"
  #include "published_socket_connector.h"
++#include "unsupported_coordinate_translator.h"
++
  #include "mir/frontend/protobuf_connection_creator.h"
  #include "mir/frontend/session_authorizer.h"
  #include "mir/options/configuration.h"
@@ -29,6 +31,7 @@
  namespace mf = mir::frontend;
  namespace mg = mir::graphics;
++namespace ms = mir::scene;
  std::shared_ptr<mf::ConnectionCreator>
  mir::DefaultServerConfiguration::the_connection_creator()
@@ -137,13 +140,23 @@
  mir::DefaultServerConfiguration::new_ipc_factory(
      std::shared_ptr<mf::SessionAuthorizer> const& session_authorizer)
+ {
++    std::shared_ptr<ms::CoordinateTranslator> translator;
++    if (the_options()->is_set(options::debug_opt))
++    {
++        translator = the_coordinate_translator();
++    }
++    else
++    {
++        translator = std::make_shared<mf::UnsupportedCoordinateTranslator>();
++    }
      return std::make_shared<mf::DefaultIpcFactory>(
--        the_frontend_shell(),
--        the_session_mediator_report(),
--        the_graphics_platform(),
--        the_frontend_display_changer(),
--        the_buffer_allocator(),
--        the_screencast(),
--        session_authorizer,
--        the_cursor_images());
++                the_frontend_shell(),
++                the_session_mediator_report(),
++                the_graphics_platform(),
++                the_frontend_display_changer(),
++                the_buffer_allocator(),
++                the_screencast(),
++                session_authorizer,
++                the_cursor_images(),
++                translator);
+ }
 === modified file 'src/server/frontend/default_ipc_factory.cpp'
 --- src/server/frontend/default_ipc_factory.cpp	2014-10-01 06:25:56 +0000
 +++ src/server/frontend/default_ipc_factory.cpp	2014-10-23 13:30:17 +0000
@@ -38,7 +38,8 @@
      std::shared_ptr<mg::GraphicBufferAllocator> const& buffer_allocator,
      std::shared_ptr<Screencast> const& screencast,
      std::shared_ptr<SessionAuthorizer> const& session_authorizer,
--    std::shared_ptr<mi::CursorImages> const& cursor_images) :
++    std::shared_ptr<mi::CursorImages> const& cursor_images,
++    std::shared_ptr<scene::CoordinateTranslator> const& translator) :
      shell(shell),
      no_prompt_shell(std::make_shared<NoPromptShell>(shell)),
      sm_report(sm_report),
@@ -48,7 +49,8 @@
      buffer_allocator(buffer_allocator),
      screencast(screencast),
      session_authorizer(session_authorizer),
--    cursor_images(cursor_images)
++    cursor_images(cursor_images),
++    translator{translator}
+ {
+ }
@@ -120,5 +122,6 @@
          resource_cache(),
          effective_screencast,
          connection_context,
--        cursor_images);
++        cursor_images,
++        translator);
+ }
 === modified file 'src/server/frontend/default_ipc_factory.h'
 --- src/server/frontend/default_ipc_factory.h	2014-10-01 06:25:56 +0000
 +++ src/server/frontend/default_ipc_factory.h	2014-10-23 13:30:17 +0000
@@ -33,6 +33,11 @@
  class CursorImages;
+ }
++namespace scene
++{
++class CoordinateTranslator;
++}
++
  namespace frontend
+ {
  class Shell;
@@ -52,7 +57,8 @@
          std::shared_ptr<graphics::GraphicBufferAllocator> const& buffer_allocator,
          std::shared_ptr<Screencast> const& screencast,
          std::shared_ptr<SessionAuthorizer> const& session_authorizer,
--        std::shared_ptr<input::CursorImages> const& cursor_images);
++        std::shared_ptr<input::CursorImages> const& cursor_images,
++        std::shared_ptr<scene::CoordinateTranslator> const& translator);
      std::shared_ptr<detail::DisplayServer> make_ipc_server(
          SessionCredentials const& creds,
@@ -83,6 +89,7 @@
      std::shared_ptr<Screencast> const screencast;
      std::shared_ptr<SessionAuthorizer> const session_authorizer;
      std::shared_ptr<input::CursorImages> const cursor_images;
++    std::shared_ptr<scene::CoordinateTranslator> const translator;
  };
+ }
+ }
 === modified file 'src/server/frontend/protobuf_message_processor.cpp'
 --- src/server/frontend/protobuf_message_processor.cpp	2014-10-22 18:55:40 +0000
 +++ src/server/frontend/protobuf_message_processor.cpp	2014-10-23 13:30:17 +0000
@@ -21,6 +21,7 @@
  #include "mir/frontend/message_processor_report.h"
  #include "mir/frontend/protobuf_message_sender.h"
  #include "mir/frontend/template_protobuf_message_processor.h"
++#include "mir/frontend/unsupported_feature_exception.h"
  #include "mir_protobuf_wire.pb.h"
@@ -104,6 +105,24 @@
          self->send_response(invocation.id(), result_message);
+     }
+ }
++
++// A partial-specialisation to handle error cases.
++template<class Self, class ServerX, class ParameterMessage, class ResultMessage>
++void invoke(
++    Self* self,
++    std::string* error,
++    void (ServerX::*/*function*/)(
++        ::google::protobuf::RpcController* controller,
++        const ParameterMessage* request,
++        ResultMessage* response,
++        ::google::protobuf::Closure* done),
++        Invocation const& invocation)
++{
++    ResultMessage result_message;
++    result_message.set_error(error->c_str());
++    self->send_response(invocation.id(), &result_message);
++}
++
+ }
+ }
+ }
@@ -206,6 +225,24 @@
              invoke(this, display_server.get(), &DisplayServer::disconnect, invocation);
              result = false;
+         }
++        else if ("translate_surface_to_screen" == invocation.method_name())
++        {
++            try
++            {
++                auto debug_interface = dynamic_cast<mir::protobuf::Debug*>(display_server.get());
++                invoke(this, debug_interface, &mir::protobuf::Debug::translate_surface_to_screen, invocation);
++            }
++            catch (mir::frontend::unsupported_feature const&)
++            {
++                std::string message{"Server does not support the client debugging interface"};
++                invoke(this,
++                       &message,
++                       &mir::protobuf::Debug::translate_surface_to_screen,
++                       invocation);
++                std::runtime_error err{"Client attempted to use unavailable debug interface"};
++                report->exception_handled(display_server.get(), invocation.id(), err);
++            }
++        }
          else
+         {
              report->unknown_method(display_server.get(), invocation.id(), invocation.method_name());
 === modified file 'src/server/frontend/session_mediator.cpp'
 --- src/server/frontend/session_mediator.cpp	2014-10-21 16:21:14 +0000
 +++ src/server/frontend/session_mediator.cpp	2014-10-23 13:30:17 +0000
@@ -17,13 +17,13 @@
   */
  #include "session_mediator.h"
--#include "client_buffer_tracker.h"
  #include "mir/frontend/session_mediator_report.h"
  #include "mir/frontend/shell.h"
  #include "mir/frontend/session.h"
  #include "mir/frontend/surface.h"
  #include "mir/scene/surface_creation_parameters.h"
++#include "mir/scene/coordinate_translator.h"
  #include "mir/frontend/display_changer.h"
  #include "resource_cache.h"
  #include "mir_toolkit/common.h"
@@ -73,7 +73,8 @@
      std::shared_ptr<MessageResourceCache> const& resource_cache,
      std::shared_ptr<Screencast> const& screencast,
      ConnectionContext const& connection_context,
--    std::shared_ptr<mi::CursorImages> const& cursor_images) :
++    std::shared_ptr<mi::CursorImages> const& cursor_images,
++    std::shared_ptr<scene::CoordinateTranslator> const& translator) :
      client_pid_(0),
      shell(shell),
      graphics_platform(graphics_platform),
@@ -86,6 +87,7 @@
      screencast(screencast),
      connection_context(connection_context),
      cursor_images(cursor_images),
++    translator{translator},
      surface_tracker{static_cast<size_t>(client_buffer_cache_size)}
+ {
+ }
@@ -551,6 +553,32 @@
      done->Run();
+ }
++void mir::frontend::SessionMediator::translate_surface_to_screen(
++    ::google::protobuf::RpcController* ,
++    ::mir::protobuf::CoordinateTranslationRequest const* request,
++    ::mir::protobuf::CoordinateTranslationResponse* response,
++    ::google::protobuf::Closure *done)
++{
++    {
++        std::unique_lock<std::mutex> lock(session_mutex);
++
++        auto session = weak_session.lock();
++
++        if (session.get() == nullptr)
++            BOOST_THROW_EXCEPTION(std::logic_error("Invalid application session"));
++
++        auto const id = frontend::SurfaceId(request->surfaceid().value());
++
++        auto const coords = translator->surface_to_screen(session->get_surface(id),
++                                                          request->x(),
++                                                          request->y());
++
++        response->set_x(coords.x.as_uint32_t());
++        response->set_y(coords.y.as_uint32_t());
++    }
++    done->Run();
++}
++
  void mf::SessionMediator::drm_auth_magic(
      google::protobuf::RpcController* /*controller*/,
      const mir::protobuf::DRMMagic* request,
 === modified file 'src/server/frontend/session_mediator.h'
 --- src/server/frontend/session_mediator.h	2014-10-21 16:21:14 +0000
 +++ src/server/frontend/session_mediator.h	2014-10-23 13:30:17 +0000
@@ -46,6 +46,11 @@
  class CursorImages;
+ }
++namespace scene
++{
++class CoordinateTranslator;
++}
++
  /// Frontend interface. Mediates the interaction between client
  /// processes and the core of the mir system.
  namespace frontend
@@ -62,7 +67,7 @@
  class PromptSession;
  // SessionMediator relays requests from the client process into the server.
--class SessionMediator : public detail::DisplayServer
++class SessionMediator : public detail::DisplayServer, public mir::protobuf::Debug
+ {
  public:
@@ -76,7 +81,8 @@
          std::shared_ptr<MessageResourceCache> const& resource_cache,
          std::shared_ptr<Screencast> const& screencast,
          ConnectionContext const& connection_context,
--        std::shared_ptr<input::CursorImages> const& cursor_images);
++        std::shared_ptr<input::CursorImages> const& cursor_images,
++        std::shared_ptr<scene::CoordinateTranslator> const& translator);
      ~SessionMediator() noexcept;
@@ -167,6 +173,13 @@
          ::mir::protobuf::SocketFD* response,
          ::google::protobuf::Closure* done) override;
++    // TODO: Split this into a separate thing
++    void translate_surface_to_screen(
++        ::google::protobuf::RpcController* controller,
++        ::mir::protobuf::CoordinateTranslationRequest const* request,
++        ::mir::protobuf::CoordinateTranslationResponse* response,
++        ::google::protobuf::Closure *done) override;
++
  private:
      void pack_protobuf_buffer(protobuf::Buffer& protobuf_buffer,
                                graphics::Buffer* graphics_buffer,
@@ -193,6 +206,7 @@
      std::shared_ptr<Screencast> const screencast;
      ConnectionContext const connection_context;
      std::shared_ptr<input::CursorImages> const cursor_images;
++    std::shared_ptr<scene::CoordinateTranslator> const translator;
      SurfaceTracker surface_tracker;
 === added file 'src/server/frontend/unsupported_coordinate_translator.cpp'
 --- src/server/frontend/unsupported_coordinate_translator.cpp	1970-01-01 00:00:00 +0000
 +++ src/server/frontend/unsupported_coordinate_translator.cpp	2014-10-23 13:30:17 +0000
@@ -0,0 +1,32 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#include "unsupported_coordinate_translator.h"
++#include "mir/frontend/unsupported_feature_exception.h"
++
++#include <boost/exception/all.hpp>
++
++namespace mg = mir::geometry;
++namespace mf = mir::frontend;
++
++mg::Point mf::UnsupportedCoordinateTranslator::surface_to_screen(
++        std::shared_ptr<mf::Surface> /*surface*/,
++        int32_t /*x*/, int32_t /*y*/)
++{
++    BOOST_THROW_EXCEPTION(mir::frontend::unsupported_feature());
++}
 === added file 'src/server/frontend/unsupported_coordinate_translator.h'
 --- src/server/frontend/unsupported_coordinate_translator.h	1970-01-01 00:00:00 +0000
 +++ src/server/frontend/unsupported_coordinate_translator.h	2014-10-23 13:30:17 +0000
@@ -0,0 +1,37 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#ifndef MIR_FRONTEND_UNSUPPORTED_COORDINATE_TRANSLATOR_H_
++#define MIR_FRONTEND_UNSUPPORTED_COORDINATE_TRANSLATOR_H_
++
++#include "mir/scene/coordinate_translator.h"
++
++namespace mir
++{
++namespace frontend
++{
++class UnsupportedCoordinateTranslator : public scene::CoordinateTranslator
++{
++    geometry::Point surface_to_screen(std::shared_ptr<Surface> surface,
++                                      int32_t x, int32_t y) override;
++};
++
++}
++}
++
++#endif // MIR_FRONTEND_UNSUPPORTED_COORDINATE_TRANSLATOR_H_
 === modified file 'src/server/scene/CMakeLists.txt'
 --- src/server/scene/CMakeLists.txt	2014-10-01 06:25:56 +0000
 +++ src/server/scene/CMakeLists.txt	2014-10-23 13:30:17 +0000
@@ -23,4 +23,5 @@
    prompt_session_container.cpp
    prompt_session_manager_impl.cpp
    rendering_tracker.cpp
++  default_coordinate_translator.cpp
+ )
 === modified file 'src/server/scene/default_configuration.cpp'
 --- src/server/scene/default_configuration.cpp	2014-10-01 06:25:56 +0000
 +++ src/server/scene/default_configuration.cpp	2014-10-23 13:30:17 +0000
@@ -36,6 +36,7 @@
  #include "surface_stack.h"
  #include "threaded_snapshot_strategy.h"
  #include "prompt_session_manager_impl.h"
++#include "default_coordinate_translator.h"
  namespace mc = mir::compositor;
  namespace mf = mir::frontend;
@@ -238,3 +239,13 @@
                  the_prompt_session_listener());
          });
+ }
++
++std::shared_ptr<ms::CoordinateTranslator>
++mir::DefaultServerConfiguration::the_coordinate_translator()
++{
++    return coordinate_translator(
++        [this]()
++        {
++            return std::make_shared<ms::DefaultCoordinateTranslator>();
++        });
++}
 === added file 'src/server/scene/default_coordinate_translator.cpp'
 --- src/server/scene/default_coordinate_translator.cpp	1970-01-01 00:00:00 +0000
 +++ src/server/scene/default_coordinate_translator.cpp	2014-10-23 13:30:17 +0000
@@ -0,0 +1,32 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#include "default_coordinate_translator.h"
++#include "basic_surface.h"
++
++namespace geom = mir::geometry;
++namespace mf = mir::frontend;
++namespace ms = mir::scene;
++
++geom::Point ms::DefaultCoordinateTranslator::surface_to_screen(std::shared_ptr<mf::Surface> surface, int32_t x,
++                                                               int32_t y)
++{
++    auto const scene_surface = std::dynamic_pointer_cast<ms::BasicSurface>(surface);
++
++    return geom::Point{x + scene_surface->top_left().x.as_int(), y + scene_surface->top_left().y.as_int()};
++}
 === added file 'src/server/scene/default_coordinate_translator.h'
 --- src/server/scene/default_coordinate_translator.h	1970-01-01 00:00:00 +0000
 +++ src/server/scene/default_coordinate_translator.h	2014-10-23 13:30:17 +0000
@@ -0,0 +1,37 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#ifndef MIR_FRONTEND_DEFAULT_COORDINATE_TRANSLATOR_H_
++#define MIR_FRONTEND_DEFAULT_COORDINATE_TRANSLATOR_H_
++
++#include "mir/scene/coordinate_translator.h"
++
++namespace mir
++{
++namespace scene
++{
++class DefaultCoordinateTranslator : public CoordinateTranslator
++{
++public:
++    geometry::Point surface_to_screen(std::shared_ptr<frontend::Surface> surface, int32_t x, int32_t y);
++};
++
++}
++}
++
++#endif // MIR_FRONTEND_DEFAULT_COORDINATE_TRANSLATOR_H_
 === modified file 'src/server/symbols.map'
 --- src/server/symbols.map	2014-10-22 18:55:40 +0000
 +++ src/server/symbols.map	2014-10-23 13:30:17 +0000
@@ -39,6 +39,7 @@
      mir::DefaultServerConfiguration::the_connection_creator*;
      mir::DefaultServerConfiguration::the_connector*;
      mir::DefaultServerConfiguration::the_connector_report*;
++    mir::DefaultServerConfiguration::the_coordinate_translator*;
      mir::DefaultServerConfiguration::the_cursor*;
      mir::DefaultServerConfiguration::the_cursor_images*;
      mir::DefaultServerConfiguration::the_cursor_listener*;
@@ -244,6 +245,9 @@
      mir::report_exception*;
      mir::run_mir*;
      mir::scene::a_surface*;
++    mir::scene::CoordinateTranslator::?CoordinateTranslator*;
++    mir::scene::CoordinateTranslator::CoordinateTranslator*;
++    mir::scene::CoordinateTranslator::surface_to_screen*;
      mir::scene::NullSurfaceObserver::alpha_set_to*;
      mir::scene::NullSurfaceObserver::attrib_changed*;
      mir::scene::NullSurfaceObserver::cursor_image_set_to*;
@@ -648,6 +652,7 @@
      typeinfo?for?mir::input::TouchVisualizer;
      typeinfo?for?mir::input::TouchVisualizer::Spot;
      typeinfo?for?mir::MainLoop;
++    typeinfo?for?mir::scene::CoordinateTranslator;
      typeinfo?for?mir::scene::NullSurfaceObserver;
      typeinfo?for?mir::scene::Observer;
      typeinfo?for?mir::scene::PlacementStrategy;
@@ -706,6 +711,7 @@
      vtable?for?mir::input::TouchVisualizer;
      vtable?for?mir::input::TouchVisualizer::Spot;
      vtable?for?mir::MainLoop;
++    vtable?for?mir::scene::CoordinateTranslator;
      vtable?for?mir::scene::NullSurfaceObserver;
      vtable?for?mir::scene::Observer;
      vtable?for?mir::scene::PlacementStrategy;
 === modified file 'tests/acceptance-tests/CMakeLists.txt'
 --- tests/acceptance-tests/CMakeLists.txt	2014-10-21 16:21:14 +0000
 +++ tests/acceptance-tests/CMakeLists.txt	2014-10-23 13:30:17 +0000
@@ -42,6 +42,7 @@
    test_server_without_active_outputs.cpp
    test_client_input.cpp
    test_server_startup.cpp
++  test_debug_api.cpp
    test_unresponsive_client.cpp
+ )
@@ -67,6 +68,7 @@
    mirserver
    mirclient
++  mirclient-debug-extension
    mirplatform
    mir-test
 === modified file 'tests/acceptance-tests/test_client_library.cpp'
 --- tests/acceptance-tests/test_client_library.cpp	2014-10-21 16:21:14 +0000
 +++ tests/acceptance-tests/test_client_library.cpp	2014-10-23 13:30:17 +0000
@@ -17,7 +17,6 @@
   */
  #include "mir_toolkit/mir_client_library.h"
--#include "mir_toolkit/mir_client_library_debug.h"
  #include "mir_test_framework/stubbed_server_configuration.h"
  #include "mir_test_framework/in_process_server.h"
 === modified file 'tests/acceptance-tests/test_client_surface_events.cpp'
 --- tests/acceptance-tests/test_client_surface_events.cpp	2014-10-01 06:25:56 +0000
 +++ tests/acceptance-tests/test_client_surface_events.cpp	2014-10-23 13:30:17 +0000
@@ -17,7 +17,7 @@
   */
  #include "mir_toolkit/mir_client_library.h"
--#include "mir_toolkit/mir_client_library_debug.h"
++#include "mir_toolkit/debug/surface.h"
  #include "mir/shell/surface_coordinator_wrapper.h"
 === modified file 'tests/acceptance-tests/test_client_surfaces.cpp'
 --- tests/acceptance-tests/test_client_surfaces.cpp	2014-10-01 06:25:56 +0000
 +++ tests/acceptance-tests/test_client_surfaces.cpp	2014-10-23 13:30:17 +0000
@@ -17,7 +17,7 @@
   */
  #include "mir_toolkit/mir_client_library.h"
--#include "mir_toolkit/mir_client_library_debug.h"
++#include "mir_toolkit/debug/surface.h"
  #include "mir_test_framework/stubbed_server_configuration.h"
  #include "mir_test_framework/basic_client_server_fixture.h"
 === modified file 'tests/acceptance-tests/test_custom_input_dispatcher.cpp'
 --- tests/acceptance-tests/test_custom_input_dispatcher.cpp	2014-10-01 06:25:56 +0000
 +++ tests/acceptance-tests/test_custom_input_dispatcher.cpp	2014-10-23 13:30:17 +0000
@@ -133,7 +133,7 @@
      // may be stopped before events have been dispatched.
      run_in_test_process([&]
+     {
--        server_config.dispatching_done.wait_for_signal_ready_for(std::chrono::seconds{5});
++        server_config.dispatching_done.wait_for_signal_ready_for(std::chrono::seconds{50});
      });
+ }
@@ -179,6 +179,6 @@
      launch_client_process(client);
      run_in_test_process([&]
+     {
--        server_config.dispatching_done.wait_for_signal_ready_for(std::chrono::seconds{5});
++        server_config.dispatching_done.wait_for_signal_ready_for(std::chrono::seconds{50});
      });
+ }
 === added file 'tests/acceptance-tests/test_debug_api.cpp'
 --- tests/acceptance-tests/test_debug_api.cpp	1970-01-01 00:00:00 +0000
 +++ tests/acceptance-tests/test_debug_api.cpp	2014-10-23 13:30:17 +0000
@@ -0,0 +1,231 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License version 3 as
++ * published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#include "mir/options/option.h"
++#include "mir/scene/surface_creation_parameters.h"
++#include "mir/scene/placement_strategy.h"
++#include "mir/scene/surface.h"
++#include "src/server/scene/session_container.h"
++#include "mir/shell/surface_coordinator_wrapper.h"
++
++#include "mir_test/wait_condition.h"
++#include "mir_test/client_event_matchers.h"
++#include "mir_test/barrier.h"
++#include "mir_test_framework/deferred_in_process_server.h"
++#include "mir_test_framework/stubbed_server_configuration.h"
++
++#include "mir_toolkit/mir_client_library.h"
++#include "mir_toolkit/debug/surface.h"
++
++#include <gtest/gtest.h>
++#include <gmock/gmock.h>
++
++namespace ms = mir::scene;
++
++namespace mtf = mir_test_framework;
++
++namespace
++{
++class SimpleConfigurablePlacementStrategy : public ms::PlacementStrategy
++{
++public:
++    ms::SurfaceCreationParameters place(ms::Session const& /*session*/,
++                                        ms::SurfaceCreationParameters const& request_parameters) override
++    {
++        auto placed = request_parameters;
++        placed.top_left = placement.top_left;
++        placed.size = placement.size;
++        return placed;
++    }
++
++    mir::geometry::Rectangle placement;
++};
++
++class SurfacePlacingConfiguration : public mtf::StubbedServerConfiguration
++{
++public:
++    SurfacePlacingConfiguration()
++        : placement_strategy{std::make_shared<SimpleConfigurablePlacementStrategy>()}
++    {
++    }
++
++    std::shared_ptr<ms::PlacementStrategy> the_placement_strategy() override
++    {
++        return placement_strategy;
++    }
++
++    void set_surface_placement(mir::geometry::Rectangle const& where)
++    {
++        placement_strategy->placement = where;
++    }
++
++    bool is_debug_server()
++    {
++        return the_options()->is_set("debug");
++    }
++
++private:
++    std::shared_ptr<SimpleConfigurablePlacementStrategy> placement_strategy;
++};
++
++char const* debugenv = "MIR_SERVER_DEBUG";
++
++void dont_kill_me_bro(MirConnection* /*unused*/, MirLifecycleState /*unused*/, void* /*unused*/)
++{
++}
++
++class TestDebugAPI : public mtf::DeferredInProcessServer
++{
++public:
++    TestDebugAPI()
++        : old_debug_env{::getenv(debugenv)}
++    {
++        mir::geometry::Rectangle surface_location;
++        surface_location.top_left.x = mir::geometry::X{0};
++        surface_location.top_left.y = mir::geometry::Y{0};
++        surface_location.size.width = mir::geometry::Width{100};
++        surface_location.size.height = mir::geometry::Height{100};
++
++        server_configuration.set_surface_placement(surface_location);
++    }
++
++    ~TestDebugAPI()
++    {
++        ::unsetenv(debugenv);
++        if (old_debug_env)
++        {
++            ::setenv(debugenv, old_debug_env, 1);
++        }
++    }
++
++    void start_server_with_debug(bool debug)
++    {
++        if (debug)
++        {
++            ::setenv(debugenv, "", 1);
++        }
++        else
++        {
++            ::unsetenv(debugenv);
++        }
++
++        if (server_configuration.is_debug_server() != debug)
++        {
++            throw std::runtime_error{"Failed to set the debug flag correctly. Have you overriden this with --debug?"};
++        }
++
++        start_server();
++        connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
++        if (!mir_connection_is_valid(connection))
++        {
++            throw std::runtime_error{std::string{"Failed to connect to test server:"} +
++                                     mir_connection_get_error_message(connection)};
++        }
++        mir_connection_set_lifecycle_event_callback(connection, dont_kill_me_bro, nullptr);
++    }
++
++    void TearDown() override
++    {
++        if (connection)
++        {
++            mir_connection_release(connection);
++        }
++        DeferredInProcessServer::TearDown();
++    }
++
++    mir::DefaultServerConfiguration& server_config() override
++    {
++        return server_configuration;
++    }
++
++    SurfacePlacingConfiguration server_configuration;
++    MirConnection* connection{nullptr};
++
++private:
++    char const* old_debug_env;
++};
++}
++
++TEST_F(TestDebugAPI, TranslatesSurfaceCoordinatesToScreenCoordinates)
++{
++    start_server_with_debug(true);
++
++    mir::geometry::Rectangle surface_location;
++    surface_location.top_left.x = mir::geometry::X{200};
++    surface_location.top_left.y = mir::geometry::Y{100};
++    surface_location.size.width = mir::geometry::Width{800};
++    surface_location.size.height = mir::geometry::Height{600};
++
++    server_configuration.set_surface_placement(surface_location);
++
++    ASSERT_TRUE(mir_connection_is_valid(connection));
++
++    MirSurfaceParameters const creation_parameters = {
++        __PRETTY_FUNCTION__,
++        800, 600,
++        mir_pixel_format_argb_8888,
++        mir_buffer_usage_hardware,
++        mir_display_output_id_invalid
++    };
++    auto surf = mir_connection_create_surface_sync(connection, &creation_parameters);
++    ASSERT_TRUE(mir_surface_is_valid(surf));
++
++    int screen_x, screen_y, x, y;
++    x = 35, y = 21;
++
++    ASSERT_TRUE(mir_debug_surface_coords_to_screen(surf, x, y, &screen_x, &screen_y));
++    EXPECT_EQ(x + surface_location.top_left.x.as_int(), screen_x);
++    EXPECT_EQ(y + surface_location.top_left.y.as_int(), screen_y);
++
++    mir_surface_release_sync(surf);
++
++    surface_location.top_left.x = mir::geometry::X{100};
++    surface_location.top_left.y = mir::geometry::Y{250};
++
++    server_configuration.set_surface_placement(surface_location);
++
++    surf = mir_connection_create_surface_sync(connection, &creation_parameters);
++    ASSERT_TRUE(mir_surface_is_valid(surf));
++
++    ASSERT_TRUE(mir_debug_surface_coords_to_screen(surf, x, y, &screen_x, &screen_y));
++    EXPECT_EQ(x + surface_location.top_left.x.as_int(), screen_x);
++    EXPECT_EQ(y + surface_location.top_left.y.as_int(), screen_y);
++
++    mir_surface_release_sync(surf);
++}
++
++TEST_F(TestDebugAPI, ApiIsUnavaliableWhenServerNotStartedWithDebug)
++{
++    start_server_with_debug(false);
++
++    MirSurfaceParameters const creation_parameters = {
++        __PRETTY_FUNCTION__,
++        800, 600,
++        mir_pixel_format_argb_8888,
++        mir_buffer_usage_hardware,
++        mir_display_output_id_invalid
++    };
++    auto surf = mir_connection_create_surface_sync(connection, &creation_parameters);
++    ASSERT_TRUE(mir_surface_is_valid(surf));
++
++    int screen_x, screen_y;
++
++    EXPECT_FALSE(mir_debug_surface_coords_to_screen(surf, 0, 0, &screen_x, &screen_y));
++
++    mir_surface_release_sync(surf);
++}
 === added file 'tests/include/mir_test_doubles/mock_coordinate_translator.h'
 --- tests/include/mir_test_doubles/mock_coordinate_translator.h	1970-01-01 00:00:00 +0000
 +++ tests/include/mir_test_doubles/mock_coordinate_translator.h	2014-10-23 13:30:17 +0000
@@ -0,0 +1,42 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
++ */
++
++#ifndef MIR_TEST_DOUBLES_MOCK_COORDINATE_TRANSLATOR_H_
++#define MIR_TEST_DOUBLES_MOCK_COORDINATE_TRANSLATOR_H_
++
++#include "src/server/frontend/coordinate_translator.h"
++
++#include <gmock/gmock.h>
++
++namespace mir
++{
++namespace test
++{
++namespace doubles
++{
++class MockCoordinateTranslator : public mir::frontend::CoordinateTranslator
++{
++    MOCK_METHOD3(surface_to_screen, geometry::Point(std::shared_ptr<frontend::Surface>, uint32_t, uint32_t));
++};
++
++using StubCoordinateTranslator = testing::NiceMock<MockCoordinateTranslator>;
++}
++}
++}
++
++#endif /* MIR_TEST_DOUBLES_MOCK_COORDINATE_TRANSLATOR_H_ */
 === modified file 'tests/include/mir_test_doubles/mock_surface.h'
 --- tests/include/mir_test_doubles/mock_surface.h	2014-10-01 06:25:56 +0000
 +++ tests/include/mir_test_doubles/mock_surface.h	2014-10-23 13:30:17 +0000
@@ -21,6 +21,7 @@
  #include "src/server/scene/basic_surface.h"
  #include "src/server/report/null_report_factory.h"
++#include "mock_buffer_stream.h"
  #include <gmock/gmock.h>
@@ -38,7 +39,7 @@
              {},
              {{},{}},
              true,
--            {},
++            std::make_shared<testing::NiceMock<MockBufferStream>>(),
              {},
              {},
              {},
 === modified file 'tests/integration-tests/CMakeLists.txt'
 --- tests/integration-tests/CMakeLists.txt	2014-10-21 16:21:14 +0000
 +++ tests/integration-tests/CMakeLists.txt	2014-10-23 13:30:17 +0000
@@ -82,6 +82,7 @@
    mir-test
    mir-test-framework
    mir-test-doubles
++  mirclient-debug-extension
    mirdraw
    mirtestdraw
 === modified file 'tests/integration-tests/test_stale_frames.cpp'
 --- tests/integration-tests/test_stale_frames.cpp	2014-10-21 16:21:14 +0000
 +++ tests/integration-tests/test_stale_frames.cpp	2014-10-23 13:30:17 +0000
@@ -17,7 +17,7 @@
   */
  #include "mir_toolkit/mir_client_library.h"
--#include "mir_toolkit/mir_client_library_debug.h"
++#include "mir_toolkit/debug/surface.h"
  #include "mir/compositor/compositor.h"
  #include "mir/compositor/renderer_factory.h"
 === modified file 'tests/unit-tests/frontend/test_session_mediator.cpp'
 --- tests/unit-tests/frontend/test_session_mediator.cpp	2014-10-21 16:21:14 +0000
 +++ tests/unit-tests/frontend/test_session_mediator.cpp	2014-10-23 13:30:17 +0000
@@ -259,7 +259,7 @@
              shell, graphics_platform, graphics_changer,
              surface_pixel_formats, report,
              std::make_shared<mtd::NullEventSink>(),
--            resource_cache, stub_screencast, &connector, nullptr}
++            resource_cache, stub_screencast, &connector, nullptr, nullptr}
+     {
          using namespace ::testing;
@@ -316,7 +316,7 @@
          shell, graphics_platform, graphics_changer,
          surface_pixel_formats, report,
          std::make_shared<mtd::NullEventSink>(),
--        resource_cache, stub_screencast, context, nullptr};
++        resource_cache, stub_screencast, context, nullptr, nullptr};
      EXPECT_THAT(connects_handled_count, Eq(0));
@@ -421,7 +421,7 @@
          surface_pixel_formats, report,
          std::make_shared<mtd::NullEventSink>(),
          resource_cache, std::make_shared<mtd::NullScreencast>(),
--        nullptr, nullptr);
++        nullptr, nullptr, nullptr);
      mediator.connect(nullptr, &connect_parameters, &connection, null_callback.get());
      EXPECT_THAT(connection.display_configuration(), mt::DisplayConfigMatches(std::cref(config)));
@@ -629,7 +629,7 @@
          surface_pixel_formats, report,
          std::make_shared<mtd::NullEventSink>(), resource_cache,
          std::make_shared<mtd::NullScreencast>(),
--         nullptr, nullptr};
++         nullptr, nullptr, nullptr};
      mediator.connect(nullptr, &connect_parameters, &connection, null_callback.get());
@@ -876,7 +876,7 @@
          shell, graphics_platform, graphics_changer,
          surface_pixel_formats, report,
          std::make_shared<mtd::NullEventSink>(),
--        mt::fake_shared(mock_cache), stub_screencast, nullptr, nullptr};
++        mt::fake_shared(mock_cache), stub_screencast, &connector, nullptr, nullptr};
      mediator.connect(nullptr, &connect_parameters, &connection, null_callback.get());
      mediator.create_surface(nullptr, &surface_parameters, &surface_response, null_callback.get());
 === modified file 'tests/unit-tests/frontend/test_session_mediator_android.cpp'
 --- tests/unit-tests/frontend/test_session_mediator_android.cpp	2014-10-01 06:25:56 +0000
 +++ tests/unit-tests/frontend/test_session_mediator_android.cpp	2014-10-23 13:30:17 +0000
@@ -61,7 +61,7 @@
            mediator{shell, graphics_platform, display_changer,
                     surface_pixel_formats, report,
                     std::make_shared<mtd::NullEventSink>(),
--                   resource_cache, std::make_shared<mtd::NullScreencast>(), nullptr, nullptr},
++                   resource_cache, std::make_shared<mtd::NullScreencast>(), nullptr, nullptr, nullptr},
            null_callback{google::protobuf::NewPermanentCallback(google::protobuf::DoNothing)}
+     {
+     }
 === modified file 'tests/unit-tests/frontend/test_session_mediator_mesa.cpp'
 --- tests/unit-tests/frontend/test_session_mediator_mesa.cpp	2014-10-21 16:21:14 +0000
 +++ tests/unit-tests/frontend/test_session_mediator_mesa.cpp	2014-10-23 13:30:17 +0000
@@ -74,7 +74,8 @@
            mediator{shell, mock_platform, display_changer,
                     surface_pixel_formats, report,
                     std::make_shared<mtd::NullEventSink>(),
--                   resource_cache, std::make_shared<mtd::NullScreencast>(), nullptr, nullptr},
++                   resource_cache, std::make_shared<mtd::NullScreencast>(), nullptr, nullptr,
++                   nullptr},
            null_callback{google::protobuf::NewPermanentCallback(google::protobuf::DoNothing)}
+     {
+     }