[Mesa-dev] [PATCH 1/2] autotools+dri3: allow building against older xcb (v4)

[Daniel Stone]

Hi,

On 14 March 2018 at 13:04, Rob Clark <robdclark at gmail.com> wrote:
> I'm not sure everyone wants to be updating their dri3 in a forced
> march setting, this allows a nicer approach, esp when you want
> to build on distro that aren't brand new.

I don't have that much of an opinion on whether the dependency should
be mandatory or not. I originally had #ifdefs and removed them when
reviewers asked me to. If people want to add them back, fine by me.

That being said, these patches need changes, per comments below. One
thing missing entirely is making the version negotiation conditional:
when we call query_version for DRI3/Present, we need to make the
version we pass in conditional on whether or not we have new XCB.
Probably also wise to ifdef the multiplane_available variables, so
it's really obvious where any users are missing ifdefs.

I'm happy to test this tomorrow and submit a new version if that's
easier for people.

> @@ -327,6 +327,7 @@ dri3_create_image_khr_pixmap_from_buffers(_EGLDisplay *disp, _EGLContext *ctx,
>                                            EGLClientBuffer buffer,
>                                            const EGLint *attr_list)
>  {
> +#ifdef HAVE_DRI3_MODIFIERS
>     struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
>     struct dri2_egl_image *dri2_img;
>     xcb_dri3_buffers_from_pixmap_cookie_t bp_cookie;
> @@ -376,6 +377,9 @@ dri3_create_image_khr_pixmap_from_buffers(_EGLDisplay *disp, _EGLContext *ctx,
>     }
>
>     return &dri2_img->base;
> +#else
> +   return NULL;
> +#endif
>  }

Just ifdef out the entire function, don't return NULL.

> @@ -1272,6 +1276,7 @@ dri3_alloc_render_buffer(struct loader_dri3_drawable *draw, unsigned int format,
>     pixmap = xcb_generate_id(draw->conn);
>     if (draw->multiplanes_available &&
>         buffer->modifier != DRM_FORMAT_MOD_INVALID) {
> +#ifdef HAVE_DRI3_MODIFIERS
>        xcb_dri3_pixmap_from_buffers(draw->conn,
>                                     pixmap,
>                                     draw->drawable,
> @@ -1284,6 +1289,7 @@ dri3_alloc_render_buffer(struct loader_dri3_drawable *draw, unsigned int format,
>                                     depth, buffer->cpp * 8,
>                                     buffer->modifier,
>                                     buffer_fds);
> +#endif
>     } else {
>        xcb_dri3_pixmap_from_buffer(draw->conn,
>                                    pixmap,

This ifdef needs to wrap the branch, so that the single-buffer
xcb_dri3_pixmap_from_buffer() always gets called if we built against
old XCB, else new-server + old-XCB-Mesa never allocates a render
buffer for X11 surfaces.

> @@ -1567,7 +1575,7 @@ dri3_get_pixmap_buffer(__DRIdrawable *driDrawable, unsigned int format,
>                            (sync_fence = xcb_generate_id(draw->conn)),
>                            false,
>                            fence_fd);
> -
> +#ifdef HAVE_DRI3_MODIFIERS
>     if (draw->multiplanes_available &&
>         draw->ext->image->base.version >= 15 &&
>         draw->ext->image->createImageFromDmaBufs2) {
> @@ -1586,7 +1594,9 @@ dri3_get_pixmap_buffer(__DRIdrawable *driDrawable, unsigned int format,
>        width = bps_reply->width;
>        height = bps_reply->height;
>        free(bps_reply);
> -   } else {
> +   } else
> +#endif
> +   {
>        xcb_dri3_buffer_from_pixmap_cookie_t bp_cookie;
>        xcb_dri3_buffer_from_pixmap_reply_t *bp_reply;

Jason complained about control flow being intermingled with #ifdefs
like this. I don't have any suggestions as to how to do it better
though, which is why I did it like this in the first place.

Cheers,
Daniel