<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
On 09/09/2016 04:27 AM, Emil Velikov wrote:<br>
<blockquote
cite="mid:CACvgo52zRf7S_v02YKsfDEvChPTB=GaBEuy1JdTf91isFTFrvQ@mail.gmail.com"
type="cite">
<pre wrap="">On 8 September 2016 at 18:46, Adam Jackson <a class="moz-txt-link-rfc2396E" href="mailto:ajax@redhat.com"><ajax@redhat.com></a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="">From: Kyle Brenneman <a class="moz-txt-link-rfc2396E" href="mailto:kbrenneman@nvidia.com"><kbrenneman@nvidia.com></a>
Added a label to the _EGLThreadInfo, _EGLDisplay, and EGLResource
structs. Implemented the function eglLabelObjectKHR.
</pre>
</blockquote>
<pre wrap="">Coding style of the new hunk follows the GLVND one, which is _not_
what we use in mesa/egl. Please don't do that ?
</pre>
<blockquote type="cite">
<pre wrap="">Reviewed-by: Adam Jackson <a class="moz-txt-link-rfc2396E" href="mailto:ajax@redhat.com"><ajax@redhat.com></a>
---
src/egl/main/eglapi.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++
src/egl/main/eglcurrent.c | 9 +++++++
src/egl/main/eglcurrent.h | 4 +++
src/egl/main/egldisplay.h | 4 +++
4 files changed, 80 insertions(+)
diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index df2dcd6..31b842f 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -1791,6 +1791,68 @@ eglExportDMABUFImageMESA(EGLDisplay dpy, EGLImage image,
RETURN_EGL_EVAL(disp, ret);
}
+static EGLint EGLAPIENTRY
+eglLabelObjectKHR(
+ EGLDisplay dpy,
+ EGLenum objectType,
+ EGLObjectKHR object,
+ EGLLabelKHR label)
</pre>
</blockquote>
<pre wrap="">Incorrect alignment.
</pre>
<blockquote type="cite">
<pre wrap="">+{
+ if (objectType == EGL_OBJECT_THREAD_KHR) {
+ _EGLThreadInfo *t = _eglGetCurrentThread();
</pre>
</blockquote>
<pre wrap="">Add blank newline after variable declarations.
</pre>
<blockquote type="cite">
<pre wrap="">+ if (!_eglIsCurrentThreadDummy()) {
+ t->Label = label;
+ }
</pre>
</blockquote>
<pre wrap="">Drop {} when they encapsulate a single line on code (unless the other
side of the conditional has them).
</pre>
<blockquote type="cite">
<pre wrap="">+ return EGL_SUCCESS;
+ } else {
</pre>
</blockquote>
<pre wrap="">This else can do. This and the above two apply for the rest of the patch.
</pre>
<blockquote type="cite">
<pre wrap="">+ _EGLDisplay *disp = _eglLookupDisplay(dpy);
+ if (disp == NULL) {
+ _eglError(EGL_BAD_DISPLAY, "eglLabelObjectKHR");
+ return EGL_BAD_DISPLAY;
+ }
+
+ if (objectType == EGL_OBJECT_DISPLAY_KHR) {
+ if (dpy != (EGLDisplay) object) {
+ _eglError(EGL_BAD_PARAMETER, "eglLabelObjectKHR");
+ return EGL_BAD_PARAMETER;
+ }
+ disp->Label = label;
+ return EGL_SUCCESS;
+ } else {
+ _EGLResourceType type;
+ switch (objectType)
+ {
+ case EGL_OBJECT_CONTEXT_KHR:
+ type = _EGL_RESOURCE_CONTEXT;
+ break;
+ case EGL_OBJECT_SURFACE_KHR:
+ type = _EGL_RESOURCE_SURFACE;
+ break;
+ case EGL_OBJECT_IMAGE_KHR:
+ type = _EGL_RESOURCE_IMAGE;
+ break;
+ case EGL_OBJECT_SYNC_KHR:
+ type = _EGL_RESOURCE_SYNC;
+ break;
+ case EGL_OBJECT_STREAM_KHR:
+ default:
+ _eglError(EGL_BAD_PARAMETER, "eglLabelObjectKHR");
+ return EGL_BAD_PARAMETER;
+ }
+
</pre>
</blockquote>
<pre wrap=""> The <display> does not need to be initialized for <objectType>
EGL_OBJECT_THREAD_KHR, or EGL_OBJECT_DISPLAY_KHR; However for all other
types it must be initialized in order to validate the <object> for
attaching a label.
</pre>
<blockquote type="cite">
<pre wrap="">+ if (_eglCheckResource(object, type, disp)) {
</pre>
</blockquote>
<pre wrap="">With the above said, I'm not sure of this will be sufficient.
_eglCheckResource checks if the resource is linked, with unlinking
happening via
a) destroy<objectType>
Mildly related: we seem to have a bug for EGL_OBJECT_SYNC_KHR, where
we unconditionally unblock and destroy, while we should flag for
deletion.
b) eglTerminate
It detaches/unlinks only contexts and surfaces (bug?). Thus even when
the display is no longer initialized we will get get to this point and
_eglCheckResource() will return true.
c) eglReleaseThread
Takes care of the error, bound API and current ctx state.
</pre>
</blockquote>
Would it be sufficient to call _eglLockDisplay instead of
_eglLookupDisplay, and then check the disp->Initialized flag for
the other object types? That way, it could just return
EGL_NOT_INITIALIZED for an uninitialized display, even if the
EGLObjectKHR handle points to some object that's still lingering
after an eglTerminate call.<br>
<br>
<blockquote
cite="mid:CACvgo52zRf7S_v02YKsfDEvChPTB=GaBEuy1JdTf91isFTFrvQ@mail.gmail.com"
type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+/**
+ * Returns the label set for the current thread.
+ */
+EGLLabelKHR _eglGetThreadLabel(void)
+{
+ _EGLThreadInfo *t = _eglGetCurrentThread();
+ return t->Label;
</pre>
</blockquote>
<pre wrap="">Shouldn't the label be cleared in eglReleaseThread ?</pre>
</blockquote>
It is, albeit indirectly. eglReleaseThread frees the whole
_EGLThreadInfo struct, so the next call to _eglGetCurrentThread will
return an _EGLThreadInfo struct without a thread label. Though,
_eglGetThreadLabel probably should call _eglIsCurrentThreadDummy
first so that it doesn't try to create create a new _EGLThreadInfo
if it doesn't have to.<br>
<br>
</body>
</html>