On 8 September 2016 at 18:46, Adam Jackson <a...@redhat.com> wrote:
> From: Kyle Brenneman <kbrenne...@nvidia.com>
>
> Added a label to the _EGLThreadInfo, _EGLDisplay, and EGLResource
> structs. Implemented the function eglLabelObjectKHR.
>
Coding style of the new hunk follows the GLVND one, which is _not_
what we use in mesa/egl. Please don't do that ?
> Reviewed-by: Adam Jackson <a...@redhat.com>
> ---
> 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)
Incorrect alignment.
> +{
> + if (objectType == EGL_OBJECT_THREAD_KHR) {
> + _EGLThreadInfo *t = _eglGetCurrentThread();
Add blank newline after variable declarations.
> + if (!_eglIsCurrentThreadDummy()) {
> + t->Label = label;
> + }
Drop {} when they encapsulate a single line on code (unless the other
side of the conditional has them).
> + return EGL_SUCCESS;
> + } else {
This else can do. This and the above two apply for the rest of the patch.
> + _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;
> + }
> +
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.
> + if (_eglCheckResource(object, type, disp)) {
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.
> +/**
> + * Returns the label set for the current thread.
> + */
> +EGLLabelKHR _eglGetThreadLabel(void)
> +{
> + _EGLThreadInfo *t = _eglGetCurrentThread();
> + return t->Label;
Shouldn't the label be cleared in eglReleaseThread ?
Regards,
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
