Mir

Merge lp:~vanvugt/mir/simplify-alpha into lp:mir

Proposed by Daniel van Vugt
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
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.

To post a comment you must log in.
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

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

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

Needs discussion:
I left my thoughts on the Renderable interface here:
https://code.launchpad.net/~kdub/mir/fix-1373698/+merge/236023/comments/578069

Needs fixing:
95 + if (renderable->shaped() || renderable->alpha() < 1.0f)
108 + renderable->shaped() || renderable->alpha() < 1.0f,
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_list_is_hwc_incompatible prevents these lines from being reached if plane alpha is present.

80 + if (// renderable->shaped() || // Unsafe to uncomment yet LP: #1374358
HWC can do alpha blending, so the commented out line should be removed.

62 -bool plane_alpha_is_translucent(mg::Renderable const& renderable)
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.

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Fixed.

And in case anyone's wondering, the regression test for LP: #1373698 already exists in test_gl_renderer.cpp where we mock shaped() and check for calls to glEnable(GL_BLEND).

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

Looks good.

review: Approve
Revision history for this message
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.

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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

Subscribers

People subscribed via source and target branches