Merge lp:~vanvugt/mir/fix-1339471 into lp:mir

[Daniel van Vugt]

Revert r1687 as it is producing visibly incorrect colours when compared
to r1686. (LP: #1339471)

This reverts enhancement LP: #1318852, which needs to be approached
differently. Aside from anything else it's infeasible to modify all existing
alpha-blending apps and toolkits to pre-multiply alpha values like r1687
necessitated. So simply modifying the colours used in
mir_demo_client_multiwin wouldn't scale and fix all other toolkits' apps in
the world.

The default blending function must be (GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA)
but we're free to add options that change that for particular special
surfaces if necessary. So LP: #1318852 can be re-implemented in some other
way.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1754
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2102/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/874
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/880
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/873
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/623
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/623/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/621
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/621/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2786
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2786/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2017
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9517

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Qt/Qml apps render with premultiplied alpha by default. Cairo does premultiplied alpha by default too.

I think it's reasonable to demand (and document clearly) that surfaces with alpha passed to Mir are premultiplied.

Perhaps we could let the clients select per surface, if we find that important clients can't do premultiplied alpha. Even so, premultiplied alpha would a reasonable default.

review: Disapprove

[Alberto Aguirre (albaguirre)]

" So simply modifying the colours used in
mir_demo_client_multiwin wouldn't scale and fix all other toolkits' apps in
the world."

What are these toolkits? Most of the libraries I know do pre-multiplied alpha (for good reason) by default.

review: Disapprove

[Daniel van Vugt (vanvugt)]

Alright, clearly my graphics knowledge is a decade or so behind the those newer tookits.

Unmerged revisions

1754. By Daniel van Vugt on 2014-07-09

Revert r1687 as it is producing visibly incorrect colours when compared
to r1686. (LP: #1339471)

=== modified file 'examples/demo-shell/demo_renderer.cpp'
--- examples/demo-shell/demo_renderer.cpp	2014-06-18 12:50:31 +0000
+++ examples/demo-shell/demo_renderer.cpp	2014-07-09 06:44:55 +0000
@@ -94,6 +94,15 @@
         {
             Color col = color;
 
+            // Cut out the corner in a circular shape.
+            if (x < cx && y < cy)
+            {
+                int dx = cx - x;
+                int dy = cy - y;
+                if (dx * dx + dy * dy >= radius_sqr)
+                    col.a = 0;
+            }
+
             // Set gradient
             if (y < cy)
             {
@@ -105,15 +114,6 @@
                 col.b += (highlight - col.b) * brighten;
             }
 
-            // Cut out the corner in a circular shape.
-            if (x < cx && y < cy)
-            {
-                int dx = cx - x;
-                int dy = cy - y;
-                if (dx * dx + dy * dy >= radius_sqr)
-                    col = {0, 0, 0, 0};
-            }
-
             image[y * width + x] = col;
         }
     }
@@ -164,7 +164,7 @@
     if (opaque)
         glClearColor(0.2f, 0.2f, 0.2f, 1.0f);
     else
-        glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
+        glClearColor(0.2f, 0.2f, 0.2f, 0.0f);
 
     glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
     glClear(GL_COLOR_BUFFER_BIT);
@@ -273,7 +273,7 @@
 
     // Shadows always need blending...
     glEnable(GL_BLEND);
-    glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA);
+    glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
 }
 
 void DemoRenderer::tessellate_frame(std::vector<graphics::GLPrimitive>& primitives,

=== modified file 'examples/eglcounter.cpp'
--- examples/eglcounter.cpp	2014-06-05 21:28:43 +0000
+++ examples/eglcounter.cpp	2014-07-09 06:44:55 +0000
@@ -201,7 +201,7 @@
 };
 
 /* Colours from http://design.ubuntu.com/brand/colour-palette */
-#define MID_AUBERGINE(x) x*0.368627451f, x*0.152941176f, x*0.31372549f
+#define MID_AUBERGINE 0.368627451f, 0.152941176f, 0.31372549f
 #define ORANGE        0.866666667f, 0.282352941f, 0.141414141f
 
 int main(int argc, char *argv[])
@@ -254,8 +254,7 @@
         return 2;
     }
 
-    float const opacity = mir_eglapp_background_opacity;
-    glClearColor(MID_AUBERGINE(opacity), opacity);
+    glClearColor(MID_AUBERGINE, mir_eglapp_background_opacity);
     glViewport(0, 0, width, height);
 
     glUseProgram(prog);

=== modified file 'examples/eglflash.c'
--- examples/eglflash.c	2014-06-05 21:28:43 +0000
+++ examples/eglflash.c	2014-07-09 06:44:55 +0000
@@ -23,11 +23,6 @@
 #include <unistd.h>
 #include <GLES2/gl2.h>
 
-typedef struct Color
-{
-    GLfloat r, g, b, a;
-} Color;
-
 int main(int argc, char *argv[])
 {
     unsigned int width = 0, height = 0;
@@ -35,25 +30,20 @@
     if (!mir_eglapp_init(argc, argv, &width, &height))
         return 1;
 
-    float const opacity = mir_eglapp_background_opacity;
-    Color red = {opacity, 0.0f, 0.0f, opacity};
-    Color green = {0.0f, opacity, 0.0f, opacity};
-    Color blue = {0.0f, 0.0f, opacity, opacity};
-
     /* This is probably the simplest GL you can do */
     while (mir_eglapp_running())
     {
-        glClearColor(red.r, red.g, red.b, red.a);
-        glClear(GL_COLOR_BUFFER_BIT);
-        mir_eglapp_swap_buffers();
-        sleep(1);
-
-        glClearColor(green.r, green.g, green.b, green.a);
-        glClear(GL_COLOR_BUFFER_BIT);
-        mir_eglapp_swap_buffers();
-        sleep(1);
-
-        glClearColor(blue.r, blue.g, blue.b, blue.a);
+        glClearColor(1.0f, 0.0f, 0.0f, mir_eglapp_background_opacity);
+        glClear(GL_COLOR_BUFFER_BIT);
+        mir_eglapp_swap_buffers();
+        sleep(1);
+
+        glClearColor(0.0f, 1.0f, 0.0f, mir_eglapp_background_opacity);
+        glClear(GL_COLOR_BUFFER_BIT);
+        mir_eglapp_swap_buffers();
+        sleep(1);
+
+        glClearColor(0.0f, 0.0f, 1.0f, mir_eglapp_background_opacity);
         glClear(GL_COLOR_BUFFER_BIT);
         mir_eglapp_swap_buffers();
         sleep(1);

=== modified file 'examples/egltriangle.c'
--- examples/egltriangle.c	2014-06-09 17:16:32 +0000
+++ examples/egltriangle.c	2014-07-09 06:44:55 +0000
@@ -44,7 +44,7 @@
 }
 
 /* Colours from http://design.ubuntu.com/brand/colour-palette */
-#define MID_AUBERGINE(x) x*0.368627451f, x*0.152941176f, x*0.31372549f
+#define MID_AUBERGINE 0.368627451f, 0.152941176f, 0.31372549f
 #define ORANGE        0.866666667f, 0.282352941f, 0.141414141f
 
 int main(int argc, char *argv[])
@@ -105,8 +105,7 @@
         return 2;
     }
 
-    float const opacity = mir_eglapp_background_opacity;
-    glClearColor(MID_AUBERGINE(opacity), opacity);
+    glClearColor(MID_AUBERGINE, mir_eglapp_background_opacity);
     glViewport(0, 0, width, height);
 
     glUseProgram(prog);

=== modified file 'src/server/compositor/gl_renderer.cpp'
--- src/server/compositor/gl_renderer.cpp	2014-06-18 12:50:31 +0000
+++ src/server/compositor/gl_renderer.cpp	2014-07-09 06:44:55 +0000
@@ -64,7 +64,7 @@
     "varying vec2 v_texcoord;\n"
     "void main() {\n"
     "   vec4 frag = texture2D(tex, v_texcoord);\n"
-    "   gl_FragColor = alpha*frag;\n"
+    "   gl_FragColor = vec4(frag.xyz, frag.a * alpha);\n"
     "}\n"
 };
 }
@@ -125,7 +125,7 @@
     if (renderable.shaped() || renderable.alpha() < 1.0f)
     {
         glEnable(GL_BLEND);
-        glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA);
+        glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
     }
     else
     {

=== modified file 'tests/unit-tests/compositor/test_gl_renderer.cpp'
--- tests/unit-tests/compositor/test_gl_renderer.cpp	2014-06-18 12:50:31 +0000
+++ tests/unit-tests/compositor/test_gl_renderer.cpp	2014-07-09 06:44:55 +0000
@@ -183,7 +183,7 @@
     EXPECT_CALL(*renderable, shaped())
         .WillOnce(Return(true));
     EXPECT_CALL(mock_gl, glEnable(GL_BLEND));
-    EXPECT_CALL(mock_gl, glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA));
+    EXPECT_CALL(mock_gl, glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA));
     EXPECT_CALL(mock_gl, glActiveTexture(GL_TEXTURE0));
 
     EXPECT_CALL(mock_gl, glUniform2f(centre_uniform_location, _, _));