Hi guys,
There's a bunch of outstanding style nitpicks (come to think of it
13/14 could use the same)
Those aside: there's a bunch of serious suggestions which I missed last time.
On 12 September 2016 at 23:19, Adam Jackson <a...@redhat.com> wrote:
From: Kyle Brenneman <kbrenne...@nvidia.com>
Wire up the debug entrypoints to EGL dispatch, and add the extension
string to the client extension list.
---
src/egl/main/eglapi.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++
src/egl/main/eglglobals.c | 3 +-
2 files changed, 142 insertions(+), 1 deletion(-)
diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index 0a6ebe7..6b0fd2e 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -1987,6 +1987,143 @@ eglExportDMABUFImageMESA(EGLDisplay dpy, EGLImage image,
RETURN_EGL_EVAL(disp, ret);
}
+static EGLint EGLAPIENTRY
+eglLabelObjectKHR(EGLDisplay dpy, EGLenum objectType, EGLObjectKHR object,
+ EGLLabelKHR label)
+{
+ _EGL_FUNC_START(NULL, EGL_NONE, NULL, EGL_BAD_ALLOC);
+
+ if (objectType == EGL_OBJECT_THREAD_KHR) {
+ _EGLThreadInfo *t = _eglGetCurrentThread();
+
+ if (!_eglIsCurrentThreadDummy()) {
+ t->Label = label;
+ return EGL_SUCCESS;
+ } else {
+ _eglDebugReportFull(EGL_BAD_ALLOC, __func__, __func__,
+ EGL_DEBUG_MSG_CRITICAL_KHR, NULL, NULL);
+ return EGL_BAD_ALLOC;
+ }
Nit: Please use the same style as the "objectType ==
EGL_OBJECT_DISPLAY_KHR" case.
+ } else {
Nit: You can also drop the "else" and flatten (indent one level less)
all of the following code.
+ _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 {
Nit: kill this "else {" as well ?
+ _EGLResourceType type;
+
+ switch (objectType)
+ {
Nit: move to previous line
+ 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;
+ }
+
+ if (_eglCheckResource(object, type, disp)) {
+ _EGLResource *res = (_EGLResource *) object;
+ res->Label = label;
+ return EGL_SUCCESS;
+ } else {
+ _eglError(EGL_BAD_PARAMETER, "eglLabelObjectKHR");
+ return EGL_BAD_PARAMETER;
+ }
Nit: coding style.
+ }
+ }
+}
+
+static EGLint
Missing EGLAPIENTRY
+eglDebugMessageControlKHR(EGLDEBUGPROCKHR callback,
+ const EGLAttrib *attrib_list)
+{
+ unsigned int newEnabled;
+
+ _EGL_FUNC_START(NULL, EGL_NONE, NULL, EGL_BAD_ALLOC);
+
+ mtx_lock(_eglGlobal.Mutex);
+
+ newEnabled = _eglGlobal.debugTypesEnabled;
+ if (attrib_list != NULL) {
+ int i;
+
+ for (i = 0; attrib_list[i] != EGL_NONE; i += 2) {
Don't think we check it elsewhere (and/or if we should care too much) but still:
Check if i overflows or use unsigned type ?
+ if (attrib_list[i] >= EGL_DEBUG_MSG_CRITICAL_KHR &&
+ attrib_list[i] <= EGL_DEBUG_MSG_INFO_KHR) {
+ if (attrib_list[i + 1]) {
+ newEnabled |= DebugBitFromType(attrib_list[i]);
+ } else {
+ newEnabled &= ~DebugBitFromType(attrib_list[i]);
+ }
Nit: break; ?
+ } else {
+ // On error, set the last error code, call the current
+ // debug callback, and return the error code.
+ mtx_unlock(_eglGlobal.Mutex);
+ _eglReportError(EGL_BAD_ATTRIBUTE, NULL,
+ "Invalid attribute 0x%04lx", (unsigned long) attrib_list[i]);
+ return EGL_BAD_ATTRIBUTE;
+ }
+ }
+ }
+
+ if (callback != NULL) {
+ _eglGlobal.debugCallback = callback;
+ _eglGlobal.debugTypesEnabled = newEnabled;
+ } else {
+ _eglGlobal.debugCallback = NULL;
+ _eglGlobal.debugTypesEnabled = _EGL_DEBUG_BIT_CRITICAL |
_EGL_DEBUG_BIT_ERROR;
+ }
+
+ mtx_unlock(_eglGlobal.Mutex);
+ return EGL_SUCCESS;
+}
+
+static EGLBoolean
Missing EGLAPIENTRY
+eglQueryDebugKHR(EGLint attribute, EGLAttrib *value)
+{
+ _EGL_FUNC_START(NULL, EGL_NONE, NULL, EGL_BAD_ALLOC);
+
+ mtx_lock(_eglGlobal.Mutex);
+ if (attribute >= EGL_DEBUG_MSG_CRITICAL_KHR &&
+ attribute <= EGL_DEBUG_MSG_INFO_KHR) {
+ if (_eglGlobal.debugTypesEnabled & DebugBitFromType(attribute)) {
+ *value = EGL_TRUE;
+ } else {
+ *value = EGL_FALSE;
+ }
+ } else if (attribute == EGL_DEBUG_CALLBACK_KHR) {
+ *value = (EGLAttrib) _eglGlobal.debugCallback;
+ } else {
+ mtx_unlock(_eglGlobal.Mutex);
+ _eglReportError(EGL_BAD_ATTRIBUTE, NULL,
+ "Invalid attribute 0x%04lx", (unsigned long) attribute);
+ return EGL_FALSE;
+ }
Nit: Switch statement will be a lot easier to read.
-Emil
