Bug 27628 - glxgears prints bogus swap interval information
glxgears prints bogus swap interval information
Status: RESOLVED FIXED
Product: Mesa
Classification: Unclassified
Component: Demos
git
Other All
: medium normal
Assigned To: mesa-dev
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-04-14 04:14 UTC by Michal Schmidt
Modified: 2010-04-19 09:57 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
[PATCH] Initialize swap_interval to 0 (609 bytes, patch)
2010-04-14 04:14 UTC, Michal Schmidt
Details | Splinter Review
[PATCH] Initialize swap_interval to 0 (410 bytes, patch)
2010-04-14 04:16 UTC, Michal Schmidt
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Michal Schmidt 2010-04-14 04:14:32 UTC
Created attachment 34997 [details] [review]
[PATCH] Initialize swap_interval to 0

$ glxgears 
Running synchronized to the vertical refresh.  The framerate should be
approximately 1/1920103026 the monitor refresh rate.

The number is obviously nonsense. This is on current Fedora 13 (mesa-7.8.1-1.fc13).

I looked into it and here's what I found:
 - glxgears gets this number using GLX_MESA_swap_control from glXGetSwapIntervalMESA().
 - glXGetSwapIntervalMESA() calls into src/glx/dri2_glx.c:dri2GetSwapInterval() which returns priv->swap_interval - this value is not initialized. Valgrind agrees:

==25411== Conditional jump or move depends on uninitialised value(s)
==25411==    at 0x402C9A: main (glxgears.c:619)

Should pdraw->swap_interval be initialized to 0 in dri2CreateDrawable(), like in the attached patch?
Comment 1 Michal Schmidt 2010-04-14 04:16:42 UTC
Created attachment 34998 [details] [review]
[PATCH] Initialize swap_interval to 0

Sorry, attached wrong file before.
Comment 2 Kristian Høgsberg 2010-04-14 07:16:11 UTC
Thanks, looks good. Committed:

commit 0863c7e499a553c2d8e7bcbd09c5de88e396fcd0
Author: Michael Schmidt <mschmidt@redhat.com>
Date:   Wed Apr 14 10:12:42 2010 -0400

    Initialize DRI2 swap interval to 0
    
    https://bugs.freedesktop.org/show_bug.cgi?id=27628
Comment 3 Jesse Barnes 2010-04-14 09:10:27 UTC
On Wed, 14 Apr 2010 17:57:07 +0200
Mario Kleiner <mario.kleiner@tuebingen.mpg.de> wrote:

> Hmm. The patch inits mesa's local cached copy of swap_interval to  
> zero. The DRI2CreateDrawable() function inside the xserver's xserver/ 
> hw/xfree86/dri2/dri2.c implementation inits its own copy of  
> swap_interval to 1, at least in the DRI2 patch series of Jesse and  
> mine that is supposed be pulled of the next x-server release.
> 
> So until client code calls glXSetSwapIntervalMESA() or  
> glXSetSwapIntervalSGI() the first time, the returned value will not  
> match the true setting.
> 
> Either we need to match the defaults, or propagate the mesa setting  
> to the server at drawable creation time, or query the server's  
> setting to init mesa's copy.

Yeah was just talking with Kristian about this.  According to the
SGI_swap_interval spec, the default value should be 1.  For direct
rendered clients we should probably just make them send a swap interval
protocol request so that the server will match up and all will be in
sync.  That way we won't have to keep the server and mesa in sync by
hand.

I'll get to it later today unless Kristian already has a fix.
Comment 4 Jesse Barnes 2010-04-19 09:57:49 UTC
On Wed, 14 Apr 2010 17:57:07 +0200
Mario Kleiner <mario.kleiner@tuebingen.mpg.de> wrote:

> Hmm. The patch inits mesa's local cached copy of swap_interval to  
> zero. The DRI2CreateDrawable() function inside the xserver's xserver/ 
> hw/xfree86/dri2/dri2.c implementation inits its own copy of  
> swap_interval to 1, at least in the DRI2 patch series of Jesse and  
> mine that is supposed be pulled of the next x-server release.
> 
> So until client code calls glXSetSwapIntervalMESA() or  
> glXSetSwapIntervalSGI() the first time, the returned value will not  
> match the true setting.
> 
> Either we need to match the defaults, or propagate the mesa setting  
> to the server at drawable creation time, or query the server's  
> setting to init mesa's copy.

Got distracted by other things last week, but the fix is pushed now to
both 7.8 and master.

commit 385e2896ebf54ac0b016132fe513f21a5b67ba4f
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Mon Apr 19 09:54:08 2010 -0700

    DRI2: synchronize swap interval with server at startup time