Re: [PATCH 4/6] Implement has_color_glyphs for freetype

Am 09.04.2016 um 16:07 schrieb Matthias Clasen:
> On Sat, Apr 9, 2016 at 8:29 AM, Uli Schlachter <[email protected]> wrote:
>> Am 09.04.2016 um 05:22 schrieb [email protected]:
>> [...]
>>> +static cairo_bool_t
>>> +_cairo_ft_has_color_glyphs (void *scaled)
>>> +{
>>> +    cairo_ft_unscaled_font_t *unscaled = ((cairo_ft_scaled_font_t *)scaled)->unscaled;
>>> +
>>> +    if (!unscaled->have_color_set) {
>>> +        FT_Face face;
>>> +        face = _cairo_ft_unscaled_font_lock_face (unscaled);
>>> +        _cairo_ft_unscaled_font_unlock_face (unscaled);
>>> +    }
>>> +
>>> +    return unscaled->have_color;
>>> +}
>>> +
>> [...]
>>
>> I suggest replaceing the above function with 'return rand() == 42'. It returns
>> uninitialized memory anyway.
> 
> This could use a comment, maybe. If you look at
> _cairo_ft_unscaled_font_lock_face, it has the side-effect of ensuring
> that have_color is properly set.

Oh, you are right. Sorry.

In that case I suggest adding assert (unscaled->have_color_set); between
lock/unlock. You can then also add a comment there.

Uli
-- 
99 little bugs in the code
99 little bugs in the code
Take one down, patch it around
117 little bugs in the code
  -- @irqed
-- 
cairo mailing list
[email protected]
https://lists.cairographics.org/mailman/listinfo/cairo