[v2] libcamera: egl: Don't redefine EGL_NO_X11
diff mbox series

Message ID 20261014164859.820451-1-joel.winarske@linux.com
State Accepted
Headers show
Series
  • [v2] libcamera: egl: Don't redefine EGL_NO_X11
Related show

Commit Message

Joel Winarske Oct. 14, 2026, 4:48 p.m. UTC 
Mesa built without the X11 platform exports -DEGL_NO_X11 via egl.pc
Cflags, from mesa!2226 (19.3) until mesa!18848 (22.3) moved Xlib
inclusion behind USE_X11. This affects non-glvnd builds, where egl.pc
comes from Mesa rather than libglvnd, e.g. the PowerVR mesa-pvr (22.1)
used on StarFive JH7110.

The -D defines the macro as 1, so the unconditional empty define in
egl.h is a redefinition, which is an error with -Werror:

  include/libcamera/internal/egl.h:23:9: error: 'EGL_NO_X11' redefined [-Werror]

The define is still needed for EGL headers predating
KhronosGroup/EGL-Registry#130, which otherwise pull in Xlib.h, so only
define it when it isn't already set.

Signed-off-by: Joel Winarske <joel.winarske@linux.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
Changes in v2:
- Expand commit message: Mesa versions/MRs, non-glvnd, why the define is kept
- Collect Reviewed-by tags

 include/libcamera/internal/egl.h | 2 ++
 1 file changed, 2 insertions(+)

Comments

Laurent Pinchart Oct. 14, 2026, 6:45 p.m. UTC | #1 
On Wed, Oct 14, 2026 at 09:48:59AM -0700, Joel Winarske wrote:
> Mesa built without the X11 platform exports -DEGL_NO_X11 via egl.pc
> Cflags, from mesa!2226 (19.3) until mesa!18848 (22.3) moved Xlib

Merge request numbers are not a good way to identify versions, they're
not available in the git history. You could use the commit ID, but let's
just drop that as you have tags: "from 19.3 until 22.3".

> inclusion behind USE_X11. This affects non-glvnd builds, where egl.pc
> comes from Mesa rather than libglvnd, e.g. the PowerVR mesa-pvr (22.1)
> used on StarFive JH7110.
> 
> The -D defines the macro as 1, so the unconditional empty define in
> egl.h is a redefinition, which is an error with -Werror:

Where do you see it defined as 1 ?

>   include/libcamera/internal/egl.h:23:9: error: 'EGL_NO_X11' redefined [-Werror]
> 
> The define is still needed for EGL headers predating
> KhronosGroup/EGL-Registry#130, which otherwise pull in Xlib.h, so only
> define it when it isn't already set.
> 
> Signed-off-by: Joel Winarske <joel.winarske@linux.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
> Changes in v2:
> - Expand commit message: Mesa versions/MRs, non-glvnd, why the define is kept
> - Collect Reviewed-by tags
> 
>  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>
Joel Winarske Oct. 14, 2026, 8:23 p.m. UTC | #2 
> Merge request numbers are not a good way to identify versions, they're
> not available in the git history. You could use the commit ID, but let's
> just drop that as you have tags: "from 19.3 until 22.3".

Copy that

> Where do you see it defined as 1 ?

-D without a value defines it as 1, per the GCC docs: "-D name: Predefine
name as a macro, with definition 1." The redefinition warning comes from
the empty #define not matching that. An identical redefinition is silent:

  $ printf '#define EGL_NO_X11\n' > a.c; gcc -DEGL_NO_X11 -Werror -c a.c
  a.c:1:9: error: 'EGL_NO_X11' redefined [-Werror]
  $ printf '#define EGL_NO_X11 1\n' > b.c; gcc -DEGL_NO_X11 -Werror -c b.c
  $

On Wed, Oct 14, 2026 at 11:45 AM Laurent Pinchart <
laurent.pinchart@ideasonboard.com> wrote:

> On Wed, Oct 14, 2026 at 09:48:59AM -0700, Joel Winarske wrote:
> > Mesa built without the X11 platform exports -DEGL_NO_X11 via egl.pc
> > Cflags, from mesa!2226 (19.3) until mesa!18848 (22.3) moved Xlib
>
> Merge request numbers are not a good way to identify versions, they're
> not available in the git history. You could use the commit ID, but let's
> just drop that as you have tags: "from 19.3 until 22.3".
>
> > inclusion behind USE_X11. This affects non-glvnd builds, where egl.pc
> > comes from Mesa rather than libglvnd, e.g. the PowerVR mesa-pvr (22.1)
> > used on StarFive JH7110.
> >
> > The -D defines the macro as 1, so the unconditional empty define in
> > egl.h is a redefinition, which is an error with -Werror:
>
> Where do you see it defined as 1 ?
>
> >   include/libcamera/internal/egl.h:23:9: error: 'EGL_NO_X11' redefined
> [-Werror]
> >
> > The define is still needed for EGL headers predating
> > KhronosGroup/EGL-Registry#130, which otherwise pull in Xlib.h, so only
> > define it when it isn't already set.
> >
> > Signed-off-by: Joel Winarske <joel.winarske@linux.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> > ---
> > Changes in v2:
> > - Expand commit message: Mesa versions/MRs, non-glvnd, why the define is
> kept
> > - Collect Reviewed-by tags
> >
> >  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>
>
> --
> Regards,
>
> Laurent Pinchart
>
Barnabás Pőcze Oct. 21, 2026, 8:19 a.m. UTC | #3 
Hi

2026. 10. 14. 22:23 keltezéssel, Joel Winarske írta:
>  > Merge request numbers are not a good way to identify versions, they're
>  > not available in the git history. You could use the commit ID, but let's
>  > just drop that as you have tags: "from 19.3 until 22.3".
> 
> Copy that
> 
>  > Where do you see it defined as 1 ?
> 
> -D without a value defines it as 1, per the GCC docs: "-D name: Predefine name as a macro, with definition 1." The redefinition warning comes from the empty #define not matching that. An identical redefinition is silent:
> 
>    $ printf '#define EGL_NO_X11\n' > a.c; gcc -DEGL_NO_X11 -Werror -c a.c
>    a.c:1:9: error: 'EGL_NO_X11' redefined [-Werror]
>    $ printf '#define EGL_NO_X11 1\n' > b.c; gcc -DEGL_NO_X11 -Werror -c b.c
>    $

I have adjusted the commit message and I'm planning to merge it soon unless someone complains:

```
     libcamera: egl: Don't redefine EGL_NO_X11
     
     Between versions 19.3 and 22.3, the egl pkgconf file provided by mesa included
     the definition of `EGL_NO_X11` in cflags. However, since a5e01866ae17
     ("egl: Add workaround for Mesa <= 22.2") `EGL_NO_X11` is unconditionally defined
     in the egl.h header file with an empty value.
     
     Since a definition on the command line implicitly has a value of 1, this
     is a redefinition error due to the different values (empty vs 1).
     
     Note, that this only affects setups where mesa and not libglvd provides
     the egl pkgconf file, for example the PowerVR mesa-pvr (22.1) used on the
     StarFive JH7110. And since mesa 22.3 the preprocessor define is no longer
     present, due to upstream changes.
     
     Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/2226
     Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18848
     Link: https://github.com/KhronosGroup/EGL-Registry/pull/130
```




> 
> On Wed, Oct 14, 2026 at 11:45 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com <mailto:laurent.pinchart@ideasonboard.com>> wrote:
> 
>     On Wed, Oct 14, 2026 at 09:48:59AM -0700, Joel Winarske wrote:
>      > Mesa built without the X11 platform exports -DEGL_NO_X11 via egl.pc
>      > Cflags, from mesa!2226 (19.3) until mesa!18848 (22.3) moved Xlib
> 
>     Merge request numbers are not a good way to identify versions, they're
>     not available in the git history. You could use the commit ID, but let's
>     just drop that as you have tags: "from 19.3 until 22.3".
> 
>      > inclusion behind USE_X11. This affects non-glvnd builds, where egl.pc
>      > comes from Mesa rather than libglvnd, e.g. the PowerVR mesa-pvr (22.1)
>      > used on StarFive JH7110.
>      >
>      > The -D defines the macro as 1, so the unconditional empty define in
>      > egl.h is a redefinition, which is an error with -Werror:
> 
>     Where do you see it defined as 1 ?
> 
>      >   include/libcamera/internal/egl.h:23:9: error: 'EGL_NO_X11' redefined [-Werror]
>      >
>      > The define is still needed for EGL headers predating
>      > KhronosGroup/EGL-Registry#130, which otherwise pull in Xlib.h, so only
>      > define it when it isn't already set.
>      >
>      > Signed-off-by: Joel Winarske <joel.winarske@linux.com <mailto:joel.winarske@linux.com>>
>      > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com <mailto:kieran.bingham@ideasonboard.com>>
>      > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com <mailto:barnabas.pocze@ideasonboard.com>>
>      > ---
>      > Changes in v2:
>      > - Expand commit message: Mesa versions/MRs, non-glvnd, why the define is kept
>      > - Collect Reviewed-by tags
>      >
>      >  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 <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>
> 
>     -- 
>     Regards,
> 
>     Laurent Pinchart
>
Kieran Bingham Oct. 21, 2026, 8:29 a.m. UTC | #4 
Quoting Barnabás Pőcze (2026-10-21 09:19:29)
> Hi
> 
> 2026. 10. 14. 22:23 keltezéssel, Joel Winarske írta:
> >  > Merge request numbers are not a good way to identify versions, they're
> >  > not available in the git history. You could use the commit ID, but let's
> >  > just drop that as you have tags: "from 19.3 until 22.3".
> > 
> > Copy that
> > 
> >  > Where do you see it defined as 1 ?
> > 
> > -D without a value defines it as 1, per the GCC docs: "-D name: Predefine name as a macro, with definition 1." The redefinition warning comes from the empty #define not matching that. An identical redefinition is silent:
> > 
> >    $ printf '#define EGL_NO_X11\n' > a.c; gcc -DEGL_NO_X11 -Werror -c a.c
> >    a.c:1:9: error: 'EGL_NO_X11' redefined [-Werror]
> >    $ printf '#define EGL_NO_X11 1\n' > b.c; gcc -DEGL_NO_X11 -Werror -c b.c
> >    $
> 
> I have adjusted the commit message and I'm planning to merge it soon unless someone complains:
> 
> ```
>      libcamera: egl: Don't redefine EGL_NO_X11
>      
>      Between versions 19.3 and 22.3, the egl pkgconf file provided by mesa included
>      the definition of `EGL_NO_X11` in cflags. However, since a5e01866ae17
>      ("egl: Add workaround for Mesa <= 22.2") `EGL_NO_X11` is unconditionally defined
>      in the egl.h header file with an empty value.
>      
>      Since a definition on the command line implicitly has a value of 1, this
>      is a redefinition error due to the different values (empty vs 1).
>      
>      Note, that this only affects setups where mesa and not libglvd provides
>      the egl pkgconf file, for example the PowerVR mesa-pvr (22.1) used on the
>      StarFive JH7110. And since mesa 22.3 the preprocessor define is no longer
>      present, due to upstream changes.
>      
>      Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/2226
>      Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18848
>      Link: https://github.com/KhronosGroup/EGL-Registry/pull/130
> ```

Looks fine to me!

--
Kieran

Patch
diff mbox series 

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>