[Xcb] xcb_render_util_composite_text

[Ian Osgood]

On Oct 14, 2006, at 7:09 PM, Jamey Sharp wrote:

> On Sat, Oct 14, 2006 at 02:47:20PM -0700, Ian Osgood wrote:
>> On Oct 14, 2006, at 1:47 PM, Jamey Sharp wrote:
>>> You haven't indicated how errors should be handled: mainly, what if
>>> the caller tries to pass in more glyphs or glyphsets than they
>>> allocated space for?
>>
>> The initial buffer would grow, so I don't consider this an error
>> condition.  Preallocation is just an optimization.
>
> Ah. I didn't understand that.
>
> Still, errors can occur: realloc can fail, for example. Should the
> stream free resources and go into an inert error state, or should the
> current glyph sequence be ignored, or what?

Good point. We could have the glyph and glyphset functions return  
success/fail.
>
>>> If we don't free it when sent, should we allow more glyphs to be  
>>> added
>>> afterwards? I'm thinking no: sending the stream makes it constant.
>>
>> There is nothing in my current implementation that would prevent  
>> this.
>
> Sure, but promising to allow glyphs to be added after send constrains
> the implementation to support that in the future. Do we want that?

I'm ambivalent. The interface is as good as what we already have.
>
>> My current implementation is laying out the wire protocol request
>> into the stream on each call. Changing sizes mid-stream would require
>> me to regeneratethe stream from scratch (doable, but tedious,
>
> Yup, I'd pick a slightly different implementation if we were going  
> to do
> that. I'd probably use one of the padding bytes in the glyph header to
> store the size in bytes of the glyphs in the current chunk, but write
> the next header with enough space for the same number of 32-bit  
> glyphs.
> While adding glyphs I'd record the largest one seen yet, and when
> sending I'd compact the stream to the smallest legal size.
>
> That's why I don't want to allow more glyphs to be added after the
> stream has been sent: at that point its representation may have become
> one that isn't easy to add to.
>
> We could provide the ability to clone a stream, though: if you have an
> immutable stream and you clone it, you get one that's mutable  
> again. Or
> we could just expand the stream again when new glyphs are added to it;
> perhaps only if the new glyphs can't be represented in the current  
> size.
>
>> especially since it is in network byte order).
>
> Where did you get that idea?

Um.

Because it's a network protocol, I guess? (I swear I saw some  
swapping going on.)

Feel free to fix that up if I don't get to it soon.

> (By this question I mean, "what docs do we
> need to go fix?") My reading of the server implementation is that all
> fields in the glyphs requests are expected to be in the client's  
> native
> byte order: the server will byte-swap them. That includes glyphsets,
> which I had thought were always sent MSB-first, but maybe I was  
> thinking
> of FONTs in PolyText{8,16}.
>
>> For the first iteration, I'll just support one size and consider
>> mixing an error. I don't think this will be much of a problem, since
>> current users of XRenderCompositeText must check ahead of time  
>> anyway.
>
> Anything that's considered an error now can be given working semantics
> later without an ABI change, so this is fine.
>
> By the way, I see you've limited glyph_8 chunks to 252 glyphs  
> instead of
> 254. The reason that libXrender does that is only because if you pack
> 254 glyphs into one chunk, you waste two bytes of padding:  
> deferring the
> last two to the next chunk saves a tiny bit of bandwidth. The last  
> chunk
> should always have up to 254, though, and the protocol allows this.  
> (I'm
> not sure libXrender takes advantage here though.)
>
> I've reviewed your code, and committed a simplification of
> xcb_render_util_composite_text and an implementation of
> xcb_render_util_composite_text_checked.

Yeah, I thought of that, but I was lazy.

> gcc with -Wall notices that
> htons and htonl aren't declared, but I don't think you needed them
> anyway. gcc with -pedantic notices that your structure initializers
> aren't computable at load-time, which should probably be fixed.
> Otherwise this looks good to me!
>
> --Jamey
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb