glamor regression caused by 9c679d06 (Re: xserver: Branch 'master' - 5 commits)

[Eric Anholt]

Michel Dänzer <michel at daenzer.net> writes:

> On 30.06.2015 13:02, Keith Packard wrote:
>> 
>> commit 9c679d06055cc62aa9209318705e87dc33fba4c8
>> Author: Eric Anholt <eric at anholt.net>
>> Date:   Sun May 31 16:07:01 2015 -0700
>> 
>>     glamor: Skip actual FBO setup in our glyph atlas.
>>     
>>     VC4 (and many GLES2 renderers) can't render to GL_ALPHA, so our pixmap
>>     would end up as GLAMOR_MEMORY and our dereference of the FBO would
>>     setfault.  Instead, tell the pixmap creation that we don't need an FBO
>>     at all.  Our glyph upload path was already glTexImage for non-a1, and
>>     a more general software fallback for a1 (since the glyph is also in
>>     system memory).
>>     
>>     Signed-off-by: Eric Anholt <eric at anholt.net>
>>     Reviewed-by: Keith Packard <keithp at keithp.com>
>> 
>> diff --git a/glamor/glamor_composite_glyphs.c b/glamor/glamor_composite_glyphs.c
>> index 47bf647..c30cbed 100644
>> --- a/glamor/glamor_composite_glyphs.c
>> +++ b/glamor/glamor_composite_glyphs.c
>> @@ -112,7 +112,8 @@ glamor_glyph_atlas_init(ScreenPtr screen, struct glamor_glyph_atlas *atlas)
>>      PictFormatPtr               format = atlas->format;
>>  
>>      atlas->atlas = glamor_create_pixmap(screen, glamor_priv->glyph_atlas_dim,
>> -                                        glamor_priv->glyph_atlas_dim, format->depth, 0);
>> +                                        glamor_priv->glyph_atlas_dim, format->depth,
>> +                                        GLAMOR_CREATE_FBO_NO_FBO);
>>      atlas->x = 0;
>>      atlas->y = 0;
>>      atlas->row_height = 0;
>
> This change broke text rendering of GTK+2 apps (at least
> iceweasel/Firefox, icedove/Thunderbird and the Xfce panel) for me with
> the radeon driver. Most of the text disappears (some of it only "after a
> while").
>
> Looking at the glamor code, it doesn't seem to be as simple as setting
> GLAMOR_CREATE_FBO_NO_FBO and not getting an FBO. E.g.
> glamor_glyphs_flush() uses glamor_pixmap_fbo_at() to get the atlas
> texture identifier.

You still get the glamor_pixmap_fbo struct, with the ->tex field.  It's
just that the fbo->fb isn't created.  Yeah, naming in glamor is
terrible.

I've been poking around trying to find anything wrong with this change,
and haven't come up with anything (though I've come up with a giant pile
of other cleanups).  I also haven't managed to reproduce anything on my
Intel development system, including scrolling around in gtkterm and
resizing my webpages to try to blow out the glyph cache.

Can you figure out anything else that might be happening when the
failure occurs?  Atlas replacement seems like the obvious thing.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20150630/9e0a6f1f/attachment.sig>