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
=== modified file 'debian/changelog'
--- debian/changelog 2014-10-01 03:37:18 +0000
+++ debian/changelog 2014-10-01 03:43:21 +0000
@@ -82,6 +82,7 @@
82 . demo shell: Keep colours consistent, regardless of the physical pixel82 . demo shell: Keep colours consistent, regardless of the physical pixel
83 format of your framebuffer (LP: #1375660)83 format of your framebuffer (LP: #1375660)
84 . tests: Fix SharedLibraryProber test runs on i386. (LP: #1375829)84 . tests: Fix SharedLibraryProber test runs on i386. (LP: #1375829)
85 . Touchspots rendered as squares on some devices (LP: #1373698)
8586
86 -- Daniel van Vugt <daniel.van.vugt@canonical.com> Tue, 30 Sep 2014 16:08:44 +080087 -- Daniel van Vugt <daniel.van.vugt@canonical.com> Tue, 30 Sep 2014 16:08:44 +0800
8788
8889
=== modified file 'include/platform/mir/graphics/renderable.h'
--- include/platform/mir/graphics/renderable.h 2014-09-19 03:08:20 +0000
+++ include/platform/mir/graphics/renderable.h 2014-10-01 03:43:21 +0000
@@ -48,7 +48,6 @@
48 */48 */
49 virtual std::shared_ptr<Buffer> buffer() const = 0;49 virtual std::shared_ptr<Buffer> buffer() const = 0;
5050
51 virtual bool alpha_enabled() const = 0;
52 virtual geometry::Rectangle screen_position() const = 0;51 virtual geometry::Rectangle screen_position() const = 0;
5352
54 // These are from the old CompositingCriteria. There is a little bit53 // These are from the old CompositingCriteria. There is a little bit
5554
=== modified file 'platform-ABI-sha1sums'
--- platform-ABI-sha1sums 2014-09-30 04:37:00 +0000
+++ platform-ABI-sha1sums 2014-10-01 03:43:21 +0000
@@ -32,7 +32,7 @@
32f90a35371e236a6cfec8e9a8474dbb3305c7621e include/platform/mir/graphics/internal_client.h32f90a35371e236a6cfec8e9a8474dbb3305c7621e include/platform/mir/graphics/internal_client.h
33c9730cac4a3a101f9706ec6f444958abe047fd88 include/platform/mir/graphics/internal_surface.h33c9730cac4a3a101f9706ec6f444958abe047fd88 include/platform/mir/graphics/internal_surface.h
34f47dac961f060eb29a44ed3c8c18a49a3d353bb1 include/platform/mir/graphics/platform.h34f47dac961f060eb29a44ed3c8c18a49a3d353bb1 include/platform/mir/graphics/platform.h
3515f201741a465de33e55ffc1ea775b507a5be950 include/platform/mir/graphics/renderable.h3584c063346b3bd51b4624d9f940008d4c3f8be066 include/platform/mir/graphics/renderable.h
36f5746dab3336266cfd410ce4e4d01333df6e5b99 include/platform/mir/options/configuration.h36f5746dab3336266cfd410ce4e4d01333df6e5b99 include/platform/mir/options/configuration.h
3747007c783c174f8e94d332c4b13c6b01358b48fb include/platform/mir/options/default_configuration.h3747007c783c174f8e94d332c4b13c6b01358b48fb include/platform/mir/options/default_configuration.h
38b45f14082c4f8b29efaa1b13de795dcb29deb738 include/platform/mir/options/option.h38b45f14082c4f8b29efaa1b13de795dcb29deb738 include/platform/mir/options/option.h
3939
=== modified file 'playground/render_overlays.cpp'
--- playground/render_overlays.cpp 2014-09-19 03:08:20 +0000
+++ playground/render_overlays.cpp 2014-10-01 03:43:21 +0000
@@ -115,11 +115,6 @@
115 return client->last_rendered();115 return client->last_rendered();
116 }116 }
117117
118 bool alpha_enabled() const override
119 {
120 return false;
121 }
122
123 geom::Rectangle screen_position() const override118 geom::Rectangle screen_position() const override
124 {119 {
125 return position;120 return position;
126121
=== modified file 'server-ABI-sha1sums'
--- server-ABI-sha1sums 2014-09-30 04:37:00 +0000
+++ server-ABI-sha1sums 2014-10-01 03:43:21 +0000
@@ -32,7 +32,7 @@
32f90a35371e236a6cfec8e9a8474dbb3305c7621e include/platform/mir/graphics/internal_client.h32f90a35371e236a6cfec8e9a8474dbb3305c7621e include/platform/mir/graphics/internal_client.h
33c9730cac4a3a101f9706ec6f444958abe047fd88 include/platform/mir/graphics/internal_surface.h33c9730cac4a3a101f9706ec6f444958abe047fd88 include/platform/mir/graphics/internal_surface.h
34f47dac961f060eb29a44ed3c8c18a49a3d353bb1 include/platform/mir/graphics/platform.h34f47dac961f060eb29a44ed3c8c18a49a3d353bb1 include/platform/mir/graphics/platform.h
3515f201741a465de33e55ffc1ea775b507a5be950 include/platform/mir/graphics/renderable.h3584c063346b3bd51b4624d9f940008d4c3f8be066 include/platform/mir/graphics/renderable.h
36f5746dab3336266cfd410ce4e4d01333df6e5b99 include/platform/mir/options/configuration.h36f5746dab3336266cfd410ce4e4d01333df6e5b99 include/platform/mir/options/configuration.h
3747007c783c174f8e94d332c4b13c6b01358b48fb include/platform/mir/options/default_configuration.h3747007c783c174f8e94d332c4b13c6b01358b48fb include/platform/mir/options/default_configuration.h
38b45f14082c4f8b29efaa1b13de795dcb29deb738 include/platform/mir/options/option.h38b45f14082c4f8b29efaa1b13de795dcb29deb738 include/platform/mir/options/option.h
3939
=== modified file 'src/platform/graphics/android/hwc_device.cpp'
--- src/platform/graphics/android/hwc_device.cpp 2014-09-30 04:37:00 +0000
+++ src/platform/graphics/android/hwc_device.cpp 2014-10-01 03:43:21 +0000
@@ -43,7 +43,7 @@
43 {43 {
44 1.0f/(2.0 * static_cast<float>(std::numeric_limits<decltype(hwc_layer_1_t::planeAlpha)>::max()))44 1.0f/(2.0 * static_cast<float>(std::numeric_limits<decltype(hwc_layer_1_t::planeAlpha)>::max()))
45 };45 };
46 return renderable.alpha_enabled() && (renderable.alpha() < 1.0f - tolerance);46 return (renderable.alpha() < 1.0f - tolerance);
47}47}
4848
49bool renderable_list_is_hwc_incompatible(mg::RenderableList const& list)49bool renderable_list_is_hwc_incompatible(mg::RenderableList const& list)
5050
=== modified file 'src/platform/graphics/android/hwc_fallback_gl_renderer.cpp'
--- src/platform/graphics/android/hwc_fallback_gl_renderer.cpp 2014-09-11 05:51:44 +0000
+++ src/platform/graphics/android/hwc_fallback_gl_renderer.cpp 2014-10-01 03:43:21 +0000
@@ -111,7 +111,7 @@
111111
112 for(auto const& renderable : renderlist)112 for(auto const& renderable : renderlist)
113 {113 {
114 if (renderable->alpha_enabled())114 if (renderable->shaped()) // TODO: support alpha() in future
115 glEnable(GL_BLEND);115 glEnable(GL_BLEND);
116 else116 else
117 glDisable(GL_BLEND);117 glDisable(GL_BLEND);
118118
=== modified file 'src/platform/graphics/android/hwc_layerlist.cpp'
--- src/platform/graphics/android/hwc_layerlist.cpp 2014-09-11 05:51:44 +0000
+++ src/platform/graphics/android/hwc_layerlist.cpp 2014-10-01 03:43:21 +0000
@@ -74,7 +74,7 @@
74 it->needs_commit = it->layer.setup_layer(74 it->needs_commit = it->layer.setup_layer(
75 mga::LayerType::gl_rendered,75 mga::LayerType::gl_rendered,
76 renderable->screen_position(),76 renderable->screen_position(),
77 renderable->alpha_enabled(),77 renderable->shaped(), // TODO: support alpha() in future too
78 *renderable->buffer());78 *renderable->buffer());
79 it++;79 it++;
80 }80 }
@@ -89,7 +89,7 @@
89 mga::HWCLayer(89 mga::HWCLayer(
90 mga::LayerType::gl_rendered,90 mga::LayerType::gl_rendered,
91 renderable->screen_position(),91 renderable->screen_position(),
92 renderable->alpha_enabled(),92 renderable->shaped(), // TODO: support alpha() in future
93 *renderable->buffer(),93 *renderable->buffer(),
94 hwc_representation, i++), true);94 hwc_representation, i++), true);
95 }95 }
9696
=== modified file 'src/platform/symbols.map'
--- src/platform/symbols.map 2014-09-30 04:37:00 +0000
+++ src/platform/symbols.map 2014-10-01 03:43:21 +0000
@@ -185,7 +185,6 @@
185 mir::graphics::Platform::Platform*;185 mir::graphics::Platform::Platform*;
186 mir::graphics::red_channel_depth*;186 mir::graphics::red_channel_depth*;
187 mir::graphics::Renderable::alpha*;187 mir::graphics::Renderable::alpha*;
188 mir::graphics::Renderable::alpha_enabled*;
189 mir::graphics::Renderable::buffer*;188 mir::graphics::Renderable::buffer*;
190 mir::graphics::Renderable::buffers_ready_for_compositor*;189 mir::graphics::Renderable::buffers_ready_for_compositor*;
191 mir::graphics::Renderable::id*;190 mir::graphics::Renderable::id*;
192191
=== modified file 'src/server/input/touchspot_controller.cpp'
--- src/server/input/touchspot_controller.cpp 2014-09-23 17:01:30 +0000
+++ src/server/input/touchspot_controller.cpp 2014-10-01 03:43:21 +0000
@@ -57,11 +57,6 @@
57 return buffer_;57 return buffer_;
58 }58 }
59 59
60 bool alpha_enabled() const override
61 {
62 return false;
63 }
64
65 geom::Rectangle screen_position() const60 geom::Rectangle screen_position() const
66 {61 {
67 return {position, buffer_->size()};62 return {position, buffer_->size()};
6863
=== modified file 'src/server/scene/basic_surface.cpp'
--- src/server/scene/basic_surface.cpp 2014-09-19 03:08:20 +0000
+++ src/server/scene/basic_surface.cpp 2014-10-01 03:43:21 +0000
@@ -616,14 +616,12 @@
616 geom::Rectangle const& position,616 geom::Rectangle const& position,
617 glm::mat4 const& transform,617 glm::mat4 const& transform,
618 bool visible,618 bool visible,
619 bool alpha_enabled,
620 float alpha,619 float alpha,
621 bool shaped,620 bool shaped,
622 mg::Renderable::ID id)621 mg::Renderable::ID id)
623 : underlying_buffer_stream{stream},622 : underlying_buffer_stream{stream},
624 compositor_buffer{nullptr},623 compositor_buffer{nullptr},
625 compositor_id{compositor_id},624 compositor_id{compositor_id},
626 alpha_enabled_{alpha_enabled},
627 alpha_{alpha},625 alpha_{alpha},
628 shaped_{shaped},626 shaped_{shaped},
629 visible_{visible},627 visible_{visible},
@@ -650,9 +648,6 @@
650 bool visible() const override648 bool visible() const override
651 { return visible_; }649 { return visible_; }
652650
653 bool alpha_enabled() const override
654 { return alpha_enabled_; }
655
656 geom::Rectangle screen_position() const override651 geom::Rectangle screen_position() const override
657 { return screen_position_; }652 { return screen_position_; }
658653
@@ -671,7 +666,6 @@
671 std::shared_ptr<mc::BufferStream> const underlying_buffer_stream;666 std::shared_ptr<mc::BufferStream> const underlying_buffer_stream;
672 std::shared_ptr<mg::Buffer> mutable compositor_buffer;667 std::shared_ptr<mg::Buffer> mutable compositor_buffer;
673 void const*const compositor_id;668 void const*const compositor_id;
674 bool const alpha_enabled_;
675 float const alpha_;669 float const alpha_;
676 bool const shaped_;670 bool const shaped_;
677 bool const visible_;671 bool const visible_;
@@ -685,7 +679,6 @@
685{679{
686 std::unique_lock<std::mutex> lk(guard);680 std::unique_lock<std::mutex> lk(guard);
687681
688 auto const shaped = nonrectangular || (surface_alpha < 1.0f);
689 return std::unique_ptr<mg::Renderable>(682 return std::unique_ptr<mg::Renderable>(
690 new SurfaceSnapshot(683 new SurfaceSnapshot(
691 surface_buffer_stream,684 surface_buffer_stream,
@@ -693,7 +686,6 @@
693 surface_rect,686 surface_rect,
694 transformation_matrix,687 transformation_matrix,
695 visible(lk),688 visible(lk),
696 shaped,
697 surface_alpha,689 surface_alpha,
698 nonrectangular, 690 nonrectangular,
699 this));691 this));
700692
=== modified file 'tests/include/mir_test_doubles/fake_renderable.h'
--- tests/include/mir_test_doubles/fake_renderable.h 2014-09-19 03:08:20 +0000
+++ tests/include/mir_test_doubles/fake_renderable.h 2014-10-01 03:43:21 +0000
@@ -107,11 +107,6 @@
107 return buf;107 return buf;
108 }108 }
109109
110 bool alpha_enabled() const override
111 {
112 return shaped() || alpha() < 1.0f;
113 }
114
115 geometry::Rectangle screen_position() const override110 geometry::Rectangle screen_position() const override
116 {111 {
117 return rect;112 return rect;
118113
=== modified file 'tests/include/mir_test_doubles/mock_renderable.h'
--- tests/include/mir_test_doubles/mock_renderable.h 2014-09-19 03:08:20 +0000
+++ tests/include/mir_test_doubles/mock_renderable.h 2014-10-01 03:43:21 +0000
@@ -49,7 +49,6 @@
4949
50 MOCK_CONST_METHOD0(id, ID());50 MOCK_CONST_METHOD0(id, ID());
51 MOCK_CONST_METHOD0(buffer, std::shared_ptr<graphics::Buffer>());51 MOCK_CONST_METHOD0(buffer, std::shared_ptr<graphics::Buffer>());
52 MOCK_CONST_METHOD0(alpha_enabled, bool());
53 MOCK_CONST_METHOD0(screen_position, geometry::Rectangle());52 MOCK_CONST_METHOD0(screen_position, geometry::Rectangle());
54 MOCK_CONST_METHOD0(alpha, float());53 MOCK_CONST_METHOD0(alpha, float());
55 MOCK_CONST_METHOD0(transformation, glm::mat4());54 MOCK_CONST_METHOD0(transformation, glm::mat4());
5655
=== modified file 'tests/include/mir_test_doubles/stub_renderable.h'
--- tests/include/mir_test_doubles/stub_renderable.h 2014-09-19 03:08:20 +0000
+++ tests/include/mir_test_doubles/stub_renderable.h 2014-10-01 03:43:21 +0000
@@ -68,10 +68,6 @@
68 {68 {
69 return stub_buffer;69 return stub_buffer;
70 }70 }
71 bool alpha_enabled() const
72 {
73 return false;
74 }
75 geometry::Rectangle screen_position() const71 geometry::Rectangle screen_position() const
76 {72 {
77 return rect;73 return rect;
@@ -133,7 +129,7 @@
133129
134struct StubTranslucentRenderable : public StubRenderable130struct StubTranslucentRenderable : public StubRenderable
135{131{
136 bool alpha_enabled() const override132 bool shaped() const override
137 {133 {
138 return true;134 return true;
139 }135 }
@@ -141,10 +137,6 @@
141137
142struct PlaneAlphaRenderable : public StubRenderable138struct PlaneAlphaRenderable : public StubRenderable
143{139{
144 bool alpha_enabled() const override
145 {
146 return true;
147 }
148 float alpha() const override140 float alpha() const override
149 {141 {
150 //approx 99% alpha 142 //approx 99% alpha

Subscribers

People subscribed via source and target branches