From 2027f7a739f77478e57ca3c3790f630103c23bfb Mon Sep 17 00:00:00 2001
From: aspioupiou <stephane@sbtech.fr>
Date: Thu, 30 Apr 2020 18:52:19 +0200
Subject: [PATCH] Refactor and comments

---
 .../Platform/WindowlessEglApplication.cpp     | 29 +++++++++++--------
 .../Platform/WindowlessEglApplication.h       | 29 ++++++++++++-------
 2 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/src/Magnum/Platform/WindowlessEglApplication.cpp b/src/Magnum/Platform/WindowlessEglApplication.cpp
index 5d1ec933d..dba0a81a0 100644
--- a/src/Magnum/Platform/WindowlessEglApplication.cpp
+++ b/src/Magnum/Platform/WindowlessEglApplication.cpp
@@ -84,6 +84,10 @@ bool extensionSupported(const char* const extensions, Containers::ArrayView<cons
 WindowlessEglContext::WindowlessEglContext(const Configuration& configuration, GLContext* const magnumContext) {
     #ifndef MAGNUM_TARGET_WEBGL
     
+    /** early check to avoid shared EGL context issues */
+    CORRADE_CONSTEXPR_ASSERT((configuration.sharedContext() == EGL_NO_CONTEXT) == (configuration.sharedDisplay() == EGL_NO_DISPLAY),
+	"Platform::WindowlessEglContext::Configuration::setSharedContext(): either both context have to be valid or both invalid.");
+	
 	/**
 	 * The user provided a shared context and its associated display, so we don't query the display
 	 */
@@ -93,14 +97,6 @@ WindowlessEglContext::WindowlessEglContext(const Configuration& configuration, G
    		_display = configuration.sharedDisplay();
    	    _sharedContext = true; 
    	}
-   	/**
-     * The user provided only the shared context or display, or provided both but one of them is wrong
-     */
-    else if (configuration.sharedContext() == EGL_NO_CONTEXT && configuration.sharedDisplay() != EGL_NO_DISPLAY ||
-             configuration.sharedContext() != EGL_NO_CONTEXT && configuration.sharedDisplay() == EGL_NO_DISPLAY)
-   {
-     	Error{} << "Platform::WindowlessEglContext: Either context or display is not valid. Context is " << configuration.sharedContext() << " Display is " << configuration.sharedDisplay();  	    
-   }   
      
     /* If relevant extensions are supported, try to find some display using
        those APIs, as that works reliably also when running headless. This
@@ -109,7 +105,7 @@ WindowlessEglContext::WindowlessEglContext(const Configuration& configuration, G
        we'd need to have a code path for 1.4 *and* 1.5, plus do complicated
        version parsing from a string. Not feeling like doing that today, no. */
     const char* const extensions = eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS);
-    if(extensions &&
+    if(!_sharedContext && extensions &&
         /* eglQueryDevicesEXT(). NVidia exposes only EGL_EXT_device_base, which
            is an older version of EGL_EXT_device_enumeration before it got
            split to that and EGL_EXT_device_query, so test for both. */
@@ -169,14 +165,15 @@ WindowlessEglContext::WindowlessEglContext(const Configuration& configuration, G
         Containers::Array<EGLDeviceEXT> devices{configuration.device() + 1};
         CORRADE_INTERNAL_ASSERT_OUTPUT(eglQueryDevices(configuration.device() + 1, devices, &count));
 
-   	   if(!_sharedContext && !(_display = reinterpret_cast<EGLDisplay(*)(EGLenum, void*, const EGLint*)>(eglGetProcAddress("eglGetPlatformDisplayEXT"))(EGL_PLATFORM_DEVICE_EXT, devices[configuration.device()], nullptr))) {
+   	   if(!(_display = reinterpret_cast<EGLDisplay(*)(EGLenum, void*, const EGLint*)>(eglGetProcAddress("eglGetPlatformDisplayEXT"))(EGL_PLATFORM_DEVICE_EXT, devices[configuration.device()], nullptr))) {
             Error{} << "Platform::WindowlessEglApplication::tryCreateContext(): cannot get platform display for a device:" << Implementation::eglErrorString(eglGetError());
             return;
         }
-    } else
-    #endif
+    } 
+    else if (!_sharedContext)    
     /* Otherwise initialize the classic way. WebGL doesn't have any of the
        above, so no need to compile that at all. */
+    #endif
     {
         #ifndef MAGNUM_TARGET_WEBGL
         if(configuration.device() != 0) {
@@ -422,6 +419,14 @@ bool WindowlessEglApplication::tryCreateContext(const Configuration& configurati
     return true;
 }
 
+WindowlessEglContext::Configuration& WindowlessEglContext::Configuration::setSharedContext(EGLDisplay display, EGLContext context) {
+	CORRADE_ASSERT((context == EGL_NO_CONTEXT) == (display == EGL_NO_DISPLAY),
+	"Platform::WindowlessEglContext::Configuration::setSharedContext(): either both context have to be valid or both invalid.", *this);
+     _sharedContext = context;
+     _sharedDisplay = display;
+     return *this;
+}
+
 WindowlessEglApplication::~WindowlessEglApplication() = default;
 
 }}
diff --git a/src/Magnum/Platform/WindowlessEglApplication.h b/src/Magnum/Platform/WindowlessEglApplication.h
index d7eb217d3..1b61a421b 100644
--- a/src/Magnum/Platform/WindowlessEglApplication.h
+++ b/src/Magnum/Platform/WindowlessEglApplication.h
@@ -288,35 +288,33 @@ class WindowlessEglContext::Configuration {
          *      @ref WindowlessEglApplication::glContext()
          * @requires_gles Context sharing is not available in WebGL.
          */
-        Configuration& setSharedContext(EGLContext context, EGLDisplay display) {
-            _sharedContext = context;
-            _sharedDisplay = display;
-            return *this;
-        }
+        Configuration& setSharedContext(EGLDisplay display, EGLContext context);
 
         /**
-         * @brief Shared context
+         * @brief Shared display
          * @m_since_latest
          *
          * @requires_gles Context sharing is not available in WebGL.
          */
-        EGLContext sharedContext() const { return _sharedContext; }
+        EGLContext sharedDisplay() const { return _sharedDisplay; } 
         
         /**
-         * @brief Shared display
+         * @brief Shared context
          * @m_since_latest
          *
          * @requires_gles Context sharing is not available in WebGL.
          */
-        EGLContext sharedDisplay() const { return _sharedDisplay; } 
+        EGLContext sharedContext() const { return _sharedContext; }
+        
+
         #endif
 
     private:
         #ifndef MAGNUM_TARGET_WEBGL
         Flags _flags;
         UnsignedInt _device;
-        EGLContext _sharedContext = EGL_NO_CONTEXT;
         EGLDisplay _sharedDisplay = EGL_NO_DISPLAY;
+        EGLContext _sharedContext = EGL_NO_CONTEXT;
         #endif
 };
 
@@ -451,6 +449,17 @@ filter named devices, so the best you can do is checking reported device count
 printed by the `--magnum-log verbose` @ref GL-Context-command-line "command-line option",
 and then going from `0` up to figure out the desired device ID.
 
+@section Platform-WindowlessEglApplication-device-selection Shared context setup
+EGL shared context handling is not the same than GLX or WGL. On these, providing the context handle and a unique display context
+(deviceContext or  GLXDrawable on windows and linux respectively) created by the WindowlessEglApplication is sufficient to create, 
+manage, and destroy the shared OpenGL context. EGL behaves differently, and returns the same EGLDisplay when querying twice for a display.
+This does not make a problem for single context setup, but creating a shared context with the same EGLDisplay than the parent one can be 
+dangerous, especially if you accidentally destroy the Display whereas someone still needs it. To avoid this, WindowlessEglApplication is guarded
+and won't kill the EGLDisplay if you create a shared context.
+Moreover, you must provide the EGLDisplay as well since we can't be sure the EGLContext is current (so we can't query the current display) and 
+and eglMakeCurrent() must receive the current EGLDisplay.
+
+
 @m_class{m-block m-danger}
 
 @par No EGL devices found