GNOME Bugzilla – Bug 594049
fix up allocation handling in workspaces.js
Last modified: 2010-12-15 12:56:44 UTC
This is an offshoot of the multihead work; I'm cleaning up workspaces.js in preparation for having separate Workspaces actors on separate monitors. How it works now: - Workspaces is a Clutter.Group that contains the workspace grid (Workspace objects) and the AddWorkspaceButton. Workspaces.actor is full-screen-sized, but only uses the subset of the screen that Overview tells it to. It figures out how the Workspace objects should be arranged, but does not actually arrange them itself. - Workspace is a Clutter.Group that contains the DesktopClone and WindowClones. It sets its position and scale according to the arrangement that Workspaces came up with, and also sets the position and scale of its children when zooming in and out. The last workspace also includes the (-) button, when appropriate. The selected workspace draws a highlight around itself. - When workspaces are added, removed, or resized, Workspaces decides what needs to be done, and then tells the relevant Workspace objects what to do. (Eg, Workspaces calls this._workspaces[i].slideOut(), and the Workspace does the actual animation.) - WindowClone is a Clutter.Clone. It gets its size from its source window, and accepts the position and scale assigned to it by Workspace. - WindowClone titles are managed by WindowClone, but placed into the WindowClone's parent actor (ie, Workspace.actor) and then scaled up to counteract the Workspace scaling. - WindowClone icons are managed by Workspace, but placed into Workspace's parent actor (ie, Workspaces.actor), to bypass the Workspace scaling. The eventual plan: - AddWorkspaceButton moves to Overview - Workspaces (or WorkspaceGrid? Now would be a good time to rename it if we were going to do that) is a Shell.GenericContainer, and there will be one on each monitor (but only the primary monitor will have a dash and an AddWorkspaceButton). It accepts an allocation from the Overview, and allocates space to its children, basically like a table widget. It also draws the "active workspace" highlight. When workspaces are added or removed, it deals with tweening the child actors into/out of their grid position. When the grid needs to be resized, it tweens the child allocations. It does not scale itself or its children. - Workspace is also a Shell.GenericContainer. It lays out and "scales" its WindowClone children, but as with Workspaces, it does this just by assigning them propportionally small allocations, not by adjusting their scaling properties I think Workspace still handles the (-) button... not 100% sure yet that it won't move to Workspaces though. - WindowClone is *also* a Shell.GenericContainer, containing a Clutter.Clone, plus the title and icon. WindowClone sets the Clutter.Clone's scale_x and scale_y properties to make it fit in the WindowClone's allocation (the Clutter.Clones are the only actors in the entire Workspace View that have non-1.0 scale). Using allocation correctly will also make it easier to support monitor size/rotation changes. The allocation-vs-scaling changes are a little awkward, but I think overall they will be an improvement, particularly in handling the window clone titles and icons. First two patches coming up, then more later.
Created attachment 142416 [details] [review] Move the add workspace button out of the Workspaces object, into Overview
Created attachment 142417 [details] [review] Set Workspaces.actor's size and position correctly Previously it was always screen-sized, but only drew within the rectangle indicated by the overview. Now it actually allocates itself to the given size/position.
Comment on attachment 142416 [details] [review] Move the add workspace button out of the Workspaces object, into Overview > Created an attachment (id=142416) [details] > Move the add workspace button out of the Workspaces object, into Overview Looks good.
Comment on attachment 142417 [details] [review] Set Workspaces.actor's size and position correctly - // Note we only round the first part, because we're still going to be - // positioned relative to the parent. By subtracting a possibly - // non-integral parent X/Y we cancel it out. - let x = Math.round(cloneX + cloneWidth - icon.width) - parentX; - let y = Math.round(cloneY + cloneHeight - icon.height) - parentY; + let x = Math.round(cloneX + cloneWidth - icon.width); + let y = Math.round(cloneY + cloneHeight - icon.height); Don't you need to make sure that the workspaces and workspace are positioned at integral positions if you are going to simplify the pixel-grid snapping code like this? Otherwise looks fine. I'd prefer to see the patch in bug 591849 land first - I don't think it conflicts significantly with this - unlike the promised follow-on window work - but there might be a small breakage or two.
(In reply to comment #4) > Don't you need to make sure that the workspaces and workspace are positioned at > integral positions if you are going to simplify the pixel-grid snapping code > like this? Oh, *that's* what that comment meant. Will fix. > I'd prefer to see the patch in bug 591849 land first OK, I'll push that forward.