Merge lp:~alan-griffiths/mir/BufferStreamArrangement.can_be_specified_when_creating_surface into lp:mir

[Alan Griffiths]

Add BufferStreamArrangement.can_be_specified_when_creating_surface test and fix implementation to pass.

In looking to fix lp:1580555 I found the code I wanted to change ins SessionMediator::create_surface() wasn't under test. Here is the missing test.

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3507
https://mir-jenkins.ubuntu.com/job/mir-ci/976/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/1057/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1104
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1095
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1095
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1067
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1067/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1067
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1067/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1067
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1067/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1067
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1067/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1067/console

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

review: Needs Fixing (continuous-integration)

[Cemil Azizoglu (cemil-azizoglu)]

Difficult to review right now as mir_connection.cpp changes are conflicting with those in https://code.launchpad.net/~kdub/mir/fix-1577967/+merge/294283.

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3507
https://mir-jenkins.ubuntu.com/job/mir-ci/977/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/1058/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1105
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1096/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1096/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1068/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1068/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1068/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1068/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1068/console

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

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

> Difficult to review right now as mir_connection.cpp changes are conflicting
> with those in https://code.launchpad.net/~kdub/mir/fix-1577967/+merge/294283.

The mir_connection.cpp changes here are not needed with that branch. So you can try:

$ bzr resolve --take-this src/client/mir_connection.cpp

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3507
https://mir-jenkins.ubuntu.com/job/mir-ci/978/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/1059/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1106
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1097/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1097/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1069/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1069/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1069/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1069/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1069/console

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

review: Needs Fixing (continuous-integration)

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3507
https://mir-jenkins.ubuntu.com/job/mir-ci/979/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/1061
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1108
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1099
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1099
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1071
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1071/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1071
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1071/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1071
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1071/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1071
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1071/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1071
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1071/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

lgtm, more-or-less a subset of what is needed by fix-1577967 (which tacks on some further clarifications)

review: Approve

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3509
https://mir-jenkins.ubuntu.com/job/mir-ci/986/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/1068
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1115
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1106
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1106
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1078
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1078/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1078
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1078/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1078
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1078/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1078
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1078/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1078
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1078/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

=== modified file 'src/client/mir_connection.cpp'
--- src/client/mir_connection.cpp	2016-05-09 14:27:58 +0000
+++ src/client/mir_connection.cpp	2016-05-13 08:57:27 +0000
@@ -381,10 +381,13 @@
         std::string name{spec.surface_name.is_set() ?
                          spec.surface_name.value() : ""};
 
-        stream = std::make_shared<mcl::BufferStream>(
-            this, request->wh, server, platform, surface_map, buffer_factory,
-            surface_proto->buffer_stream(), make_perf_report(logger), name,
-            mir::geometry::Size{surface_proto->width(), surface_proto->height()}, nbuffers);
+        if (surface_proto->has_buffer_stream())
+        {
+            stream = std::make_shared<mcl::BufferStream>(
+                this, request->wh, server, platform, surface_map, buffer_factory,
+                surface_proto->buffer_stream(), make_perf_report(logger), name,
+                mir::geometry::Size{surface_proto->width(), surface_proto->height()}, nbuffers);
+        }
     }
     catch (std::exception const& error)
     {
@@ -417,7 +420,9 @@
             this, server, &debug, stream, input_platform, spec, *surface_proto, request->wh);
 
         surface_map->insert(mf::SurfaceId{surface_proto->id().value()}, surf);
-        surface_map->insert(mf::BufferStreamId{surface_proto->buffer_stream().id().value()}, stream);
+
+        if (stream)
+            surface_map->insert(mf::BufferStreamId{surface_proto->buffer_stream().id().value()}, stream);
     }
 
     callback(surf.get(), context);

=== modified file 'src/server/frontend/session_mediator.cpp'
--- src/server/frontend/session_mediator.cpp	2016-05-10 10:48:28 +0000
+++ src/server/frontend/session_mediator.cpp	2016-05-13 08:57:27 +0000
@@ -335,19 +335,15 @@
     auto const surf_id = shell->create_surface(session, params, sink);
 
     auto surface = session->get_surface(surf_id);
-    auto stream = session->get_buffer_stream(buffer_stream_id);
     auto const& client_size = surface->client_size();
     response->mutable_id()->set_value(surf_id.as_value());
     response->set_width(client_size.width.as_uint32_t());
     response->set_height(client_size.height.as_uint32_t());
 
     // TODO: Deprecate
-    response->set_pixel_format(stream->pixel_format());
+    response->set_pixel_format(request->pixel_format());
     response->set_buffer_usage(request->buffer_usage());
 
-    response->mutable_buffer_stream()->set_pixel_format(stream->pixel_format());
-    response->mutable_buffer_stream()->set_buffer_usage(request->buffer_usage());
-
     if (surface->supports_input())
         response->add_fd(surface->client_input_fd());
     
@@ -364,6 +360,9 @@
     {
         buffer_stream_tracker.set_default_stream(surf_id, buffer_stream_id);
         response->mutable_buffer_stream()->mutable_id()->set_value(buffer_stream_id.as_value());
+        response->mutable_buffer_stream()->set_pixel_format(legacy_stream->pixel_format());
+        response->mutable_buffer_stream()->set_buffer_usage(request->buffer_usage());
+
         advance_buffer(buffer_stream_id, *legacy_stream, buffer_stream_tracker.last_buffer(buffer_stream_id),
             [this, buffering_sender, response, done, session]
             (graphics::Buffer* client_buffer, graphics::BufferIpcMsgType msg_type)

=== modified file 'tests/acceptance-tests/test_buffer_stream_arrangement.cpp'
--- tests/acceptance-tests/test_buffer_stream_arrangement.cpp	2016-01-29 08:18:22 +0000
+++ tests/acceptance-tests/test_buffer_stream_arrangement.cpp	2016-05-13 08:57:27 +0000
@@ -66,15 +66,6 @@
 
 struct Stream
 {
-    Stream(MirBufferStream* stream, geom::Point pt, geom::Size sz) :
-        stream(stream),
-        pos{pt},
-        stream_size{sz},
-        needs_release{false}
-    {
-        mir_buffer_stream_swap_buffers_sync(stream);
-    }
-
     Stream(MirConnection* connection, geom::Rectangle rect) :
         stream(mir_connection_create_buffer_stream_sync(
             connection,
@@ -212,8 +203,7 @@
         ConnectedClientWithASurface::SetUp();
         server.the_cursor()->hide();
 
-        streams.emplace_back(
-            std::make_unique<Stream>(mir_surface_get_buffer_stream(surface), geom::Point{0,0}, surface_size));
+        streams.emplace_back(std::make_unique<Stream>(connection, geom::Rectangle{geom::Point{0,0}, surface_size}));
         int const additional_streams{3};
         for (auto i = 0; i < additional_streams; i++)
         {
@@ -235,6 +225,32 @@
 };
 }
 
+TEST_F(BufferStreamArrangement, can_be_specified_when_creating_surface)
+{
+    using namespace testing;
+    std::vector<MirBufferStreamInfo> infos(streams.size());
+    auto i = 0u;
+    for (auto const& stream : streams)
+    {
+        infos[i++] = MirBufferStreamInfo{
+            stream->handle(),
+            stream->position().x.as_int(),
+            stream->position().y.as_int()};
+    }
+
+    mir_surface_release_sync(surface);
+
+    auto const spec = mir_connection_create_spec_for_normal_surface(
+        connection, surface_size.width.as_int(), surface_size.height.as_int(), mir_pixel_format_abgr_8888);
+    mir_surface_spec_set_name(spec, "BufferStreamArrangement.can_be_specified_when_creating_surface");
+    mir_surface_spec_set_buffer_usage(spec, mir_buffer_usage_hardware);
+    mir_surface_spec_set_streams(spec, infos.data(), infos.size());
+
+    surface = mir_surface_create_sync(spec);
+    mir_surface_spec_release(spec);
+    EXPECT_TRUE(mir_surface_is_valid(surface)) << mir_surface_get_error_message(surface);
+}
+
 TEST_F(BufferStreamArrangement, arrangements_are_applied)
 {
     using namespace testing;