[cairo] [PATCH] xcb: Initialize font options from Xft resources

[Lukáš Lalinský]

On Tue, Sep 9, 2014 at 11:10 AM, Uli Schlachter <psychon at znc.in> wrote:
> Should this also ignore trailing spaces? I have no clue about this resources
> stuff...

I did some testing and xrdb seems to strip those automatically when
reading ~/.Xresources, so they never end up there. I don't have that
from any official documentation though.

>> +static void
>> +resource_parser_done (struct resource_parser *parser)
>> +{
>> +    if (parser->bytes_in_buffer > 0) {
>> +     parser->buffer[parser->bytes_in_buffer] = 0;
>
> This zero byte is already inserted in resource_parser_update, no need to add it
> here again.

I think this is actually not the case, because when I move the
remaining data to the front of the buffer, I do not move the null
char. I move only the data that is passed to the function (which might
not contain a trailing null).

> Feel free to leave this as-is and for someone else to fix up later, but in this
> case it would be nice to update the above comment.

I did leave this unchanged. This is better to be done by somebody who
knows what they are doing, not just guessing.

>> +    *options = *_cairo_xcb_screen_get_font_options (surface->screen);
>
> Still requires a pair of _cairo_xcb_connection_acquire() /
> _cairo_xcb_connection_release() somewhere. Cairo-xlib does this in
> _cairo_xlib_screen_get_font_options(), but you put an assert into
> _cairo_xcb_screen_get_font_options() making this the caller's responsibility.
> Either place is fine with me to do the necessary locking.

Thanks, I was not completely clear on how to do this before. I added
it to _cairo_xcb_surface_get_font_options, as that seems consistent
with the rest of the XCB code.

> Nice patch and once again thanks for working on this. Since the above are just
> some minor things that can easily be addressed, this patch (with those things
> fixed) is:
>
> Reviewed-by: Uli Schlachter <psychon at znc.in>

Thanks again for the review. Updated patch attached.

Lukas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-xcb-Initialize-font-options-from-Xft-resources.patch
Type: text/x-patch
Size: 17120 bytes
Desc: not available
URL: <http://lists.cairographics.org/archives/cairo/attachments/20140912/3b8c719b/attachment-0001.bin>