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 563844 - add some basic window management effects
add some basic window management effects
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2008-12-09 13:40 UTC by Jonathan Matthew
Modified: 2008-12-29 04:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
some mostly working effects (15.19 KB, patch)
2008-12-09 13:44 UTC, Jonathan Matthew
needs-work Details | Review
updated (18.43 KB, patch)
2008-12-11 12:37 UTC, Jonathan Matthew
needs-work Details | Review
remove tweens (18.86 KB, patch)
2008-12-19 03:48 UTC, Jonathan Matthew
committed Details | Review

Description Jonathan Matthew 2008-12-09 13:40:41 UTC
I picked this off the todo list because it looked pretty easy and I wanted to get some idea of how this stuff fits together.

I ended up implementing the effects mostly as they were in the scratch plugin.  It looks OK, except for maximizing (scaling window contents up to fullscreen is obviously not the thing to do), and unmaximizing (no effect, plus everything locks up for a few seconds, but that seems to be a GL thing).
Comment 1 Jonathan Matthew 2008-12-09 13:44:33 UTC
Created attachment 124269 [details] [review]
some mostly working effects

I don't think I've ever written more than 10 lines of javascript at once before, so I don't really know what I'm doing there.
Comment 2 Dan Winship 2008-12-09 17:56:51 UTC
(In reply to comment #1)
> I don't think I've ever written more than 10 lines of javascript at once
> before, so I don't really know what I'm doing there.

Yeah, we're kinda learning as we go too.

The biggest javascript bug I saw is the way you're trying to use objects as array indexes:

+        this._minimizing[actor] = 1;

+        if (actor in this._minimizing) {

+            delete this._minimizing[actor]

that doesn't actually work, because indexes have to be integers or strings, so it will call actor.toString() to convert the object, and you end up using the string "[object _private_Meta_MutterWindow]" as the index for every window. This is why switchWorkspace does the thing with the array of { window: ..., parent: ...} objects instead. (Maybe there's a better way?)


Also, private methods (ie, all of the Done methods) should have a "_" before their name. (Yes, switchWorkspaceDone was wrong before and should be fixed with the rest.)


Re: shell_wm_completed_effect, my plan was to have a separate complete method for each effect, like with shell_wm_completed_switch_workspaces() now.


(In reply to comment #0)
> unmaximizing (no effect, plus everything
> locks up for a few seconds, but that seems to be a GL thing).

I think the problem is that you have to call the completed() method *after* returning from the signal handler. Try adding a dummy tween (not changing any properties, just getting it to call unmaximizeWindowDone after a brief delay, and call completed() from there).
Comment 3 Jonathan Matthew 2008-12-10 11:57:09 UTC
Thanks for taking a look over the patch.  I'll update it once I get everything working again..

(In reply to comment #2)
> The biggest javascript bug I saw is the way you're trying to use objects as
> array indexes:
> 
> +        this._minimizing[actor] = 1;
> 
> +        if (actor in this._minimizing) {
> 
> +            delete this._minimizing[actor]
> 
> that doesn't actually work, because indexes have to be integers or strings, so
> it will call actor.toString() to convert the object, and you end up using the
> string "[object _private_Meta_MutterWindow]" as the index for every window.
> This is why switchWorkspace does the thing with the array of { window: ...,
> parent: ...} objects instead. (Maybe there's a better way?)

I've replaced those with arrays and things are working a bit better now.  No more 'error in <effect> accounting' messages, at least.
 
> 
> Also, private methods (ie, all of the Done methods) should have a "_" before
> their name. (Yes, switchWorkspaceDone was wrong before and should be fixed with
> the rest.)

OK, I'll fix that.  Aren't all of the methods private?  They're only called by the signal handler closures and as tweener completion callbacks.

> 
> Re: shell_wm_completed_effect, my plan was to have a separate complete method
> for each effect, like with shell_wm_completed_switch_workspaces() now.

Sounds sensible enough.
 
> 
> (In reply to comment #0)
> > unmaximizing (no effect, plus everything
> > locks up for a few seconds, but that seems to be a GL thing).
> 
> I think the problem is that you have to call the completed() method *after*
> returning from the signal handler. Try adding a dummy tween (not changing any
> properties, just getting it to call unmaximizeWindowDone after a brief delay,
> and call completed() from there).

I tried running the maximize effect for unmaximize (which should at least have done something), but it didn't help.  I don't think calling completed() is the problem - I'm doing that for every effect for non-META_COMP_WINDOW_NORMAL windows, and those all work OK.

This looks like a GL (or clutter?) thing because window operations start to get really slow once one window gets near full-screen (1900x1200) size, whether I actually maximize it or not.  It starts taking ~5s to switch between the large window and any other, and similar amounts of time to update the window.  I'll try it with a smaller display size and see what happens.
Comment 4 Dan Winship 2008-12-10 14:04:04 UTC
(In reply to comment #3)
> OK, I'll fix that.  Aren't all of the methods private?  They're only called by
> the signal handler closures and as tweener completion callbacks.

Oops. Yes.

> This looks like a GL (or clutter?) thing because window operations start to get
> really slow once one window gets near full-screen (1900x1200) size, whether I
> actually maximize it or not.  It starts taking ~5s to switch between the large
> window and any other, and similar amounts of time to update the window.  I'll
> try it with a smaller display size and see what happens.

Hm... what video hardware?

You always have a full-screen window, btw, the desktop. Of course, maybe it's just that you only have enough RAM for *2* full-screen windows...
Comment 5 Jonathan Matthew 2008-12-10 22:49:14 UTC
(In reply to comment #4)

> Hm... what video hardware?
> 
> You always have a full-screen window, btw, the desktop. Of course, maybe it's
> just that you only have enough RAM for *2* full-screen windows...

This is a nvidia 8400GS with 512M RAM, using nvidia proprietary glx.

Using a smaller display (960x600) doesn't seem to help.  Under normal conditions, I have at least one window that size or larger, so, uh..
Comment 6 Owen Taylor 2008-12-10 23:02:11 UTC
That sounds like it might posibly be something I debugged last Friday with
the nvidia proprietary drivers ... they can get into a state where
things are *really* slow. In:

 clutter/glx/clutter-glx-texture-pixmap.c:clutter_glx_texture_pixmap_create_glx_pixmap

Try moving the 

  if (priv->glx_pixmap)
    clutter_glx_texture_pixmap_free_glx_pixmap (texture);

To the top of the function and see if that helps.
Comment 7 Jonathan Matthew 2008-12-10 23:56:14 UTC
Thanks, that fixes it.
Comment 8 Jonathan Matthew 2008-12-11 12:37:26 UTC
Created attachment 124414 [details] [review]
updated

The minimize effect doesn't work, and I'm guessing it's related to the FIXME in clutter_cmp_set_window_hidden() - it's hiding the window immediately, rather than when the effect finishes.
Comment 9 Jonathan Matthew 2008-12-11 13:29:24 UTC
I've hacked that a bit and now the minimize effect at least runs.  The window seems to move to some other location on the screen and to the bottom of the stack before the effect starts, so most of the time it's not really visible.
Comment 10 Dan Winship 2008-12-11 15:01:04 UTC
OK, this looks mostly good. The last thing I noticed is that you need to call Tweener.removeTweens() in the Done methods, to clean things up in the case where the kill signal got emitted. (Otherwise tweener will keep updating the window after the wm told you to stop the effect.)
Comment 11 Jonathan Matthew 2008-12-19 03:48:16 UTC
Created attachment 124981 [details] [review]
remove tweens

I finally realised the ugliness of the minimize effect is caused by mutter's resize_win function not ignoring windows that are being minimized like it does windows being maximized, unmaximized, or mapped.
Comment 12 Dan Winship 2008-12-19 14:00:35 UTC
Looks good. Thanks!
Comment 13 Owen Taylor 2008-12-22 22:11:26 UTC
Will need some (straightforward) fixing up for the changes from bug 565386
Comment 14 Jonathan Matthew 2008-12-29 04:44:11 UTC
Updated and committed.