Hi,
In detectRefreshRateOfScreen(), we have
s->redrawTime = 1000 / s->opt[COMP_SCREEN_OPTION_REFRESH_RATE].value.i;
When the refresh rate is autodetected through RandR's XRRConfigCurrentRate
function, 0 can be returned in certain cases, leading to division by zero. An
example of this situation would be the DynamicTwinView option in the nVidia
proprietary drivers[0] (note that this is the default option in TwinView
configurations). Apparently, it can happen in other situations as well.
A simple patch that falls back to the manually-set refresh rate when a SIGFPE
is imminent is attached, but I don't think it's really the best way to
deal
with this situation. In particular, where the index doesn't = 0,
there'll be
no crash but the refresh rate will be stupidly slow. A better solution might
be to fall back to the manual setting where a stupid (e.g. < 50 ?) refresh
rate has been autodetected.
Technically, it's the driver behaving in a broken way, but it'd be nice
to
avoid the crash.
/Nick ("lupine" / "lupine_85")
[0]http://http.download.nvidia.com/XFree86/Linux-x86/1.0-9755/README/appendix-d.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-fpe.patch
Type: text/x-diff
Size: 462 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/compiz/attachments/20070402/32ae7c76/fix-fpe.bin
On Mon, 2007-04-02 at 12:41 +0100, Nicholas Thomas wrote:> Hi, > > In detectRefreshRateOfScreen(), we have > > s->redrawTime = 1000 / s->opt[COMP_SCREEN_OPTION_REFRESH_RATE].value.i; > > When the refresh rate is autodetected through RandR's XRRConfigCurrentRate > function, 0 can be returned in certain cases, leading to division by zero. An > example of this situation would be the DynamicTwinView option in the nVidia > proprietary drivers[0] (note that this is the default option in TwinView > configurations). Apparently, it can happen in other situations as well. > > A simple patch that falls back to the manually-set refresh rate when a SIGFPE > is imminent is attached, but I don't think it's really the best way to deal > with this situation. In particular, where the index doesn't = 0, there'll be > no crash but the refresh rate will be stupidly slow. A better solution might > be to fall back to the manual setting where a stupid (e.g. < 50 ?) refresh > rate has been autodetected. > > Technically, it's the driver behaving in a broken way, but it'd be nice to > avoid the crash.Sure, we should avoid the crash. I think + if (value.i == 0) + value.i = defaultRefreshRate; is a bit more appropriate. - David