Bug 30095

Summary: [GLX] Fullscreen OpenGL game now has occasional flicker (bisected)
Product: Mesa Reporter: Chris Rankin <rankincj>
Component: GLXAssignee: Kristian Høgsberg <krh>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium    
Version: git   
Hardware: x86 (IA32)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: Optimize out makecurrent with same context and drawables

Description Chris Rankin 2010-09-08 18:14:31 UTC 
Playing World of Warcraft fullscreen with my RV350, and the screen is now occasionally turning pitch black for a fraction of a second. No other side-effects noticed, but the "flicker" is annoying.

The pitch black screen isn't completely blank - it looks like the minimap is being rendered (briefly!) in the bottom LH corner of the screen.

My limited git-fu has narrowed the problem down to this commit:

16887d042a917fa4773e4d853f50051b54e9948c is the first bad commit
commit 16887d042a917fa4773e4d853f50051b54e9948c
Author: Kristian Høgsberg <krh@bitplanet.net>
Date:   Fri Aug 27 12:40:11 2010 -0400

    glx: Drop broken drawable garbage collection
    
    Doesn't work for pixmaps, was looking up the GLX XID and was never thread
    safe.  Instead, just destroy the client side structures when the
    drawable is no long current for a context.

:040000 040000 2500fd85901fe387ae1cedc6a0819ca9ffe003b7 7b1e3579557f00281007e0f05df4e7a7e0f1cba5 M	src
Comment 1 Kristian Høgsberg 2010-09-09 05:11:25 UTC 
Created attachment 38575 [details] [review]
Optimize out makecurrent with same context and drawables

Does this patch help?
Comment 2 Andy Furniss 2010-09-09 08:26:10 UTC 
(In reply to comment #1)
> Created an attachment (id=38575) [details]
> Optimize out makecurrent with same context and drawables
> 
> Does this patch help?

I just bisected an issue with r600c to this commit and the patch fixes it for me (there is a typo in the patch, I changed oldGc to oldGC).

The issue was that some mesa demos totally blocked output to the screen - no mouse, or vt switch (sysrq worked).

It turned out that pkill the demo would fix so it was easy to bisect like -

 (./tunnel &)  && sleep 10 && pkill tunnel
Comment 3 Andy Furniss 2010-09-09 08:39:25 UTC 
(In reply to comment #2)

> I just bisected an issue with r600c to this commit and the patch fixes it for
> me.

Spoke too soon it fixes tunnel and geartrain which previously would "lock", all other demos now work apart from -

tunnel2 which still "locks"
Comment 4 Chris Rankin 2010-09-09 12:08:24 UTC 
(In reply to comment #1) 
> Does this patch help?

No, sorry. The flickering still happens with this patch.
Comment 5 Chris Rankin 2010-09-09 16:00:12 UTC 
(In reply to comment #1)
> Does this patch help?

I should add that this flickering doesn't happen everywhere - only in places where WoW displays the "indoors" minimap. This *might* imply that the problem is related to pbuffers, since (IIRC) the "indoors" minimap needed pbuffers and KMS to be implemented in order to work in the first place
Comment 6 Kristian Høgsberg 2010-09-09 16:07:25 UTC 
(In reply to comment #5)
> (In reply to comment #1)
> > Does this patch help?
> 
> I should add that this flickering doesn't happen everywhere - only in places
> where WoW displays the "indoors" minimap. This *might* imply that the problem
> is related to pbuffers, since (IIRC) the "indoors" minimap needed pbuffers and
> KMS to be implemented in order to work in the first place

That's good info, I suspect it's because WoW uses glXMakeCurrent to switch to  different drawables, which triggers the more aggressive cleanup introduced in that patch.
Comment 7 Kristian Høgsberg 2010-09-10 05:11:54 UTC 
Can you try the patch in #30109 please?
Comment 8 Andy Furniss 2010-09-10 06:00:18 UTC 
(In reply to comment #7)
> Can you try the patch in #30109 please?

It doesn't fix tunnel2 for me.
Comment 9 Chris Rankin 2010-09-11 04:24:31 UTC 
(In reply to comment #7)
> Can you try the patch in #30109 please?

The flickering is still present with this patch.
Comment 10 Andy Furniss 2010-09-21 07:43:46 UTC 
(In reply to comment #8)
> (In reply to comment #7)
> > Can you try the patch in #30109 please?
> 
> It doesn't fix tunnel2 for me.

Mesa master commit 
441344ba7ed2a1d162ee33ac4bac4bf645188ceb
glx: Hold on to drawables if we're just switching to another context

fixes tunnel2 for me.
Comment 11 Kristian Høgsberg 2010-09-21 08:59:05 UTC 
(In reply to comment #9)
> (In reply to comment #7)
> > Can you try the patch in #30109 please?
> 
> The flickering is still present with this patch.

Chris, does that commit also fix the WoW flickering you're seeing?
Comment 12 Chris Rankin 2010-09-21 13:01:58 UTC 
(In reply to comment #11)
> Chris, does that commit also fix the WoW flickering you're seeing?

Unfortunately it doesn't, although the flicker might be quicker now.
Comment 13 Chris Rankin 2010-09-29 14:10:31 UTC 
commit 4b70fe8421f5132c585ff1dfb8d90229be26e71f
Author: Kristian Høgsberg <krh@bitplanet.net>
Date:   Wed Sep 29 08:32:12 2010 -0400

    glx: Only remove drawables from the hash when we actually delete them
    
This commit seems to have fixed my flicker problem... so far...
Comment 14 Kristian Høgsberg 2010-09-29 16:36:28 UTC 
(In reply to comment #13)
> commit 4b70fe8421f5132c585ff1dfb8d90229be26e71f
> Author: Kristian Høgsberg <krh@bitplanet.net>
> Date:   Wed Sep 29 08:32:12 2010 -0400
> 
>     glx: Only remove drawables from the hash when we actually delete them
> 
> This commit seems to have fixed my flicker problem... so far...

Woo, good news.  The commit you bisected to should have been safe if I hadn't messed it up.  4b70 fixes the brain-fart in the original commit, and it's good to hear that it fixes the flickering.

I'll close this bug, but reopen if you come across other flickering caused by the 1688 commit.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.