Bug 2655 - glxgears should call glFinish before glXSwapBuffers
Summary: glxgears should call glFinish before glXSwapBuffers
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Demos (show other bugs)
Version: git
Hardware: x86 (IA32) Linux (All)
: high normal
Assignee: Xorg Project Team
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-03-05 19:34 UTC by Felix Kühling
Modified: 2011-10-15 16:04 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
glFinish before glXSwapBuffers (1001 bytes, patch)
2005-03-05 19:36 UTC, Felix Kühling
Details | Splinter Review

Description Felix Kühling 2005-03-05 19:34:52 UTC
When I changed the Savage driver to flush primitives less frequently to the
hardware glxgears performance decreased, which is counter intuitive. The reason
is that glFinish currently waits for the blit from back to front buffer and
after that the hardware waits for glxgears and the driver to prepare the next frame.

Moving the call to glFinish before glXSwapBuffers fixes this and improves
performance a lot on this hardware (330fps -> 355fps). Now glxgears and the
driver can prepare the next frame while the hardware is busy with the blit (the
driver is smart enough to flush after expensive primitives like swap and clear).

I'll attach a patch for consideration.
Comment 1 Felix Kühling 2005-03-05 19:36:41 UTC
Created attachment 2023 [details] [review]
glFinish before glXSwapBuffers
Comment 2 Adam Jackson 2005-03-07 08:16:11 UTC
please make sure to also apply this to the glxgears in Mesa, since that's where
it lives in the new world order.
Comment 3 Roland Mainz 2005-03-07 11:44:36 UTC
ajax wrote:
> please make sure to also apply this to the glxgears in Mesa,

Both versions need to be sync'ed anyway as the Xorg version already carries a
couple of things not available in the stock Mesa version...

> since that's where it lives in the new world order.

ajax:
How is this patch different from the one I did in bug 2220 ?
Comment 4 Felix Kühling 2005-03-07 14:05:33 UTC
Ah, I see that the resolution of bug #2220 which was just committed to CVS fixes
this one too. Closing as fixed. Thanks!
Comment 5 Roland Mainz 2005-03-07 14:07:57 UTC
(In reply to comment #4)
> Ah, I see that the resolution of bug #2220 which was just committed to CVS fixes
> this one too. Closing as fixed. Thanks!

Uhm... bug #2220 was a backout of a previous patch in that bug... are you sure
the backout fixed that issue (note that the code does not call |glFinish()| now) ?
Comment 6 Felix Kühling 2005-03-07 14:20:40 UTC
(In reply to comment #5)
> Uhm... bug #2220 was a backout of a previous patch in that bug... are you sure
> the backout fixed that issue (note that the code does not call |glFinish()| now) ?

Calling glFinish in a different place was just a workaround for decreased
performance without changing the latency (I wasn't sure of the purpose of
glFinish, so I tried to change as little as possible). Naturally, not calling
glFinish at all removes all performance penalties, which makes me perfectly happy.


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.