Mir

Merge lp:~kdub/mir/mesa-reconstruct-buffer into lp:mir

mesa-reconstruct-buffer
Merge into development-branch

Proposed by Kevin DuBois on 2015-04-09

Status:	Merged
Approved by:	Alexandros Frantzis on 2015-04-15
Approved revision:	2478
Merged at revision:	2487
Proposed branch:	lp:~kdub/mir/mesa-reconstruct-buffer
Merge into:	lp:mir
Diff against target:	164 lines (+105/-0) 5 files modified src/platforms/mesa/server/buffer_allocator.cpp (+31/-0) src/platforms/mesa/server/buffer_allocator.h (+2/-0) tests/include/mir_test_doubles/mock_gbm.h (+1/-0) tests/mir_test_doubles/mock_gbm.cpp (+5/-0) tests/unit-tests/graphics/mesa/test_buffer_allocator.cpp (+66/-0)
To merge this branch:	bzr merge lp:~kdub/mir/mesa-reconstruct-buffer
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Alexandros Frantzis (community)			Approve on 2015-04-15
Alan Griffiths			Approve on 2015-04-15
PS Jenkins bot	continuous-integration		Approve on 2015-04-14
Robert Carr (community)			Approve on 2015-04-13
Chris Halse Rogers		2015-04-09	Approve on 2015-04-13
Review via email: mp+255722@code.launchpad.net

Commit Message

mesa: provide a way for a nested server to reconstruct a mgm::Buffer from a MirNativeBuffer.

Description of the Change

mesa: provide a way for a nested server to reconstruct a mgm::Buffer from a MirNativeBuffer.

The nested server gets access to the native buffer type from mir_buffer_stream_current_buffer(), and the type that is returned is platform-dependendent, so allow each platform to reconstruct the mg::Buffer.

PS Jenkins bot (ps-jenkins) wrote on 2015-04-09:

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

review: Needs Fixing (continuous-integration)

Chris Halse Rogers (raof) wrote on 2015-04-10:

27 + BOOST_THROW_EXCEPTION(std::runtime_error("Failed to import MirBufferPackage"));

This would more usefully be a std::system_error; gbm_bo_import will set errno in case of failure.

Otherwise looks good to me.

review: Needs Fixing

lp:~kdub/mir/mesa-reconstruct-buffer updated on 2015-04-10

2472. By Kevin DuBois on 2015-04-10: use system error instead of runtime_error

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

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

review: Needs Fixing (continuous-integration)

lp:~kdub/mir/mesa-reconstruct-buffer updated on 2015-04-10

2473. By Kevin DuBois on 2015-04-10: correct clang error

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

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

review: Needs Fixing (continuous-integration)

Chris Halse Rogers (raof) wrote on 2015-04-13:

LGTM

review: Approve

Alan Griffiths (alan-griffiths) wrote on 2015-04-13:

*nitpicking*

31+ std::shared_ptr<gbm_bo> bo(
32+ gbm_bo_import(device, GBM_BO_IMPORT_FD, &data, package->flags),
33+ [](gbm_bo* bo){ gbm_bo_destroy(bo); });
34+ if (!bo)
35+ BOOST_THROW_EXCEPTION(
36+ std::system_error(errno, std::system_category(), "Failed to import MirBufferPackage"));

Easier to read with a blank line before the if, and braces around the multi-line conditional.

review: Needs Fixing

lp:~kdub/mir/mesa-reconstruct-buffer updated on 2015-04-13

2474. By Kevin DuBois on 2015-04-13: braces and spaces
2475. By Kevin DuBois on 2015-04-13: merge mir

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

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

review: Needs Fixing (continuous-integration)

Robert Carr (robertcarr) wrote on 2015-04-13:

LGTM.

Nits:
l22
+ BOOST_THROW_EXCEPTION(std::runtime_error("Failed to create mgm::Buffer from MirBufferPackage"));

I think because the caller can verify this, we can say its a precondition violation and this is thus a logic_error.

review: Approve

Kevin DuBois (kdub) wrote on 2015-04-13:

@jenkins, I thought this was fixed?
11: [ RUN ] SimpleDispatchThreadTest.keeps_dispatching_after_signal_interruption

lp:~kdub/mir/mesa-reconstruct-buffer updated on 2015-04-13

2476. By Kevin DuBois on 2015-04-13: use logic_error instead of runtime_error

Kevin DuBois (kdub) wrote on 2015-04-13:

@Robert, fair enough, changed to logic_error

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

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

review: Needs Fixing (continuous-integration)

Alexandros Frantzis (afrantzis) wrote on 2015-04-14:

Download full text (3.2 KiB)

> @jenkins, I thought this was fixed?
> 11: [ RUN ] SimpleDispatchThreadTest.keeps_dispatching_after_signal_interruption

Because this test forks a new process, it catches valgrind errors detected in previous tests. In this case the real issue is:

11: [ RUN ] MesaBufferAllocatorTest.reconstructs_from_native_type
11: ==32764== Conditional jump or move depends on uninitialised value(s)
11: ==32764== at 0x15CFAC8: Matches (gmock-spec-builders.h:1088)
11: ==32764== by 0x15CFAC8: ShouldHandleArguments (gmock-spec-builders.h:1101)
11: ==32764== by 0x15CFAC8: FindMatchingExpectationLocked (gmock-spec-builders.h:1667)
11: ==32764== by 0x15CFAC8: testing::internal::FunctionMockerBase<gbm_bo* (gbm_device*, unsigned int, void*, unsigned int)>::UntypedFindMatchingExpectation(void const*, void const**, bool*, std::ostream*, std::ostream*) (gmock-spec-builders.h:1631)
11: ==32764== by 0x161E937: testing::internal::UntypedFunctionMockerBase::UntypedInvokeWith(void const*) (in /tmp/buildd/mir-0.13.0bzr2476pkg0vivid1509/obj-x86_64-linux-gnu/bin/.mir_unit_tests-uninstalled)
11: ==32764== by 0x15B8142: InvokeWith (gmock-spec-builders.h:1529)
11: ==32764== by 0x15B8142: Invoke (gmock-generated-function-mockers.h:162)
11: ==32764== by 0x15B8142: gbm_bo_import (mock_gbm.h:79)
11: ==32764== by 0x15B8142: gbm_bo_import (mock_gbm.cpp:174)
11: ==32764== by 0x13BA657: mir::graphics::mesa::BufferAllocator::reconstruct_from(MirBufferPackage*, MirPixelFormat) (buffer_allocator.cpp:249)
11: ==32764== by 0x10A7CCF: MesaBufferAllocatorTest_reconstructs_from_native_type_Test::TestBody() (test_buffer_allocator.cpp:321)
11: ==32764== by 0x160FA49: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (in /tmp/buildd/mir-0.13.0bzr2476pkg0vivid1509/obj-x86_64-linux-gnu/bin/.mir_unit_tests-uninstalled)
11: ==32764== by 0x160A71E: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (in /tmp/buildd/mir-0.13.0bzr2476pkg0vivid1509/obj-x86_64-linux-gnu/bin/.mir_unit_tests-uninstalled)
11: ==32764== by 0x15F038A: testing::Test::Run() (in /tmp/buildd/mir-0.13.0bzr2476pkg0vivid1509/obj-x86_64-linux-gnu/bin/.mir_unit_tests-uninstalled)
11: ==32764== by 0x15F0C28: testing::TestInfo::Run() (in /tmp/buildd/mir-0.13.0bzr2476pkg0vivid1509/obj-x86_64-linux-gnu/bin/.mir_unit_tests-uninstalled)
11: ==32764== by 0x15F1331: testing::TestCase::Run() (in /tmp/buildd/mir-0.13.0bzr2476pkg0vivid1509/obj-x86_64-linux-gnu/bin/.mir_unit_tests-uninstalled)
11: ==32764== by 0x15F8203: testing::internal::UnitTestImpl::RunAllTests() (in /tmp/buildd/mir-0.13.0bzr2476pkg0vivid1509/obj-x86_64-linux-gnu/bin/.mir_unit_tests-uninstalled)
11: ==32764== by 0x1611014: bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (in /tmp/buildd/mir-0.13.0bzr2476pkg0vivid1509/obj-x86_64-linux-gnu/bin/.mir_unit_tests-uninstalled)
11: ==32764==
11: [ OK ] Mesa...

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:

Emanuele Antonio Faraone

Gerry Boland

Kevin DuBois

Mir development team

 === modified file 'src/platforms/mesa/server/buffer_allocator.cpp'
 --- src/platforms/mesa/server/buffer_allocator.cpp	2015-04-13 14:07:18 +0000
 +++ src/platforms/mesa/server/buffer_allocator.cpp	2015-04-14 12:24:42 +0000
@@ -33,6 +33,7 @@
  #include <algorithm>
  #include <stdexcept>
++#include <system_error>
  #include <gbm.h>
  #include <cassert>
@@ -227,3 +228,33 @@
      return iter != formats.end();
+ }
++
++std::unique_ptr<mg::Buffer> mgm::BufferAllocator::reconstruct_from(
++    MirBufferPackage* package,
++    MirPixelFormat format)
++{
++    if (package->fd_items != 1)
++        BOOST_THROW_EXCEPTION(std::logic_error("Failed to create mgm::Buffer from invalid MirBufferPackage"));
++
++    gbm_import_fd_data data;
++    data.fd = package->fd[0];
++    data.width  = package->width;
++    data.height = package->height;
++    data.stride = package->stride;
++    data.format = format;
++
++    std::shared_ptr<gbm_bo> bo(
++        gbm_bo_import(device, GBM_BO_IMPORT_FD, &data, package->flags),
++        [](gbm_bo* bo){ gbm_bo_destroy(bo); });
++
++    if (!bo)
++    {
++        BOOST_THROW_EXCEPTION(
++            std::system_error(errno, std::system_category(), "Failed to import MirBufferPackage"));
++    }
++
++    return std::make_unique<mgm::GBMBuffer>(
++        bo,
++        package->flags,
++        std::make_unique<EGLImageBufferTextureBinder>(bo, egl_extensions));
++}
 === modified file 'src/platforms/mesa/server/buffer_allocator.h'
 --- src/platforms/mesa/server/buffer_allocator.h	2015-01-22 09:00:14 +0000
 +++ src/platforms/mesa/server/buffer_allocator.h	2015-04-14 12:24:42 +0000
@@ -46,6 +46,8 @@
      virtual std::shared_ptr<Buffer> alloc_buffer(
          graphics::BufferProperties const& buffer_properties);
++    std::unique_ptr<Buffer> reconstruct_from(MirBufferPackage* package, MirPixelFormat format);
++
      std::vector<MirPixelFormat> supported_pixel_formats();
  private:
 === modified file 'tests/include/mir_test_doubles/mock_gbm.h'
 --- tests/include/mir_test_doubles/mock_gbm.h	2013-08-28 03:41:48 +0000
 +++ tests/include/mir_test_doubles/mock_gbm.h	2015-04-14 12:24:42 +0000
@@ -76,6 +76,7 @@
      MOCK_METHOD1(gbm_bo_get_user_data, void*(struct gbm_bo *bo));
      MOCK_METHOD3(gbm_bo_write, bool(struct gbm_bo *bo, const void *buf, size_t count));
      MOCK_METHOD1(gbm_bo_destroy, void(struct gbm_bo *bo));
++    MOCK_METHOD4(gbm_bo_import, struct gbm_bo*(struct gbm_device*, uint32_t, void*, uint32_t));
      FakeGBMResources fake_gbm;
 === modified file 'tests/mir_test_doubles/mock_gbm.cpp'
 --- tests/mir_test_doubles/mock_gbm.cpp	2013-08-28 03:41:48 +0000
 +++ tests/mir_test_doubles/mock_gbm.cpp	2015-04-14 12:24:42 +0000
@@ -168,3 +168,8 @@
+ {
      return global_mock->gbm_bo_destroy(bo);
+ }
++
++struct gbm_bo* gbm_bo_import(struct gbm_device* device, uint32_t type, void* data, uint32_t flags)
++{
++    return global_mock->gbm_bo_import(device, type, data, flags);
++}
 === modified file 'tests/unit-tests/graphics/mesa/test_buffer_allocator.cpp'
 --- tests/unit-tests/graphics/mesa/test_buffer_allocator.cpp	2015-03-31 02:35:42 +0000
 +++ tests/unit-tests/graphics/mesa/test_buffer_allocator.cpp	2015-04-14 12:24:42 +0000
@@ -31,6 +31,7 @@
  #include <cstdlib>
  #include <memory>
  #include <stdexcept>
++#include <system_error>
  #include <algorithm>
  #include <gtest/gtest.h>
  #include <gmock/gmock.h>
@@ -281,3 +282,68 @@
          allocator->alloc_buffer(mg::BufferProperties{size, mir_pixel_format_abgr_8888, usage});
      }, std::runtime_error);
+ }
++
++MATCHER_P(GbmImportMatch, value, "import data matches")
++{
++    using namespace testing;
++    auto data = reinterpret_cast<struct gbm_import_fd_data*>(arg);
++    EXPECT_THAT(data->fd, Eq(value.fd));
++    EXPECT_THAT(data->width, Eq(value.width));
++    EXPECT_THAT(data->height, Eq(value.height));
++    EXPECT_THAT(data->stride, Eq(value.stride));
++    EXPECT_THAT(data->format, Eq(value.format));
++    return !(::testing::Test::HasFailure());
++}
++
++TEST_F(MesaBufferAllocatorTest, reconstructs_from_native_type)
++{
++    using namespace testing;
++    MirNativeBuffer native_buffer;
++    uint32_t stride {22};
++    native_buffer.fd_items = 1;
++    native_buffer.fd[0] = 33;
++    native_buffer.width = size.width.as_uint32_t();
++    native_buffer.height = size.height.as_uint32_t();
++    native_buffer.stride = stride;
++    native_buffer.flags = GBM_BO_USE_RENDERING;
++
++    struct gbm_import_fd_data expected_data {
++        native_buffer.fd[0],
++        size.width.as_uint32_t(),
++        size.height.as_uint32_t(),
++        stride,
++        pf
++    };
++
++    EXPECT_CALL(mock_gbm, gbm_bo_import(_, GBM_BO_IMPORT_FD, GbmImportMatch(expected_data), native_buffer.flags ))
++        .WillOnce(Return(mock_gbm.fake_gbm.bo));
++    EXPECT_CALL(mock_gbm, gbm_bo_destroy(mock_gbm.fake_gbm.bo));
++
++    auto buffer = allocator->reconstruct_from(&native_buffer, pf);
++    ASSERT_THAT(buffer, Ne(nullptr));
++}
++
++TEST_F(MesaBufferAllocatorTest, reconstruct_throws_with_invalid_native_type)
++{
++    MirNativeBuffer native_buffer;
++    native_buffer.fd_items = 0;
++    EXPECT_THROW({
++        auto buffer = allocator->reconstruct_from(&native_buffer, pf);
++    }, std::logic_error);
++    native_buffer.fd_items = 2;
++    EXPECT_THROW({
++        auto buffer = allocator->reconstruct_from(&native_buffer, pf);
++    }, std::logic_error);
++}
++
++TEST_F(MesaBufferAllocatorTest, reconstruct_throws_if_gbm_cannot_import)
++{
++    using namespace testing;
++    EXPECT_CALL(mock_gbm, gbm_bo_import(_, _, _, _))
++        .WillOnce(Return(nullptr));
++    MirNativeBuffer native_buffer;
++    native_buffer.fd_items = 1;
++    EXPECT_THROW({
++        auto buffer = allocator->reconstruct_from(&native_buffer, pf);
++    }, std::system_error);
++}