GNOME Bugzilla – Bug 563844
add some basic window management effects
Last modified: 2008-12-29 04:44:11 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).
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.
(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).
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.
(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...
(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..
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.
Thanks, that fixes it.
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.
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.
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.)
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.
Looks good. Thanks!
Will need some (straightforward) fixing up for the changes from bug 565386
Updated and committed.