GNOME Bugzilla – Bug 600999
fade out desktop icons if necessary when showing overview
Last modified: 2010-02-15 20:41:42 UTC
If nautilus is set to show the desktop we should fade out its desktop icons if necessary when showing the overview. I think the icons should be gone by the time the zoom animation is complete (currently 0.25 sec). This will dramatically reduce the visual noise and clutter in the window picker.
This is easy to implement, but as a complete guess, is going to reduce frame rate going into the overlay by 20% or so. (Isn't there a bug about this already?)
Hmm, poop. Well maybe then just hide them right off the bat at the start of the animation.
*** Bug 583574 has been marked as a duplicate of this bug. ***
583574 called for a "dimmed/blurred" background. What's the current desired design? Hide the icons immediately when going into the overview, but leave the background there displayed normally?
I don't think hiding the icons and dimming/blurring the background are necessary mutually exclusive. The point is that they both potentially distract from identifying windows. Right now they do. We'll have to try a few things to make it better I suspect.
(In reply to comment #2) > Hmm, poop. Well maybe then just hide them right off the bat at the start of > the animation. Turns out to look really bad if you do this (you can test with): == diff --git a/js/ui/workspace.js b/js/ui/workspace.js index 9a7f019..1aa17d2 100644 --- a/js/ui/workspace.js +++ b/js/ui/workspace.js @@ -551,7 +551,7 @@ Workspace.prototype = { // Find the desktop window for (let i = 0; i < windows.length; i++) { - if (windows[i].get_window_type() == Meta.WindowType.DESKTOP) { + if (false && windows[i].get_window_type() == Meta.WindowType.DESKT this._desktop = new DesktopClone(windows[i]); break; } === It's very distacting to have the icons blink out. Well, it looks bad in the case where there is no maximized window. If there's a maximized window it looks good, since you can't see the icons anyways. Since performance going into the overview is worst when there's a bunch of maximized windows, I wonder if it would work to just show the fade-out when there is no maximized window.
(In reply to comment #6) > It's very distacting to have the icons blink out. > > Well, it looks bad in the case where there is no maximized window. If there's a > maximized window it looks good, since you can't see the icons anyways. Since > performance going into the overview is worst when there's a bunch of maximized > windows, I wonder if it would work to just show the fade-out when there is no > maximized window. OK, I'll give it a shot - I already have a patch which does fading, I'll update it to check for maximized windows ...
Created attachment 153168 [details] [review] Fade out desktop icons when showing overview As desktop icons don't have any purpose in the overview (except for distracting the user), fade them out when entering the overview. Unfortunately, the fading effect affects performance, therefore hide icons directly when there are maximized windows on the desktop.
Created attachment 153195 [details] [review] Fade out desktop icons when showing overview In shell_window_tracker_is_window_maximized: Trap X errors during the call to XGetWindowProperty().
Review of attachment 153195 [details] [review]: Trying it out, it looks very good in practice. It's really surprising how much better the fade looks than abruptly removing the icons. Some suggestions about code organization below, and I think we want to check for maximized windows in a slightly different way (requiring a Mutter addition) ::: js/ui/workspace.js @@ +287,3 @@ + this.actor.add_actor(new Clutter.Clone({ source: Main.background.source })); + + this._windows = windows; The set of windows could change while the overview is up, right? I think it's better to pass in the workspace object. Note that the fade-out or fade-in should only ever apply to the current workspace. Other workspaces should simply never have the icons. @@ +303,3 @@ + if (Main.overview.animationInProgress + && !this._haveMaximizedWindows()) + Tweener.addTween(clone, It's a bit odd to me to set up an animation in _init() - creating an object shouldn't start something time running going. It should just create an object. What I'd do is make the desktop clone have zoomToOverview and zoomFromOverview methods and call them from the corresponding methods in Workspace. ::: src/shell-window-tracker.c @@ +325,3 @@ + + meta_error_trap_push_with_return (display); + status = XGetWindowProperty (dpy, w, net_wm_atoms[_net_wm_state], _NET_WM_STATE is something set by Mutter. It's not really OK for us to do round trips to the X server and parse atoms to find out something that we (our process) set ourselves. Unfortunately, this information doesn't yet seem to be exposed from MetaWindow so you'll have to extend it to do that. The whole maximized-vertically vs. maximized horizontally things make that more complicated than it would be otherwise. I think I'd just add independent GObject properties for both (maximized-vertically, maximized-horizontally), and then you can write: if (metaWindow.maximized_horizontally && metaWindow.maximized_vertically) from the JS code.
Created attachment 153674 [details] [review] Fade out desktop icons when showing overview (In reply to comment #10) > The set of windows could change while the overview is up, right? I think it's > better to pass in the workspace object. Mmmh, I don't like circular references - nevermind, I think I found a better solution anyway. Otherwise the patch follows the review. > _NET_WM_STATE is something set by Mutter. It's not really OK for us to do round > trips to the X server and parse atoms to find out something that we (our > process) set ourselves. Yeah, I knew you would say so - it was just too good an oportunity to loose my XLib virginity, sorry about that ... > I think I'd just add independent GObject properties for both > (maximized-vertically, maximized-horizontally) Done. Patch attached to bug 590706
Created attachment 153853 [details] [review] Fade out desktop icons when showing overview Minor update: when checking for maximized windows on a desktop, skip minimized windows.
Review of attachment 153853 [details] [review]: Looks really good! ::: js/ui/workspace.js @@ +301,3 @@ }, + zoomFromOverview: function(fadeIn) { I think the parameters might be slightly better as fadeInIcons/fadeOutIcons, since the background itself doesn't fadeIn/FadeOut.
Attachment 153853 [details] pushed as 03a7501 - Fade out desktop icons when showing overview Renamed fadeIn/fadeOut to fadeInIcons/fadeOutIcons before commiting.