[PATCH weston] simple-dmabuf-drm: fix build with --disable-egl

[Pekka Paalanen]

On Tue, 3 Jul 2018 12:35:30 +0200
Emilio Pozuelo Monfort <pochu27 at gmail.com> wrote:

> On 03/07/18 11:00, Pekka Paalanen wrote:
> > On Mon,  2 Jul 2018 17:22:30 +0200
> > Emilio Pozuelo Monfort <pochu27 at gmail.com> wrote:
> >   
> >> Signed-off-by: Emilio Pozuelo Monfort <emilio.pozuelo at collabora.co.uk>
> >> ---
> >> I tried a build with --disable-egl as I didn't have the headers
> >> installed, and it broke here. The EGL usage here seemed optional so I
> >> did that, but I didn't run-test the result. If it would make more sense
> >> to disable the client if EGL support is disabled I can do that.
> >>
> >>  clients/simple-dmabuf-drm.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
> >> index fcab30e5..1ae3a2dd 100644
> >> --- a/clients/simple-dmabuf-drm.c
> >> +++ b/clients/simple-dmabuf-drm.c
> >> @@ -863,6 +863,7 @@ create_display(int opts, int format)
> >>  	display->req_dmabuf_immediate = opts & OPT_IMMEDIATE;
> >>  	display->req_dmabuf_modifiers = (format == DRM_FORMAT_NV12);
> >>  
> >> +#if ENABLE_EGL
> >>  	/*
> >>  	 * hard code format if the platform egl doesn't support format
> >>  	 * querying / advertising.
> >> @@ -871,6 +872,7 @@ create_display(int opts, int format)
> >>  	if (extensions && !weston_check_egl_extension(extensions,
> >>  				"EGL_EXT_image_dma_buf_import_modifiers"))
> >>  		display->xrgb8888_format_found = 1;
> >> +#endif
> >>  
> >>  	display->registry = wl_display_get_registry(display->display);
> >>  	wl_registry_add_listener(display->registry,  
> > 
> > Hi,
> > 
> > that's very strange. This program does not use EGL or even GBM, that's
> > a completely dead hunk of code you're ifdeffing there as I can see.
> > Would be better to just remove it completely, and make sure the build
> > does not link libEGL or GBM. Include for shared/platform.h should be
> > useless too.  
> 
> It's not dead code, it's a fallback mechanism in case the EGL platform doesn't
> support EGL_EXT_image_dma_buf_import_modifiers. The problem is that configure.ac
> checks for EGL for simple-dmabuf-drm-client, but the client is not disabled if
> one builds with --disable-egl. Perhaps I should do that instead. After all,
> disable-egl disables the gl-renderer, which is needed for zwp_linux_dmabuf,
> which is needed by simple-dmabuf-drm-client.

What I meant was that nothing calls eglGetDisplay or equivalent. It may
be inspecting the EGL Client extensions, but it won't do anything with
that information. The Wayland extension advertises modifier support
completely independently.

In fact, if the compositor did not advertise xrgb8888 through
zwp_linux_dmabuf, the flag for it should never be set. I believe you'd
be fixing a bug by simply deleting that code. No, two bugs: the flag
and the build. :-)


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180703/5054e9d3/attachment.sig>