[PATCH 7/8] dri: add __DRIimageLoaderExtension and __DRIimageDriverExtension

[Keith Packard]

Kristian Høgsberg <hoegsberg at gmail.com> writes:

> It just the two older create context functions (which fall back to
> calling driCreateContextAtribs) and allocateBuffer and releaseBuffer.
> The two buffer functions are __DRIbuffer specific of course, but we
> can implement them in terms of __DRIimage in dri_util.c now.

I guess the benefit is that we could remove the DRIdri2Extension
functions from each driver and just use the DRIimage-based wrappers in
dri_util then?

We're still stuck with leaving the DRIdri2Extension as the interface
From the loader though.

> There is a third option, which is to pull the functions we need from
> __DRIcoreExtension into the __DRIimageDriverExtension, which then is
> all you need along with __DRIimageExtension.  This is as painful in
> the short term as the current __DRIimageDriverExtension, but it lets
> of cut loose of the DRI1 (__DRIcoreExtension has the DRI1
> createNewScreen in it) and DRI2 baggage properly later on.  And
> pulling out the functions into a loader private struct as you suggest
> will make it a lot less painful.  The functions we move from core to
> _DRIimageDriverExtension will share the same implementation in
> dri_util.c so there's no new code.

That doesn't seem like a crazy plan; at least Image-based loaders would
be simple then; find the DRIimageDriverExtension and that's it.

> Right - I actually like the clean break idea, but if we're going to
> take that pain I want to get rid of all the junk and avoid the awkward
> "use some stuff from __DRIcoreExtension and other stuff from
> __DRIimageDriverExtension" setup.  So we should either 1) make
> __DRIimageDriverExtension completely replace __DRIcoreExtension and
> __DRIdri2Extension, or 2) just do a minimal, incremental change (just
> the extension to indicate the support for __DRIimage based
> getbuffers).

If we're going to get drivers to add DRIimageExtension to the list of
exported extensions, then it doesn't seem like it matters which way we
go here -- we can move from 2) to 1) in the future without changing any
drivers, only the dri_util bits and the loaders.

However, if we think that 1) is the way to go, we might as well just do
it as that'd avoid having to ever fix the loaders.

> The difference is that there the loader returns a packed array of the
> buffers the driver asked for. Now we're using a struct which can be
> sparsely populated, so the driver should only look at the fields it
> asked for.

My concern is that the DRI2 drivers always return a front buffer for
pixmap drawables, and I think this is actually required for things to
work right (I have vague memories of hacking at this when I started
this).

How about I just stick the set of returned images back into the
DRIimageList struct:

diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
index 2601249..2a873d8 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -1319,6 +1319,7 @@ enum __DRIimageBufferMask {
 };
 
 struct __DRIimageList {
+   uint32_t image_mask;
    __DRIimage *back;
    __DRIimage *front;
 };
diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
index 1bb8241..7de7abd 100644
--- a/src/glx/dri3_glx.c
+++ b/src/glx/dri3_glx.c
@@ -1208,6 +1208,7 @@ dri3_get_buffers(__DRIdrawable *driDrawable,
    struct dri3_drawable *priv = loaderPrivate;
    struct dri3_buffer   *front, *back;
 
+   buffers->image_mask = 0;
    buffers->front = NULL;
    buffers->back = NULL;
 
@@ -1254,12 +1255,15 @@ dri3_get_buffers(__DRIdrawable *driDrawable,
    }
 
    if (front) {
+      buffers->image_mask |= __DRI_IMAGE_BUFFER_FRONT;
       buffers->front = front->image;
       priv->have_fake_front = !priv->is_pixmap;
    }
 
-   if (back)
+   if (back) {
+      buffers->image_mask |= __DRI_IMAGE_BUFFER_BACK;
       buffers->back = back->image;
+   }
 
    priv->stamp = stamp;
 
diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
index 90bbbfc..273d455 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -1338,7 +1338,7 @@ intel_update_image_buffers(struct brw_context *brw, __DRIdrawable *drawable)
                                         buffer_mask,
                                         &images);
 
-   if (images.front) {
+   if (images.image_mask & __DRI_IMAGE_BUFFER_FRONT) {
       assert(front_rb);
       drawable->w = images.front->width;
       drawable->h = images.front->height;
@@ -1348,7 +1348,7 @@ intel_update_image_buffers(struct brw_context *brw, __DRIdrawable *drawable)
                                 images.front,
                                 __DRI_IMAGE_BUFFER_FRONT);
    }
-   if (images.back) {
+   if (images.image_mask & __DRI_IMAGE_BUFFER_BACK) {
       drawable->w = images.back->width;
       drawable->h = images.back->height;
       intel_update_image_buffer(brw,

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131106/2d6834d3/attachment.pgp>