Merge lp:~kdub/mir/fix-1661521-0.26.1 into lp:~mir-team/mir/0.26-old

[Kevin DuBois]

Make sure native format is properly communicated all the way from client through nested to host and back again. The native format was getting lost, resulting in a failed EGL buffer mapping, resulting in black nested clients.

fixes: LP: #1661521

Make sure native format is properly communicated all the way from client through nested to host and back again. The native format was getting lost, resulting in a failed EGL buffer mapping, resulting in black nested clients.

fixes: LP: #1661521

=== modified file 'src/client/buffer_factory.cpp'
--- src/client/buffer_factory.cpp	2017-01-24 07:37:03 +0000
+++ src/client/buffer_factory.cpp	2017-02-03 19:56:51 +0000
@@ -34,8 +34,21 @@
     native_buffer_factory(native_buffer_factory),
     connection(connection),
     size(size),
-    format(format),
-    usage(usage),
+    sw_request(SoftwareRequest{format, usage}),
+    cb(cb),
+    cb_context(cb_context)
+{
+}
+
+mcl::BufferFactory::AllocationRequest::AllocationRequest(
+    std::shared_ptr<mcl::ClientBufferFactory> const& native_buffer_factory,
+    MirConnection* connection,
+    geom::Size size, uint32_t native_format, uint32_t native_flags,
+    MirBufferCallback cb, void* cb_context) :
+    native_buffer_factory(native_buffer_factory),
+    connection(connection),
+    size(size),
+    native_request(NativeRequest{native_format, native_flags}),
     cb(cb),
     cb_context(cb_context)
 {
@@ -55,6 +68,20 @@
         std::make_unique<AllocationRequest>(factory, connection, size, format, usage, cb, cb_context));
 }
 
+void mcl::BufferFactory::expect_buffer(
+    std::shared_ptr<mcl::ClientBufferFactory> const& factory,
+    MirConnection* connection,
+    geometry::Size size,
+    uint32_t native_format,
+    uint32_t native_flags,
+    MirBufferCallback cb,
+    void* cb_context)
+{
+    std::lock_guard<decltype(mutex)> lk(mutex);
+    allocation_requests.emplace_back(
+        std::make_unique<AllocationRequest>(factory, connection, size, native_format, native_flags, cb, cb_context));
+}
+
 std::unique_ptr<mcl::MirBuffer> mcl::BufferFactory::generate_buffer(mir::protobuf::Buffer const& buffer)
 {
     std::lock_guard<decltype(mutex)> lk(mutex);
@@ -76,13 +103,30 @@
     }
     else
     {
+        auto factory = (*request_it)->native_buffer_factory;
+        std::shared_ptr<mcl::ClientBuffer> client_buffer;
+        auto usage = mir_buffer_usage_hardware;
+        if ((*request_it)->native_request.is_set())
+        {
+            auto& req = (*request_it)->native_request.value();
+            client_buffer = factory->create_buffer(
+                mcl::protobuf_to_native_buffer(buffer), req.native_format, req.native_flags);
+        }
+        else if ((*request_it)->sw_request.is_set())
+        {
+            auto& req = (*request_it)->sw_request.value();
+            usage = req.usage;
+            client_buffer = factory->create_buffer(
+                mcl::protobuf_to_native_buffer(buffer), (*request_it)->size, req.format);
+        }
+        else
+        {
+            BOOST_THROW_EXCEPTION(std::logic_error("could not create buffer"));
+        }
+
         b = std::make_unique<Buffer>(
-            (*request_it)->cb, (*request_it)->cb_context,
-            buffer.buffer_id(),
-            (*request_it)->native_buffer_factory->create_buffer(
-                mcl::protobuf_to_native_buffer(buffer),
-                (*request_it)->size, (*request_it)->format),
-                (*request_it)->connection, (*request_it)->usage);
+            (*request_it)->cb, (*request_it)->cb_context, buffer.buffer_id(),
+            client_buffer, (*request_it)->connection, usage);
     }
 
     allocation_requests.erase(request_it);

=== modified file 'src/client/buffer_factory.h'
--- src/client/buffer_factory.h	2017-01-24 07:37:03 +0000
+++ src/client/buffer_factory.h	2017-02-03 19:56:51 +0000
@@ -20,6 +20,7 @@
 #define MIR_CLIENT_MIR_BUFFER_FACTORY_H
 
 #include "mir/geometry/size.h"
+#include "mir/optional_value.h"
 #include "mir_protobuf.pb.h"
 #include "buffer.h"
 #include <mutex>
@@ -45,6 +46,14 @@
         MirBufferUsage usage,
         MirBufferCallback cb,
         void* cb_context) = 0;
+    virtual void expect_buffer(
+        std::shared_ptr<ClientBufferFactory> const& native_buffer_factory,
+        MirConnection* connection,
+        geometry::Size size,
+        uint32_t native_format,
+        uint32_t native_flags,
+        MirBufferCallback cb,
+        void* cb_context) = 0;
     virtual void cancel_requests_with_context(void*) = 0;
 
 private:
@@ -64,13 +73,34 @@
         MirBufferUsage usage,
         MirBufferCallback cb,
         void* cb_context) override;
+    void expect_buffer(
+        std::shared_ptr<ClientBufferFactory> const& native_buffer_factory,
+        MirConnection* connection,
+        geometry::Size size,
+        uint32_t native_format,
+        uint32_t native_flags,
+        MirBufferCallback cb,
+        void* cb_context) override;
     void cancel_requests_with_context(void*) override;
 
 private:
     std::mutex mutex;
     int error_id { -1 };
+
     struct AllocationRequest
     {
+        struct NativeRequest
+        {
+            uint32_t native_format;
+            uint32_t native_flags;
+        };
+
+        struct SoftwareRequest
+        {
+            MirPixelFormat format;
+            MirBufferUsage usage;
+        };
+
         AllocationRequest(
             std::shared_ptr<ClientBufferFactory> const& native_buffer_factory,
             MirConnection* connection,
@@ -79,12 +109,20 @@
             MirBufferUsage usage,
             MirBufferCallback cb,
             void* cb_context);
+        AllocationRequest(
+            std::shared_ptr<ClientBufferFactory> const& native_buffer_factory,
+            MirConnection* connection,
+            geometry::Size size,
+            uint32_t native_format,
+            uint32_t native_flags,
+            MirBufferCallback cb,
+            void* cb_context);
 
         std::shared_ptr<ClientBufferFactory> const native_buffer_factory;
         MirConnection* connection;
         geometry::Size size;
-        MirPixelFormat format;
-        MirBufferUsage usage;
+        optional_value<NativeRequest> native_request;
+        optional_value<SoftwareRequest> sw_request;
         MirBufferCallback cb;
         void* cb_context;
     };

=== modified file 'src/client/mir_connection.cpp'
--- src/client/mir_connection.cpp	2017-02-02 19:38:06 +0000
+++ src/client/mir_connection.cpp	2017-02-03 19:56:51 +0000
@@ -1301,7 +1301,7 @@
         client_buffer_factory = platform->create_buffer_factory();
     buffer_factory->expect_buffer(
         client_buffer_factory, this,
-        size, mir_pixel_format_invalid, mir_buffer_usage_hardware,
+        size, native_format, native_flags,
         callback, context);
     server.allocate_buffers(&request, ignored.get(), gp::NewCallback(ignore));
 }

=== modified file 'src/include/client/mir/client_buffer_factory.h'
--- src/include/client/mir/client_buffer_factory.h	2015-02-22 07:46:25 +0000
+++ src/include/client/mir/client_buffer_factory.h	2017-02-03 19:56:51 +0000
@@ -47,6 +47,8 @@
      */
     virtual std::shared_ptr<ClientBuffer> create_buffer(std::shared_ptr<MirBufferPackage> const& package,
                                                         geometry::Size size, MirPixelFormat pf) = 0;
+    virtual std::shared_ptr<ClientBuffer> create_buffer(std::shared_ptr<MirBufferPackage> const& package,
+                                                        unsigned int native_pf, unsigned int native_flags) = 0;
 
 protected:
     ClientBufferFactory() = default;

=== modified file 'src/platforms/android/client/android_client_buffer_factory.cpp'
--- src/platforms/android/client/android_client_buffer_factory.cpp	2015-02-22 07:46:25 +0000
+++ src/platforms/android/client/android_client_buffer_factory.cpp	2017-02-03 19:56:51 +0000
@@ -18,12 +18,14 @@
  */
 
 #include "android_client_buffer_factory.h"
+#include "android_format_conversion-inl.h"
 #include "buffer_registrar.h"
 #include "buffer.h"
 
 namespace mcl=mir::client;
 namespace mcla=mir::client::android;
 namespace geom=mir::geometry;
+namespace mga=mir::graphics::android;
 
 mcla::AndroidClientBufferFactory::AndroidClientBufferFactory(
     std::shared_ptr<BufferRegistrar> const& buffer_registrar) :
@@ -36,3 +38,10 @@
     (void)size; // TODO: remove this unused parameter
     return std::make_shared<mcla::Buffer>(registrar, *package, pf);
 }
+
+std::shared_ptr<mcl::ClientBuffer> mcla::AndroidClientBufferFactory::create_buffer(
+    std::shared_ptr<MirBufferPackage> const& package,
+    unsigned int native_pf, unsigned int)
+{
+    return std::make_shared<mcla::Buffer>(registrar, *package, mga::to_mir_format(native_pf));
+}

=== modified file 'src/platforms/android/client/android_client_buffer_factory.h'
--- src/platforms/android/client/android_client_buffer_factory.h	2015-02-22 07:46:25 +0000
+++ src/platforms/android/client/android_client_buffer_factory.h	2017-02-03 19:56:51 +0000
@@ -45,6 +45,9 @@
         std::shared_ptr<MirBufferPackage> const& package,
         geometry::Size size,
         MirPixelFormat pf) override;
+    std::shared_ptr<ClientBuffer> create_buffer(
+        std::shared_ptr<MirBufferPackage> const& package,
+        unsigned int native_pf, unsigned int native_flags) override;
 private:
     std::shared_ptr<BufferRegistrar> const registrar;
 };

=== modified file 'src/platforms/eglstream-kms/client/client_buffer_factory.cpp'
--- src/platforms/eglstream-kms/client/client_buffer_factory.cpp	2016-05-31 01:21:42 +0000
+++ src/platforms/eglstream-kms/client/client_buffer_factory.cpp	2017-02-03 19:56:51 +0000
@@ -19,6 +19,8 @@
 
 #include "client_buffer_factory.h"
 #include "client_buffer.h"
+#include <boost/throw_exception.hpp>
+#include <stdexcept>
 
 namespace mcl=mir::client;
 namespace mcle=mir::client::eglstream;
@@ -34,3 +36,11 @@
         geometry::Size{package->width, package->height},
         pf);
 }
+
+std::shared_ptr<mcl::ClientBuffer>
+mcle::ClientBufferFactory::create_buffer(
+    std::shared_ptr<MirBufferPackage> const&,
+    unsigned int, unsigned int)
+{
+    BOOST_THROW_EXCEPTION(std::runtime_error("no native buffers on eglstream platform"));
+}

=== modified file 'src/platforms/eglstream-kms/client/client_buffer_factory.h'
--- src/platforms/eglstream-kms/client/client_buffer_factory.h	2016-05-31 01:21:42 +0000
+++ src/platforms/eglstream-kms/client/client_buffer_factory.h	2017-02-03 19:56:51 +0000
@@ -36,7 +36,10 @@
 
     std::shared_ptr<client::ClientBuffer> create_buffer(
         std::shared_ptr<MirBufferPackage> const& package,
-        geometry::Size size, MirPixelFormat pf);
+        geometry::Size size, MirPixelFormat pf) override;
+    std::shared_ptr<ClientBuffer> create_buffer(
+        std::shared_ptr<MirBufferPackage> const& package,
+        unsigned int native_pf, unsigned int native_flags) override;
 };
 
 }

=== modified file 'src/platforms/mesa/client/client_buffer.cpp'
--- src/platforms/mesa/client/client_buffer.cpp	2017-01-18 02:29:37 +0000
+++ src/platforms/mesa/client/client_buffer.cpp	2017-02-03 19:56:51 +0000
@@ -103,6 +103,9 @@
         EGL_DMA_BUF_PLANE0_PITCH_EXT, static_cast<const EGLint>(creation_package->stride),
         EGL_NONE}
 {
+    if (pf == mir_pixel_format_invalid)
+        BOOST_THROW_EXCEPTION(std::invalid_argument("cannot create buffer with mir_pixel_format_invalid"));
+        
     if (package->fd_items != 1)
     {
         BOOST_THROW_EXCEPTION(std::runtime_error(
@@ -110,6 +113,27 @@
     }
 }
 
+mclm::ClientBuffer::ClientBuffer(
+    std::shared_ptr<BufferFileOps> const& buffer_file_ops,
+    std::shared_ptr<MirBufferPackage> const& package,
+    geometry::Size size,
+    unsigned int native_pf, unsigned int /*native_flags*/) :
+    buffer_file_ops{buffer_file_ops},
+    creation_package{to_native_buffer(*package)},
+    rect({geom::Point{0, 0}, size}),
+    buffer_pf{mir::graphics::mesa::gbm_format_to_mir_format(native_pf)},
+    egl_image_attrs{
+        EGL_IMAGE_PRESERVED_KHR, EGL_TRUE,
+        EGL_WIDTH, static_cast<const EGLint>(creation_package->width),
+        EGL_HEIGHT, static_cast<const EGLint>(creation_package->height),
+        EGL_LINUX_DRM_FOURCC_EXT, static_cast<const EGLint>(native_pf),
+        EGL_DMA_BUF_PLANE0_FD_EXT, creation_package->fd[0],
+        EGL_DMA_BUF_PLANE0_OFFSET_EXT, 0,
+        EGL_DMA_BUF_PLANE0_PITCH_EXT, static_cast<const EGLint>(creation_package->stride),
+        EGL_NONE}
+{
+}
+
 mclm::ClientBuffer::~ClientBuffer() noexcept
 {
     // TODO (@raof): Error reporting? It should not be possible for this to fail; if it does,

=== modified file 'src/platforms/mesa/client/client_buffer.h'
--- src/platforms/mesa/client/client_buffer.h	2017-01-18 02:29:37 +0000
+++ src/platforms/mesa/client/client_buffer.h	2017-02-03 19:56:51 +0000
@@ -44,6 +44,10 @@
                  std::shared_ptr<MirBufferPackage> const& buffer_package,
                  geometry::Size size,
                  MirPixelFormat pf);
+    ClientBuffer(std::shared_ptr<BufferFileOps> const& buffer_file_ops,
+                 std::shared_ptr<MirBufferPackage> const& buffer_package,
+                 geometry::Size size,
+                 unsigned int native_pf, unsigned int native_flags);
 
     ~ClientBuffer() noexcept;
 

=== modified file 'src/platforms/mesa/client/client_buffer_factory.cpp'
--- src/platforms/mesa/client/client_buffer_factory.cpp	2014-03-06 06:05:17 +0000
+++ src/platforms/mesa/client/client_buffer_factory.cpp	2017-02-03 19:56:51 +0000
@@ -42,3 +42,15 @@
         geometry::Size{package->width, package->height},
         pf);
 }
+
+std::shared_ptr<mcl::ClientBuffer>
+mclm::ClientBufferFactory::create_buffer(
+    std::shared_ptr<MirBufferPackage> const& package,
+    unsigned int native_pf, unsigned int native_flags)
+{
+    return std::make_shared<mclm::ClientBuffer>(
+        buffer_file_ops,
+        package,
+        geometry::Size{package->width, package->height},
+        native_pf, native_flags);
+}

=== modified file 'src/platforms/mesa/client/client_buffer_factory.h'
--- src/platforms/mesa/client/client_buffer_factory.h	2015-02-22 07:46:25 +0000
+++ src/platforms/mesa/client/client_buffer_factory.h	2017-02-03 19:56:51 +0000
@@ -39,6 +39,9 @@
     std::shared_ptr<client::ClientBuffer> create_buffer(
         std::shared_ptr<MirBufferPackage> const& package,
         geometry::Size size, MirPixelFormat pf);
+    std::shared_ptr<client::ClientBuffer> create_buffer(
+        std::shared_ptr<MirBufferPackage> const& package,
+        unsigned int native_pf, unsigned int native_flags);
 private:
     std::shared_ptr<BufferFileOps> const buffer_file_ops;
 };

=== modified file 'tests/include/mir/test/doubles/mock_client_buffer_factory.h'
--- tests/include/mir/test/doubles/mock_client_buffer_factory.h	2015-06-17 05:20:42 +0000
+++ tests/include/mir/test/doubles/mock_client_buffer_factory.h	2017-02-03 19:56:51 +0000
@@ -36,6 +36,8 @@
                  std::shared_ptr<client::ClientBuffer>(
                     std::shared_ptr<MirBufferPackage> const& /*package*/,
                     mir::geometry::Size /*size*/, MirPixelFormat /*pf*/));
+    MOCK_METHOD3(create_buffer, std::shared_ptr<client::ClientBuffer>(
+        std::shared_ptr<MirBufferPackage> const&, uint32_t, uint32_t));
 };
 }
 }

=== modified file 'tests/include/mir/test/doubles/stub_client_buffer_factory.h'
--- tests/include/mir/test/doubles/stub_client_buffer_factory.h	2016-08-09 13:02:44 +0000
+++ tests/include/mir/test/doubles/stub_client_buffer_factory.h	2017-02-03 19:56:51 +0000
@@ -37,6 +37,13 @@
     {
         return std::make_shared<StubClientBuffer>(package, size, pf, nullptr);
     }
+
+    std::shared_ptr<client::ClientBuffer> create_buffer(
+        std::shared_ptr<MirBufferPackage> const& package, uint32_t, uint32_t)
+    {
+        return std::make_shared<StubClientBuffer>(
+            package, geometry::Size{package->width, package->height}, mir_pixel_format_abgr_8888, nullptr);
+    }
 };
 
 }

=== modified file 'tests/mir_test_framework/stub_client_platform_factory.cpp'
--- tests/mir_test_framework/stub_client_platform_factory.cpp	2017-02-02 19:38:06 +0000
+++ tests/mir_test_framework/stub_client_platform_factory.cpp	2017-02-03 19:56:51 +0000
@@ -138,6 +138,17 @@
             return std::make_shared<mtd::StubClientBuffer>(package, size, pf,
                 std::make_shared<mtf::NativeBuffer>(properties));
         }
+
+        std::shared_ptr<mcl::ClientBuffer> create_buffer(
+            std::shared_ptr<MirBufferPackage> const& package, uint32_t, uint32_t) override
+        {
+            mir::graphics::BufferUsage usage = mir::graphics::BufferUsage::hardware;
+            geom::Size size {package->width, package->height};
+            auto pf = mir_pixel_format_abgr_8888;
+            mir::graphics::BufferProperties properties { size, pf, usage }; 
+            return std::make_shared<mtd::StubClientBuffer>(package, size, pf,
+                std::make_shared<mtf::NativeBuffer>(properties));
+        }
     };
     return std::make_shared<StubPlatformBufferFactory>();
 }

=== modified file 'tests/unit-tests/client/test_buffer_vault.cpp'
--- tests/unit-tests/client/test_buffer_vault.cpp	2017-01-18 02:29:37 +0000
+++ tests/unit-tests/client/test_buffer_vault.cpp	2017-02-03 19:56:51 +0000
@@ -49,7 +49,7 @@
 {
     MockClientBufferFactory()
     {
-        ON_CALL(*this, create_buffer(_,_,_))
+        ON_CALL(*this, create_buffer(_,An<geom::Size>(),_))
             .WillByDefault(Invoke([](
                     std::shared_ptr<MirBufferPackage> const&, geom::Size size, MirPixelFormat)
                 {
@@ -61,6 +61,8 @@
     }
     MOCK_METHOD3(create_buffer, std::shared_ptr<mcl::ClientBuffer>(
         std::shared_ptr<MirBufferPackage> const&, geom::Size, MirPixelFormat));
+    MOCK_METHOD3(create_buffer, std::shared_ptr<mcl::ClientBuffer>(
+        std::shared_ptr<MirBufferPackage> const&, uint32_t, uint32_t));
 };
 
 struct MockServerRequests : mcl::ServerBufferRequests
@@ -272,7 +274,7 @@
     ON_CALL(*mock_buffer, size())
         .WillByDefault(Return(size));
     EXPECT_CALL(*mock_buffer, increment_age());
-    ON_CALL(mock_platform_factory, create_buffer(_,_,_))
+    ON_CALL(mock_platform_factory, create_buffer(_,An<uint32_t>(),_))
         .WillByDefault(Return(mock_buffer));
     auto b1 = std::make_shared<mcl::Buffer>(
         ignore, nullptr, package.buffer_id(), mock_buffer, nullptr, mir_buffer_usage_software);
@@ -289,7 +291,7 @@
     ON_CALL(*mock_buffer, size())
         .WillByDefault(Return(size));
     EXPECT_CALL(*mock_buffer, mark_as_submitted());
-    ON_CALL(mock_platform_factory, create_buffer(_,_,_))
+    ON_CALL(mock_platform_factory, create_buffer(_,An<uint32_t>(),_))
         .WillByDefault(Return(mock_buffer));
     auto b1 = std::make_shared<mcl::Buffer>(
         ignore, nullptr, package.buffer_id(), mock_buffer, nullptr, mir_buffer_usage_software);

=== modified file 'tests/unit-tests/client/test_client_buffer_stream.cpp'
--- tests/unit-tests/client/test_client_buffer_stream.cpp	2017-01-25 15:34:05 +0000
+++ tests/unit-tests/client/test_client_buffer_stream.cpp	2017-02-03 19:56:51 +0000
@@ -167,7 +167,7 @@
 {
     ClientBufferStream()
     {
-        ON_CALL(mock_factory, create_buffer(_,_,_))
+        ON_CALL(mock_factory, create_buffer(_,An<geom::Size>(),_))
             .WillByDefault(Return(std::make_shared<mtd::NullClientBuffer>()));
     }
 
@@ -305,7 +305,7 @@
 
 TEST_F(ClientBufferStream, uses_buffer_message_from_server)
 {
-    EXPECT_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package),_,_))
+    EXPECT_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package),An<geom::Size>(),_))
         .WillOnce(Return(std::make_shared<mtd::NullClientBuffer>()));
     mcl::BufferStream bs(
         nullptr, nullptr, wait_handle, mock_protobuf_server,
@@ -382,10 +382,10 @@
     auto protobuf_bs = a_protobuf_buffer_stream(default_pixel_format, default_buffer_usage);
     
     Sequence seq;
-    EXPECT_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package_1),_,_))
+    EXPECT_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package_1),An<geom::Size>(),_))
         .InSequence(seq)
         .WillOnce(Return(client_buffer_1));
-    EXPECT_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package_2),_,_))
+    EXPECT_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package_2),An<geom::Size>(),_))
         .InSequence(seq)
         .WillOnce(Return(client_buffer_2));
 
@@ -448,7 +448,7 @@
     NiceMock<mtd::MockClientBuffer> mock_client_buffer;
     ON_CALL(mock_client_buffer, size())
         .WillByDefault(Return(size));
-    EXPECT_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package),_,_))
+    EXPECT_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package),An<geom::Size>(),_))
         .WillOnce(Return(mt::fake_shared(mock_client_buffer)));
 
     mcl::BufferStream bs(
@@ -470,7 +470,7 @@
     NiceMock<mtd::MockClientBuffer> mock_client_buffer;
     ON_CALL(mock_client_buffer, size())
         .WillByDefault(Return(size));
-    ON_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package),_,_))
+    ON_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package),An<geom::Size>(),_))
         .WillByDefault(Return(mt::fake_shared(mock_client_buffer)));
     mcl::BufferStream bs(
         nullptr, nullptr, wait_handle, mock_protobuf_server,
@@ -568,7 +568,7 @@
     NiceMock<mtd::MockClientBuffer> second_mock_client_buffer;
     ON_CALL(second_mock_client_buffer, size())
         .WillByDefault(Return(size));
-    EXPECT_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package),_,_))
+    EXPECT_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package),An<geom::Size>(),_))
         .WillOnce(Return(mt::fake_shared(mock_client_buffer)));
 
     mcl::BufferStream bs(
@@ -582,7 +582,7 @@
     another_buffer_package.set_buffer_id(id);
     another_buffer_package.set_width(size.width.as_int());
     another_buffer_package.set_height(size.height.as_int());
-    EXPECT_CALL(mock_factory, create_buffer(_,_,_))
+    EXPECT_CALL(mock_factory, create_buffer(_,An<geom::Size>(),_))
         .WillOnce(Return(mt::fake_shared(second_mock_client_buffer)));
     EXPECT_CALL(mock_protobuf_server, submit_buffer(_,_,_))
         .WillOnce(mtd::RunProtobufClosure());
@@ -599,7 +599,7 @@
     NiceMock<mtd::MockClientBuffer> mock_client_buffer;
     ON_CALL(mock_client_buffer, size())
         .WillByDefault(Return(size));
-    ON_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package),_,_))
+    ON_CALL(mock_factory, create_buffer(BufferPackageMatches(buffer_package),An<geom::Size>(),_))
         .WillByDefault(Return(mt::fake_shared(mock_client_buffer)));
     ON_CALL(mock_protobuf_server, submit_buffer(_,_,_))
         .WillByDefault(mtd::RunProtobufClosure());
@@ -741,7 +741,7 @@
 {
     auto format = mir_pixel_format_bgr_888;
     response.set_pixel_format(format);
-    EXPECT_CALL(mock_factory, create_buffer(_,_,format))
+    EXPECT_CALL(mock_factory, create_buffer(_,_,TypedEq<MirPixelFormat>(format)))
         .WillRepeatedly(Return(std::make_shared<mtd::NullClientBuffer>()));
     mcl::BufferStream bs(
         nullptr, nullptr, wait_handle, mock_protobuf_server,
@@ -755,7 +755,7 @@
 
 TEST_F(ClientBufferStream, keeps_accurate_buffer_id)
 {
-    ON_CALL(mock_factory, create_buffer(_,_,_))
+    ON_CALL(mock_factory, create_buffer(_,An<geom::Size>(),_))
         .WillByDefault(Return(std::make_shared<mtd::NullClientBuffer>(size)));
     mcl::BufferStream stream(
         nullptr, nullptr, wait_handle, mock_protobuf_server,

=== modified file 'tests/unit-tests/client/test_mir_connection.cpp'
--- tests/unit-tests/client/test_mir_connection.cpp	2017-02-02 19:38:06 +0000
+++ tests/unit-tests/client/test_mir_connection.cpp	2017-02-03 19:56:51 +0000
@@ -93,6 +93,12 @@
         MirBufferUsage usage,
         MirBufferCallback cb,
         void* cb_context));
+    MOCK_METHOD7(expect_buffer, void(
+        std::shared_ptr<mcl::ClientBufferFactory> const& native_buffer_factory,
+        MirConnection* connection,
+        geom::Size size, uint32_t, uint32_t,
+        MirBufferCallback cb,
+        void* cb_context));
 };
 
 struct MockRpcChannel : public mir::client::rpc::MirBasicRpcChannel,
@@ -849,7 +855,7 @@
     params->set_buffer_usage(usage);
     params->set_pixel_format(format);
     EXPECT_CALL(*mock_channel, allocate_buffers(BufferAllocationMatches(mp_alloc)));
-    EXPECT_CALL(*mock_buffer_allocator, expect_buffer(_, connection.get(), size, format, usage, nullptr, nullptr));
+    EXPECT_CALL(*mock_buffer_allocator, expect_buffer(_, connection.get(), size, TypedEq<MirPixelFormat>(format), usage, nullptr, nullptr));
 
     connection->allocate_buffer(size, format, nullptr, nullptr);
 }
@@ -869,7 +875,7 @@
     params->set_native_format(native_format);
     params->set_flags(native_flags);
     EXPECT_CALL(*mock_channel, allocate_buffers(BufferAllocationMatches(mp_alloc)));
-    EXPECT_CALL(*mock_buffer_allocator, expect_buffer(_, connection.get(), size, _, mir_buffer_usage_hardware, nullptr, nullptr));
+    EXPECT_CALL(*mock_buffer_allocator, expect_buffer(_, connection.get(), size, TypedEq<uint32_t>(native_format), TypedEq<uint32_t>(native_flags), nullptr, nullptr));
 
     connection->allocate_buffer(size, native_format, native_flags, nullptr, nullptr);
 }

=== modified file 'tests/unit-tests/client/test_protobuf_rpc_channel.cpp'
--- tests/unit-tests/client/test_protobuf_rpc_channel.cpp	2017-01-24 07:55:14 +0000
+++ tests/unit-tests/client/test_protobuf_rpc_channel.cpp	2017-02-03 19:56:51 +0000
@@ -688,6 +688,10 @@
         MirConnection*,
         mir::geometry::Size, MirPixelFormat, MirBufferUsage,
         MirBufferCallback, void*));
+    MOCK_METHOD7(expect_buffer, void(
+        std::shared_ptr<mcl::ClientBufferFactory> const&,
+        MirConnection*, mir::geometry::Size, uint32_t, uint32_t,
+        MirBufferCallback, void*));
 };
 
 namespace

=== modified file 'tests/unit-tests/client/test_screencast_stream.cpp'
--- tests/unit-tests/client/test_screencast_stream.cpp	2017-01-18 02:29:37 +0000
+++ tests/unit-tests/client/test_screencast_stream.cpp	2017-02-03 19:56:51 +0000
@@ -75,7 +75,7 @@
 {
     ScreencastStream()
     {
-        ON_CALL(mock_factory, create_buffer(_,_,_))
+        ON_CALL(mock_factory, create_buffer(_,An<geom::Size>(),_))
             .WillByDefault(Return(std::make_shared<mtd::NullClientBuffer>()));
     }
 
@@ -170,6 +170,12 @@
                 return nullptr;
         }
 
+        std::shared_ptr<mcl::ClientBuffer> create_buffer(
+            std::shared_ptr<MirBufferPackage> const&, uint32_t, uint32_t)
+        {
+            return create_buffer(nullptr, geom::Size{0,0}, mir_pixel_format_invalid);
+        }
+
         void start_failing()
         {
             fail = true;

=== modified file 'tests/unit-tests/platforms/mesa/client/test_client_buffer.cpp'
--- tests/unit-tests/platforms/mesa/client/test_client_buffer.cpp	2017-01-18 02:29:37 +0000
+++ tests/unit-tests/platforms/mesa/client/test_client_buffer.cpp	2017-02-03 19:56:51 +0000
@@ -217,3 +217,38 @@
     EXPECT_THAT(egl_buffer, Eq(nullptr));
     EXPECT_THAT(attrs, mir::test::doubles::AttrMatches(expected_image_attrs));
 }
+
+//LP: #1661521
+TEST_F(MesaClientBufferTest, suggests_dma_import_with_native_format)
+{
+    auto format = GBM_FORMAT_ABGR8888;
+    static EGLint expected_image_attrs[] =
+    {
+        EGL_IMAGE_PRESERVED_KHR, EGL_TRUE,
+        EGL_WIDTH, static_cast<const EGLint>(package->width),
+        EGL_HEIGHT, static_cast<const EGLint>(package->height),
+        EGL_LINUX_DRM_FOURCC_EXT, static_cast<const EGLint>(format),
+        EGL_DMA_BUF_PLANE0_FD_EXT, package->fd[0],
+        EGL_DMA_BUF_PLANE0_OFFSET_EXT, 0,
+        EGL_DMA_BUF_PLANE0_PITCH_EXT, static_cast<const EGLint>(package->stride),
+        EGL_NONE
+    };
+
+    EGLenum type;
+    EGLClientBuffer egl_buffer;
+    EGLint* attrs;
+    
+    mclg::ClientBuffer buffer(buffer_file_ops, package, size, format, 0);
+    buffer.egl_image_creation_parameters(&type, &egl_buffer, &attrs);
+
+    EXPECT_THAT(type, Eq(static_cast<EGLenum>(EGL_LINUX_DMA_BUF_EXT)));
+    EXPECT_THAT(egl_buffer, Eq(nullptr));
+    EXPECT_THAT(attrs, mir::test::doubles::AttrMatches(expected_image_attrs));
+}
+
+TEST_F(MesaClientBufferTest, invalid_pixel_format_throws)
+{
+    EXPECT_THROW({ 
+        mclg::ClientBuffer buffer(buffer_file_ops, package, size, mir_pixel_format_invalid);
+    }, std::invalid_argument);
+}