Mir

Merge lp:~alan-griffiths/mir/scene-can-identify-surface-under-cursor into lp:mir

scene-can-identify-surface-under-cursor
Merge into development-branch

Proposed by Alan Griffiths on 2015-02-13

Status:	Merged
Approved by:	Alan Griffiths on 2015-02-20
Approved revision:	2337
Merged at revision:	2331
Proposed branch:	lp:~alan-griffiths/mir/scene-can-identify-surface-under-cursor
Merge into:	lp:mir
Diff against target:	340 lines (+112/-32) 11 files modified include/server/mir/scene/surface_coordinator.h (+4/-0) src/server/scene/surface_controller.cpp (+6/-0) src/server/scene/surface_controller.h (+2/-0) src/server/scene/surface_stack.cpp (+33/-0) src/server/scene/surface_stack.h (+2/-0) src/server/scene/surface_stack_model.h (+5/-1) src/server/symbols.map (+7/-31) tests/include/mir_test_doubles/mock_surface_coordinator.h (+1/-0) tests/unit-tests/scene/test_application_session.cpp (+4/-0) tests/unit-tests/scene/test_surface_controller.cpp (+1/-0) tests/unit-tests/scene/test_surface_stack.cpp (+47/-0)
To merge this branch:	bzr merge lp:~alan-griffiths/mir/scene-can-identify-surface-under-cursor
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Daniel van Vugt		2015-02-13	Needs Fixing on 2015-02-20
Robert Carr (community)			Approve on 2015-02-19
Alberto Aguirre			Approve on 2015-02-19
PS Jenkins bot	continuous-integration		Approve on 2015-02-19
Andreas Pokorny (community)			Abstain on 2015-02-19
Alexandros Frantzis (community)			Approve on 2015-02-17
Alan Griffiths			Abstain on 2015-02-17
Chris Halse Rogers			Approve on 2015-02-17
Review via email: mp+249696@code.launchpad.net

Commit Message

scene: Introduce mechanism to find the surface under a cursor

Description of the Change

scene: Introduce mechanism to find the surface under a cursor

There'll be an acceptance test along shortly to expose this functionality for window management, but this bit splits off cleanly.

PS Jenkins bot (ps-jenkins) wrote on 2015-02-13:

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

review: Approve (continuous-integration)

Daniel van Vugt (vanvugt) wrote on 2015-02-16:

Sounds redundant. We already have such logic to send input events to the right window. Can we just reuse that?

review: Needs Information

Daniel van Vugt (vanvugt) wrote on 2015-02-16:

Oh you are using that logic already :)

70 + if (surface->input_bounds().contains(cursor))
71 + result = surface;

but that's only a rectangle.

I think the right answer is to test the free-form input region, which we have a setter for but no getter yet:

virtual void set_input_region(std::vector<geometry::Rectangle> const& region) = 0;

It's not a big deal, but if we wanted to support custom shaped surfaces properly then we would test the region and not just the bounding rectangle.

review: Needs Information

Daniel van Vugt (vanvugt) wrote on 2015-02-16:

Aha. The right function already exists.

Instead of:
surface->input_bounds().contains(cursor)
we want something like:
surface->input_area_contains(cursor)
which will test the region proper and also checks the visible() flag properly.

Furthermore, we don't need to pay attention to this flag:
67 + if (surface->query(mir_surface_attrib_visibility) ==
68 + MirSurfaceVisibility::mir_surface_visibility_exposed)
Instead just make sure you're walking the stack from top-to-bottom (reverse?), and break out of the loop when you find the first hit.

review: Needs Fixing

PS Jenkins bot (ps-jenkins) wrote on 2015-02-16:

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

review: Approve (continuous-integration)

Chris Halse Rogers (raof) wrote on 2015-02-17:

Looks sensible to me.

review: Approve

Daniel van Vugt (vanvugt) wrote on 2015-02-17:

Looks good. Just missing server ABI bump to 30 due to the ABI break.

review: Needs Fixing

Alan Griffiths (alan-griffiths) wrote on 2015-02-17:

Actually, as Daniel indirectly pointed out on another thread, this shouldn't be scanning for the input area under the cursor but for the whole (visible) surface area.

review: Needs Fixing

Alan Griffiths (alan-griffiths) wrote on 2015-02-17:

> Looks good. Just missing server ABI bump to 30 due to the ABI break.

Done.

Alan Griffiths (alan-griffiths) on 2015-02-17:

review: Abstain

PS Jenkins bot (ps-jenkins) wrote on 2015-02-17:

FAILED: Continuous integration, rev:2326
http://jenkins.qa.ubuntu.com/job/mir-ci/2990/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1320
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1319
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1276/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/987/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1276/console

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

review: Needs Fixing (continuous-integration)

lp:~alan-griffiths/mir/scene-can-identify-surface-under-cursor updated on 2015-02-17

2327. By Alan Griffiths on 2015-02-17: Update debian/*

PS Jenkins bot (ps-jenkins) wrote on 2015-02-17:

FAILED: Continuous integration, rev:2327
http://jenkins.qa.ubuntu.com/job/mir-ci/2995/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1325
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1324
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1281/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/992/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1281/console

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

review: Needs Fixing (continuous-integration)

Alexandros Frantzis (afrantzis) wrote on 2015-02-17:

>361 + stub_surface1->configure(mir_surface_attrib_visibility, MirSurfaceVisibility::mir_surface_visibility_exposed);
... etc

Not needed.

review: Needs Fixing

lp:~alan-griffiths/mir/scene-can-identify-surface-under-cursor updated on 2015-02-17

2328. By Alan Griffiths on 2015-02-17: Update debian/*
2329. By Alan Griffiths on 2015-02-17: Remove unnecessary setup

Alan Griffiths (alan-griffiths) wrote on 2015-02-17:

> >361 + stub_surface1->configure(mir_surface_attrib_visibility,
> MirSurfaceVisibility::mir_surface_visibility_exposed);
> ... etc
>
> Not needed.

Fixed, but the diff refuses to show it

lp:~alan-griffiths/mir/scene-can-identify-surface-under-cursor updated on 2015-02-17

2330. By Alan Griffiths on 2015-02-17: Empty checkin to fool lp

Alexandros Frantzis (afrantzis) wrote on 2015-02-17:

> Fixed, but the diff refuses to show it

Oh, no, we have broken LP! :)

review: Approve

PS Jenkins bot (ps-jenkins) wrote on 2015-02-17:

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

review: Approve (continuous-integration)

Robert Carr (robertcarr) wrote on 2015-02-17:

lgtm

review: Approve

PS Jenkins bot (ps-jenkins) wrote on 2015-02-17:

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

review: Approve (continuous-integration)

Daniel van Vugt (vanvugt) wrote on 2015-02-18:

This is now incorrect:

133 + if (surface->visible() &&
134 + geom::Rectangle{surface->top_left(), surface->size()}.contains(cursor))
135 + return surface;

A shaped surface occupies less space than its rectangle (e.g. try mir_demo_client_egltriangle -b0). So your new algorithm does not support shaped surfaces that express a reduced input region. Using input_area_contains() would solve this.

Then again, I think we lack any client API to express an input shape at all yet...

review: Needs Fixing

Alan Griffiths (alan-griffiths) wrote on 2015-02-18:

> This is now incorrect:
>
> 133 + if (surface->visible() &&
> 134 + geom::Rectangle{surface->top_left(),
> surface->size()}.contains(cursor))
> 135 + return surface;
>
> A shaped surface occupies less space than its rectangle (e.g. try
> mir_demo_client_egltriangle -b0). So your new algorithm does not support
> shaped surfaces that express a reduced input region. Using
> input_area_contains() would solve this.

What I want is the surface visible under the cursor position, not the input area. Using is wrong as you described yourself at lp:~alan-griffiths/mir/canonical-window-manager/+merge/249849/comments/619311

"BTW, if you're resizing then the input area the client has designated is probably irrelevant. Instead you want to consider the whole surface rectangle..."

> Then again, I think we lack any client API to express an input shape at all
> yet...

The region I want here includes, for example, decorations that are not part of the input area - maybe we can refine the implementation in future, but the proposed algorithm is more correct than using nput_area_contains().

lp:~alan-griffiths/mir/scene-can-identify-surface-under-cursor updated on 2015-02-18

2331. By Alan Griffiths on 2015-02-18: TODO comment explaining implementation choice
2332. By Alan Griffiths on 2015-02-18: merge lp:mir

PS Jenkins bot (ps-jenkins) wrote on 2015-02-18:

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

review: Needs Fixing (continuous-integration)

Robert Carr (robertcarr) wrote on 2015-02-18:

I think using input_area_contains is probably correct.

The origin of the input area was to allow unity8 (pre qt-compositor) to punch an input hole in the always on top shell surface. If client API were added I imagine it would also be used for things like a circular clock app e.g. the X shape extension...

Essentially it seems like a bit of a misnomer to say a surface is under the cursor when cursor events will pass through the surface.

review: Needs Information

Alberto Aguirre (albaguirre) wrote on 2015-02-18:

"A shaped surface occupies less space than its rectangle (e.g. try mir_demo_client_egltriangle -b0). So your new algorithm does not support shaped surfaces that express a reduced input region. Using input_area_contains() would solve this."

After a brief scan of the code, I don't see how input_area_contains tracks client opacity. It would be just as broken if the proposed implementation "used input_area_contains".

@Robert,
A shell surface like that sounds like it should have it's own type or state.

@Alan,
Perhaps we can have:

std::shared_ptr<Surface> ms::SurfaceStack::surface_find(std::function<bool(ms::Surface*)> predicate)

And then the demo window manager can have the simple implementation proposed here.

review: Needs Information

Robert Carr (robertcarr) wrote on 2015-02-18:

>> A shell surface like that sounds like it should have it's own type or state.

Sure I was just outlining the history of the code. I think the upcoming use case of say a circular clock app is valid though.

Robert Carr (robertcarr) wrote on 2015-02-18:

The suggestion not being that input_area_contains will track opacity but rather that the client will have the option of updating its input area via specifying a set of rectangles.

Perhaps this should be it's input/output area. I know at least OSX (Circa 2009 lol) and X11 support setting input shape exclusively.

Daniel van Vugt (vanvugt) wrote on 2015-02-19:

Alberto says:
"After a brief scan of the code, I don't see how input_area_contains tracks client opacity. It would be just as broken if the proposed implementation "used input_area_contains".

That's right. It's a feature and not a bug. Monitoring pixel opacity could be prohibitively expensive if done regularly. It's just the client's responsibility to designate an input area that matches its visibly opaque area.

As Robert points out, this is based on X11's input area. The goal is noble, but the implementation can be expensive, because in both X11 and Mir the input area is designated by a set of rectangles. But we don't need to be tied to the set-of-rectangles implementation everyone uses right now. The beauty of input_area_contains() is that it's nice and opaque. We can modify the implementation in future.

Long term, I suggest a nice solution would be to resample pixel opacity once per surface resize and use that as a mask for the default input region (which then can be any shape). You can also optimize for the common case of rectangular clients so only surfaces that are actually shaped need to have a mask stored for them.

At least input_area_contains() has a chance of supporting shaped surfaces (like a circular clock). Just testing a single rectangle we've got no chance.

That said, testing a single rectangle might make sense for a purely tiling shell, for ease of use. By definition of the tiling shell there are no "gaps". But a traditional floating window shell should honour surface shapes more accurately.

review: Needs Fixing

Andreas Pokorny (andreas-pokorny) wrote on 2015-02-19:

oh a classic you cannot to this but you also cannot do the opposite.

Hm most of what was proposed and discussed here might fit.. based on the ambivalent nature of mir::scene::Surface and its processing components.

What I do not like about the MP is that we are running into scene and surface locks while dispatching user input. But I will not block based on that. We can still drive that onto a different implementation path, while keeping the interface.

review: Abstain

lp:~alan-griffiths/mir/scene-can-identify-surface-under-cursor updated on 2015-02-19

2333. By Alan Griffiths on 2015-02-19: Revert debian/* changes
2334. By Alan Griffiths on 2015-02-19: Fix symbol map without updating to MIR_SERVER_30
2335. By Alan Griffiths on 2015-02-19: Back to surface->input_area_contains(cursor) with a note of misgivings
2336. By Alan Griffiths on 2015-02-19: merge lp:mir

Alan Griffiths (alan-griffiths) wrote on 2015-02-19:

I'm not yet convinced that input_area_contains() can support all the usecases: Some (like the motivating case of the shell determining which window is clicked on) require it includes decorations and I suspect others will exclude decorations.

But input_area_contains() is good enough for now. And decorations are not yet supported.

I've also reverted the ABI bump as that's causing CI problems and can be landed separately.

PS Jenkins bot (ps-jenkins) wrote on 2015-02-19:

FAILED: Continuous integration, rev:2336
http://jenkins.qa.ubuntu.com/job/mir-ci/3018/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1356
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1355
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1312/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1015/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1312/console

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

review: Needs Fixing (continuous-integration)

lp:~alan-griffiths/mir/scene-can-identify-surface-under-cursor updated on 2015-02-19

2337. By Alan Griffiths on 2015-02-19: Forgotten reverts

PS Jenkins bot (ps-jenkins) wrote on 2015-02-19:

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

review: Approve (continuous-integration)

Alan Griffiths (alan-griffiths) wrote on 2015-02-19:

> What I do not like about the MP is that we are running into scene and surface
> locks while dispatching user input. But I will not block based on that. We can
> still drive that onto a different implementation path, while keeping the
> interface.

The scene needs some re-architecting to reduce lock contention. Having C++14 sharable mutexes will help I think (as only the shell should be updating the scene).

Alan Griffiths (alan-griffiths) wrote on 2015-02-19:

> Perhaps we can have:
>
> std::shared_ptr<Surface>
> ms::SurfaceStack::surface_find(std::function<bool(ms::Surface*)> predicate)
>
> And then the demo window manager can have the simple implementation proposed
> here.

I don't think it makes sense for the window manager to decide which surface is under a click. At least not until we understand how decorations are represented in the scene.

Alberto Aguirre (albaguirre) wrote on 2015-02-19:

OK.

LGTM then. We can improve this as we enhance the scene.

review: Approve

Andreas Pokorny (andreas-pokorny) wrote on 2015-02-19:

> > What I do not like about the MP is that we are running into scene and
> surface
> > locks while dispatching user input. But I will not block based on that. We
> can
> > still drive that onto a different implementation path, while keeping the
> > interface.
>
> The scene needs some re-architecting to reduce lock contention. Having C++14
> sharable mutexes will help I think (as only the shell should be updating the
> scene).

I think our scenes are rather small we can avoid contention-free acquiring of a lot of locks. So I thought of transforming the grand central scene to an input scene when changes happen. the reduced input scene would be for input dispatch & input related window management only.

Alan Griffiths (alan-griffiths) wrote on 2015-02-19:

> I think our scenes are rather small we can avoid contention-free acquiring of
> a lot of locks. So I thought of transforming the grand central scene to an
> input scene when changes happen. the reduced input scene would be for input
> dispatch & input related window management only.

We digress, but input can also update an input scene model what it needs on the basis of observers instead of transforming the grand central scene whenever changes happen.

Robert Carr (robertcarr) wrote on 2015-02-19:

>> I think our scenes are rather small we can avoid contention-free acquiring of a lot of locks. So I >> thought of transforming the grand central scene to an input scene when changes happen. the reduced >> input scene would be for input dispatch & input related window management only.

Android actually uses an approach like this...we used to use..InputDispatcher::setInputTargets(vector<WindowHandle>)

Robert Carr (robertcarr) wrote on 2015-02-19:

Anyway LGTM

review: Approve

Daniel van Vugt (vanvugt) wrote on 2015-02-20:

I think we're almost there. Not too concerned about input on decorations as I've recently come to the realization that might be more the shell's job to detect (Unity8 does already). In fact, I already started designing for that and stopped (see TODOs in basic_surface.cpp).

Probable needs fixing: Functions removed from symbols.map without an ABI bump :)

review: Needs Fixing

Alan Griffiths (alan-griffiths) wrote on 2015-02-20:

> Probable needs fixing: Functions removed from symbols.map without an ABI bump
> :)

I think not:

1. these functions couldn't be used as they don't appear in pubic headers; and,
2. most (if not all) were not actually exported because the symbols were not generated.

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:

Alan Griffiths

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file 'include/server/mir/scene/surface_coordinator.h'
 --- include/server/mir/scene/surface_coordinator.h	2015-01-21 07:34:50 +0000
 +++ include/server/mir/scene/surface_coordinator.h	2015-02-19 13:21:41 +0000
@@ -24,6 +24,7 @@
  namespace mir
+ {
++namespace geometry { class Point; }
  namespace scene
+ {
  class Surface;
@@ -41,6 +42,9 @@
      virtual void raise(std::weak_ptr<Surface> const& surface) = 0;
      virtual void remove_surface(std::weak_ptr<Surface> const& surface) = 0;
++
++    virtual auto surface_at(geometry::Point) const -> std::shared_ptr<Surface> = 0;
++
  protected:
      SurfaceCoordinator() = default;
      virtual ~SurfaceCoordinator() = default;
 === modified file 'src/server/scene/surface_controller.cpp'
 --- src/server/scene/surface_controller.cpp	2015-02-13 06:12:34 +0000
 +++ src/server/scene/surface_controller.cpp	2015-02-19 13:21:41 +0000
@@ -49,3 +49,9 @@
+ {
      surface_stack->raise(surface);
+ }
++
++auto ms::SurfaceController::surface_at(geometry::Point cursor) const
++-> std::shared_ptr<Surface>
++{
++    return surface_stack->surface_at(cursor);
++}
 === modified file 'src/server/scene/surface_controller.h'
 --- src/server/scene/surface_controller.h	2015-02-13 06:12:34 +0000
 +++ src/server/scene/surface_controller.h	2015-02-19 13:21:41 +0000
@@ -46,6 +46,8 @@
      void raise(std::weak_ptr<Surface> const& surface) override;
++    auto surface_at(geometry::Point) const -> std::shared_ptr<Surface> override;
++
  private:
      std::shared_ptr<SurfaceFactory> const surface_factory;
      std::shared_ptr<SurfaceStackModel> const surface_stack;
 === modified file 'src/server/scene/surface_stack.cpp'
 --- src/server/scene/surface_stack.cpp	2015-02-13 06:12:34 +0000
 +++ src/server/scene/surface_stack.cpp	2015-02-19 13:21:41 +0000
@@ -277,6 +277,39 @@
      // TODO: error logging when surface not found
+ }
++namespace
++{
++template <typename Container>
++struct InReverse {
++    Container& container;
++    auto begin() -> decltype(container.rbegin()) { return container.rbegin(); }
++    auto end() -> decltype(container.rend()) { return container.rend(); }
++};
++
++template <typename Container>
++InReverse<Container> in_reverse(Container& container) { return InReverse<Container>{container}; }
++}
++
++auto ms::SurfaceStack::surface_at(geometry::Point cursor) const
++-> std::shared_ptr<Surface>
++{
++    std::lock_guard<decltype(guard)> lg(guard);
++    for (auto &layer : in_reverse(layers_by_depth))
++    {
++        for (auto const& surface : in_reverse(layer.second))
++        {
++            // TODO There's a lack of clarity about how the input area will
++            // TODO be maintained and whether this test will detect clicks on
++            // TODO decorations (it should) as these may be outside the area
++            // TODO known to the client.  But it works for now.
++            if (surface->input_area_contains(cursor))
++                    return surface;
++        }
++    }
++
++    return {};
++}
++
  void ms::SurfaceStack::for_each(std::function<void(std::shared_ptr<mi::Surface> const&)> const& callback)
+ {
      std::lock_guard<decltype(guard)> lg(guard);
 === modified file 'src/server/scene/surface_stack.h'
 --- src/server/scene/surface_stack.h	2015-01-21 09:51:37 +0000
 +++ src/server/scene/surface_stack.h	2015-02-19 13:21:41 +0000
@@ -89,6 +89,8 @@
          DepthId depth,
          input::InputReceptionMode input_mode) override;
++    auto surface_at(geometry::Point) const -> std::shared_ptr<Surface> override;
++
      void add_observer(std::shared_ptr<Observer> const& observer) override;
      void remove_observer(std::weak_ptr<Observer> const& observer) override;
 === modified file 'src/server/scene/surface_stack_model.h'
 --- src/server/scene/surface_stack_model.h	2015-01-21 07:34:50 +0000
 +++ src/server/scene/surface_stack_model.h	2015-02-19 13:21:41 +0000
@@ -27,6 +27,8 @@
  namespace mir
+ {
++namespace geometry { class Point; }
++
  namespace scene
+ {
  class Surface;
@@ -34,7 +36,7 @@
  class SurfaceStackModel
+ {
  public:
--    virtual ~SurfaceStackModel() {}
++    virtual ~SurfaceStackModel() = default;
      virtual void add_surface(
          std::shared_ptr<Surface> const& surface,
@@ -45,6 +47,8 @@
      virtual void raise(std::weak_ptr<Surface> const& surface) = 0;
++    virtual auto surface_at(geometry::Point) const -> std::shared_ptr<Surface> = 0;
++
  protected:
      SurfaceStackModel() = default;
      SurfaceStackModel(const SurfaceStackModel&) = delete;
 === modified file 'src/server/symbols.map'
 --- src/server/symbols.map	2015-02-13 06:12:34 +0000
 +++ src/server/symbols.map	2015-02-19 13:21:41 +0000
@@ -213,6 +213,7 @@
      mir::scene::SurfaceBufferAccess::?SurfaceBufferAccess*;
      mir::scene::SurfaceBufferAccess::SurfaceBufferAccess*;
      mir::scene::SurfaceBufferAccess::with_most_recent_buffer_do*;
++    mir::scene::Surface::buffers_ready_for_compositor*;
      mir::scene::Surface::client_size*;
      mir::scene::Surface::compositor_snapshot*;
      mir::scene::SurfaceConfigurator::attribute_set*;
@@ -377,9 +378,6 @@
      mir::shell::FocusController::focussed_application*;
      mir::shell::FocusController::operator*;
      mir::shell::FocusController::set_focus_to*;
--    mir::shell::FocusSetter::FocusSetter*;
--    mir::shell::FocusSetter::operator*;
--    mir::shell::FocusSetter::set_focus_to*;
      mir::shell::HostLifecycleEventListener::?HostLifecycleEventListener*;
      mir::shell::HostLifecycleEventListener::HostLifecycleEventListener*;
      mir::shell::HostLifecycleEventListener::lifecycle_event_occurred*;
@@ -389,16 +387,6 @@
      mir::shell::InputTargeter::?InputTargeter*;
      mir::shell::InputTargeter::InputTargeter*;
      mir::shell::InputTargeter::operator*;
--    mir::shell::SessionCoordinatorWrapper::add_prompt_provider_for*;
--    mir::shell::SessionCoordinatorWrapper::close_session*;
--    mir::shell::SessionCoordinatorWrapper::focus_next*;
--    mir::shell::SessionCoordinatorWrapper::focussed_application*;
--    mir::shell::SessionCoordinatorWrapper::handle_surface_created*;
--    mir::shell::SessionCoordinatorWrapper::open_session*;
--    mir::shell::SessionCoordinatorWrapper::SessionCoordinatorWrapper*;
--    mir::shell::SessionCoordinatorWrapper::set_focus_to*;
--    mir::shell::SessionCoordinatorWrapper::start_prompt_session_for*;
--    mir::shell::SessionCoordinatorWrapper::stop_prompt_session*;
      mir::shell::Shell::add_prompt_provider_for*;
      mir::shell::Shell::close_session*;
      mir::shell::Shell::create_surface*;
@@ -423,10 +411,6 @@
      mir::shell::ShellWrapper::ShellWrapper*;
      mir::shell::ShellWrapper::start_prompt_session_for*;
      mir::shell::ShellWrapper::stop_prompt_session*;
--    mir::shell::SurfaceCoordinatorWrapper::add_surface*;
--    mir::shell::SurfaceCoordinatorWrapper::raise*;
--    mir::shell::SurfaceCoordinatorWrapper::remove_surface*;
--    mir::shell::SurfaceCoordinatorWrapper::SurfaceCoordinatorWrapper*;
      mir::terminate_with_current_exception*;
      mir::time::Alarm::?Alarm*;
      mir::time::Alarm::Alarm*;
@@ -497,11 +481,6 @@
      non-virtual?thunk?to?mir::shell::FocusController::?FocusController*;
      non-virtual?thunk?to?mir::shell::HostLifecycleEventListener::?HostLifecycleEventListener*;
      non-virtual?thunk?to?mir::shell::InputTargeter::?InputTargeter*;
--    non-virtual?thunk?to?mir::shell::SessionCoordinatorWrapper::close_session*;
--    non-virtual?thunk?to?mir::shell::SessionCoordinatorWrapper::focus_next*;
--    non-virtual?thunk?to?mir::shell::SessionCoordinatorWrapper::focussed_application*;
--    non-virtual?thunk?to?mir::shell::SessionCoordinatorWrapper::open_session*;
--    non-virtual?thunk?to?mir::shell::SessionCoordinatorWrapper::set_focus_to*;
      non-virtual?thunk?to?mir::shell::ShellWrapper::add_prompt_provider_for*;
      non-virtual?thunk?to?mir::shell::ShellWrapper::close_session*;
      non-virtual?thunk?to?mir::shell::ShellWrapper::create_surface*;
@@ -515,9 +494,6 @@
      non-virtual?thunk?to?mir::shell::ShellWrapper::set_surface_attribute*;
      non-virtual?thunk?to?mir::shell::ShellWrapper::start_prompt_session_for*;
      non-virtual?thunk?to?mir::shell::ShellWrapper::stop_prompt_session*;
--    non-virtual?thunk?to?mir::shell::SurfaceCoordinatorWrapper::add_surface*;
--    non-virtual?thunk?to?mir::shell::SurfaceCoordinatorWrapper::raise*;
--    non-virtual?thunk?to?mir::shell::SurfaceCoordinatorWrapper::remove_surface*;
      non-virtual?thunk?to?mir::time::Alarm::?Alarm*;
      non-virtual?thunk?to?mir::time::Timer::?Timer*;
      typeinfo?for?mir::compositor::Compositor;
@@ -566,13 +542,10 @@
      typeinfo?for?mir::shell::AbstractShell;
      typeinfo?for?mir::shell::DisplayLayout;
      typeinfo?for?mir::shell::FocusController;
--    typeinfo?for?mir::shell::FocusSetter;
      typeinfo?for?mir::shell::HostLifecycleEventListener;
      typeinfo?for?mir::shell::InputTargeter;
--    typeinfo?for?mir::shell::SessionCoordinatorWrapper;
      typeinfo?for?mir::shell::Shell;
      typeinfo?for?mir::shell::ShellWrapper;
--    typeinfo?for?mir::shell::SurfaceCoordinatorWrapper;
      typeinfo?for?mir::time::Alarm;
      typeinfo?for?mir::time::Timer;
      vtable?for?mir::compositor::Compositor;
@@ -621,13 +594,10 @@
      vtable?for?mir::shell::AbstractShell;
      vtable?for?mir::shell::DisplayLayout;
      vtable?for?mir::shell::FocusController;
--    vtable?for?mir::shell::FocusSetter;
      vtable?for?mir::shell::HostLifecycleEventListener;
      vtable?for?mir::shell::InputTargeter;
--    vtable?for?mir::shell::SessionCoordinatorWrapper;
      vtable?for?mir::shell::Shell;
      vtable?for?mir::shell::ShellWrapper;
--    vtable?for?mir::shell::SurfaceCoordinatorWrapper;
      vtable?for?mir::time::Alarm;
      vtable?for?mir::time::Timer;
@@ -765,3 +735,9 @@
    };
   local: *;
  };
++
++MIR_SERVER_29.1 {
++  global:
++    mir::scene::SurfaceCoordinator::surface_at*;
++  local: *;
++} MIR_SERVER_29;
 === modified file 'tests/include/mir_test_doubles/mock_surface_coordinator.h'
 --- tests/include/mir_test_doubles/mock_surface_coordinator.h	2015-02-13 06:12:34 +0000
 +++ tests/include/mir_test_doubles/mock_surface_coordinator.h	2015-02-19 13:21:41 +0000
@@ -41,6 +41,7 @@
          scene::Session* session));
      MOCK_METHOD1(remove_surface, void(std::weak_ptr<scene::Surface> const& surface));
++    MOCK_CONST_METHOD1(surface_at, std::shared_ptr<scene::Surface>(geometry::Point));
  };
+ }
 === modified file 'tests/unit-tests/scene/test_application_session.cpp'
 --- tests/unit-tests/scene/test_application_session.cpp	2015-02-13 06:12:34 +0000
 +++ tests/unit-tests/scene/test_application_session.cpp	2015-02-19 13:21:41 +0000
@@ -97,6 +97,10 @@
      void remove_surface(std::weak_ptr<ms::Surface> const&) override
+     {
+     }
++    auto surface_at(mir::geometry::Point) const -> std::shared_ptr<ms::Surface>
++    {
++        return std::shared_ptr<ms::Surface>{};
++    }
  };
  struct ApplicationSession : public testing::Test
 === modified file 'tests/unit-tests/scene/test_surface_controller.cpp'
 --- tests/unit-tests/scene/test_surface_controller.cpp	2015-02-13 06:12:34 +0000
 +++ tests/unit-tests/scene/test_surface_controller.cpp	2015-02-19 13:21:41 +0000
@@ -52,6 +52,7 @@
          mir::input::InputReceptionMode input_mode));
      MOCK_METHOD1(remove_surface, void(std::weak_ptr<ms::Surface> const&));
      MOCK_METHOD1(raise, void(std::weak_ptr<ms::Surface> const&));
++    MOCK_CONST_METHOD1(surface_at, std::shared_ptr<ms::Surface>(geom::Point));
  };
  struct SurfaceController : testing::Test
 === modified file 'tests/unit-tests/scene/test_surface_stack.cpp'
 --- tests/unit-tests/scene/test_surface_stack.cpp	2015-02-13 06:12:34 +0000
 +++ tests/unit-tests/scene/test_surface_stack.cpp	2015-02-19 13:21:41 +0000
@@ -882,3 +882,50 @@
      stack.for_each(count_exposed_surfaces);
      EXPECT_THAT(num_exposed_surfaces, Eq(1));
+ }
++
++using namespace ::testing;
++
++TEST_F(SurfaceStack, returns_top_surface_under_cursor)
++{
++    geom::Point const cursor_over_all {100, 100};
++    geom::Point const cursor_over_12  {200, 100};
++    geom::Point const cursor_over_1   {600, 600};
++    geom::Point const cursor_over_none{999, 999};
++
++    stack.add_surface(stub_surface1, default_params.depth, default_params.input_mode);
++    stack.add_surface(stub_surface2, default_params.depth, default_params.input_mode);
++    stack.add_surface(stub_surface3, default_params.depth, default_params.input_mode);
++
++    stub_surface1->resize({900, 900});
++    stub_surface2->resize({500, 200});
++    stub_surface3->resize({200, 500});
++
++    EXPECT_THAT(stack.surface_at(cursor_over_all),  Eq(stub_surface3));
++    EXPECT_THAT(stack.surface_at(cursor_over_12),   Eq(stub_surface2));
++    EXPECT_THAT(stack.surface_at(cursor_over_1),    Eq(stub_surface1));
++    EXPECT_THAT(stack.surface_at(cursor_over_none).get(), IsNull());
++}
++
++TEST_F(SurfaceStack, returns_top_visible_surface_under_cursor)
++{
++    geom::Point const cursor_over_all {100, 100};
++    geom::Point const cursor_over_12  {200, 100};
++    geom::Point const cursor_over_1   {600, 600};
++    geom::Point const cursor_over_none{999, 999};
++
++    stack.add_surface(stub_surface1, default_params.depth, default_params.input_mode);
++    stack.add_surface(stub_surface2, default_params.depth, default_params.input_mode);
++    stack.add_surface(stub_surface3, default_params.depth, default_params.input_mode);
++    stack.add_surface(invisible_stub_surface, default_params.depth, default_params.input_mode);
++
++    stub_surface1->resize({900, 900});
++    stub_surface2->resize({500, 200});
++    stub_surface3->resize({200, 500});
++    invisible_stub_surface->resize({999, 999});
++
++    EXPECT_THAT(stack.surface_at(cursor_over_all),  Eq(stub_surface3));
++    EXPECT_THAT(stack.surface_at(cursor_over_12),   Eq(stub_surface2));
++    EXPECT_THAT(stack.surface_at(cursor_over_1),    Eq(stub_surface1));
++    EXPECT_THAT(stack.surface_at(cursor_over_none).get(), IsNull());
++}
++