GNOME Bugzilla – Bug 584609
Zooming the whole overlay in transitions
Last modified: 2009-08-11 00:24:56 UTC
Currently when showing or hiding the overlay, only the workspaces are moving and changing size. It might be better to give the whole overlay a zooming animation, while keeping the workspaces fixed on the overlay plane. I think this gives the user a better mental model of where the overlay lives when hidden, and removes the amount of objects confusingly sliding around. It also seems to come close to what Jeremy Perry describes on his mockup page for application browsing: > When the user first opens this scene, the animation could give the sense > that you've pulled back, so the desktop recedes to the right, and the items > on the left appear to come in from above left http://live.gnome.org/GnomeShell/DesignerPlayground/AppBrowsingAlternative I started playing with the idea in the 'zoom' branch at: http://github.com/sander/gnome-shell/tree/zoom The code is currently very ugly and it only works when the first workspace is activated, but it might already give a good idea of how it would look. One concern mentioned at IRC is that the huge labels in the menu would look too bad. I didn't find it disturbing, especially with ANIMATION_TIME = 0.25 as proposed in #583572.
I rather like the effect - with the shorter time and the EaseInQuad animation it's pretty hard to see the initial portions where the text would be large. It's a bit hard to evaluate though without fixing up code that makes the windows move linearly on the transition since it's pretty disturbing to have windows swooping around as you go in.
With 5 workspaces, the text becomes really large, but that's probably not a use case that should be optimized for. And you still don't get to see the text pixelated. What do you mean by the windows moving linearly? Both the workspace and the windows currently have an easeOutQuad transition, so it seems their movements correspond linearly. (That's higher relativity theory though, so I might be wrong on this time of the day.) Just setting the windows' transition to "linear" doesn't improve the situation. For further experimenting, see Workspace._positionWindows() in workspaces.js.
(In reply to comment #2) > What do you mean by the windows moving linearly? So, imagine that you have four workspaces, and you're on workspace 1, and you have a window in the top-right corner of the screen, which the overlay is going to place in the bottom-left corner of the exposé-ed workspace in the overlay. But since there are 4 workspaces, the bottom-left corner of the first workspace will end up being only halfway down the screen, right? What you want is for the window to uniformly move from its original position at the top-right directly to its final position at center-left. But if you move the windows independently of the workspaces, then what will happen is that the window will start moving down and to the left, but the workspace will simultaneously be moving upward and shrinking, and eventually the downward motion of the window will be scaled to the point where it's less than the upward motion of the workspace, and so the overall path of the window as you enter the overlay is U shaped instead of linear. And when you have lots of windows doing this, it's sort of confusing. The current code uses the workspaceRelative tweener hack at the end of overlay.js to compensate for this by actually moving the window in a curve rather than a straight line, so as to exactly cancel out the effect of moving and shrinking the workspace. I haven't looked at your code, but if this isn't working any more, then probably it's because you changed the way that the workspace moves, but didn't change workspaceRelative as well, and so now it's compensating incorrectly.
Created attachment 138252 [details] [review] Patch for zooming the whole overlay Thanks a lot for the clear explanation! I think I've got that issue fixed now by making sure that workspaceRelative uses the correct workspace position. I'm not sure whether I should change the window transition curve too, but keeping easeOutQuad looked best to me. Zooming in and out from other workspaces than the first one was easily implemented by using the right workspace positioning variables (gridX instead of fullSizeX). The attached patch is for the master branch, and obsoletes my original Git branch.
Mostly the patch looks really good to me. Few small comments: - Instead of returning a dictionary of x,y for the position, maybe return an array [x, y] ? With Spidermonkdy, you can do: posX, posY = someFunctionThatReturnsAnArray() We don't do a lot of either right now so I can't really say definitively what our "style" is for this. (Workspace._computeWindowPosition() returns an array of [x,y,scale], so it's sort of similar.) - Did you mean to remove Workspaces._updateInOverlay? It looks like the link between the Workspaces.updatePosition() function you removed as no-longer-used, and the Workspace.updateInOverlay function you removed as no-longer-used. These removals could use explicit commenting in the commit message. - I think the code that fades in the remove button could use a comment explaining why we need to fade it in (to avoid it appearing huge initially?) - You probably should removeTweens before fading in the remove button; right now it looks like you'll sometimes get competing tweens. If they have the same time, it probably won't be apparent, but maybe we'd use different times for the remove button. And the workspaceRelativeModifier code looks a little finish; I believe what you have ends up as a no-op since workspace.actor.x/y remain constant. Let: w_x, w_y: The position of the workspace actor (fixed during the animation) overlay_x0, overlay_y0: The initial position of the overlay actor overlay_scale0: The initial scale of the overlay actor overlay_x1, overlay_y1: The final position of the overlay actor overlay_scale1: The final scale of the overlay actor The initial screen X position of the window actor is: (begin + w_x) * overly_scale0 + overlay_x0 The final screen X position is: (end + w_x) * overly_scale1 + overlay_x1 If we interpolate linearly, the screen position at time t is: screen_x = (1-t) * [(begin + w_x) * overly_scale0 + overlay_x0] + t * [(end + w_x) * overly_scale1 + overlay_x1] The position in workspace coordinates of the window is then: x = (screen_x - overlay.actor.x) / overlay.actor.scale - w_x [ That, I think, gets the actors upper left corner to move in a straight line. We probably really want the center to move in a straight line. Which could be accomplished with actor.move_anchor_point_from_gravity(). Not sure if the difference would be visible. ] One other thing to think about - for actors not on the active workspace do we want them to just stay in their "spread out" positions and not animate as we zoom in or out?
(In reply to comment #5) > - Instead of returning a dictionary of x,y for the position, maybe > return an array [x, y] ? With Spidermonkdy, you can do: > > posX, posY = someFunctionThatReturnsAnArray() > > We don't do a lot of either right now so I can't really say > definitively what our "style" is for this. You have to say [posX, posY] = ... I think the returning-an-array is implicitly our style, since that's what gjs does for multiple-out-args, and we already have stuff like: [width, height] = actor.get_transformed_size() > - I think the code that fades in the remove button could use a comment > explaining why we need to fade it in (to avoid it appearing huge > initially?) IIRC it's because the same code gets used when you remove a workspace, and it has to add a remove button to the newly-last workspace. > [ That, I think, gets the actors upper left corner to move in a straight line. > We probably really want the center to move in a straight line. Which could be > accomplished with actor.move_anchor_point_from_gravity(). Not sure if the > difference would be visible. ] as noted in bug 571109 comment 16 there seem to be problems with changing anchor points in the overlay
Created attachment 138565 [details] [review] Updated patch Thanks for the comments! I have to catch a bus, so had to quickly implement the changes. The mathematical proof looks convincing, though I don't have time to check, but the animation looks the same with the workspaceRelative code removed indeed. If there are any issues with my patch, feel free to modify it. Otherwise, please commit it. See you in 2.5 weeks! :-)
My math wasn't meant to prove that what you have was a no-op - I just noted that from inspection. It was meant to be the non-noop-code that is actually needed to get windows to move linearly in screen coordinates.
Created attachment 140070 [details] [review] Updated with lineair window movement Oops, that's what I get when coding in a hurry... This patch implements your new window movement formulas. I agree with them now and it seems like they really work too. This updated patch re-introduces workspaceRelative with the new math and adds Overlay.getZoomedIn[Scale|Position]() and Overlay.get[Scale|Position]() functions for use in workspaceRelative. I needed to delay positionWindows() until after Workspaces has been created, because it depends on Main.overlay._workspaces. For this I added a 'workspaces-created' signal to Overlay.
It looks like this patch needs to be rebased on top of http://git.gnome.org/cgit/gnome-shell/commit/?id=4d52c1095816bfba51715b5e836f29483960ea13 I had to apply it manually ( I was too curious to try it with the hot corner :), and then the application icons were not positioned correctly when the overlay was first entered. They did get re-positioned correctly as workspaces were added or windows were moved around.
Created attachment 140193 [details] [review] Rebased patch I didn't notice the icon positioning bug you described after doing this rebase and changing some things, so I hope that's fixed now. In this patch I also removed the new 'workspaces-created' signal in favor of connecting to 'showing', and changed the initial workspaceRelative positions in order to improve the cancel-opening-the-overlay animation.
Created attachment 140335 [details] [review] Rebased and some changes The patch needed to be rebased to 06d17e08c06acb3168750251e85e76c51171a1b5 I also applied the change to _workspaceRelativeGet as suggested by Owen on IRC, to interpolate the overlay's dimensions instead of relying on Overlay.getPosition() and Overlay.getScale(). I removed one trailing comma that was in the previous patch after + overlayScale: overlayScale } }
Created attachment 140392 [details] [review] With Dash fading During the transitions, Dash is faded in and out now, based on a mockup by Jeremy and Jon: http://www.gnome.org/~mccann/shell/mockups/20090810/shellzoom4.html
Created attachment 140396 [details] [review] The right patch The previous patch accidentally was the before-previous patch, this is the described one.
Behavior and patch look great. Good to commit! + // Returns the scale of the overlay when zoomed in to a workspace. This is just a bit unclear to me. Maybe: // Returns the scale the overlay has when we just start zooming out // to overview mode. That is, when just the active workspace is showing. + // Position/scale the desktop windows and their children after the + // workspaces have been created. This cannot be done first because + // window movement depends on the Workspaces object being accessible + // as an Overlay member. + Main.overlay.connect('showing', + Lang.bind(this, function() { + for (let w = 0; w < this._workspaces.length; w++) + this._workspaces[w].zoomToOverlay(); + })); This is just a bit convoluted, but I don't have a better easy suggestion, so it's OK. If we go to something like bug 586549, then we'll want to split the workspaces constructor and workspaces.zoomtoOverlay(), and then this becomes trivial.
Made the comment change and committed. Thanks!