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

[Daniel van Vugt]

Avoid __attribute__((weak)) as that will fail to resolve symbols from
lazily loaded libraries (like Qt platform plugins apparently).
(LP: #1510218)

This implementation is written such that the tests introduced in r2771
still work. But if you want a new test specifically for RTLD_LAZY,
that's going to take a whole lot more work. I'm not sure it's worth it
as long as the initial tests from r2771 still work.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3064
http://jenkins.qa.ubuntu.com/job/mir-ci/5410/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4651
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3558
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4593/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/551/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1564
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1564/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/551
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/551/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4594
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4594/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7214/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24590
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/552/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5410/rebuild

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

code does not need to be inlined in a header

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3065
http://jenkins.qa.ubuntu.com/job/mir-ci/5413/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4654
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3561
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4596/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/554/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1567
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1567/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/554
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/554/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4597
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4597/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7217/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24595
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/555/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5413/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3066
http://jenkins.qa.ubuntu.com/job/mir-ci/5420/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4668
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3575
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4610
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/568/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1574
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1574/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/568
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/568/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4611
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4611/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7231
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24634
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/569/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5420/rebuild

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

Nit:

+namespace mir { namespace client {

in a .cpp isn't our "house style"

review: Approve

[Alberto Aguirre (albaguirre)]

Given that the API requires an EGLConfig parameter - which needs to be obtained from another call to libEGL, I guess it's not unreasonable to expect the caller of mir_connection_get_egl_pixel_format has linked to libEGL.

review: Approve

=== modified file 'src/platforms/android/client/android_client_platform.cpp'
--- src/platforms/android/client/android_client_platform.cpp	2015-07-21 04:30:03 +0000
+++ src/platforms/android/client/android_client_platform.cpp	2015-10-28 07:23:08 +0000
@@ -24,6 +24,7 @@
 #include "android_client_buffer_factory.h"
 #include "egl_native_surface_interpreter.h"
 
+#include "mir/weak_egl.h"
 #include <EGL/egl.h>
 
 #include <boost/throw_exception.hpp>
@@ -124,19 +125,6 @@
     return buf->anwb();
 }
 
-/*
- * Driver modules get dlopened with RTLD_NOW, meaning that if the below egl
- * functions aren't found in memory the driver fails to load. This would
- * normally prevent software clients (those not linked to libEGL) from
- * successfully loading our client module, but if we mark the undefined
- * egl function symbols as "weak" then their absence is no longer an error,
- * even with RTLD_NOW.
- */
-extern "C" EGLAPI EGLBoolean EGLAPIENTRY
-    eglGetConfigAttrib(EGLDisplay dpy, EGLConfig config,
-                       EGLint attribute, EGLint *value)
-    __attribute__((weak));
-
 MirPixelFormat mcla::AndroidClientPlatform::get_egl_pixel_format(
     EGLDisplay disp, EGLConfig conf) const
 {
@@ -144,7 +132,8 @@
 
     // EGL_KHR_platform_android says this will always work...
     EGLint vis = 0;
-    if (eglGetConfigAttrib(disp, conf, EGL_NATIVE_VISUAL_ID, &vis))
+    mcl::WeakEGL weak;
+    if (weak.eglGetConfigAttrib(disp, conf, EGL_NATIVE_VISUAL_ID, &vis))
         mir_format = mir::graphics::android::to_mir_format(vis);
 
     return mir_format;

=== modified file 'src/platforms/common/client/mir/CMakeLists.txt'
--- src/platforms/common/client/mir/CMakeLists.txt	2015-01-19 14:16:46 +0000
+++ src/platforms/common/client/mir/CMakeLists.txt	2015-10-28 07:23:08 +0000
@@ -2,4 +2,5 @@
 
 add_library(client_platform_common OBJECT
   aging_buffer.cpp
+  weak_egl.cpp
 )

=== added file 'src/platforms/common/client/mir/weak_egl.cpp'
--- src/platforms/common/client/mir/weak_egl.cpp	1970-01-01 00:00:00 +0000
+++ src/platforms/common/client/mir/weak_egl.cpp	2015-10-28 07:23:08 +0000
@@ -0,0 +1,65 @@
+/*
+ * EGL without any linkage requirements!
+ * ~~~
+ * Copyright © 2015 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Daniel van Vugt <[e-mail]>
+ */
+
+#include "weak_egl.h"
+#include <dlfcn.h>
+
+namespace mir { namespace client {
+
+WeakEGL::WeakEGL()
+    : egl1(nullptr)
+    , pGetConfigAttrib(nullptr)
+{
+}
+
+WeakEGL::~WeakEGL()
+{
+    if (egl1)
+        dlclose(egl1);
+}
+
+EGLBoolean WeakEGL::eglGetConfigAttrib(EGLDisplay dpy, EGLConfig config,
+                                       EGLint attribute, EGLint* value)
+{
+    if (find("eglGetConfigAttrib", (void**)&pGetConfigAttrib))
+        return pGetConfigAttrib(dpy, config, attribute, value);
+    else
+        return EGL_FALSE;
+}
+
+bool WeakEGL::find(char const* name, void** func)
+{
+    if (!*func)
+    {
+        // RTLD_DEFAULT is first choice to support wrappers like MockEGL
+        if (!(*func = dlsym(RTLD_DEFAULT, name)))
+        {
+            // This will work more in real-world situations if the library
+            // is hidden behind an RTLD_LOCAL (e.g. a Qt plugin)
+            if (!egl1)
+                egl1 = dlopen("libEGL.so.1", RTLD_NOLOAD|RTLD_LAZY);
+            if (egl1)
+                *func = dlsym(egl1, name);
+        }
+    }
+    return *func != nullptr;
+}
+
+}} // namespace mir::client

=== added file 'src/platforms/common/client/mir/weak_egl.h'
--- src/platforms/common/client/mir/weak_egl.h	1970-01-01 00:00:00 +0000
+++ src/platforms/common/client/mir/weak_egl.h	2015-10-28 07:23:08 +0000
@@ -0,0 +1,47 @@
+/*
+ * EGL without any linkage requirements!
+ * ~~~
+ * Copyright © 2015 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Daniel van Vugt <[e-mail]>
+ */
+
+#ifndef MIR_CLIENT_WEAK_EGL_H_
+#define MIR_CLIENT_WEAK_EGL_H_
+
+#include <EGL/egl.h>
+#include <dlfcn.h>
+
+namespace mir { namespace client {
+
+class WeakEGL
+{
+public:
+    WeakEGL();
+    ~WeakEGL();
+    EGLBoolean eglGetConfigAttrib(EGLDisplay dpy, EGLConfig config,
+                                  EGLint attribute, EGLint* value);
+
+private:
+    bool find(char const* name, void** func);
+
+    void* egl1;
+    EGLBoolean (*pGetConfigAttrib)(EGLDisplay dpy, EGLConfig config,
+                                   EGLint attribute, EGLint* value);
+};
+
+}} // namespace mir::client
+
+#endif // MIR_CLIENT_WEAK_EGL_H_

=== modified file 'src/platforms/mesa/client/client_platform.cpp'
--- src/platforms/mesa/client/client_platform.cpp	2015-10-19 12:10:38 +0000
+++ src/platforms/mesa/client/client_platform.cpp	2015-10-28 07:23:08 +0000
@@ -23,6 +23,7 @@
 #include "native_surface.h"
 #include "mir/client_buffer_factory.h"
 #include "mir/client_context.h"
+#include "mir/weak_egl.h"
 #include "mir_toolkit/mesa/platform_operation.h"
 
 #include <cstring>
@@ -163,19 +164,6 @@
 }
 
 
-/*
- * Driver modules get dlopened with RTLD_NOW, meaning that if the below egl
- * functions aren't found in memory the driver fails to load. This would
- * normally prevent software clients (those not linked to libEGL) from
- * successfully loading our client module, but if we mark the undefined
- * egl function symbols as "weak" then their absence is no longer an error,
- * even with RTLD_NOW.
- */
-extern "C" EGLAPI EGLBoolean EGLAPIENTRY
-    eglGetConfigAttrib(EGLDisplay dpy, EGLConfig config,
-                       EGLint attribute, EGLint *value)
-    __attribute__((weak));
-
 MirPixelFormat mclm::ClientPlatform::get_egl_pixel_format(
     EGLDisplay disp, EGLConfig conf) const
 {
@@ -192,10 +180,11 @@
      * EGL_NATIVE_VISUAL_ID, so ignore that for now.
      */
     EGLint r = 0, g = 0, b = 0, a = 0;
-    eglGetConfigAttrib(disp, conf, EGL_RED_SIZE, &r);
-    eglGetConfigAttrib(disp, conf, EGL_GREEN_SIZE, &g);
-    eglGetConfigAttrib(disp, conf, EGL_BLUE_SIZE, &b);
-    eglGetConfigAttrib(disp, conf, EGL_ALPHA_SIZE, &a);
+    mcl::WeakEGL weak;
+    weak.eglGetConfigAttrib(disp, conf, EGL_RED_SIZE, &r);
+    weak.eglGetConfigAttrib(disp, conf, EGL_GREEN_SIZE, &g);
+    weak.eglGetConfigAttrib(disp, conf, EGL_BLUE_SIZE, &b);
+    weak.eglGetConfigAttrib(disp, conf, EGL_ALPHA_SIZE, &a);
 
     if (r == 8 && g == 8 && b == 8)
     {