Merge lp:~cemil-azizoglu/mir/fix-lp1589632 into lp:mir

[Cemil Azizoglu]

Read depth and Stencil values from the GLConfig as in other platforms

Fixes lp#1589632

Read depth and Stencil values from the GLConfig as in other platforms

Fixes lp#1589632

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3532
https://mir-jenkins.ubuntu.com/job/mir-ci/1128/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/1250
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1300
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1291
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1291
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1260
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1260/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1260
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1260/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1260
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1260/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1260
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1260/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1260
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1260/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1128/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

Should we have a unit test?

+ std::shared_ptr<GLConfig> const& gl_config)
could just be a reference, we don't seem to take the shared_ptr.

94 +
unneeded addition?

review: Needs Fixing

[Cemil Azizoglu (cemil-azizoglu)]

> Should we have a unit test?
>
Fixed.

> + std::shared_ptr<GLConfig> const& gl_config)
> could just be a reference, we don't seem to take the shared_ptr.
Fixed.

>
> 94 +
> unneeded addition?

This is to match the blank line at the top of the class declaration.

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3533
https://mir-jenkins.ubuntu.com/job/mir-ci/1134/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/1258
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1308
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1299
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1299
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1268
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1268/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1268
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1268/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1268
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1268/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1268
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1268/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1268
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1268/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1134/rebuild

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

> > 94 +
> > unneeded addition?
>
> This is to match the blank line at the top of the class declaration.

Is this explanation a thinko? The line is added at the end of the X namespace, not the end of any class. (I don't see a need for it in either case.)

[Kevin DuBois (kdub)]

Still don't think the line is necessary, but primary topics addressed, so lgtm

review: Approve

[Mir CI Bot (mir-ci-bot)]

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/315/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/1287/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/341/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1337
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1328
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1328
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1297
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1297/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1297/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1297
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1297/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1297
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1297/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1297
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1297/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)

[Cemil Azizoglu (cemil-azizoglu)]

Failure appears unrelated. Filed lp:1591354

[Mir CI Bot (mir-ci-bot)]

review: Approve (continuous-integration)

=== modified file 'src/platforms/mesa/server/x11/graphics/display.cpp'
--- src/platforms/mesa/server/x11/graphics/display.cpp	2016-06-02 05:33:50 +0000
+++ src/platforms/mesa/server/x11/graphics/display.cpp	2016-06-09 14:11:22 +0000
@@ -21,6 +21,7 @@
 #include "mir/graphics/egl_resources.h"
 #include "mir/graphics/egl_error.h"
 #include "mir/graphics/virtual_output.h"
+#include "mir/graphics/gl_config.h"
 #include "display_configuration.h"
 #include "display.h"
 #include "display_buffer.h"
@@ -60,7 +61,10 @@
     return egl_dpy;
 }
 
-mgx::X11Window::X11Window(::Display* x_dpy, EGLDisplay egl_dpy, geom::Size const size)
+mgx::X11Window::X11Window(::Display* x_dpy,
+                          EGLDisplay egl_dpy,
+                          geom::Size const size,
+                          GLConfig const& gl_config)
     : x_dpy{x_dpy}
 {
     EGLint const att[] = {
@@ -69,8 +73,8 @@
         EGL_GREEN_SIZE, 8,
         EGL_BLUE_SIZE, 8,
         EGL_ALPHA_SIZE, 8,
-        EGL_DEPTH_SIZE, 0,
-        EGL_STENCIL_SIZE, 0,
+        EGL_DEPTH_SIZE, gl_config.depth_buffer_bits(),
+        EGL_STENCIL_SIZE, gl_config.stencil_buffer_bits(),
         EGL_RENDERABLE_TYPE, MIR_SERVER_EGL_OPENGL_BIT,
         EGL_NONE
     };
@@ -242,12 +246,15 @@
     return egl_surf;
 }
 
-mgx::Display::Display(::Display* x_dpy, geom::Size const size)
+mgx::Display::Display(::Display* x_dpy,
+                      geom::Size const size,
+                      GLConfig const& gl_config)
     : egl_display{X11EGLDisplay(x_dpy)},
       size{size},
       win{X11Window(x_dpy,
                     egl_display,
-                    size)},
+                    size,
+                    gl_config)},
       egl_context{X11EGLContext(egl_display,
                                 win.egl_config())},
       egl_surface{X11EGLSurface(egl_display,

=== modified file 'src/platforms/mesa/server/x11/graphics/display.h'
--- src/platforms/mesa/server/x11/graphics/display.h	2016-04-22 02:39:57 +0000
+++ src/platforms/mesa/server/x11/graphics/display.h	2016-06-09 14:11:22 +0000
@@ -32,6 +32,9 @@
 {
 namespace graphics
 {
+
+class GLConfig;
+
 namespace X
 {
 
@@ -50,7 +53,10 @@
 class X11Window
 {
 public:
-    X11Window(::Display* const x_dpy, EGLDisplay egl_dpy, mir::geometry::Size const size);
+    X11Window(::Display* const x_dpy,
+              EGLDisplay egl_dpy,
+              geometry::Size const size,
+              GLConfig const& gl_config);
     ~X11Window();
 
     operator Window() const;
@@ -93,7 +99,9 @@
 class Display : public graphics::Display
 {
 public:
-    explicit Display(::Display* x_dpy, mir::geometry::Size const size);
+    explicit Display(::Display* x_dpy,
+                     geometry::Size const size,
+                     GLConfig const& gl_config);
     ~Display() noexcept;
 
     void for_each_display_sync_group(std::function<void(graphics::DisplaySyncGroup&)> const& f) override;
@@ -129,6 +137,7 @@
 };
 
 }
+
 }
 }
 #endif /* MIR_GRAPHICS_X_DISPLAY_H_ */

=== modified file 'src/platforms/mesa/server/x11/graphics/platform.cpp'
--- src/platforms/mesa/server/x11/graphics/platform.cpp	2016-01-29 08:18:22 +0000
+++ src/platforms/mesa/server/x11/graphics/platform.cpp	2016-06-09 14:11:22 +0000
@@ -46,9 +46,9 @@
 
 mir::UniqueModulePtr<mg::Display> mgx::Platform::create_display(
     std::shared_ptr<DisplayConfigurationPolicy> const& /*initial_conf_policy*/,
-    std::shared_ptr<GLConfig> const& /*gl_config*/)
+    std::shared_ptr<GLConfig> const& gl_config)
 {
-    return make_module_ptr<mgx::Display>(x11_connection.get(), size);
+    return make_module_ptr<mgx::Display>(x11_connection.get(), size, *gl_config);
 }
 
 mir::UniqueModulePtr<mg::PlatformIpcOperations> mgx::Platform::make_ipc_operations() const

=== modified file 'tests/unit-tests/graphics/mesa/x11/test_display.cpp'
--- tests/unit-tests/graphics/mesa/x11/test_display.cpp	2016-01-29 08:18:22 +0000
+++ tests/unit-tests/graphics/mesa/x11/test_display.cpp	2016-06-09 14:11:22 +0000
@@ -22,6 +22,7 @@
 #include "src/platforms/mesa/server/x11/graphics/display.h"
 #include "mir/test/doubles/mock_egl.h"
 #include "mir/test/doubles/mock_x11.h"
+#include "mir/test/doubles/mock_gl_config.h"
 
 namespace mg=mir::graphics;
 namespace mgx=mg::X;
@@ -78,11 +79,15 @@
 
     std::shared_ptr<mgx::Display> create_display()
     {
-        return std::make_shared<mgx::Display>(mock_x11.fake_x11.display, size);
+        return std::make_shared<mgx::Display>(
+                   mock_x11.fake_x11.display,
+                   size,
+                   mock_gl_config);
     }
 
     ::testing::NiceMock<mtd::MockEGL> mock_egl;
     ::testing::NiceMock<mtd::MockX11> mock_x11;
+    mtd::MockGLConfig mock_gl_config;
 };
 
 }
@@ -111,3 +116,28 @@
 
     auto display = create_display();
 }
+
+TEST_F(X11DisplayTest, respects_gl_config)
+{
+    using namespace testing;
+
+    EGLint const depth_bits{24};
+    EGLint const stencil_bits{8};
+
+    EXPECT_CALL(mock_gl_config, depth_buffer_bits())
+        .Times(AtLeast(1))
+        .WillRepeatedly(Return(depth_bits));
+    EXPECT_CALL(mock_gl_config, stencil_buffer_bits())
+        .Times(AtLeast(1))
+        .WillRepeatedly(Return(stencil_bits));
+
+    EXPECT_CALL(mock_egl,
+                eglChooseConfig(
+                    _,
+                    AllOf(mtd::EGLConfigContainsAttrib(EGL_DEPTH_SIZE, depth_bits),
+                          mtd::EGLConfigContainsAttrib(EGL_STENCIL_SIZE, stencil_bits)),
+                    _,_,_))
+        .Times(AtLeast(1));
+
+    auto display = create_display();
+}