-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

David Reveman wrote:
> This patch adds direct and indirect rendering support for
> GLX_MESA_copy_sub_buffer. Intel, r200 and r300 drivers are the only
> drivers I've implemented support for so far. I've only tried the intel
> implementation and that seems to be working OK. I've attached a patch
> for CVS head and one for 6.4.2.

I think the spec for this extension and the code implementing it needs a
slight update.  The problem is that the extension assumes that
SwapBuffers is implemented by copying the back buffer to the front.
However, this is *not* always the case.  The result is that this
extension does not produce the desired results with page flipping.

However, it is possible for an application to detect / request the copy
behavior by using GLX_OML_swap_method.  An fbconfig with copy semantics
can be requested by using "GLX_SWAP_METHOD_OML, GLX_SWAP_COPY_OML" in
the attribs list.  If not particular method is requested, the current
method can be queried via glXGetFBConfigAttrib.

The only rub is that most of our drivers only advertise a
GLX_SWAP_METHOD_OML of GLX_SWAP_UNDEFINED_OML.  This is because, in the
drivers that support page-flipping, we switch between GLX_SWAP_COPY_OML
and GLX_SWAP_EXCHANGE_OML on the fly.  We can easilly modify the drivers
to adverteise GLX_SWAP_COPY_OML fbconfigs and enforce that behavior.

Currently none of the drivers actually advertise GLX_SWAP_EXCHANGE_OML
fbconfigs.

http://oss.sgi.com/projects/ogl-sample/registry/OML/glx_swap_method.txt

My opinion is that we should modify GLX_MESA_copy_sub_buffer to, at the
very least, note that the desired behavior is only guaranteed for the
GLX_SWAP_COPY_OML case.  We may even want to say that, if
GLX_OML_swap_method is supported, it is an error to call
glXCopySubBufferMESA when GLX_SWAP_METHOD_OML is not GLX_SWAP_COPY_OML.

Dunno.

I have a few comments about the patches, but nothing too major...

> --- include/GL/internal/dri_interface.h       29 Nov 2005 23:01:43 -0000      
> 1.17
> +++ include/GL/internal/dri_interface.h       12 Mar 2006 15:58:04 -0000
> @@ -472,6 +472,9 @@
>       * \since Internal API version 20030317.
>       */
>      unsigned swap_interval;
> +
> +    void (*copySubBuffer)(__DRInativeDisplay *dpy, void *drawablePrivate,
> +                       int x, int y, int w, int h);
>  };

Please add a \since comment here and bump the internal version.  You'll
also need to update __glXGetInternalVersion (glxcmds.c).

> --- src/glx/x11/glxextensions.c       24 Feb 2006 15:36:24 -0000      1.13
> +++ src/glx/x11/glxextensions.c       12 Mar 2006 15:58:41 -0000
> @@ -79,7 +79,7 @@
>     { GLX(EXT_visual_rating),           VER(0,0), Y, Y, N, N },
>     { GLX(MESA_agp_offset),             VER(0,0), N, N, N, Y }, /* Deprecated 
> */
>     { GLX(MESA_allocate_memory),        VER(0,0), Y, N, N, Y },
> -   { GLX(MESA_copy_sub_buffer),        VER(0,0), N, N, N, N }, /* 
> Deprecated? */
> +   { GLX(MESA_copy_sub_buffer),        VER(0,0), Y, Y, N, N }, /* 
> Deprecated? */

This should be "Y, N, N, N".  Setting direct_support here means that the
extension is enable without the driver explicitly enabling it.  Since
not all drivers will set the copySubBuffer pointer, I assume that's not
what you want. :)

I'll update the field comments in the extension_info structure today.
It's not very clear.  Sorry.

>     { GLX(MESA_pixmap_colormap),        VER(0,0), N, N, N, N }, /* Deprecated 
> */
>     { GLX(MESA_release_buffers),        VER(0,0), N, N, N, N }, /* Deprecated 
> */
>     { GLX(MESA_set_3dfx_mode),          VER(0,0), N, N, N, N }, /* Deprecated 
> */
> --- src/mesa/drivers/dri/common/dri_util.c    29 Nov 2005 23:01:43 -0000      
> 1.29
> +++ src/mesa/drivers/dri/common/dri_util.c    12 Mar 2006 15:58:46 -0000
> @@ -547,6 +547,13 @@
>                                                             remainder );
>  }
>  
> +static void driCopySubBuffer( __DRInativeDisplay *dpy, void *drawablePrivate,
> +                           int x, int y, int w, int h)
> +{
> +    __DRIdrawablePrivate *dPriv = (__DRIdrawablePrivate *) drawablePrivate;
> +    dPriv->driScreenPriv->DriverAPI.CopySubBuffer(dPriv, x, y, w, h);
> +    (void) dpy;
> +}
>  
>  /**
>   * This is called via __DRIscreenRec's createNewDrawable pointer.
> @@ -622,6 +629,7 @@
>      pdraw->swapBuffersMSC = driSwapBuffersMSC;
>      pdraw->frameTracking = NULL;
>      pdraw->queryFrameTracking = driQueryFrameTracking;
> +    pdraw->copySubBuffer = driCopySubBuffer;

After bumping the API version, this code will need to verify that
api_ver is new enough for this field to exist.

>  
>      /* This special default value is replaced with the configured
>       * default value when the drawable is first bound to a direct
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (GNU/Linux)

iD8DBQFEFb3kX1gOwKyEAw8RAq88AJwIalYqOmmatygb894V6MMHQemjDQCdEsjr
G06AvFIu3z9Ac5yg+JX1e7c=
=Rrd2
-----END PGP SIGNATURE-----


-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Reply via email to