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

[Michel Dänzer]

On 01.07.2015 07:09, Eric Anholt wrote:
> 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.

On this machine, logging into an Xfce session, starting iceweasel and
opening a random website seems pretty reliable for reproducing it. Text
quickly starts disappearing all over the place in iceweasel and in the
Xfce panel.

On another machine, I haven't been able to reproduce it yet either.
However, I've seen another radeon user on IRC mention it, so it doesn't
seem to be just me.


> Can you figure out anything else that might be happening when the
> failure occurs?  Atlas replacement seems like the obvious thing.

It doesn't fill up an atlas or even switch between atlases for different
formats inside glamor_composite_glyphs.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer