| Message ID | 20261013223403.1242372-1-joel.winarske@linux.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Joel Winarske (2026-10-13 23:34:03) > Mesa <= 22.2 (and forks based on it, e.g. the PowerVR mesa-pvr used on > StarFive JH7110) export -DEGL_NO_X11 via egl.pc Cflags. The > unconditional define in egl.h then redefines it with a different value, > which is an error with -Werror: > > include/libcamera/internal/egl.h:23:9: error: 'EGL_NO_X11' redefined [-Werror] > > Only define it when it isn't already set. > > Signed-off-by: Joel Winarske <joel.winarske@linux.com> Does this mean we don't need the define at all ? I guess we want it for something or we wouldn't have added it - but maybe we should be getting a different flag from pkg-config ? Anyway if we can't rely on it, then this looks like a clear solution so: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > include/libcamera/internal/egl.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h > index 7d062b32d..9d2c9b5bc 100644 > --- a/include/libcamera/internal/egl.h > +++ b/include/libcamera/internal/egl.h > @@ -21,7 +21,9 @@ > * Workaround for build issues on Mesa <= 22.2, see > * https://github.com/KhronosGroup/EGL-Registry/pull/130 > */ > +#ifndef EGL_NO_X11 > #define EGL_NO_X11 > +#endif > > #define EGL_EGLEXT_PROTOTYPES > #include <EGL/egl.h> > -- > 2.56.0 >
Hi 2026. 10. 14. 0:34 keltezéssel, Joel Winarske írta: > Mesa <= 22.2 (and forks based on it, e.g. the PowerVR mesa-pvr used on > StarFive JH7110) export -DEGL_NO_X11 via egl.pc Cflags. The > unconditional define in egl.h then redefines it with a different value, > which is an error with -Werror: > > include/libcamera/internal/egl.h:23:9: error: 'EGL_NO_X11' redefined [-Werror] > > Only define it when it isn't already set. > > Signed-off-by: Joel Winarske <joel.winarske@linux.com> > --- This looks ok to me. The relevant mesa change appears to be https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/2226 although I wonder why this hasn't come up so far... Probably because this only affects non-glvnd mesa builds, as far as I can see? Maybe it makes sense to extend the commit message mentioning that the egl.pc provided by mesa sets this macro in cflags until mesa 22.3. (https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18848 I think) Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > include/libcamera/internal/egl.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h > index 7d062b32d..9d2c9b5bc 100644 > --- a/include/libcamera/internal/egl.h > +++ b/include/libcamera/internal/egl.h > @@ -21,7 +21,9 @@ > * Workaround for build issues on Mesa <= 22.2, see > * https://github.com/KhronosGroup/EGL-Registry/pull/130 > */ > +#ifndef EGL_NO_X11 > #define EGL_NO_X11 > +#endif > > #define EGL_EGLEXT_PROTOTYPES > #include <EGL/egl.h>
Thanks both. The define is still needed: EGL headers predating EGL-Registry#130 include Xlib.h unless EGL_NO_X11 is set. We can't get it from pkg-config either, since only Mesa built without the X11 platform puts -DEGL_NO_X11 in egl.pc, and with glvnd egl.pc comes from libglvnd. Barnabás, yes, it's non-glvnd only; mesa-pvr 22.1.3 here is non-glvnd with X11 disabled. v2 expands the commit message along those lines. On Wed, Oct 14, 2026 at 4:20 AM Barnabás Pőcze < barnabas.pocze@ideasonboard.com> wrote: > Hi > > 2026. 10. 14. 0:34 keltezéssel, Joel Winarske írta: > > Mesa <= 22.2 (and forks based on it, e.g. the PowerVR mesa-pvr used on > > StarFive JH7110) export -DEGL_NO_X11 via egl.pc Cflags. The > > unconditional define in egl.h then redefines it with a different value, > > which is an error with -Werror: > > > > include/libcamera/internal/egl.h:23:9: error: 'EGL_NO_X11' redefined > [-Werror] > > > > Only define it when it isn't already set. > > > > Signed-off-by: Joel Winarske <joel.winarske@linux.com> > > --- > > This looks ok to me. The relevant mesa change appears to be > > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/2226 > > although I wonder why this hasn't come up so far... Probably because this > only > affects non-glvnd mesa builds, as far as I can see? Maybe it makes sense > to extend > the commit message mentioning that the egl.pc provided by mesa sets this > macro > in cflags until mesa 22.3. ( > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18848 I think) > > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > > include/libcamera/internal/egl.h | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/include/libcamera/internal/egl.h > b/include/libcamera/internal/egl.h > > index 7d062b32d..9d2c9b5bc 100644 > > --- a/include/libcamera/internal/egl.h > > +++ b/include/libcamera/internal/egl.h > > @@ -21,7 +21,9 @@ > > * Workaround for build issues on Mesa <= 22.2, see > > * https://github.com/KhronosGroup/EGL-Registry/pull/130 > > */ > > +#ifndef EGL_NO_X11 > > #define EGL_NO_X11 > > +#endif > > > > #define EGL_EGLEXT_PROTOTYPES > > #include <EGL/egl.h> > >
Hello Joel, Thank you for the patch. On Tue, Oct 13, 2026 at 03:34:03PM -0700, Joel Winarske wrote: > Mesa <= 22.2 (and forks based on it, e.g. the PowerVR mesa-pvr used on > StarFive JH7110) export -DEGL_NO_X11 via egl.pc Cflags. The > unconditional define in egl.h then redefines it with a different value, Looking at Mesa 22.2, egl.pc contains exactly "-DEGL_NO_X11". libcamera redefines it indeed, which causes a warning, but not to a different value. Let's just drop "with a different value". With that, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> No need to send a v2, this can be addressed when applying the patch. > which is an error with -Werror: > > include/libcamera/internal/egl.h:23:9: error: 'EGL_NO_X11' redefined [-Werror] > > Only define it when it isn't already set. > > Signed-off-by: Joel Winarske <joel.winarske@linux.com> > --- > include/libcamera/internal/egl.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h > index 7d062b32d..9d2c9b5bc 100644 > --- a/include/libcamera/internal/egl.h > +++ b/include/libcamera/internal/egl.h > @@ -21,7 +21,9 @@ > * Workaround for build issues on Mesa <= 22.2, see > * https://github.com/KhronosGroup/EGL-Registry/pull/130 > */ > +#ifndef EGL_NO_X11 > #define EGL_NO_X11 > +#endif > > #define EGL_EGLEXT_PROTOTYPES > #include <EGL/egl.h>
diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h index 7d062b32d..9d2c9b5bc 100644 --- a/include/libcamera/internal/egl.h +++ b/include/libcamera/internal/egl.h @@ -21,7 +21,9 @@ * Workaround for build issues on Mesa <= 22.2, see * https://github.com/KhronosGroup/EGL-Registry/pull/130 */ +#ifndef EGL_NO_X11 #define EGL_NO_X11 +#endif #define EGL_EGLEXT_PROTOTYPES #include <EGL/egl.h>
Mesa <= 22.2 (and forks based on it, e.g. the PowerVR mesa-pvr used on StarFive JH7110) export -DEGL_NO_X11 via egl.pc Cflags. The unconditional define in egl.h then redefines it with a different value, which is an error with -Werror: include/libcamera/internal/egl.h:23:9: error: 'EGL_NO_X11' redefined [-Werror] Only define it when it isn't already set. Signed-off-by: Joel Winarske <joel.winarske@linux.com> --- include/libcamera/internal/egl.h | 2 ++ 1 file changed, 2 insertions(+)