[Xcb] [PATCH] dri2: Update to version 1.3

[Fredrik Höglund]

On Tuesday 06 December 2011, Peter Harris wrote:
> Thanks for the patch. Review inline.
> 
> On 2011-12-06 13:24, Fredrik Höglund wrote:
> > +
> > +  <!-- Version 1.2 -->
> 
> All the 1.2 (Or should that be 2.2?) and newer requests are listed as
> "opcode=7" in the spec. This is clearly a bug in the spec, but it means
> I can't (easily) review the opcodes used here. Also, every 1.2 and newer
> request is listed as "length=8". Either padding is missing from most
> request descriptions, or the spec is wrong. I'll assume for now that the
> spec is wrong.

There are a lot of issues with the spec so I decided to rely more on the
dri2proto.h header and the code in the server.

> > +  <request name="SwapBuffers" opcode="8">
> > +    <field type="DRAWABLE" name="drawable" />
> > +    <field type="CARD32" name="target_msc_hi" />
> > +    <field type="CARD32" name="target_msc_lo" />
> > +    <field type="CARD32" name="divisor_hi" />
> > +    <field type="CARD32" name="divisor_lo" />
> > +    <field type="CARD32" name="remainder_hi" />
> > +    <field type="CARD32" name="remainder_lo" />
> > +    <reply>
> > +      <pad bytes="1" />
> > +      <field type="CARD32" name="swap_hi" />
> > +      <field type="CARD32" name="swap_lo" />
> > +    </reply>
> 
> The spec has two different SwapBuffers requests (is one a copy-and-paste
> bug?). Both have a LISTofDRI2BUFFER at the end of the reply. One pads
> out the standard 32-byte reply header, and the other does not. Which is
> correct?

The answer is neither. The LISTofDRI2BUFFER was removed in favor of sending
SwapComplete + InvalidateBuffers events. The client has to send a GetBuffers
request to get the new buffers.

The reply is supposed to be padded out though. I was under the impression
that xcb took care of that automatically.

> > +  <!-- Events -->
> > +
> > +  <event name="BufferSwapComplete" number="0">
> > +    <field type="CARD8" name="pad0" />
> > +    <field type="CARD16" name="event_type" />
> > +    <field type="CARD16" name="pad1" />
> 
> We usually spell that <pad bytes="2" />. These padding bytes are missing
> from the spec, but the padding bytes are more likely than unaligned fields.

Yes, they're present in dri2proto.h. I'll replace them with <pad>'s.
 
> > +    <field type="DRAWABLE" name="drawable" />
> > +    <field type="CARD32" name="ust_hi" />
> > +    <field type="CARD32" name="ust_lo" />
> > +    <field type="CARD32" name="msc_hi" />
> > +    <field type="CARD32" name="msc_lo" />
> > +    <field type="CARD32" name="sbc" />
> 
> Spec still says sbc_hi and sbc_lo, even though there isn't enough space
> in non-"generic" events for sbc_lo. Is this just sbc_hi? Or is it
> something else?

sbc was respecified to be 32 bits, presumably for this reason.

The protocol header actually has a SwapComplete and a SwapComplete2
event, both with the same number. SwapComplete2 has the 32 bit sbc,
and it's the one the server generates. The reason for adding SwapComplete2
instead of fixing SwapComplete was apparently to avoid breaking source
compatibility.

sbc is still 64 bits in all other requests and replies.

> > +  <!-- Version 2.3 -->
> 
> Mismatch between comments and headers. This should be 1.3 (or <xcb
> major-version= > above should be 2).

Good catch, it should be 1.3.

Regards,
Fredrik