[cairo] segment fault when rendering text with space charactors onto glitz surface.

[sunmoon1997]

sunmoon1997 wrote:

> Vladimir Vukicevic wrote:
>
>> On 8/9/05, David Reveman <davidr at novell.com> wrote:
>>  
>>
>>> On Tue, 2005-08-09 at 09:51 -0700, Vladimir Vukicevic wrote:
>>>   
>>>
>>>> Here's an updated patch for this issue, as well as not initializing
>>>> the memory entry member (I sent both patches separately earlier, this
>>>> is both combined and updated to latest CVS).  Carl/David, let me know
>>>> if it's ok to commit this..
>>>>     
>>>
>>> It's OK commit it.
>>>
>>> I'm not sure the:
>>> (area == &_empty_area)
>>> check is necessary as the code should never try top move in areas with
>>> width == 0 or height == 0 but I might be wrong so it's OK to commit it
>>> as it is, I'll change it later if it turns out not necessary.
>>>   
>>
>>
>> Hmm, so applying this patch to current CVS (with my patch checked in)
>> still caused a slightly different crash with 0-space characters.  The
>> attached patch fixes it, but at this point I'm just playing
>> whack-a-mole without a full understanding of the glitz glyph caching
>> implementation... in any case I'd like to check the patch in, since
>> text seems to render with it, but I have no idea about the correctness
>> -- specifically, I'm not sure why we ever have an image_entry->image
>> for a space character.  The move_in check isn't needed with this...
>>
>>    - Vlad
>>  
>>
>> ------------------------------------------------------------------------
>>
>> Index: cairo-glitz-surface.c
>> ===================================================================
>> RCS file: /cvs/cairo/cairo/src/cairo-glitz-surface.c,v
>> retrieving revision 1.57
>> diff -u -8 -p -r1.57 cairo-glitz-surface.c
>> --- cairo-glitz-surface.c    9 Aug 2005 20:42:54 -0000    1.57
>> +++ cairo-glitz-surface.c    9 Aug 2005 21:25:11 -0000
>> @@ -1261,19 +1261,16 @@ struct _cairo_glitz_root_area {
>>     cairo_glitz_area_t           *area;
>>     const cairo_glitz_area_funcs_t *funcs;
>> };
>>
>> static cairo_status_t
>> _cairo_glitz_area_move_in (cairo_glitz_area_t *area,
>>                void              *closure)
>> {
>> -    if (area == &_empty_area)
>> -    return CAIRO_STATUS_SUCCESS;
>> -
>>     area->closure = closure;
>>     area->state   = CAIRO_GLITZ_AREA_OCCUPIED;
>>         return (*area->root->funcs->move_in) (area, area->closure);
>> }
>>
>> static void
>> _cairo_glitz_area_move_out (cairo_glitz_area_t *area)
>> @@ -1780,17 +1777,18 @@ _cairo_glitz_cache_glyph (cairo_glitz_gl
>>     int             am, rm, gm, bm;
>>
>>     entry->size = image_entry->size;
>>         if (entry->size.width  > GLYPH_CACHE_MAX_WIDTH ||
>>     entry->size.height > GLYPH_CACHE_MAX_HEIGHT)
>>     return CAIRO_STATUS_SUCCESS;
>>
>> -    if (!image_entry->image)
>> +    if ((entry->size.width  == 0 && entry->size.height == 0) ||
>> +        !image_entry->image)
>>     {
>>     entry->area = &_empty_area;
>>     return CAIRO_STATUS_SUCCESS;
>>     }
>>         format = pixman_image_get_format 
>> (image_entry->image->pixman_image);
>>     if (!format)
>>     return CAIRO_STATUS_NO_MEMORY;
>>  
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> cairo mailing list
>> cairo at cairographics.org
>> http://lists.freedesktop.org/mailman/listinfo/cairo
>>  
>>
> Hi,
>    This patch doesn't fix the problem completely. My test program 
> still get segment fault. I attach a  small patch to complete the fix.
>
> Many thanks.
> Jinghua Luo
>
>------------------------------------------------------------------------
>
>Index: src/cairo-glitz-surface.c
>===================================================================
>RCS file: /cvs/cairo/cairo/src/cairo-glitz-surface.c,v
>retrieving revision 1.57
>diff -u -r1.57 cairo-glitz-surface.c
>--- src/cairo-glitz-surface.c	9 Aug 2005 20:42:54 -0000	1.57
>+++ src/cairo-glitz-surface.c	10 Aug 2005 06:43:48 -0000
>@@ -1785,7 +1785,9 @@
> 	entry->size.height > GLYPH_CACHE_MAX_HEIGHT)
> 	return CAIRO_STATUS_SUCCESS;
> 
>-    if (!image_entry->image)
>+    if (!image_entry->image ||
>+	entry->size.width == 0 ||
>+	entry->size.height == 0)
>     {
> 	entry->area = &_empty_area;
> 	return CAIRO_STATUS_SUCCESS;
>  
>
Hi,
   Sorry for not reading your message carefully, I found you already fix 
this though it's not in CVS, I just update cairo from CVS repository and 
find it still not work.  From my understanding, cairo-glitz create a big 
surface(root erea) first,  then divid it into small ereas and cache 
glyphs in the small area.  For 0-space characters we don't  need the 
erea. Please point out if  it's wrong.:)

regards
Jinghua Luo