Mir

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

Proposed by Alexandros Frantzis on 2013-08-07
Status: Merged
Approved by: Alan Griffiths on 2013-08-08
Approved revision: 937
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 on 2013-08-07
Alan Griffiths 2013-08-07 Approve on 2013-08-07
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.
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
937. By Alexandros Frantzis on 2013-08-07

gbm: Use an enumeration constant for invalid_gbm_format

Alexandros Frantzis (afrantzis) wrote :

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

Fixed.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/server/graphics/gbm/gbm_buffer.cpp'
2--- src/server/graphics/gbm/gbm_buffer.cpp 2013-07-22 02:18:21 +0000
3+++ src/server/graphics/gbm/gbm_buffer.cpp 2013-08-07 11:53:28 +0000
4@@ -27,7 +27,6 @@
5 #include <boost/exception/errinfo_errno.hpp>
6 #include <boost/throw_exception.hpp>
7
8-#include <limits>
9 #include <stdexcept>
10
11 namespace mgg=mir::graphics::gbm;
12@@ -68,8 +67,7 @@
13 gbm_pf = GBM_FORMAT_XRGB8888;
14 break;
15 default:
16- /* There is no explicit invalid GBM pixel format! */
17- gbm_pf = std::numeric_limits<uint32_t>::max();
18+ gbm_pf = mgg::invalid_gbm_format;
19 break;
20 }
21
22
23=== modified file 'src/server/graphics/gbm/gbm_buffer.h'
24--- src/server/graphics/gbm/gbm_buffer.h 2013-07-22 02:18:21 +0000
25+++ src/server/graphics/gbm/gbm_buffer.h 2013-08-07 11:53:28 +0000
26@@ -25,6 +25,7 @@
27 #include <gbm.h>
28
29 #include <memory>
30+#include <limits>
31
32 namespace mir
33 {
34@@ -35,6 +36,7 @@
35
36 geometry::PixelFormat gbm_format_to_mir_format(uint32_t format);
37 uint32_t mir_format_to_gbm_format(geometry::PixelFormat format);
38+enum : uint32_t { invalid_gbm_format = std::numeric_limits<uint32_t>::max() };
39
40 class BufferTextureBinder;
41
42
43=== modified file 'src/server/graphics/gbm/gbm_buffer_allocator.cpp'
44--- src/server/graphics/gbm/gbm_buffer_allocator.cpp 2013-07-31 14:01:17 +0000
45+++ src/server/graphics/gbm/gbm_buffer_allocator.cpp 2013-08-07 11:53:28 +0000
46@@ -31,6 +31,7 @@
47 #include <GLES2/gl2.h>
48 #include <GLES2/gl2ext.h>
49
50+#include <algorithm>
51 #include <stdexcept>
52 #include <gbm.h>
53 #include <cassert>
54@@ -119,6 +120,15 @@
55 {
56 uint32_t bo_flags{GBM_BO_USE_RENDERING};
57
58+ uint32_t gbm_format = mgg::mir_format_to_gbm_format(buffer_properties.format);
59+
60+ if (!is_pixel_format_supported(buffer_properties.format) ||
61+ gbm_format == mgg::invalid_gbm_format)
62+ {
63+ BOOST_THROW_EXCEPTION(
64+ std::runtime_error("Trying to create GBM buffer with unsupported pixel format"));
65+ }
66+
67 /* Create the GBM buffer object */
68 if (buffer_properties.usage == BufferUsage::software)
69 bo_flags |= GBM_BO_USE_WRITE;
70@@ -127,7 +137,7 @@
71 platform->gbm.device,
72 buffer_properties.size.width.as_uint32_t(),
73 buffer_properties.size.height.as_uint32_t(),
74- mgg::mir_format_to_gbm_format(buffer_properties.format),
75+ gbm_format,
76 bo_flags);
77
78 if (!bo_raw)
79@@ -155,3 +165,12 @@
80
81 return pixel_formats;
82 }
83+
84+bool mgg::GBMBufferAllocator::is_pixel_format_supported(geom::PixelFormat format)
85+{
86+ auto formats = supported_pixel_formats();
87+
88+ auto iter = std::find(formats.begin(), formats.end(), format);
89+
90+ return iter != formats.end();
91+}
92
93=== modified file 'src/server/graphics/gbm/gbm_buffer_allocator.h'
94--- src/server/graphics/gbm/gbm_buffer_allocator.h 2013-07-31 11:18:25 +0000
95+++ src/server/graphics/gbm/gbm_buffer_allocator.h 2013-08-07 11:53:28 +0000
96@@ -47,6 +47,8 @@
97 std::vector<geometry::PixelFormat> supported_pixel_formats();
98
99 private:
100+ bool is_pixel_format_supported(geometry::PixelFormat format);
101+
102 std::shared_ptr<GBMPlatform> platform;
103 std::shared_ptr<graphics::BufferInitializer> buffer_initializer;
104 std::shared_ptr<EGLExtensions> const egl_extensions;
105
106=== modified file 'tests/unit-tests/graphics/gbm/test_gbm_buffer_allocator.cpp'
107--- tests/unit-tests/graphics/gbm/test_gbm_buffer_allocator.cpp 2013-07-31 14:01:17 +0000
108+++ tests/unit-tests/graphics/gbm/test_gbm_buffer_allocator.cpp 2013-08-07 11:53:28 +0000
109@@ -112,16 +112,6 @@
110 allocator->alloc_buffer(mg::BufferProperties{size, geom::PixelFormat::xrgb_8888, usage});
111 }
112
113-TEST_F(GBMBufferAllocatorTest, correct_buffer_format_translation_abgr_8888_to_invalid)
114-{
115- using namespace testing;
116-
117- EXPECT_CALL(mock_gbm, gbm_bo_create(_,_,_,std::numeric_limits<uint32_t>::max(),_));
118- EXPECT_CALL(mock_gbm, gbm_bo_destroy(_));
119-
120- allocator->alloc_buffer(mg::BufferProperties{size, geom::PixelFormat::abgr_8888, usage});
121-}
122-
123 MATCHER_P(has_flag_set, flag, "")
124 {
125 return arg & flag;
126@@ -244,3 +234,15 @@
127 ASSERT_FALSE(supported_pixel_formats.empty());
128 EXPECT_EQ(geom::PixelFormat::argb_8888, supported_pixel_formats[0]);
129 }
130+
131+TEST_F(GBMBufferAllocatorTest, alloc_with_unsupported_pixel_format_throws)
132+{
133+ using namespace testing;
134+
135+ /* We shouldn't try to create a buffer with an unsupported format */
136+ EXPECT_CALL(mock_gbm, gbm_bo_create(_,_,_,_,_)).Times(0);
137+
138+ EXPECT_THROW({
139+ allocator->alloc_buffer(mg::BufferProperties{size, geom::PixelFormat::abgr_8888, usage});
140+ }, std::runtime_error);
141+}

Subscribers

People subscribed via source and target branches