[Mesa-dev,11/15] egl: add eglGetSyncAttrib

Submitted by Marek Olšák on May 12, 2015, 10:54 p.m.

Details

Message ID 1431471290-7299-12-git-send-email-maraeo@gmail.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Marek Olšák May 12, 2015, 10:54 p.m.
From: Marek Olšák <marek.olsak@amd.com>

---
 src/egl/main/eglapi.c  | 14 +++++++++++++-
 src/egl/main/eglapi.h  |  2 +-
 src/egl/main/eglsync.c |  2 +-
 src/egl/main/eglsync.h |  2 +-
 4 files changed, 16 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index 544f7e4..6457798 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -1161,6 +1161,7 @@  eglGetProcAddress(const char *procname)
       { "eglCreateSync", (_EGLProc) eglCreateSync },
       { "eglDestroySync", (_EGLProc) eglDestroySync },
       { "eglClientWaitSync", (_EGLProc) eglClientWaitSync },
+      { "eglGetSyncAttrib", (_EGLProc) eglGetSyncAttrib },
       { "eglDestroyImage", (_EGLProc) eglDestroyImage },
       { "eglWaitSync", (_EGLProc) eglWaitSync },
 #endif /* _EGL_GET_CORE_ADDRESSES */
@@ -1542,7 +1543,7 @@  eglSignalSyncKHR(EGLDisplay dpy, EGLSync sync, EGLenum mode)
 
 
 EGLBoolean EGLAPIENTRY
-eglGetSyncAttribKHR(EGLDisplay dpy, EGLSync sync, EGLint attribute, EGLint *value)
+eglGetSyncAttrib(EGLDisplay dpy, EGLSync sync, EGLint attribute, EGLAttrib *value)
 {
    _EGLDisplay *disp = _eglLockDisplay(dpy);
    _EGLSync *s = _eglLookupSync(sync, disp);
@@ -1558,6 +1559,17 @@  eglGetSyncAttribKHR(EGLDisplay dpy, EGLSync sync, EGLint attribute, EGLint *valu
 }
 
 
+EGLBoolean EGLAPIENTRY
+eglGetSyncAttribKHR(EGLDisplay dpy, EGLSync sync, EGLint attribute, EGLint *value)
+{
+   EGLAttrib attrib = *value;
+   EGLBoolean result = eglGetSyncAttrib(dpy, sync, attribute, &attrib);
+
+   *value = attrib;
+   return result;
+}
+
+
 #ifdef EGL_NOK_swap_region
 
 EGLBoolean EGLAPIENTRY
diff --git a/src/egl/main/eglapi.h b/src/egl/main/eglapi.h
index 44f589d..fdd3b05 100644
--- a/src/egl/main/eglapi.h
+++ b/src/egl/main/eglapi.h
@@ -91,7 +91,7 @@  typedef EGLBoolean (*DestroySyncKHR_t)(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSy
 typedef EGLint (*ClientWaitSyncKHR_t)(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync, EGLint flags, EGLTime timeout);
 typedef EGLint (*WaitSyncKHR_t)(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync);
 typedef EGLBoolean (*SignalSyncKHR_t)(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync, EGLenum mode);
-typedef EGLBoolean (*GetSyncAttribKHR_t)(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync, EGLint attribute, EGLint *value);
+typedef EGLBoolean (*GetSyncAttribKHR_t)(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync, EGLint attribute, EGLAttrib *value);
 
 
 #ifdef EGL_NOK_swap_region
diff --git a/src/egl/main/eglsync.c b/src/egl/main/eglsync.c
index 205cdc0..a09d991 100644
--- a/src/egl/main/eglsync.c
+++ b/src/egl/main/eglsync.c
@@ -142,7 +142,7 @@  _eglInitSync(_EGLSync *sync, _EGLDisplay *dpy, EGLenum type,
 
 EGLBoolean
 _eglGetSyncAttribKHR(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync,
-                     EGLint attribute, EGLint *value)
+                     EGLint attribute, EGLAttrib *value)
 {
    if (!value)
       return _eglError(EGL_BAD_PARAMETER, "eglGetSyncAttribKHR");
diff --git a/src/egl/main/eglsync.h b/src/egl/main/eglsync.h
index 4959cf0..2cefcca 100644
--- a/src/egl/main/eglsync.h
+++ b/src/egl/main/eglsync.h
@@ -58,7 +58,7 @@  _eglInitSync(_EGLSync *sync, _EGLDisplay *dpy, EGLenum type,
 
 extern EGLBoolean
 _eglGetSyncAttribKHR(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync,
-                     EGLint attribute, EGLint *value);
+                     EGLint attribute, EGLAttrib *value);
 
 
 /**

Comments

On Wed 13 May 2015, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak@amd.com>
> 
> ---
>  src/egl/main/eglapi.c  | 14 +++++++++++++-
>  src/egl/main/eglapi.h  |  2 +-
>  src/egl/main/eglsync.c |  2 +-
>  src/egl/main/eglsync.h |  2 +-
>  4 files changed, 16 insertions(+), 4 deletions(-)
>

I see two issues below.

> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> index 544f7e4..6457798 100644
> --- a/src/egl/main/eglapi.c
> +++ b/src/egl/main/eglapi.c


> @@ -1558,6 +1559,17 @@ eglGetSyncAttribKHR(EGLDisplay dpy, EGLSync sync, EGLint attribute, EGLint *valu
>  }
>  
>  
> +EGLBoolean EGLAPIENTRY
> +eglGetSyncAttribKHR(EGLDisplay dpy, EGLSync sync, EGLint attribute, EGLint *value)
> +{
> +   EGLAttrib attrib = *value;
> +   EGLBoolean result = eglGetSyncAttrib(dpy, sync, attribute, &attrib);
> +
> +   *value = attrib;
> +   return result;
> +}

This change violates the EGL_KHR_fence_sync spec, which says this
regarding eglGetSyncAttribKHR:

    If any error occurs, <*value> is not modified.

I think it's a good idea to add this quote to the function body so that
future devs don't accidentally make the same mistake.


> diff --git a/src/egl/main/eglapi.h b/src/egl/main/eglapi.h
> index 44f589d..fdd3b05 100644
> --- a/src/egl/main/eglapi.h
> +++ b/src/egl/main/eglapi.h
> @@ -91,7 +91,7 @@ typedef EGLBoolean (*DestroySyncKHR_t)(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSy
>  typedef EGLint (*ClientWaitSyncKHR_t)(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync, EGLint flags, EGLTime timeout);
>  typedef EGLint (*WaitSyncKHR_t)(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync);
>  typedef EGLBoolean (*SignalSyncKHR_t)(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync, EGLenum mode);
> -typedef EGLBoolean (*GetSyncAttribKHR_t)(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync, EGLint attribute, EGLint *value);
> +typedef EGLBoolean (*GetSyncAttribKHR_t)(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync, EGLint attribute, EGLAttrib *value);

I think changing this change is potentially dangerous, because
post-patch GetSyncAttribKHR_t has the signature of eglGetSyncAttrib and
*not* eglGetSyncAttribKHR. The code will compile, but this discrepancy
looks like a honeypot for accidental function misuse.

And...

>  #ifdef EGL_NOK_swap_region
> diff --git a/src/egl/main/eglsync.c b/src/egl/main/eglsync.c
> index 205cdc0..a09d991 100644
> --- a/src/egl/main/eglsync.c
> +++ b/src/egl/main/eglsync.c
> @@ -142,7 +142,7 @@ _eglInitSync(_EGLSync *sync, _EGLDisplay *dpy, EGLenum type,
>  
>  EGLBoolean
>  _eglGetSyncAttribKHR(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync,
> -                     EGLint attribute, EGLint *value)
> +                     EGLint attribute, EGLAttrib *value)

I have the same issue here. Let's avoid introducing confusing
discrepancies between function names and their signatures. Just name
this one and the typedef to '_eglGetSyncAttrib' and 'GetSyncAttrib_t'.