[Xcb] [PATCH 2/2] glx: Add protocol for GLX_ARB_create_context and GLX_ARB_create_context_profile

[Peter Harris]

Thanks for doing all the hard work. Review inline below:

On 2011-11-30 13:01, Ian Romanick wrote:
>  
> +	<request name="SetClientInfoARB" opcode="33">

> +		<list type="CARD32" name="glversions">

SetClientInfo2ARB uses an underscore in gl_versions. For consistency,
you should probably add an underscore here, or remove the other one.

> +		<list type="char" name="glextensionstring">
> +			<fieldref>gl_str_len</fieldref>
> +		</list>
> +		<list type="char" name="glxextensionstring">
> +			<fieldref>glx_str_len</fieldref>
> +		</list>

There is no implicit padding between lists of the same type. This needs
something like <list type="void" name="alignment_pad"> (see
xf86vidmode.xml) between the strings.

I apologize for the awkward syntax. Unfortunately, we don't have <pad
align="4" /> yet.

> +	<request name="CreateContextAttribsARB" opcode="34">
> +		<field type="glx:CONTEXT" name="context" />
> +		<field type="CARD32" name="fbconfig" />
> +		<field type="CARD32" name="screen" />
> +		<field type="CARD32" name="share_list" />

The spec says type="glx:CONTEXT" name="share_context" instead of
"share_list". Is this a typo here or in the spec?

> +	<request name="SetClientInfo2ARB" opcode="35">

> +		<list type="CARD32" name="gl_versions">

SetClientInfoARB doesn't use the underscore in the name. See above.

> +		<list type="char" name="gl_extension_string">
> +			<fieldref>gl_str_len</fieldref>
> +		</list>
> +		<list type="char" name="glx_extension_string">
> +			<fieldref>glx_str_len</fieldref>
> +		</list>

This one needs explicit padding too.

Peter Harris
-- 
               Open Text Connectivity Solutions Group
Peter Harris                    http://connectivity.opentext.com/
Research and Development        Phone: +1 905 762 6001
pharris at opentext.com            Toll Free: 1 877 359 4866