GNOME Bugzilla – Bug 649248
Dash doesnt resize correctly
Last modified: 2011-10-17 14:15:29 UTC
Dash doesn't resize correctly and consequently doesn't show all running icons or favorites
The dash resizes correctly for us. You need to provide specific instructions and a screenshot.
Created attachment 187162 [details] Dash
Dash running on fedora 15 (updated) on a netbook (1024x600). After restart the dash resize correctly but after some minutes stops resizing and the icons become too large and not all icons are displayed.
I've seen this too once on my 1024x768 notebook running Gnome 3.0.1 under Arch Linux (i686). Incidentally, when it happened I also used a custom icon theme (Faenza) like the reporter. I have since switched back to the standard theme and haven't seen the problem since.
I test my system and i confirm that the problem appears only with some iconsets, notably with the Faenza iconset.
I've just had this happen with the standard Gnome icon set, screenshot attached.
Created attachment 187754 [details] Incorrect Dash resizing on a 1024x768 screen
I have installed gnome-shell on other netbook (running ubuntu and with resolution 1024x600) and in this netbook i have the problem with all iconsets.
I remember having run into this issue often but never quite figuring out the "logic" behind it. I'm not sure if still happens with the latest stable release. Maybe it forgets to recalculate size when some windows are opened or closed. In any case, if you can still reproduce this, it would be really nice if you could provide step by step instructions to triggering the bug. Try to investigate systematically, opening and closing windows that are favorites or not, noting how many icons are in the dash, etc...
*** Bug 655408 has been marked as a duplicate of this bug. ***
This now happens about 10 times a day for me with the latest stable release (3.0.2), but I've been unable to consistently reproduce it. It seems to happen mainly/only on small screens though. Is there nothing I can do in LookingGlass when this happens that might give some hints as to what's going wrong?
*** Bug 658963 has been marked as a duplicate of this bug. ***
I'm seeing this too, with 3.1.91
(In reply to comment #13) > I'm seeing this too, with 3.1.91 Is that on a small screen?
moderately small, yes: 1366x768
If it helps, I think my first report was on a 1024x768 display, but I am sure I've seen this at least on 1280x800 too.
I've dug into this a bit. I'm not sure what I found is the root of this bug too but is sounds like it might be. Testing was done on a 1680x1050 monitor. First I changed dash.js to just use the smaller or larger icons: let iconSizes = [ 16, 64 ]; Adding favorites 1 by 1, at some point, the dash will resize to the smaller icons. I found that, at this point, dragging 1 favorite and dropping it where it was will make the dash resize to the bigger icons although the number of icons is the same that made it resize down before. I narrowed this to be caused by the _redisplay() method being called while the tween from _adjustIconSize() is going. _redisplay() will call _adjustIconSize() and this will call get_preferred_height() on the dash which will propagate to the icon textures that are tweening and thus return their middle animation height instead of the final which wouldn't let them all fit. I'm attaching a patch which doesn't let this happen by calling _redisplay() in an idle instead of using the deferred work mechanism. But this makes the animation look like step motion so I don't really like it. I also noticed that dragging icons while the dash resizes causes lots of clutter warnings: (lt-gnome-shell-real:8171): Clutter-WARNING **: The actor 'uiGroup' is currently inside an allocation cycle; calling clutter_actor_queue_relayout() is not recommended
Created attachment 196688 [details] [review] dash: Fix resizing Ugly fix.
(In reply to comment #17) > I also noticed that dragging icons while the dash resizes causes lots of > clutter warnings: > > (lt-gnome-shell-real:8171): Clutter-WARNING **: The actor 'uiGroup' is > currently inside an allocation cycle; calling clutter_actor_queue_relayout() is > not recommended Yeah, that's probably a recent regression introduced by recent Chrome changes - to get rid of the console spam, try commenting out the call to set_tooltip_text().
Created attachment 197088 [details] [review] dash: Ignore hiding items in _adjustIconSize() Rather than relying on the caller to hide the remove target and removed items before calling _adjustIconSize(), move that logic into _adjustIconSize() itself.
Created attachment 197089 [details] [review] dash: Rework _adjustIconSize() The current code uses the dash's height and current icon size to calculate the new icon size. However, the height does not correctly relate to the icon size while the icons are animating, in which case the resulting icon size may be wrong. Rework the function to be independent from the actual icon sizes, so that a correct size is calculated even when called during an animation.
Created attachment 197090 [details] [review] dash: Add minor optimization to _adjustIconSize() In case _adjustIconSize() is called while the the dash icons are animating, some extra work is required to yield the expected result. Skip those extra steps when the icons are not actually animating.
So, I'm still unable to reproduce this reliably. However, the attached patch series should make the size calculation robust against intermediate sizes during animations.
*** Bug 658487 has been marked as a duplicate of this bug. ***
Patches work fine for me; haven't seen cutoff icons with them.
The patches in comments 20-22 solved the problem for me on Gnome 3.2.
OK, so should we try to get this in for 3.2.1? Anyone up for review?
Yes, for getting this in. Unfortunately, I'm not a shell reviewer...
Review of attachment 197088 [details] [review]: Looks good to me. ::: js/ui/dash.js @@ +422,3 @@ }); + Spurious empty line.
Review of attachment 197088 [details] [review]: This already went through an IRC review, sorry. I didn't like the name "hiding" and asked Florian to rename it to something like "aboutToBeRemoved" (I forget the exact identifier, though) and put in some comments around some of hte things like "// Don't show icons which are in a hiding animation".
Review of attachment 197089 [details] [review]: Looks sound. It's actually easier to understand now!
Review of attachment 197090 [details] [review]: And this looks pretty good too.
(In reply to comment #30) > This already went through an IRC review, sorry. I didn't like the name "hiding" > and asked Florian to rename it to something like "aboutToBeRemoved" (I forget > the exact identifier, though) and put in some comments around some of hte > things like "// Don't show icons which are in a hiding animation". Yup, agree. I think this can go in 3.2.1.
(In reply to comment #30) > Review of attachment 197088 [details] [review]: > > This already went through an IRC review, sorry. I didn't like the name "hiding" > and asked Florian to rename it to something like "aboutToBeRemoved" (I forget > the exact identifier, though) I suggested "animatingOut" (which matches the animateOutAndDestroy method name) > and put in some comments around some of hte > things like "// Don't show icons which are in a hiding animation". Does // For the icon size, we only consider children which are "proper" // icons (i.e. ignoring drag placeholders) and which are not // animating out (which means they will be destroyed at the end of // the animation) let iconChildren = this._box.get_children().filter(function(actor) { return actor._delegate.child && actor._delegate.child._delegate && actor._delegate.child._delegate.icon && !actor._delegate.animatingOut; }); look good?
Perfect, thanks.
Attachment 197088 [details] pushed as 790b9d3 - dash: Ignore hiding items in _adjustIconSize() Attachment 197089 [details] pushed as a4b69db - dash: Rework _adjustIconSize() Attachment 197090 [details] pushed as 6d0be86 - dash: Add minor optimization to _adjustIconSize()