Re: [Mesa-dev] [PATCH kmscube] Search for a suitable config
On Wed, 3 Jul 2019 at 08:16, Drew DeVault wrote:
>
> Instead of assuming the first will be suitable. kmscube fails to start
> for me without this change.
Yes please. Picking the first one is rarely the correct thing to do.
Especially when we have platform specific attributes which are exempt
from the sorting rules.
> ---
> common.c | 52 +---
> common.h | 1 +
> 2 files changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/common.c b/common.c
> index f9bd280..45c074d 100644
> --- a/common.c
> +++ b/common.c
> @@ -24,11 +24,47 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> #include "common.h"
>
> +static bool egl_get_config(EGLDisplay disp, EGLint *attribs,
Nit: "const EGLint *attribs" and keep the const notation in the parent function.
> + EGLConfig *out, EGLint visual_id) {
Nit: { on the next line
> + EGLint count = 0, matched = 0, ret;
> +
> + ret = eglGetConfigs(disp, NULL, 0, &count);
> + if (ret == EGL_FALSE || count == 0) {
> + printf("eglGetConfigs returned no configs\n");
> + return false;
> + }
> +
> + EGLConfig configs[count];
> +
VLAs are convenient, although they lead to messy binary, are not
supported on some platforms etc.
How about:
EGLConfig configs[128];
assert(count...)
> + ret = eglChooseConfig(disp, attribs, configs, count, &matched);
> + if (ret == EGL_FALSE) {
> + printf("eglChooseConfig failed\n");
> + return false;
> + }
> +
> + for (int i = 0; i < matched; ++i) {
> + EGLint visual;
> + if (!eglGetConfigAttrib(disp, configs[i],
> + EGL_NATIVE_VISUAL_ID, &visual)) {
> + continue;
> + }
> +
> + if (!visual_id || visual == visual_id) {
> + *out = configs[i];
> + return true;
> + }
> + }
> +
> + printf("no valid egl config found\n");
> + return false;
> +}
> +
> struct gbm * init_gbm(int drm_fd, int w, int h)
> {
> struct gbm *gbm = calloc(1, sizeof (struct gbm));
> @@ -59,7 +95,7 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
> EGL_NONE
> };
>
> - static const EGLint config_attribs[] = {
> + static EGLint config_attribs[] = {
Nit: Keep the const.
> EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
> EGL_RED_SIZE, 1,
> EGL_GREEN_SIZE, 1,
> @@ -81,9 +117,10 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
> get_proc(eglDestroySyncKHR);
> get_proc(eglWaitSyncKHR);
> get_proc(eglDupNativeFenceFDANDROID);
> + get_proc(eglCreatePlatformWindowSurfaceEXT);
Move this and associated changes to a separate commit, preserving the
fallback to eglCreateWindowSurface().
>
> if (egl->eglGetPlatformDisplayEXT) {
> - egl->display =
> egl->eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_KHR,
> + egl->display =
> egl->eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA,
Out of curiosity: Both constants are numerically the same. Why the change?
> gbm->dev, NULL);
> } else {
> egl->display = eglGetDisplay((void *)gbm->dev);
> @@ -106,8 +143,9 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
> return -1;
> }
>
> - if (!eglChooseConfig(egl->display, config_attribs, &egl->config, 1,
> &n) || n != 1) {
> - printf("failed to choose config: %d\n", n);
> + if (!egl_get_config(egl->display, config_attribs,
> + &egl->config, GBM_FORMAT_XRGB)) {
Let's use gbm.format instead of open-coding it.
Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH kmscube] Search for a suitable config
Hi Drew, On Wed, 3 Jul 2019 at 08:16, Drew DeVault wrote: > Instead of assuming the first will be suitable. kmscube fails to start > for me without this change. There are a couple of unrelated changes combined in here, but I think the core one is good. eglChooseConfig has some really useful properties, where it strictly specifies a sort order, does not include EGL_NATIVE_VISUAL_ID in that sort order, and silently accepts attributes which are not relevant for the sort. Your change which takes all possible configs and then queries them to make sure that the native visual ID is actually correct is the right thing to do. Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH kmscube] Search for a suitable config
Instead of assuming the first will be suitable. kmscube fails to start
for me without this change.
---
common.c | 52 +---
common.h | 1 +
2 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/common.c b/common.c
index f9bd280..45c074d 100644
--- a/common.c
+++ b/common.c
@@ -24,11 +24,47 @@
#include
#include
#include
+#include
#include
#include
#include "common.h"
+static bool egl_get_config(EGLDisplay disp, EGLint *attribs,
+ EGLConfig *out, EGLint visual_id) {
+ EGLint count = 0, matched = 0, ret;
+
+ ret = eglGetConfigs(disp, NULL, 0, &count);
+ if (ret == EGL_FALSE || count == 0) {
+ printf("eglGetConfigs returned no configs\n");
+ return false;
+ }
+
+ EGLConfig configs[count];
+
+ ret = eglChooseConfig(disp, attribs, configs, count, &matched);
+ if (ret == EGL_FALSE) {
+ printf("eglChooseConfig failed\n");
+ return false;
+ }
+
+ for (int i = 0; i < matched; ++i) {
+ EGLint visual;
+ if (!eglGetConfigAttrib(disp, configs[i],
+ EGL_NATIVE_VISUAL_ID, &visual)) {
+ continue;
+ }
+
+ if (!visual_id || visual == visual_id) {
+ *out = configs[i];
+ return true;
+ }
+ }
+
+ printf("no valid egl config found\n");
+ return false;
+}
+
struct gbm * init_gbm(int drm_fd, int w, int h)
{
struct gbm *gbm = calloc(1, sizeof (struct gbm));
@@ -59,7 +95,7 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
EGL_NONE
};
- static const EGLint config_attribs[] = {
+ static EGLint config_attribs[] = {
EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
EGL_RED_SIZE, 1,
EGL_GREEN_SIZE, 1,
@@ -81,9 +117,10 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
get_proc(eglDestroySyncKHR);
get_proc(eglWaitSyncKHR);
get_proc(eglDupNativeFenceFDANDROID);
+ get_proc(eglCreatePlatformWindowSurfaceEXT);
if (egl->eglGetPlatformDisplayEXT) {
- egl->display =
egl->eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_KHR,
+ egl->display =
egl->eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA,
gbm->dev, NULL);
} else {
egl->display = eglGetDisplay((void *)gbm->dev);
@@ -106,8 +143,9 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
return -1;
}
- if (!eglChooseConfig(egl->display, config_attribs, &egl->config, 1, &n)
|| n != 1) {
- printf("failed to choose config: %d\n", n);
+ if (!egl_get_config(egl->display, config_attribs,
+ &egl->config, GBM_FORMAT_XRGB)) {
+ printf("Failed to get EGL config\n");
return -1;
}
@@ -118,10 +156,10 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
return -1;
}
- egl->surface = eglCreateWindowSurface(egl->display, egl->config,
- (EGLNativeWindowType)gbm->surface, NULL);
+ egl->surface = egl->eglCreatePlatformWindowSurfaceEXT(
+ egl->display, egl->config, gbm->surface, NULL);
if (egl->surface == EGL_NO_SURFACE) {
- printf("failed to create egl surface\n");
+ printf("failed to create egl surface: %d\n", eglGetError());
return -1;
}
diff --git a/common.h b/common.h
index 1ddf04b..1675f98 100644
--- a/common.h
+++ b/common.h
@@ -69,6 +69,7 @@ struct egl {
EGLSurface surface;
PFNEGLGETPLATFORMDISPLAYEXTPROC eglGetPlatformDisplayEXT;
+ PFNEGLCREATEPLATFORMWINDOWSURFACEEXTPROC
eglCreatePlatformWindowSurfaceEXT;
PFNEGLCREATEIMAGEKHRPROC eglCreateImageKHR;
PFNEGLDESTROYIMAGEKHRPROC eglDestroyImageKHR;
PFNGLEGLIMAGETARGETTEXTURE2DOESPROC glEGLImageTargetTexture2DOES;
--
2.22.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
