Serve side GLX changes (was Re: xserver: Branch 'master' - 2 commits)

[Ian Romanick]

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

Kristian HXXgsberg wrote:

> diff --git a/GL/glx/glxdri.c b/GL/glx/glxdri.c
> index 4239229..6a10554 100644
> --- a/GL/glx/glxdri.c
> +++ b/GL/glx/glxdri.c
> @@ -112,12 +112,28 @@ struct __GLXDRIdrawable {
>   *            months ago. :(
>   * 20050727 - Gut all the old interfaces.  This breaks compatability with
>   *            any DRI driver built to any previous version.
> + * 20060314 - Added support for GLX_MESA_copy_sub_buffer.
>   */
> +
>  #define INTERNAL_VERSION 20050727
>  
>  static const char CREATE_NEW_SCREEN_FUNC[] =
>      "__driCreateNewScreen_" STRINGIFY (INTERNAL_VERSION);
>  
> +/* The DRI driver entry point version wasn't bumped when the
> + * copySubBuffer functionality was added to the DRI drivers, but the
> + * functionality is still conditional on the value of the
> + * internal_api_version passed to __driCreateNewScreen.  However, the
> + * screen constructor doesn't fail for a DRI driver that's older than
> + * the passed in version number, so there's no way we can know for
> + * sure that we can actually use the copySubBuffer functionality.  But
> + * since the earliest (and at this point only) released mesa version
> + * (6.5) that uses the 20050727 entry point does have copySubBuffer,
> + * we'll just settle for that.  We still have to pass in a higher to
> + * the screen constructor to enable the functionality.
> + */
> +#define COPY_SUB_BUFFER_INTERNAL_VERSION 20060314
> +

There's a difference between the API version being capable of supporting
an extension and a driver actually implementing it.  In this case, the
driver needs a certain API version to know that the structures passed in
from libGL have certain fields.  Then the driver calls
glxEnableExtension with the name of the extenion to actually enable it.

Once the driver enables the extension, the client_support,
direct_support and either server_support or direct_only bits in the GLX
extension_info (see src/glx/x11/glxextension.c) need to be set before
the extension is actually enabled.

So, I guess I'm not sure what you're trying to say with this comment.
If the field already exists in __DRIdrawableRec, then there's no need to
bump the internal version.  If the field is there, but libGL doesn't
actually support the extension, then client_support will never be set
and the extension will never be enabled.  Ditto for the driver and
direct_support.

Although, it looks like radeon drivers always enable the extension
regarless of the API version, which is a mistake.

In any case, I don't see a reason to create a new verion value.  Just
bump INTERNAL_VERSION to what it should be (i.e., 20060314).

> diff --git a/GL/glx/g_disptab.h b/GL/glx/g_disptab.h
> index 9be5bdf..a83c7a8 100644
> --- a/GL/glx/g_disptab.h
> +++ b/GL/glx/g_disptab.h
> @@ -124,4 +126,11 @@ extern __GLXdispatchRenderProcPtr __glXR
>  extern __GLXdispatchSingleProcPtr __glXSingleTable[__GLX_SINGLE_TABLE_SIZE];
>  extern __GLXdispatchRenderProcPtr __glXSwapRenderTable[__GLX_RENDER_TABLE_SIZE];
>  extern __GLXdispatchSingleProcPtr __glXSwapSingleTable[__GLX_SINGLE_TABLE_SIZE];
> +
> +/* Copied from mesa src/glx/x11/glxcmds.c
> + *
> + * Apparently there's no standardized opcode for this extension.
> + */
> +#define X_GLXvop_CopySubBufferMESA 5154 /* temporary */
> +

What is this?  Where did this opcode come from?  If it's not offically
assigned, PLEASE FOR THE LOVE OF GOD DO NOT ENABLE IT!!!!

> diff --git a/GL/glx/glxscreens.c b/GL/glx/glxscreens.c
> index 77654d3..3809af3 100644
> --- a/GL/glx/glxscreens.c
> +++ b/GL/glx/glxscreens.c
> @@ -139,6 +139,7 @@ static char GLXServerExtensions[] =
>                          "GLX_SGIX_swap_barrier "
>  #endif
>  			"GLX_SGIX_fbconfig "
> +			"GLX_MESA_copy_sub_buffer "
>  			;

For sanity sake, I like to keep the extension strings sorted by ARB,
EXT, then alphabetically by vendor.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)

iD8DBQFEsp4tX1gOwKyEAw8RAu28AKCfVepNv/2xaqTvjFSof8VS23Al8ACeNSiW
0Pn0NI/7uVo0wiIfqzs5dEE=
=Fl4/
-----END PGP SIGNATURE-----