Mir

Merge lp:~afrantzis/mir/gbm-alloc-validate-buffer-format into lp:~mir-team/mir/trunk

  1. gbm-alloc-validate-buffer-format
  2. Merge into trunk
Proposed by Alexandros Frantzis
Status: Merged
Approved by: Alan Griffiths
Approved revision: no longer in the source branch.
Merged at revision: 946
Proposed branch: lp:~afrantzis/mir/gbm-alloc-validate-buffer-format
Merge into: lp:~mir-team/mir/trunk
Diff against target: 141 lines (+37/-14)
5 files modified
src/server/graphics/gbm/gbm_buffer.cpp (+1/-3)
src/server/graphics/gbm/gbm_buffer.h (+2/-0)
src/server/graphics/gbm/gbm_buffer_allocator.cpp (+20/-1)
src/server/graphics/gbm/gbm_buffer_allocator.h (+2/-0)
tests/unit-tests/graphics/gbm/test_gbm_buffer_allocator.cpp (+12/-10)
To merge this branch: bzr merge lp:~afrantzis/mir/gbm-alloc-validate-buffer-format
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alan Griffiths Approve
Review via email: mp+178928@code.launchpad.net

Commit message

gbm: Don't try to allocate buffers with unsupported formats

Description of the change

gbm: Don't try to allocate buffers with unsupported formats

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:936
http://jenkins.qa.ubuntu.com/job/mir-ci/1241/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1639
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1524
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/479
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/479/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

17 +uint32_t mgg::invalid_gbm_format()
18 +{
19 + /* There is no explicit invalid GBM pixel format! */
20 + return std::numeric_limits<uint32_t>::max();
21 +}

Is there any need for this to be a function (as opposed to an enum constant)?

review: Approve
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

> Is there any need for this to be a function (as opposed to an enum constant)?

Fixed.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:937
http://jenkins.qa.ubuntu.com/job/mir-ci/1244/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1643
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1528
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/482
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/482/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/server/graphics/gbm/gbm_buffer.cpp'
--- src/server/graphics/gbm/gbm_buffer.cpp 2013-07-22 02:18:21 +0000
+++ src/server/graphics/gbm/gbm_buffer.cpp 2013-08-07 11:53:28 +0000
@@ -27,7 +27,6 @@
27#include <boost/exception/errinfo_errno.hpp>27#include <boost/exception/errinfo_errno.hpp>
28#include <boost/throw_exception.hpp>28#include <boost/throw_exception.hpp>
2929
30#include <limits>
31#include <stdexcept>30#include <stdexcept>
3231
33namespace mgg=mir::graphics::gbm;32namespace mgg=mir::graphics::gbm;
@@ -68,8 +67,7 @@
68 gbm_pf = GBM_FORMAT_XRGB8888;67 gbm_pf = GBM_FORMAT_XRGB8888;
69 break;68 break;
70 default:69 default:
71 /* There is no explicit invalid GBM pixel format! */70 gbm_pf = mgg::invalid_gbm_format;
72 gbm_pf = std::numeric_limits<uint32_t>::max();
73 break;71 break;
74 }72 }
7573
7674
=== modified file 'src/server/graphics/gbm/gbm_buffer.h'
--- src/server/graphics/gbm/gbm_buffer.h 2013-07-22 02:18:21 +0000
+++ src/server/graphics/gbm/gbm_buffer.h 2013-08-07 11:53:28 +0000
@@ -25,6 +25,7 @@
25#include <gbm.h>25#include <gbm.h>
2626
27#include <memory>27#include <memory>
28#include <limits>
2829
29namespace mir30namespace mir
30{31{
@@ -35,6 +36,7 @@
3536
36geometry::PixelFormat gbm_format_to_mir_format(uint32_t format);37geometry::PixelFormat gbm_format_to_mir_format(uint32_t format);
37uint32_t mir_format_to_gbm_format(geometry::PixelFormat format);38uint32_t mir_format_to_gbm_format(geometry::PixelFormat format);
39enum : uint32_t { invalid_gbm_format = std::numeric_limits<uint32_t>::max() };
3840
39class BufferTextureBinder;41class BufferTextureBinder;
4042
4143
=== modified file 'src/server/graphics/gbm/gbm_buffer_allocator.cpp'
--- src/server/graphics/gbm/gbm_buffer_allocator.cpp 2013-07-31 14:01:17 +0000
+++ src/server/graphics/gbm/gbm_buffer_allocator.cpp 2013-08-07 11:53:28 +0000
@@ -31,6 +31,7 @@
31#include <GLES2/gl2.h>31#include <GLES2/gl2.h>
32#include <GLES2/gl2ext.h>32#include <GLES2/gl2ext.h>
3333
34#include <algorithm>
34#include <stdexcept>35#include <stdexcept>
35#include <gbm.h>36#include <gbm.h>
36#include <cassert>37#include <cassert>
@@ -119,6 +120,15 @@
119{120{
120 uint32_t bo_flags{GBM_BO_USE_RENDERING};121 uint32_t bo_flags{GBM_BO_USE_RENDERING};
121122
123 uint32_t gbm_format = mgg::mir_format_to_gbm_format(buffer_properties.format);
124
125 if (!is_pixel_format_supported(buffer_properties.format) ||
126 gbm_format == mgg::invalid_gbm_format)
127 {
128 BOOST_THROW_EXCEPTION(
129 std::runtime_error("Trying to create GBM buffer with unsupported pixel format"));
130 }
131
122 /* Create the GBM buffer object */132 /* Create the GBM buffer object */
123 if (buffer_properties.usage == BufferUsage::software)133 if (buffer_properties.usage == BufferUsage::software)
124 bo_flags |= GBM_BO_USE_WRITE;134 bo_flags |= GBM_BO_USE_WRITE;
@@ -127,7 +137,7 @@
127 platform->gbm.device,137 platform->gbm.device,
128 buffer_properties.size.width.as_uint32_t(),138 buffer_properties.size.width.as_uint32_t(),
129 buffer_properties.size.height.as_uint32_t(),139 buffer_properties.size.height.as_uint32_t(),
130 mgg::mir_format_to_gbm_format(buffer_properties.format),140 gbm_format,
131 bo_flags);141 bo_flags);
132142
133 if (!bo_raw)143 if (!bo_raw)
@@ -155,3 +165,12 @@
155165
156 return pixel_formats;166 return pixel_formats;
157}167}
168
169bool mgg::GBMBufferAllocator::is_pixel_format_supported(geom::PixelFormat format)
170{
171 auto formats = supported_pixel_formats();
172
173 auto iter = std::find(formats.begin(), formats.end(), format);
174
175 return iter != formats.end();
176}
158177
=== modified file 'src/server/graphics/gbm/gbm_buffer_allocator.h'
--- src/server/graphics/gbm/gbm_buffer_allocator.h 2013-07-31 11:18:25 +0000
+++ src/server/graphics/gbm/gbm_buffer_allocator.h 2013-08-07 11:53:28 +0000
@@ -47,6 +47,8 @@
47 std::vector<geometry::PixelFormat> supported_pixel_formats();47 std::vector<geometry::PixelFormat> supported_pixel_formats();
4848
49private:49private:
50 bool is_pixel_format_supported(geometry::PixelFormat format);
51
50 std::shared_ptr<GBMPlatform> platform;52 std::shared_ptr<GBMPlatform> platform;
51 std::shared_ptr<graphics::BufferInitializer> buffer_initializer;53 std::shared_ptr<graphics::BufferInitializer> buffer_initializer;
52 std::shared_ptr<EGLExtensions> const egl_extensions;54 std::shared_ptr<EGLExtensions> const egl_extensions;
5355
=== modified file 'tests/unit-tests/graphics/gbm/test_gbm_buffer_allocator.cpp'
--- tests/unit-tests/graphics/gbm/test_gbm_buffer_allocator.cpp 2013-07-31 14:01:17 +0000
+++ tests/unit-tests/graphics/gbm/test_gbm_buffer_allocator.cpp 2013-08-07 11:53:28 +0000
@@ -112,16 +112,6 @@
112 allocator->alloc_buffer(mg::BufferProperties{size, geom::PixelFormat::xrgb_8888, usage});112 allocator->alloc_buffer(mg::BufferProperties{size, geom::PixelFormat::xrgb_8888, usage});
113}113}
114114
115TEST_F(GBMBufferAllocatorTest, correct_buffer_format_translation_abgr_8888_to_invalid)
116{
117 using namespace testing;
118
119 EXPECT_CALL(mock_gbm, gbm_bo_create(_,_,_,std::numeric_limits<uint32_t>::max(),_));
120 EXPECT_CALL(mock_gbm, gbm_bo_destroy(_));
121
122 allocator->alloc_buffer(mg::BufferProperties{size, geom::PixelFormat::abgr_8888, usage});
123}
124
125MATCHER_P(has_flag_set, flag, "")115MATCHER_P(has_flag_set, flag, "")
126{116{
127 return arg & flag;117 return arg & flag;
@@ -244,3 +234,15 @@
244 ASSERT_FALSE(supported_pixel_formats.empty());234 ASSERT_FALSE(supported_pixel_formats.empty());
245 EXPECT_EQ(geom::PixelFormat::argb_8888, supported_pixel_formats[0]);235 EXPECT_EQ(geom::PixelFormat::argb_8888, supported_pixel_formats[0]);
246}236}
237
238TEST_F(GBMBufferAllocatorTest, alloc_with_unsupported_pixel_format_throws)
239{
240 using namespace testing;
241
242 /* We shouldn't try to create a buffer with an unsupported format */
243 EXPECT_CALL(mock_gbm, gbm_bo_create(_,_,_,_,_)).Times(0);
244
245 EXPECT_THROW({
246 allocator->alloc_buffer(mg::BufferProperties{size, geom::PixelFormat::abgr_8888, usage});
247 }, std::runtime_error);
248}

Subscribers

People subscribed via source and target branches