Merge lp:~robertcarr/mir/simplify-depth-assignment into lp:~mir-team/mir/trunk

[Robert Carr]

Move the DepthId in surface creation to msh::SurfaceCreationParameters.

Simplify depth assignment so the shell can easily pass as part of the SurfaceCreationParameters rather than overriding one of our 'factory/controller' classes (SurfaceController).

Also a first pass at making things easier for the system compositor and the greeter stacking, but still need to find a way to get the session association in sensibly

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:836
http://jenkins.qa.ubuntu.com/job/mir-ci/1015/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1319/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1204/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/253/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/646

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

review: Needs Fixing (continuous-integration)

[Robert Ancell (robert-ancell)]

Shows a merge conflict

review: Needs Fixing

[Robert Ancell (robert-ancell)]

Other than that, +1

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:837
http://jenkins.qa.ubuntu.com/job/mir-ci/1016/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1320
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1205
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/254
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/254/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Robert Ancell (robert-ancell)]

review: Approve

[Alexandros Frantzis (afrantzis)]

80 bool msh::operator==(
81 const SurfaceCreationParameters& lhs,
82 const msh::SurfaceCreationParameters& rhs)

Don't we need to update operator==()? Is operator==() used at all?

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

I'm still deeply suspicious that DepthId is the wrong abstraction - so this has the feel of "shuffling deckchairs on a sinking ship".

operator==() is wrong before and after this MP

review: Abstain

[Robert Carr (robertcarr)]

Updated operator== (see changes to test_surface.cpp: (SurfaceCreationParameters, equality) as well I found the existing test misleading). DepthId is clearly on the way out eventually but at least it's less annoying here than in create_surface parameters.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:838
http://jenkins.qa.ubuntu.com/job/mir-ci/1024/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1331
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1216/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/263/console
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/263/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/653

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

review: Needs Fixing (continuous-integration)

[Kevin DuBois (kdub)]

> I'm still deeply suspicious that DepthId is the wrong abstraction - so this
> has the feel of "shuffling deckchairs on a sinking ship".

+1

I'd rather just present a z-ordering and have the shell have better tracking of what surface is where and how its stacked. I *somewhat* detect that the shell wants to group surfaces and order them more arbitrarily, and are trying to fit their square peg into SurfaceStack's round abstraction of a scenegraph

That being said, I suppose that if its already in the code base, its better that there's a way to use it.

My question is... are we planning on keeping DepthID forever, or do we think that as the shell evolves we will eliminate this? I sort of get the feeling that the stack would be a better abstraction if it was closer to a z-ordering (simpler).

review: Abstain

[Kevin DuBois (kdub)]

also though, don't want to derail this specific MP with esoteric planning about the surfacestack, I think the idea behind this specific MP is an improvement in the code :)

[Gerry Boland (gerboland)]

> My question is... are we planning on keeping DepthID forever, or do we think
> that as the shell evolves we will eliminate this? I sort of get the feeling
> that the stack would be a better abstraction if it was closer to a z-ordering
> (simpler).

Speaking from the shell perspective, I see situations in future when just DepthID and limited z-order manipulation via focus changes will be too limiting. I'd vote for z-order access.

[Alexandros Frantzis (afrantzis)]

Looks good.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:838
http://jenkins.qa.ubuntu.com/job/mir-ci/1033/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1341/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1226/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/271/console

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

review: Needs Fixing (continuous-integration)

[Gerry Boland (gerboland)]

No objection here

review: Approve

[Robert Carr (robertcarr)]

This should land after https://code.launchpad.net/~robertcarr/mir/fix-depth-tests/+merge/175410 or this diff becomes very confusing.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:838
http://jenkins.qa.ubuntu.com/job/mir-ci/1043/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1358/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1243/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/281/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/670

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:838
http://jenkins.qa.ubuntu.com/job/mir-ci/1044/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1359/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1244/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/282/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/671

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

review: Needs Fixing (continuous-integration)

[Robert Carr (robertcarr)]

Merged trunk.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:839
http://jenkins.qa.ubuntu.com/job/mir-ci/1058/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1376
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1261
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/296
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/296/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/682

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

review: Approve (continuous-integration)

=== modified file 'include/server/mir/shell/surface_creation_parameters.h'
--- include/server/mir/shell/surface_creation_parameters.h	2013-05-21 17:16:43 +0000
+++ include/server/mir/shell/surface_creation_parameters.h	2013-07-18 22:32:28 +0000
@@ -23,6 +23,7 @@
 #include "mir/geometry/point.h"
 #include "mir/geometry/size.h"
 #include "mir/compositor/buffer_properties.h"
+#include "mir/surfaces/depth_id.h"
 
 #include <memory>
 #include <string>
@@ -47,12 +48,15 @@
     SurfaceCreationParameters& of_buffer_usage(compositor::BufferUsage new_buffer_usage);
 
     SurfaceCreationParameters& of_pixel_format(geometry::PixelFormat new_pixel_format);
+    
+    SurfaceCreationParameters& of_depth(surfaces::DepthId const& new_depth);
 
     std::string name;
     geometry::Size size;
     geometry::Point top_left;
     compositor::BufferUsage buffer_usage;
     geometry::PixelFormat pixel_format;
+    surfaces::DepthId depth;
 };
 
 bool operator==(const SurfaceCreationParameters& lhs, const SurfaceCreationParameters& rhs);

=== modified file 'include/server/mir/surfaces/surface_stack.h'
--- include/server/mir/surfaces/surface_stack.h	2013-07-17 09:09:30 +0000
+++ include/server/mir/surfaces/surface_stack.h	2013-07-18 22:32:28 +0000
@@ -73,7 +73,7 @@
     void for_each(std::function<void(std::shared_ptr<input::InputChannel> const&)> const& callback);
 
     // From SurfaceStackModel 
-    virtual std::weak_ptr<Surface> create_surface(const shell::SurfaceCreationParameters& params, DepthId depth);
+    virtual std::weak_ptr<Surface> create_surface(const shell::SurfaceCreationParameters& params);
 
     virtual void destroy_surface(std::weak_ptr<Surface> const& surface);
     

=== modified file 'include/server/mir/surfaces/surface_stack_model.h'
--- include/server/mir/surfaces/surface_stack_model.h	2013-06-25 20:28:17 +0000
+++ include/server/mir/surfaces/surface_stack_model.h	2013-07-18 22:32:28 +0000
@@ -40,7 +40,7 @@
 public:
     virtual ~SurfaceStackModel() {}
 
-    virtual std::weak_ptr<Surface> create_surface(shell::SurfaceCreationParameters const& params, DepthId depth) = 0;
+    virtual std::weak_ptr<Surface> create_surface(shell::SurfaceCreationParameters const& params) = 0;
     virtual void destroy_surface(std::weak_ptr<Surface> const& surface) = 0;
 
     virtual void raise(std::weak_ptr<Surface> const& surface) = 0;

=== modified file 'src/server/shell/surface_creation_parameters.cpp'
--- src/server/shell/surface_creation_parameters.cpp	2013-07-11 16:54:41 +0000
+++ src/server/shell/surface_creation_parameters.cpp	2013-07-18 22:32:28 +0000
@@ -24,7 +24,8 @@
 
 msh::SurfaceCreationParameters::SurfaceCreationParameters()
     : name(), size(), top_left(), buffer_usage(mc::BufferUsage::undefined),
-      pixel_format(geom::PixelFormat::invalid)
+      pixel_format(geom::PixelFormat::invalid),
+      depth{0}
 {
 }
 
@@ -73,13 +74,24 @@
     return *this;
 }
 
+msh::SurfaceCreationParameters& msh::SurfaceCreationParameters::of_depth(
+    surfaces::DepthId const& new_depth)
+{
+    depth = new_depth;
+    
+    return *this;
+}
+
 bool msh::operator==(
     const SurfaceCreationParameters& lhs,
     const msh::SurfaceCreationParameters& rhs)
 {
-    return lhs.size == rhs.size &&
-           lhs.buffer_usage == rhs.buffer_usage &&
-           lhs.pixel_format == rhs.pixel_format;
+    return lhs.name == rhs.name && 
+        lhs.size == rhs.size &&
+        lhs.top_left == rhs.top_left &&
+        lhs.buffer_usage == rhs.buffer_usage &&
+        lhs.pixel_format == rhs.pixel_format &&
+        lhs.depth == rhs.depth;
 }
 
 bool msh::operator!=(

=== modified file 'src/server/surfaces/surface_controller.cpp'
--- src/server/surfaces/surface_controller.cpp	2013-06-25 20:28:17 +0000
+++ src/server/surfaces/surface_controller.cpp	2013-07-18 22:32:28 +0000
@@ -28,8 +28,7 @@
 
 std::weak_ptr<ms::Surface> ms::SurfaceController::create_surface(shell::SurfaceCreationParameters const& params)
 {
-    static ms::DepthId const default_surface_depth{0};
-    return surface_stack->create_surface(params, default_surface_depth);
+    return surface_stack->create_surface(params);
 }
 
 void ms::SurfaceController::destroy_surface(std::weak_ptr<Surface> const& surface)

=== modified file 'src/server/surfaces/surface_stack.cpp'
--- src/server/surfaces/surface_stack.cpp	2013-07-17 09:09:30 +0000
+++ src/server/surfaces/surface_stack.cpp	2013-07-18 22:32:28 +0000
@@ -73,13 +73,13 @@
     notify_change = f;
 }
 
-std::weak_ptr<ms::Surface> ms::SurfaceStack::create_surface(shell::SurfaceCreationParameters const& params, ms::DepthId depth)
+std::weak_ptr<ms::Surface> ms::SurfaceStack::create_surface(shell::SurfaceCreationParameters const& params)
 {
     auto change_cb = [this]() { emit_change_notification(); };
     auto surface = surface_factory->create_surface(params, change_cb); 
     {
         std::lock_guard<std::mutex> lg(guard);
-        layers_by_depth[depth].push_back(surface);
+        layers_by_depth[params.depth].push_back(surface);
     }
 
     input_registrar->input_channel_opened(surface->input_channel(), surface->input_surface());

=== modified file 'tests/acceptance-tests/test_client_input.cpp'
--- tests/acceptance-tests/test_client_input.cpp	2013-07-12 09:47:40 +0000
+++ tests/acceptance-tests/test_client_input.cpp	2013-07-18 22:32:28 +0000
@@ -436,26 +436,37 @@
 
 namespace
 {
-typedef std::map<std::string, geom::Rectangle> GeometryList;
+typedef std::map<std::string, geom::Rectangle> GeometryMap;
+typedef std::map<std::string, ms::DepthId> DepthMap;
 
 struct StaticPlacementStrategy : public msh::PlacementStrategy
 {
-    StaticPlacementStrategy(GeometryList const& positions)
-        : surface_geometry_by_name(positions)
+    StaticPlacementStrategy(GeometryMap const& positions,
+                            DepthMap const& depths)
+        : surface_geometry_by_name(positions),
+          surface_depths_by_name(depths)
+    {
+    }
+
+    StaticPlacementStrategy(GeometryMap const& positions)
+        : StaticPlacementStrategy(positions, DepthMap())
     {
     }
 
     msh::SurfaceCreationParameters place(msh::SurfaceCreationParameters const& request_parameters)
     {
         auto placed = request_parameters;
-        auto geometry = surface_geometry_by_name[request_parameters.name];
+        auto const& name = request_parameters.name;
+        auto geometry = surface_geometry_by_name[name];
 
         placed.top_left = geometry.top_left;
         placed.size = geometry.size;
+        placed.depth = surface_depths_by_name[name];
         
         return placed;
     }
-    GeometryList surface_geometry_by_name;
+    GeometryMap surface_geometry_by_name;
+    DepthMap surface_depths_by_name;
 };
 
 }
@@ -475,7 +486,7 @@
     {
         std::shared_ptr<msh::PlacementStrategy> the_shell_placement_strategy() override
         {
-            static GeometryList positions;
+            static GeometryMap positions;
             positions[test_client_1] = geom::Rectangle{geom::Point{0, 0},
                 geom::Size{client_width, client_height}};
             positions[test_client_2] = geom::Rectangle{geom::Point{screen_width/2, screen_height/2},
@@ -585,7 +596,7 @@
     {
         std::shared_ptr<msh::PlacementStrategy> the_shell_placement_strategy() override
         {
-            static GeometryList positions;
+            static GeometryMap positions;
             positions[test_client_name] = screen_geometry;
             
             return std::make_shared<StaticPlacementStrategy>(positions);
@@ -649,27 +660,6 @@
     launch_client_process(client_config);
 }
 
-namespace
-{
-typedef std::map<std::string, ms::DepthId> DepthList;
-
-struct StackingSurfaceController : public ms::SurfaceController
-{
-    StackingSurfaceController(std::shared_ptr<ms::SurfaceStackModel> const& surface_stack_model, DepthList const& depths)
-        : SurfaceController(surface_stack_model),
-          surface_depths_by_name(depths)
-    {
-    }
-    
-    std::weak_ptr<ms::Surface> create_surface(msh::SurfaceCreationParameters const& params) override
-    {
-        return surface_stack->create_surface(params, surface_depths_by_name[params.name]);
-    }
-    
-    DepthList surface_depths_by_name;
-};
-}
-
 TEST_F(TestClientInput, surfaces_obscure_motion_events_by_stacking)
 {
     using namespace ::testing;
@@ -687,23 +677,18 @@
     {
         std::shared_ptr<msh::PlacementStrategy> the_shell_placement_strategy() override
         {
-            static GeometryList positions;
+            static GeometryMap positions;
             positions[test_client_name_1] = screen_geometry;
             
             auto smaller_geometry = screen_geometry;
             smaller_geometry.size.width = geom::Width{screen_width/2};
             positions[test_client_name_2] = smaller_geometry;
             
-            return std::make_shared<StaticPlacementStrategy>(positions);
-       }
-        
-        std::shared_ptr<msh::SurfaceBuilder> the_surface_builder() override
-        {
-            static DepthList depths;
+            static DepthMap depths;
             depths[test_client_name_1] = ms::DepthId{0};
             depths[test_client_name_2] = ms::DepthId{1};
             
-            return std::make_shared<StackingSurfaceController>(the_surface_stack_model(), depths);
+            return std::make_shared<StaticPlacementStrategy>(positions, depths);
         }
         
         void inject_input() override

=== modified file 'tests/unit-tests/surfaces/test_surface.cpp'
--- tests/unit-tests/surfaces/test_surface.cpp	2013-07-16 15:37:58 +0000
+++ tests/unit-tests/surfaces/test_surface.cpp	2013-07-18 22:32:28 +0000
@@ -92,12 +92,12 @@
     mc::BufferUsage const usage{mc::BufferUsage::hardware};
     geom::PixelFormat const format{geom::PixelFormat::abgr_8888};
 
-    auto params0 = msh::a_surface().of_name("surface0")
+    auto params0 = msh::a_surface().of_name("surface")
                                   .of_size(size)
                                   .of_buffer_usage(usage)
                                   .of_pixel_format(format);
 
-    auto params1 = msh::a_surface().of_name("surface1")
+    auto params1 = msh::a_surface().of_name("surface")
                                   .of_size(size)
                                   .of_buffer_usage(usage)
                                   .of_pixel_format(format);

=== modified file 'tests/unit-tests/surfaces/test_surface_controller.cpp'
--- tests/unit-tests/surfaces/test_surface_controller.cpp	2013-06-25 20:28:17 +0000
+++ tests/unit-tests/surfaces/test_surface_controller.cpp	2013-07-18 22:32:28 +0000
@@ -33,7 +33,7 @@
 {
 struct MockSurfaceStackModel : public ms::SurfaceStackModel
 {
-    MOCK_METHOD2(create_surface, std::weak_ptr<ms::Surface>(msh::SurfaceCreationParameters const&, ms::DepthId depth));
+    MOCK_METHOD1(create_surface, std::weak_ptr<ms::Surface>(msh::SurfaceCreationParameters const&));
     MOCK_METHOD1(destroy_surface, void(std::weak_ptr<ms::Surface> const&));
     MOCK_METHOD1(raise, void(std::weak_ptr<ms::Surface> const&));
 };
@@ -49,7 +49,7 @@
     ms::SurfaceController controller(mt::fake_shared(model));
 
     InSequence seq;
-    EXPECT_CALL(model, create_surface(_, _)).Times(1).WillOnce(Return(null_surface));
+    EXPECT_CALL(model, create_surface(_)).Times(1).WillOnce(Return(null_surface));
     EXPECT_CALL(model, destroy_surface(_)).Times(1);
 
     auto surface = controller.create_surface(msh::a_surface());

=== modified file 'tests/unit-tests/surfaces/test_surface_stack.cpp'
--- tests/unit-tests/surfaces/test_surface_stack.cpp	2013-07-18 11:04:18 +0000
+++ tests/unit-tests/surfaces/test_surface_stack.cpp	2013-07-18 22:32:28 +0000
@@ -146,8 +146,6 @@
                                                               std::function<void()> const&)); 
 };
 
-static ms::DepthId const default_depth{0};
-
 struct StubBufferStreamFactory : public ms::BufferStreamFactory
 {
     std::shared_ptr<ms::BufferStream> create_buffer_stream(mc::BufferProperties const&)
@@ -201,7 +199,7 @@
 
     auto use_count = stub_surface1.use_count();
 
-    auto surface = stack.create_surface(default_params, default_depth);
+    auto surface = stack.create_surface(default_params);
         {
             EXPECT_EQ(stub_surface1, surface.lock());
         }
@@ -222,13 +220,13 @@
         .WillOnce(Return(stub_surface3));
 
     ms::SurfaceStack stack(mt::fake_shared(mock_surface_allocator), mt::fake_shared(input_registrar));
-    auto s1 = stack.create_surface(default_params, default_depth);
+    auto s1 = stack.create_surface(default_params);
     auto criteria1 = s1.lock()->compositing_criteria();
     auto stream1 = s1.lock()->buffer_stream();
-    auto s2 = stack.create_surface(default_params, default_depth);
+    auto s2 = stack.create_surface(default_params);
     auto criteria2 = s2.lock()->compositing_criteria();
     auto stream2 = s2.lock()->buffer_stream();
-    auto s3 = stack.create_surface(default_params, default_depth);
+    auto s3 = stack.create_surface(default_params);
     auto criteria3 = s3.lock()->compositing_criteria();
     auto stream3 = s3.lock()->buffer_stream();
 
@@ -264,13 +262,13 @@
 
     ms::SurfaceStack stack(mt::fake_shared(mock_surface_allocator), mt::fake_shared(input_registrar));
 
-    auto s1 = stack.create_surface(default_params, default_depth);
+    auto s1 = stack.create_surface(default_params);
     auto criteria1 = s1.lock()->compositing_criteria();
     auto stream1 = s1.lock()->buffer_stream();
-    auto s2 = stack.create_surface(default_params, default_depth);
+    auto s2 = stack.create_surface(default_params);
     auto criteria2 = s2.lock()->compositing_criteria();
     auto stream2 = s2.lock()->buffer_stream();
-    auto s3 = stack.create_surface(default_params, default_depth);
+    auto s3 = stack.create_surface(default_params);
     auto criteria3 = s3.lock()->compositing_criteria();
     auto stream3 = s3.lock()->buffer_stream();
 
@@ -296,7 +294,7 @@
 
     ms::SurfaceStack stack(mt::fake_shared(mock_surface_allocator), mt::fake_shared(input_registrar));
     stack.set_change_callback(std::bind(&MockCallback::call, &mock_cb));
-    auto surface = stack.create_surface(default_params, default_depth);
+    auto surface = stack.create_surface(default_params);
     stack.destroy_surface(surface);
 }
 
@@ -310,13 +308,13 @@
         .WillOnce(Return(stub_surface3));
 
     ms::SurfaceStack stack(mt::fake_shared(mock_surface_allocator), mt::fake_shared(input_registrar));
-    auto s1 = stack.create_surface(default_params, ms::DepthId{0});
+    auto s1 = stack.create_surface(default_params.of_depth(ms::DepthId{0}));
     auto criteria1 = s1.lock()->compositing_criteria();
     auto stream1 = s1.lock()->buffer_stream();
-    auto s2 = stack.create_surface(default_params, ms::DepthId{1});
+    auto s2 = stack.create_surface(default_params.of_depth(ms::DepthId{1}));
     auto criteria2 = s2.lock()->compositing_criteria();
     auto stream2 = s2.lock()->buffer_stream();
-    auto s3 = stack.create_surface(default_params, ms::DepthId{0});
+    auto s3 = stack.create_surface(default_params.of_depth(ms::DepthId{0}));
     auto criteria3 = s3.lock()->compositing_criteria();
     auto stream3 = s3.lock()->buffer_stream();
 
@@ -347,7 +345,7 @@
 
     ms::SurfaceStack stack(mt::fake_shared(mock_surface_allocator), mt::fake_shared(registrar));
     
-    auto s = stack.create_surface(msh::a_surface(), default_depth);
+    auto s = stack.create_surface(msh::a_surface());
     stack.destroy_surface(s);
 }
 
@@ -361,13 +359,13 @@
         .WillOnce(Return(stub_surface3));
 
     ms::SurfaceStack stack(mt::fake_shared(mock_surface_allocator), mt::fake_shared(input_registrar));
-    auto s1 = stack.create_surface(default_params, default_depth);
+    auto s1 = stack.create_surface(default_params);
     auto criteria1 = s1.lock()->compositing_criteria();
     auto stream1 = s1.lock()->buffer_stream();
-    auto s2 = stack.create_surface(default_params, default_depth);
+    auto s2 = stack.create_surface(default_params);
     auto criteria2 = s2.lock()->compositing_criteria();
     auto stream2 = s2.lock()->buffer_stream();
-    auto s3 = stack.create_surface(default_params, default_depth);
+    auto s3 = stack.create_surface(default_params);
     auto criteria3 = s3.lock()->compositing_criteria();
     auto stream3 = s3.lock()->buffer_stream();
 
@@ -404,13 +402,13 @@
         .WillOnce(Return(stub_surface3));
 
     ms::SurfaceStack stack(mt::fake_shared(mock_surface_allocator), mt::fake_shared(input_registrar));
-    auto s1 = stack.create_surface(default_params, ms::DepthId{0});
+    auto s1 = stack.create_surface(default_params.of_depth(ms::DepthId{0}));
     auto criteria1 = s1.lock()->compositing_criteria();
     auto stream1 = s1.lock()->buffer_stream();
-    auto s2 = stack.create_surface(default_params, ms::DepthId{0});
+    auto s2 = stack.create_surface(default_params.of_depth(ms::DepthId{0}));
     auto criteria2 = s2.lock()->compositing_criteria();
     auto stream2 = s2.lock()->buffer_stream();
-    auto s3 = stack.create_surface(default_params, ms::DepthId{1});
+    auto s3 = stack.create_surface(default_params.of_depth(ms::DepthId{1}));
     auto criteria3 = s3.lock()->compositing_criteria();
     auto stream3 = s3.lock()->buffer_stream();