Merge lp:~vanvugt/mir/simplify-alpha into lp:mir
- simplify-alpha
- Merge into development-branch
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Daniel van Vugt | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 1948 | ||||
Proposed branch: | lp:~vanvugt/mir/simplify-alpha | ||||
Merge into: | lp:mir | ||||
Diff against target: |
257 lines (+8/-41) 14 files modified
debian/changelog (+1/-0) include/platform/mir/graphics/renderable.h (+0/-1) platform-ABI-sha1sums (+1/-1) playground/render_overlays.cpp (+0/-5) server-ABI-sha1sums (+1/-1) src/platform/graphics/android/hwc_device.cpp (+1/-1) src/platform/graphics/android/hwc_fallback_gl_renderer.cpp (+1/-1) src/platform/graphics/android/hwc_layerlist.cpp (+2/-2) src/platform/symbols.map (+0/-1) src/server/input/touchspot_controller.cpp (+0/-5) src/server/scene/basic_surface.cpp (+0/-8) tests/include/mir_test_doubles/fake_renderable.h (+0/-5) tests/include/mir_test_doubles/mock_renderable.h (+0/-1) tests/include/mir_test_doubles/stub_renderable.h (+1/-9) |
||||
To merge this branch: | bzr merge lp:~vanvugt/mir/simplify-alpha | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Daniel van Vugt | Approve | ||
Kevin DuBois (community) | Approve | ||
Alberto Aguirre (community) | Approve | ||
PS Jenkins bot (community) | continuous-integration | Approve | |
Review via email: mp+236075@code.launchpad.net |
Commit message
Simplify alpha-related attributes, removing the recently added
alpha_enabled(). It adds no functionality that's not already
available via alpha() or shaped(). So drop it.
As a bonus, this also fixes LP: #1373698.
Description of the change
The two ABI breaks have already been bumped in the current 0.8 series. No further bump required.
Daniel van Vugt (vanvugt) wrote : | # |
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1942
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1942
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1943
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1944
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1944
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1944
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
Kevin DuBois (kdub) wrote : | # |
Needs discussion:
I left my thoughts on the Renderable interface here:
https:/
Needs fixing:
95 + if (renderable-
108 + renderable-
We don't support planeAlpha blending in HWC yet, so we don't need the check of alpha() here just yet. The check in renderable_
80 + if (// renderable-
HWC can do alpha blending, so the commented out line should be removed.
62 -bool plane_alpha_
Lets not remove this. I don't think the simpler check causes any 'real' problems, but since we're translating a float to an 8bit, anything greater than (1-2^9) should translate into 0xFF for the alpha value.
Daniel van Vugt (vanvugt) wrote : | # |
Fixed.
And in case anyone's wondering, the regression test for LP: #1373698 already exists in test_gl_
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1946
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1946
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1946
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1947
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1947
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1947
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
Alberto Aguirre (albaguirre) wrote : | # |
Looks good.
Kevin DuBois (kdub) wrote : | # |
okay, looks good now. I guess I'll resubmit the other branch with the name cleanup, and this branch will remove of one of the alpha functions and the bug.
Daniel van Vugt (vanvugt) : | # |
Preview Diff
1 | === modified file 'debian/changelog' |
2 | --- debian/changelog 2014-10-01 03:37:18 +0000 |
3 | +++ debian/changelog 2014-10-01 03:43:21 +0000 |
4 | @@ -82,6 +82,7 @@ |
5 | . demo shell: Keep colours consistent, regardless of the physical pixel |
6 | format of your framebuffer (LP: #1375660) |
7 | . tests: Fix SharedLibraryProber test runs on i386. (LP: #1375829) |
8 | + . Touchspots rendered as squares on some devices (LP: #1373698) |
9 | |
10 | -- Daniel van Vugt <daniel.van.vugt@canonical.com> Tue, 30 Sep 2014 16:08:44 +0800 |
11 | |
12 | |
13 | === modified file 'include/platform/mir/graphics/renderable.h' |
14 | --- include/platform/mir/graphics/renderable.h 2014-09-19 03:08:20 +0000 |
15 | +++ include/platform/mir/graphics/renderable.h 2014-10-01 03:43:21 +0000 |
16 | @@ -48,7 +48,6 @@ |
17 | */ |
18 | virtual std::shared_ptr<Buffer> buffer() const = 0; |
19 | |
20 | - virtual bool alpha_enabled() const = 0; |
21 | virtual geometry::Rectangle screen_position() const = 0; |
22 | |
23 | // These are from the old CompositingCriteria. There is a little bit |
24 | |
25 | === modified file 'platform-ABI-sha1sums' |
26 | --- platform-ABI-sha1sums 2014-09-30 04:37:00 +0000 |
27 | +++ platform-ABI-sha1sums 2014-10-01 03:43:21 +0000 |
28 | @@ -32,7 +32,7 @@ |
29 | f90a35371e236a6cfec8e9a8474dbb3305c7621e include/platform/mir/graphics/internal_client.h |
30 | c9730cac4a3a101f9706ec6f444958abe047fd88 include/platform/mir/graphics/internal_surface.h |
31 | f47dac961f060eb29a44ed3c8c18a49a3d353bb1 include/platform/mir/graphics/platform.h |
32 | -15f201741a465de33e55ffc1ea775b507a5be950 include/platform/mir/graphics/renderable.h |
33 | +84c063346b3bd51b4624d9f940008d4c3f8be066 include/platform/mir/graphics/renderable.h |
34 | f5746dab3336266cfd410ce4e4d01333df6e5b99 include/platform/mir/options/configuration.h |
35 | 47007c783c174f8e94d332c4b13c6b01358b48fb include/platform/mir/options/default_configuration.h |
36 | b45f14082c4f8b29efaa1b13de795dcb29deb738 include/platform/mir/options/option.h |
37 | |
38 | === modified file 'playground/render_overlays.cpp' |
39 | --- playground/render_overlays.cpp 2014-09-19 03:08:20 +0000 |
40 | +++ playground/render_overlays.cpp 2014-10-01 03:43:21 +0000 |
41 | @@ -115,11 +115,6 @@ |
42 | return client->last_rendered(); |
43 | } |
44 | |
45 | - bool alpha_enabled() const override |
46 | - { |
47 | - return false; |
48 | - } |
49 | - |
50 | geom::Rectangle screen_position() const override |
51 | { |
52 | return position; |
53 | |
54 | === modified file 'server-ABI-sha1sums' |
55 | --- server-ABI-sha1sums 2014-09-30 04:37:00 +0000 |
56 | +++ server-ABI-sha1sums 2014-10-01 03:43:21 +0000 |
57 | @@ -32,7 +32,7 @@ |
58 | f90a35371e236a6cfec8e9a8474dbb3305c7621e include/platform/mir/graphics/internal_client.h |
59 | c9730cac4a3a101f9706ec6f444958abe047fd88 include/platform/mir/graphics/internal_surface.h |
60 | f47dac961f060eb29a44ed3c8c18a49a3d353bb1 include/platform/mir/graphics/platform.h |
61 | -15f201741a465de33e55ffc1ea775b507a5be950 include/platform/mir/graphics/renderable.h |
62 | +84c063346b3bd51b4624d9f940008d4c3f8be066 include/platform/mir/graphics/renderable.h |
63 | f5746dab3336266cfd410ce4e4d01333df6e5b99 include/platform/mir/options/configuration.h |
64 | 47007c783c174f8e94d332c4b13c6b01358b48fb include/platform/mir/options/default_configuration.h |
65 | b45f14082c4f8b29efaa1b13de795dcb29deb738 include/platform/mir/options/option.h |
66 | |
67 | === modified file 'src/platform/graphics/android/hwc_device.cpp' |
68 | --- src/platform/graphics/android/hwc_device.cpp 2014-09-30 04:37:00 +0000 |
69 | +++ src/platform/graphics/android/hwc_device.cpp 2014-10-01 03:43:21 +0000 |
70 | @@ -43,7 +43,7 @@ |
71 | { |
72 | 1.0f/(2.0 * static_cast<float>(std::numeric_limits<decltype(hwc_layer_1_t::planeAlpha)>::max())) |
73 | }; |
74 | - return renderable.alpha_enabled() && (renderable.alpha() < 1.0f - tolerance); |
75 | + return (renderable.alpha() < 1.0f - tolerance); |
76 | } |
77 | |
78 | bool renderable_list_is_hwc_incompatible(mg::RenderableList const& list) |
79 | |
80 | === modified file 'src/platform/graphics/android/hwc_fallback_gl_renderer.cpp' |
81 | --- src/platform/graphics/android/hwc_fallback_gl_renderer.cpp 2014-09-11 05:51:44 +0000 |
82 | +++ src/platform/graphics/android/hwc_fallback_gl_renderer.cpp 2014-10-01 03:43:21 +0000 |
83 | @@ -111,7 +111,7 @@ |
84 | |
85 | for(auto const& renderable : renderlist) |
86 | { |
87 | - if (renderable->alpha_enabled()) |
88 | + if (renderable->shaped()) // TODO: support alpha() in future |
89 | glEnable(GL_BLEND); |
90 | else |
91 | glDisable(GL_BLEND); |
92 | |
93 | === modified file 'src/platform/graphics/android/hwc_layerlist.cpp' |
94 | --- src/platform/graphics/android/hwc_layerlist.cpp 2014-09-11 05:51:44 +0000 |
95 | +++ src/platform/graphics/android/hwc_layerlist.cpp 2014-10-01 03:43:21 +0000 |
96 | @@ -74,7 +74,7 @@ |
97 | it->needs_commit = it->layer.setup_layer( |
98 | mga::LayerType::gl_rendered, |
99 | renderable->screen_position(), |
100 | - renderable->alpha_enabled(), |
101 | + renderable->shaped(), // TODO: support alpha() in future too |
102 | *renderable->buffer()); |
103 | it++; |
104 | } |
105 | @@ -89,7 +89,7 @@ |
106 | mga::HWCLayer( |
107 | mga::LayerType::gl_rendered, |
108 | renderable->screen_position(), |
109 | - renderable->alpha_enabled(), |
110 | + renderable->shaped(), // TODO: support alpha() in future |
111 | *renderable->buffer(), |
112 | hwc_representation, i++), true); |
113 | } |
114 | |
115 | === modified file 'src/platform/symbols.map' |
116 | --- src/platform/symbols.map 2014-09-30 04:37:00 +0000 |
117 | +++ src/platform/symbols.map 2014-10-01 03:43:21 +0000 |
118 | @@ -185,7 +185,6 @@ |
119 | mir::graphics::Platform::Platform*; |
120 | mir::graphics::red_channel_depth*; |
121 | mir::graphics::Renderable::alpha*; |
122 | - mir::graphics::Renderable::alpha_enabled*; |
123 | mir::graphics::Renderable::buffer*; |
124 | mir::graphics::Renderable::buffers_ready_for_compositor*; |
125 | mir::graphics::Renderable::id*; |
126 | |
127 | === modified file 'src/server/input/touchspot_controller.cpp' |
128 | --- src/server/input/touchspot_controller.cpp 2014-09-23 17:01:30 +0000 |
129 | +++ src/server/input/touchspot_controller.cpp 2014-10-01 03:43:21 +0000 |
130 | @@ -57,11 +57,6 @@ |
131 | return buffer_; |
132 | } |
133 | |
134 | - bool alpha_enabled() const override |
135 | - { |
136 | - return false; |
137 | - } |
138 | - |
139 | geom::Rectangle screen_position() const |
140 | { |
141 | return {position, buffer_->size()}; |
142 | |
143 | === modified file 'src/server/scene/basic_surface.cpp' |
144 | --- src/server/scene/basic_surface.cpp 2014-09-19 03:08:20 +0000 |
145 | +++ src/server/scene/basic_surface.cpp 2014-10-01 03:43:21 +0000 |
146 | @@ -616,14 +616,12 @@ |
147 | geom::Rectangle const& position, |
148 | glm::mat4 const& transform, |
149 | bool visible, |
150 | - bool alpha_enabled, |
151 | float alpha, |
152 | bool shaped, |
153 | mg::Renderable::ID id) |
154 | : underlying_buffer_stream{stream}, |
155 | compositor_buffer{nullptr}, |
156 | compositor_id{compositor_id}, |
157 | - alpha_enabled_{alpha_enabled}, |
158 | alpha_{alpha}, |
159 | shaped_{shaped}, |
160 | visible_{visible}, |
161 | @@ -650,9 +648,6 @@ |
162 | bool visible() const override |
163 | { return visible_; } |
164 | |
165 | - bool alpha_enabled() const override |
166 | - { return alpha_enabled_; } |
167 | - |
168 | geom::Rectangle screen_position() const override |
169 | { return screen_position_; } |
170 | |
171 | @@ -671,7 +666,6 @@ |
172 | std::shared_ptr<mc::BufferStream> const underlying_buffer_stream; |
173 | std::shared_ptr<mg::Buffer> mutable compositor_buffer; |
174 | void const*const compositor_id; |
175 | - bool const alpha_enabled_; |
176 | float const alpha_; |
177 | bool const shaped_; |
178 | bool const visible_; |
179 | @@ -685,7 +679,6 @@ |
180 | { |
181 | std::unique_lock<std::mutex> lk(guard); |
182 | |
183 | - auto const shaped = nonrectangular || (surface_alpha < 1.0f); |
184 | return std::unique_ptr<mg::Renderable>( |
185 | new SurfaceSnapshot( |
186 | surface_buffer_stream, |
187 | @@ -693,7 +686,6 @@ |
188 | surface_rect, |
189 | transformation_matrix, |
190 | visible(lk), |
191 | - shaped, |
192 | surface_alpha, |
193 | nonrectangular, |
194 | this)); |
195 | |
196 | === modified file 'tests/include/mir_test_doubles/fake_renderable.h' |
197 | --- tests/include/mir_test_doubles/fake_renderable.h 2014-09-19 03:08:20 +0000 |
198 | +++ tests/include/mir_test_doubles/fake_renderable.h 2014-10-01 03:43:21 +0000 |
199 | @@ -107,11 +107,6 @@ |
200 | return buf; |
201 | } |
202 | |
203 | - bool alpha_enabled() const override |
204 | - { |
205 | - return shaped() || alpha() < 1.0f; |
206 | - } |
207 | - |
208 | geometry::Rectangle screen_position() const override |
209 | { |
210 | return rect; |
211 | |
212 | === modified file 'tests/include/mir_test_doubles/mock_renderable.h' |
213 | --- tests/include/mir_test_doubles/mock_renderable.h 2014-09-19 03:08:20 +0000 |
214 | +++ tests/include/mir_test_doubles/mock_renderable.h 2014-10-01 03:43:21 +0000 |
215 | @@ -49,7 +49,6 @@ |
216 | |
217 | MOCK_CONST_METHOD0(id, ID()); |
218 | MOCK_CONST_METHOD0(buffer, std::shared_ptr<graphics::Buffer>()); |
219 | - MOCK_CONST_METHOD0(alpha_enabled, bool()); |
220 | MOCK_CONST_METHOD0(screen_position, geometry::Rectangle()); |
221 | MOCK_CONST_METHOD0(alpha, float()); |
222 | MOCK_CONST_METHOD0(transformation, glm::mat4()); |
223 | |
224 | === modified file 'tests/include/mir_test_doubles/stub_renderable.h' |
225 | --- tests/include/mir_test_doubles/stub_renderable.h 2014-09-19 03:08:20 +0000 |
226 | +++ tests/include/mir_test_doubles/stub_renderable.h 2014-10-01 03:43:21 +0000 |
227 | @@ -68,10 +68,6 @@ |
228 | { |
229 | return stub_buffer; |
230 | } |
231 | - bool alpha_enabled() const |
232 | - { |
233 | - return false; |
234 | - } |
235 | geometry::Rectangle screen_position() const |
236 | { |
237 | return rect; |
238 | @@ -133,7 +129,7 @@ |
239 | |
240 | struct StubTranslucentRenderable : public StubRenderable |
241 | { |
242 | - bool alpha_enabled() const override |
243 | + bool shaped() const override |
244 | { |
245 | return true; |
246 | } |
247 | @@ -141,10 +137,6 @@ |
248 | |
249 | struct PlaneAlphaRenderable : public StubRenderable |
250 | { |
251 | - bool alpha_enabled() const override |
252 | - { |
253 | - return true; |
254 | - } |
255 | float alpha() const override |
256 | { |
257 | //approx 99% alpha |
I just noticed I had accidentally fixed too much in hwc_device.cpp, which stopped nested servers from overlaying. Now un-fixed hwc_device.cpp so it at least has the same old bug as before ("FIXME"), and overlays will still work :S