On Mon, 2006-03-13 at 10:45 -0800, Ian Romanick wrote:
> -----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.

Yes, I though about this and I think the best is that CopySubBuffer is
separated from buffer swapping so that no extensions that control buffer
swapping are affected by this extension. This is how the current
implementation should behave. E.g. it's not waiting for vblank or
incrementing swap counters.

Yes, the spec needs to be updated (at least with some protocol).

> 
> 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).

ok.

> 
> > --- 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. :)

yes.

> 
> 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.

ok, thanks.

-David



-------------------------------------------------------
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