GNOME Bugzilla – Bug 655746
Allow disabling window animations
Last modified: 2013-02-15 01:48:10 UTC
Include disable-window-animations in gnome-shell-extensions. https://github.com/lzap/disable-window-animations
Created attachment 193001 [details] [review] Adding disable-window-animations ext. (BZ 655746) # modified: README # modified: configure.ac # new file: extensions/disable-window-animations/Makefile.am # new file: extensions/disable-window-animations/extension.js # new file: extensions/disable-window-animations/metadata.json.in # new file: extensions/disable-window-animations/stylesheet.css
There is already a GSettings key called org.gnome.desktop.interface.enable-animations, and gnome-shell core should be patched to look at it.
Ouch :-) So you say Gnome Shell will eventually read this setting. Feel free to close it then. From what version is it working?
No, I said that gnome-shell *should* look at this. Code is missing, at the moment.
Created attachment 193949 [details] [review] windowManager: respect animations settings The org.gnome.desktop.interface schema has a setting for enabling/ disabling animations in the whole desktop. We should respect that when animating window operations.
You patch works for me in 3.0 (Fedora 15).
Review of attachment 193949 [details] [review]: Tested on Gnome 3.0 from Fedora 15. I could test it against F16 (Gnome 3.2) in few weeks as well if needed.
(In reply to comment #7) > Review of attachment 193949 [details] [review]: > > Tested on Gnome 3.0 from Fedora 15. I could test it against F16 (Gnome 3.2) in > few weeks as well if needed. I'll wait for a review from a member of the shell team before pushing.
Review of attachment 193949 [details] [review]: Makes sense to me.
Comment on attachment 193949 [details] [review] windowManager: respect animations settings Can we please wait for some designer input before pushing this? For the record, it does not make much sense to me that the key disables some random animations (mapping, minimizing, ...), but not others (in particular the overview transition, which is very much a "windows animation")
Review of attachment 193949 [details] [review]: Hmm, I'm not sure we need designer feedback, because afaik, the key is no-where exposed in configuration. Still, it seems to me that we should try to do this at the tweener level, so that we aren't always fighting some animation that leaks out. The way I'd imagine it would work is that at the first ticker callback, any active tweens would be set to complete and properties set to their final value. Like the effect of St.set_slow_down_factor(<very small number>). Trying that, it's sort of jarring in some cases - message tray, workspace thumbnail slide ins, but I think it's a whole lot of work to define "good animations" vs. "bad animations", for something that's just hidden in a non-exposed GSetting.
we did GNOME_SHELL_SLOWDOWN_FACTOR=0.001 on Matthias's slow software rendered machine, and it definitely improved the user experience in that case. One side effect, though, was the logout timer abuses tweener, and no longer waited 60 seconds.
so, where does this stand ? do we need a patch to stop tweener abuses, and another one to set a slowdown factor in response to disable-animations ?
I'm asking, because we've identified a reduced resources mode as one of the tasks for dropping fallback mode.
FYI https://extensions.gnome.org/extension/119/disable-window-animations/
(In reply to comment #13) > so, where does this stand ? > > do we need a patch to stop tweener abuses, and another one to set a slowdown > factor in response to disable-animations ? That sounds realistic, yes. Alternatively we could properly hook into tweener instead of re-using the slowdown factor for the second part, but we'd still need stopping tweener for non-animations.
Created attachment 234460 [details] [review] endSessionDialog: don't use Tweener for the timer We want to make Tweener short-circuit animations when resources are constrained, so this is not going to work. Instead, use a one-second timeout until the seconds left reach zero.
Created attachment 234461 [details] [review] tweener: follow org.gnome.desktop.interface enable-animations Disable the animations by making the animation time a very small value.
Review of attachment 234460 [details] [review]: ::: js/ui/endSessionDialog.js @@ +413,3 @@ + this._timerId = Mainloop.timeout_add_seconds(1, Lang.bind(this, + function() { + this._secondsLeft -= 1; It would be better to read current time and subtract initial time to calculate seconds left.
Review of attachment 234461 [details] [review]: ::: js/ui/tweener.js @@ +51,3 @@ function init() { Tweener.setFrameTicker(new ClutterFrameTicker()); + animationSettings = new Gio.Settings({ schema: 'org.gnome.desktop.interface' }); should watch for changes @@ +79,3 @@ + if (!animationSettings.get_boolean('enable-animations')) + tweeningParameters['time'] /= 1000000; two thoughts: 1) this could just be an assignment instead of a division right? 2) why not call Tweener.setTimeScale (0.0000001) or so at start time like the slow down factor works?
(In reply to comment #20) > Tweener.setFrameTicker(new ClutterFrameTicker()); > + animationSettings = new Gio.Settings({ schema: > 'org.gnome.desktop.interface' }); > > should watch for changes Should it? Any animation time that is big enough to justify cancellation on settings changes is probably an abuse of the tweener API ...
I think not listening for changes during an animation is fine, as long as we pick up changes between animations.
(In reply to comment #21) > (In reply to comment #20) > > Tweener.setFrameTicker(new ClutterFrameTicker()); > > + animationSettings = new Gio.Settings({ schema: > > 'org.gnome.desktop.interface' }); > > > > should watch for changes > > Should it? Any animation time that is big enough to justify cancellation on > settings changes is probably an abuse of the tweener API ... Indeed you're right, just querying it in _wrapTweening like he does is fine.
*** Bug 685452 has been marked as a duplicate of this bug. ***
would be cool to produce an updated patch and get this out of the way
Created attachment 235563 [details] [review] endSessionDialog: don't use Tweener for the timer -- Updated as for Ray's review to calculate seconds left from the monotonic time.
Created attachment 235564 [details] [review] tweener: follow org.gnome.desktop.interface enable-animations -- The reason I did it in tweener was really to avoid watching for changes, since GSettings reads should be cheap. I modified this to an assignment now.
Review of attachment 235563 [details] [review]: OK.
Review of attachment 235563 [details] [review]: Actually, I wonder if it makes sense to use something like WallClockSource from GnomeDesktop.
(In reply to comment #29) > Review of attachment 235563 [details] [review]: > > Actually, I wonder if it makes sense to use something like WallClockSource from > GnomeDesktop. WallClockSource doesn't seem to be public (and GnomeWallClock doesn't really seem like a good fit to me, we want to follow monotonic time, no?). Since we want to update the seconds left string, I really think we need a timeout here.
timerfd allows you to create a monotonic source, but maybe this is good enough.
I think it is good enough. This is not about high-precision measurements, but about giving the user some sense of a ticking clock...
Review of attachment 235563 [details] [review]: Yeah, I just realized that. If they're logging out, they also probably don't care about wakeups too much.
Review of attachment 235564 [details] [review]: Why can't we set it to 0? ::: js/ui/tweener.js @@ +79,3 @@ + if (!animationSettings.get_boolean('enable-animations')) + tweeningParameters['time'] = 0.000001; undefined
(In reply to comment #34) > ::: js/ui/tweener.js > @@ +79,3 @@ > > + if (!animationSettings.get_boolean('enable-animations')) > + tweeningParameters['time'] = 0.000001; > > undefined Ignore this part. It seems that Splinter broke.
(In reply to comment #34) > Review of attachment 235564 [details] [review]: > > Why can't we set it to 0? Other things will break if we just set it to zero (I don't remember exactly what part without trying again though).
(In reply to comment #36) > Other things will break if we just set it to zero (I don't remember exactly > what part without trying again though). We should look into that. Tweener has a fast-path if you have 0 time to end the tween at its final value and call onComplete, so maybe onUpdate handlers not getting called breaks something?
(In reply to comment #37) > We should look into that. Tweener has a fast-path if you have 0 time to end the > tween at its final value and call onComplete, so maybe onUpdate handlers not > getting called breaks something? It's not just that; we have code that assumes Tweener will give back control to the mainloop before calling onComplete, which it doesn't when the time is zero. So, code like this will suddenly break. Tweener.addTween(foo, { time: 0.30, onComplete: Lang.bind(this, this._foo) }); this._barExpectedToRunBeforeFoo(); While it might be a good idea, I think it's too late to fix all the code with such assumptions for this cycle.
Review of attachment 235564 [details] [review]: OK, let's roll with it. ::: js/ui/tweener.js @@ +79,3 @@ + if (!animationSettings.get_boolean('enable-animations')) + tweeningParameters['time'] = 0.000001; I wonder if this follow the GTK+ XSetting "gtk-enable-animations" instead. I don't mind either way.
org.gnome.desktop.interface enable-animations is what backs the gtk-enable-animations xsetting
Yes, so this would simply add the ability to override with ~/.config/gtk-3.0/settings.ini or with gnome-settings-daemon xsettings overrides Whether that's wanted or not, I don't know.
Attachment 235563 [details] pushed as 1c57c0e - endSessionDialog: don't use Tweener for the timer Attachment 235564 [details] pushed as 5f95351 - tweener: follow org.gnome.desktop.interface enable-animations Thanks for the review. The gnome-settings-daemon plugin sets the GSetting value, so I think it's OK for us to do the same.