Re: [Mesa-dev] [PATCH 13/14] egl: Track EGL_KHR_debug state when going through EGL API calls (v3)

[Adam Jackson]

On Fri, 2016-09-16 at 18:33 +0100, Emil Velikov wrote:

> But regardless, this and v2 14/14 look a lot better and are
>
> Reviewed-by: Emil Velikov 

Merged, thanks.

- ajax
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 13/14] egl: Track EGL_KHR_debug state when going through EGL API calls (v3)

[Emil Velikov]

On 14 September 2016 at 14:59, Adam Jackson  wrote:
> From: Kyle Brenneman 
>
> This decorates every EGL entrypoint with _EGL_FUNC_START, which records
> the function name and primary dispatch object label in the current
> thread state. It also adds debug report functions and calls them when
> appropriate.
>
> This would be useful enough for debugging on its own, if the user set a
> breakpoint when the report function was called. We will also need this
> state tracked in order to expose EGL_KHR_debug.
>
> v2:
> - Clear the object label in more cases in _eglSetFuncName
> - Pass draw surface (if any) to _EGL_FUNC_START in eglSwapInterval
>
> v3:
> - Set dummy thread's CurrentAPI to EGL_OPENGL_ES_API not zero
Maybe (only maybe) we want this as a one-line fix for stable.

But regardless, this and v2 14/14 look a lot better and are
Reviewed-by: Emil Velikov 

Thanks for addressing all the comment Adam !
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 13/14] egl: Track EGL_KHR_debug state when going through EGL API calls (v3)

[Kyle Brenneman]

This looks right to me.

-Kyle

On 09/14/2016 07:59 AM, Adam Jackson wrote:

From: Kyle Brenneman 

This decorates every EGL entrypoint with _EGL_FUNC_START, which records
the function name and primary dispatch object label in the current
thread state. It also adds debug report functions and calls them when
appropriate.

This would be useful enough for debugging on its own, if the user set a
breakpoint when the report function was called. We will also need this
state tracked in order to expose EGL_KHR_debug.

v2:
- Clear the object label in more cases in _eglSetFuncName
- Pass draw surface (if any) to _EGL_FUNC_START in eglSwapInterval

v3:
- Set dummy thread's CurrentAPI to EGL_OPENGL_ES_API not zero
- Less ?: in _eglSetFuncName
---
  src/egl/main/eglapi.c | 153 +++---
  src/egl/main/eglcurrent.c |  91 ++-
  src/egl/main/eglcurrent.h |  22 +++
  src/egl/main/eglglobals.h |   5 ++
  4 files changed, 258 insertions(+), 13 deletions(-)

diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index 1c62a80..cbc3841 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -250,6 +250,37 @@ _eglUnlockDisplay(_EGLDisplay *dpy)
 mtx_unlock(>Mutex);
  }
  
+static EGLBoolean

+_eglSetFuncName(const char *funcName, _EGLDisplay *disp, EGLenum objectType, 
_EGLResource *object)
+{
+   _EGLThreadInfo *thr = _eglGetCurrentThread();
+   if (!_eglIsCurrentThreadDummy()) {
+  thr->CurrentFuncName = funcName;
+  thr->CurrentObjectLabel = NULL;
+
+  if (objectType == EGL_OBJECT_THREAD_KHR)
+ thr->CurrentObjectLabel = thr->Label;
+  else if (objectType == EGL_OBJECT_DISPLAY_KHR && disp)
+ thr->CurrentObjectLabel = disp->Label;
+  else if (object)
+ thr->CurrentObjectLabel = object->Label;
+
+  return EGL_TRUE;
+   }
+
+   _eglDebugReportFull(EGL_BAD_ALLOC, funcName, funcName,
+  EGL_DEBUG_MSG_CRITICAL_KHR, NULL, NULL);
+   return EGL_FALSE;
+}
+
+#define _EGL_FUNC_START(disp, objectType, object, ret) \
+   do { \
+  if (!_eglSetFuncName(__func__, disp, objectType, (_EGLResource *) 
object)) { \
+ if (disp) \
+_eglUnlockDisplay(disp);   \
+ return ret; \
+  } \
+   } while(0)
  
  static EGLint *

  _eglConvertAttribsToInt(const EGLAttrib *attr_list)
@@ -287,6 +318,8 @@ eglGetDisplay(EGLNativeDisplayType nativeDisplay)
 _EGLDisplay *dpy;
 void *native_display_ptr;
  
+   _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);

+
 STATIC_ASSERT(sizeof(void*) == sizeof(nativeDisplay));
 native_display_ptr = (void*) nativeDisplay;
  
@@ -330,6 +363,7 @@ static EGLDisplay EGLAPIENTRY

  eglGetPlatformDisplayEXT(EGLenum platform, void *native_display,
   const EGLint *attrib_list)
  {
+   _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);
 return _eglGetPlatformDisplayCommon(platform, native_display, attrib_list);
  }
  
@@ -340,6 +374,8 @@ eglGetPlatformDisplay(EGLenum platform, void *native_display,

 EGLDisplay display;
 EGLint *int_attribs;
  
+   _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);

+
 int_attribs = _eglConvertAttribsToInt(attrib_list);
 if (attrib_list && !int_attribs)
RETURN_EGL_ERROR(NULL, EGL_BAD_ALLOC, NULL);
@@ -483,6 +519,8 @@ eglInitialize(EGLDisplay dpy, EGLint *major, EGLint *minor)
  {
 _EGLDisplay *disp = _eglLockDisplay(dpy);
  
+   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, EGL_FALSE);

+
 if (!disp)
RETURN_EGL_ERROR(NULL, EGL_BAD_DISPLAY, EGL_FALSE);
  
@@ -533,6 +571,8 @@ eglTerminate(EGLDisplay dpy)

  {
 _EGLDisplay *disp = _eglLockDisplay(dpy);
  
+   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, EGL_FALSE);

+
 if (!disp)
RETURN_EGL_ERROR(NULL, EGL_BAD_DISPLAY, EGL_FALSE);
  
@@ -560,6 +600,7 @@ eglQueryString(EGLDisplay dpy, EGLint name)

 }
  
 disp = _eglLockDisplay(dpy);

+   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, NULL);
 _EGL_CHECK_DISPLAY(disp, NULL, drv);
  
 switch (name) {

@@ -585,6 +626,8 @@ eglGetConfigs(EGLDisplay dpy, EGLConfig *configs,
 _EGLDriver *drv;
 EGLBoolean ret;
  
+   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, EGL_FALSE);

+
 _EGL_CHECK_DISPLAY(disp, EGL_FALSE, drv);
 ret = drv->API.GetConfigs(drv, disp, configs, config_size, num_config);
  
@@ -600,6 +643,8 @@ eglChooseConfig(EGLDisplay dpy, const EGLint *attrib_list, EGLConfig *configs,

 _EGLDriver *drv;
 EGLBoolean ret;
  
+   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, EGL_FALSE);

+
 _EGL_CHECK_DISPLAY(disp, EGL_FALSE, drv);
 ret = drv->API.ChooseConfig(drv, disp, attrib_list, configs,
  config_size, num_config);
@@ -617,6 +662,8 @@ eglGetConfigAttrib(EGLDisplay dpy, EGLConfig config,
 

[Mesa-dev] [PATCH 13/14] egl: Track EGL_KHR_debug state when going through EGL API calls (v3)

[Adam Jackson]

From: Kyle Brenneman 

This decorates every EGL entrypoint with _EGL_FUNC_START, which records
the function name and primary dispatch object label in the current
thread state. It also adds debug report functions and calls them when
appropriate.

This would be useful enough for debugging on its own, if the user set a
breakpoint when the report function was called. We will also need this
state tracked in order to expose EGL_KHR_debug.

v2:
- Clear the object label in more cases in _eglSetFuncName
- Pass draw surface (if any) to _EGL_FUNC_START in eglSwapInterval

v3:
- Set dummy thread's CurrentAPI to EGL_OPENGL_ES_API not zero
- Less ?: in _eglSetFuncName
---
 src/egl/main/eglapi.c | 153 +++---
 src/egl/main/eglcurrent.c |  91 ++-
 src/egl/main/eglcurrent.h |  22 +++
 src/egl/main/eglglobals.h |   5 ++
 4 files changed, 258 insertions(+), 13 deletions(-)

diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index 1c62a80..cbc3841 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -250,6 +250,37 @@ _eglUnlockDisplay(_EGLDisplay *dpy)
mtx_unlock(>Mutex);
 }
 
+static EGLBoolean
+_eglSetFuncName(const char *funcName, _EGLDisplay *disp, EGLenum objectType, 
_EGLResource *object)
+{
+   _EGLThreadInfo *thr = _eglGetCurrentThread();
+   if (!_eglIsCurrentThreadDummy()) {
+  thr->CurrentFuncName = funcName;
+  thr->CurrentObjectLabel = NULL;
+
+  if (objectType == EGL_OBJECT_THREAD_KHR)
+ thr->CurrentObjectLabel = thr->Label;
+  else if (objectType == EGL_OBJECT_DISPLAY_KHR && disp)
+ thr->CurrentObjectLabel = disp->Label;
+  else if (object)
+ thr->CurrentObjectLabel = object->Label;
+
+  return EGL_TRUE;
+   }
+
+   _eglDebugReportFull(EGL_BAD_ALLOC, funcName, funcName,
+  EGL_DEBUG_MSG_CRITICAL_KHR, NULL, NULL);
+   return EGL_FALSE;
+}
+
+#define _EGL_FUNC_START(disp, objectType, object, ret) \
+   do { \
+  if (!_eglSetFuncName(__func__, disp, objectType, (_EGLResource *) 
object)) { \
+ if (disp) \
+_eglUnlockDisplay(disp);   \
+ return ret; \
+  } \
+   } while(0)
 
 static EGLint *
 _eglConvertAttribsToInt(const EGLAttrib *attr_list)
@@ -287,6 +318,8 @@ eglGetDisplay(EGLNativeDisplayType nativeDisplay)
_EGLDisplay *dpy;
void *native_display_ptr;
 
+   _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);
+
STATIC_ASSERT(sizeof(void*) == sizeof(nativeDisplay));
native_display_ptr = (void*) nativeDisplay;
 
@@ -330,6 +363,7 @@ static EGLDisplay EGLAPIENTRY
 eglGetPlatformDisplayEXT(EGLenum platform, void *native_display,
  const EGLint *attrib_list)
 {
+   _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);
return _eglGetPlatformDisplayCommon(platform, native_display, attrib_list);
 }
 
@@ -340,6 +374,8 @@ eglGetPlatformDisplay(EGLenum platform, void 
*native_display,
EGLDisplay display;
EGLint *int_attribs;
 
+   _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);
+
int_attribs = _eglConvertAttribsToInt(attrib_list);
if (attrib_list && !int_attribs)
   RETURN_EGL_ERROR(NULL, EGL_BAD_ALLOC, NULL);
@@ -483,6 +519,8 @@ eglInitialize(EGLDisplay dpy, EGLint *major, EGLint *minor)
 {
_EGLDisplay *disp = _eglLockDisplay(dpy);
 
+   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, EGL_FALSE);
+
if (!disp)
   RETURN_EGL_ERROR(NULL, EGL_BAD_DISPLAY, EGL_FALSE);
 
@@ -533,6 +571,8 @@ eglTerminate(EGLDisplay dpy)
 {
_EGLDisplay *disp = _eglLockDisplay(dpy);
 
+   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, EGL_FALSE);
+
if (!disp)
   RETURN_EGL_ERROR(NULL, EGL_BAD_DISPLAY, EGL_FALSE);
 
@@ -560,6 +600,7 @@ eglQueryString(EGLDisplay dpy, EGLint name)
}
 
disp = _eglLockDisplay(dpy);
+   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, NULL);
_EGL_CHECK_DISPLAY(disp, NULL, drv);
 
switch (name) {
@@ -585,6 +626,8 @@ eglGetConfigs(EGLDisplay dpy, EGLConfig *configs,
_EGLDriver *drv;
EGLBoolean ret;
 
+   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, EGL_FALSE);
+
_EGL_CHECK_DISPLAY(disp, EGL_FALSE, drv);
ret = drv->API.GetConfigs(drv, disp, configs, config_size, num_config);
 
@@ -600,6 +643,8 @@ eglChooseConfig(EGLDisplay dpy, const EGLint *attrib_list, 
EGLConfig *configs,
_EGLDriver *drv;
EGLBoolean ret;
 
+   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, EGL_FALSE);
+
_EGL_CHECK_DISPLAY(disp, EGL_FALSE, drv);
ret = drv->API.ChooseConfig(drv, disp, attrib_list, configs,
 config_size, num_config);
@@ -617,6 +662,8 @@ eglGetConfigAttrib(EGLDisplay dpy, EGLConfig config,
_EGLDriver *drv;
EGLBoolean ret;
 
+   _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL, EGL_FALSE);
+
_EGL_CHECK_CONFIG(disp, conf,