Mir

Merge lp:~albaguirre/mir/render-opaque-background into lp:mir

  1. render-opaque-background
  2. Merge into development-branch
Proposed by Alberto Aguirre
Status: Merged
Approved by: Alberto Aguirre
Approved revision: no longer in the source branch.
Merged at revision: 1622
Proposed branch: lp:~albaguirre/mir/render-opaque-background
Merge into: lp:mir
Diff against target: 56 lines (+13/-0)
4 files modified
include/test/mir_test_doubles/mock_gl.h (+1/-0)
src/server/compositor/gl_renderer.cpp (+4/-0)
tests/mir_test_doubles/mock_gl.cpp (+6/-0)
tests/unit-tests/compositor/test_gl_renderer.cpp (+2/-0)
To merge this branch: bzr merge lp:~albaguirre/mir/render-opaque-background
Reviewer Review Type Date Requested Status
Alan Griffiths Approve
Daniel van Vugt Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+218712@code.launchpad.net

Commit message

Render opaque background before compositing (LP: #1317260)

Description of the change

Render opaque background before compositing (LP: #1317260)

This is needed so a nested display surface doesn't have an uninitialized or completely transparent alpha channel.

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

FAILED: Continuous integration, rev:1609
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1551/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/89
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/90/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/90
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/70
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/70/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/68
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/68/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/198
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/198/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1357
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6923

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1551/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

^-- Looks like a spurious failure:

"148/186 Test #148: mir_unit_tests.ProtobufSocketCommunicatorFD.* ..............................***Exception: SegFault 1.03 sec"

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

PASSED: Continuous integration, rev:1610
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1553/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/92
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/93
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/93
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/72
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/72/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/70
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/70/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/212
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/212/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1360
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6939

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1553/rebuild

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

Does this cause problems for nested servers that want to use alpha? (The greeter?)

review: Needs Information
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

> Does this cause problems for nested servers that want to use alpha? (The
> greeter?)

Well after pinging mterry, yes it does for the split greeter, so back to the drawing board.

Revision history for this message
Alberto Aguirre (albaguirre) wrote :

> > Does this cause problems for nested servers that want to use alpha? (The
> > greeter?)
>
> Well after pinging mterry, yes it does for the split greeter, so back to the
> drawing board.

After discussing with anpok, we actually need more work to properly support alpha-enabled framebuffers properly -
i.e. we need to assume pre-multiplied sources and change the blending equation to glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA) so the resulting framebuffer per-pixel alpha is correct.

The split greeter sliding animation could be accomplished with surface positioning in the system compositor.

Properly supporting per-pixel alpha transition effects for nested needs additional discussion and work that are outside the scope of this MP.

Currently, the most pressing issue is making unity8 render correctly - unity8 doesn't care that framebuffer is opaque.

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

Works well and doesn't increase the number of glClear's. +1

Verified bug 1301210 remains fixed.

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

> > > Does this cause problems for nested servers that want to use alpha? (The
> > > greeter?)
> >
> > Well after pinging mterry, yes it does for the split greeter, so back to the
> > drawing board.
>
> After discussing with anpok, we actually need more work to properly support
> alpha-enabled framebuffers properly -
> i.e. we need to assume pre-multiplied sources and change the blending equation
> to glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA) so the resulting framebuffer
> per-pixel alpha is correct.
>
> The split greeter sliding animation could be accomplished with surface
> positioning in the system compositor.
>
> Properly supporting per-pixel alpha transition effects for nested needs
> additional discussion and work that are outside the scope of this MP.
>
> Currently, the most pressing issue is making unity8 render correctly - unity8
> doesn't care that framebuffer is opaque.

I don't pretend to understand all the issues here (it sounds a bit like postponing problems not addressing them) but I'll accept the conclusion.

review: Approve
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

> I don't pretend to understand all the issues here (it sounds a bit like
> postponing problems not addressing them) but I'll accept the conclusion.

Yes, the problem of correctly supporting correct alpha in a display framebuffer is not being addressed with this MP so therefore postponed. As mir stands now, it is not supported. So this MP does not address it nor makes the situation any worse.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/test/mir_test_doubles/mock_gl.h'
2--- include/test/mir_test_doubles/mock_gl.h 2014-05-05 03:36:45 +0000
3+++ include/test/mir_test_doubles/mock_gl.h 2014-05-08 03:43:54 +0000
4@@ -47,6 +47,7 @@
5 void(GLenum, GLsizeiptr, const GLvoid *, GLenum));
6 MOCK_METHOD1(glCheckFramebufferStatus, GLenum(GLenum));
7 MOCK_METHOD1(glClear, void(GLbitfield));
8+ MOCK_METHOD4(glClearColor, void(GLclampf, GLclampf, GLclampf, GLclampf));
9 MOCK_METHOD4(glColorMask, void(GLboolean, GLboolean, GLboolean, GLboolean));
10 MOCK_METHOD1(glCompileShader, void(GLuint));
11 MOCK_METHOD0(glCreateProgram, GLuint());
12
13=== modified file 'src/server/compositor/gl_renderer.cpp'
14--- src/server/compositor/gl_renderer.cpp 2014-05-06 03:33:05 +0000
15+++ src/server/compositor/gl_renderer.cpp 2014-05-08 03:43:54 +0000
16@@ -289,6 +289,10 @@
17
18 void mc::GLRenderer::begin() const
19 {
20+ // Ensure background is opaque otherwise alpha artifacts may occur
21+ // when rendering a nested display buffer (LP: #1317260)
22+ glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
23+ glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
24 glClear(GL_COLOR_BUFFER_BIT);
25
26 // Ensure we don't change the framebuffer's alpha components (if any)
27
28=== modified file 'tests/mir_test_doubles/mock_gl.cpp'
29--- tests/mir_test_doubles/mock_gl.cpp 2014-05-05 03:36:45 +0000
30+++ tests/mir_test_doubles/mock_gl.cpp 2014-05-08 03:43:54 +0000
31@@ -84,6 +84,12 @@
32 global_mock_gl->glClear(mask);
33 }
34
35+void glClearColor(GLclampf red, GLclampf green, GLclampf blue, GLclampf alpha)
36+{
37+ CHECK_GLOBAL_VOID_MOCK();
38+ global_mock_gl->glClearColor(red, green, blue, alpha);
39+}
40+
41 void glColorMask(GLboolean r, GLboolean g, GLboolean b, GLboolean a)
42 {
43 CHECK_GLOBAL_VOID_MOCK();
44
45=== modified file 'tests/unit-tests/compositor/test_gl_renderer.cpp'
46--- tests/unit-tests/compositor/test_gl_renderer.cpp 2014-05-06 03:33:05 +0000
47+++ tests/unit-tests/compositor/test_gl_renderer.cpp 2014-05-08 03:43:54 +0000
48@@ -166,6 +166,8 @@
49
50 InSequence seq;
51
52+ EXPECT_CALL(mock_gl, glClearColor(_, _, _, 1.0f));
53+ EXPECT_CALL(mock_gl, glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE));
54 EXPECT_CALL(mock_gl, glClear(_));
55 EXPECT_CALL(mock_gl, glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_FALSE));
56 EXPECT_CALL(mock_gl, glUseProgram(stub_program));

Subscribers

People subscribed via source and target branches