After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 594049 - fix up allocation handling in workspaces.js
fix up allocation handling in workspaces.js
Status: RESOLVED OBSOLETE
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2009-09-03 15:41 UTC by Dan Winship
Modified: 2010-12-15 12:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Move the add workspace button out of the Workspaces object, into Overview (8.63 KB, patch)
2009-09-03 15:42 UTC, Dan Winship
committed Details | Review
Set Workspaces.actor's size and position correctly (6.19 KB, patch)
2009-09-03 15:42 UTC, Dan Winship
reviewed Details | Review

Description Dan Winship 2009-09-03 15:41:47 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.
Comment 1 Dan Winship 2009-09-03 15:42:54 UTC
Created attachment 142416 [details] [review]
Move the add workspace button out of the Workspaces object, into Overview
Comment 2 Dan Winship 2009-09-03 15:42:57 UTC
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 3 Owen Taylor 2009-09-03 21:40:51 UTC
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 4 Owen Taylor 2009-09-03 21:54:00 UTC
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.
Comment 5 Dan Winship 2009-09-04 14:03:55 UTC
(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.