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 649248 - Dash doesnt resize correctly
Dash doesnt resize correctly
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
3.0.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 655408 658487 658963 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-05-03 04:07 UTC by dmiranda
Modified: 2011-10-17 14:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Dash (325.83 KB, image/png)
2011-05-04 00:28 UTC, dmiranda
  Details
Incorrect Dash resizing on a 1024x768 screen (43.30 KB, image/png)
2011-05-13 06:53 UTC, Ulrich Keller
  Details
dash: Fix resizing (1.47 KB, patch)
2011-09-16 03:20 UTC, Rui Matos
none Details | Review
dash: Ignore hiding items in _adjustIconSize() (4.95 KB, patch)
2011-09-20 16:49 UTC, Florian Müllner
committed Details | Review
dash: Rework _adjustIconSize() (3.97 KB, patch)
2011-09-20 16:50 UTC, Florian Müllner
committed Details | Review
dash: Add minor optimization to _adjustIconSize() (2.54 KB, patch)
2011-09-20 16:50 UTC, Florian Müllner
committed Details | Review

Description dmiranda 2011-05-03 04:07:04 UTC
Dash doesn't resize correctly and consequently doesn't show all running icons or favorites
Comment 1 Owen Taylor 2011-05-03 14:24:21 UTC
The dash resizes correctly for us. You need to provide specific instructions and a screenshot.
Comment 2 dmiranda 2011-05-04 00:28:39 UTC
Created attachment 187162 [details]
Dash
Comment 3 dmiranda 2011-05-04 00:31:03 UTC
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.
Comment 4 Ulrich Keller 2011-05-04 18:49:02 UTC
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.
Comment 5 dmiranda 2011-05-04 23:05:05 UTC
I test my system and i confirm that the problem appears only with some iconsets, notably with the Faenza iconset.
Comment 6 Ulrich Keller 2011-05-13 06:52:24 UTC
I've just had this happen with the standard Gnome icon set, screenshot attached.
Comment 7 Ulrich Keller 2011-05-13 06:53:22 UTC
Created attachment 187754 [details]
Incorrect Dash resizing on a 1024x768 screen
Comment 8 dmiranda 2011-06-02 00:19:57 UTC
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.
Comment 9 Jean-François Fortin Tam 2011-07-23 22:40:51 UTC
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...
Comment 10 Florian Müllner 2011-07-27 12:10:58 UTC
*** Bug 655408 has been marked as a duplicate of this bug. ***
Comment 11 Ulrich Keller 2011-09-02 11:46:10 UTC
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?
Comment 12 Florian Müllner 2011-09-13 16:36:27 UTC
*** Bug 658963 has been marked as a duplicate of this bug. ***
Comment 13 Matthias Clasen 2011-09-13 16:51:45 UTC
I'm seeing this too, with 3.1.91
Comment 14 Florian Müllner 2011-09-13 16:53:56 UTC
(In reply to comment #13)
> I'm seeing this too, with 3.1.91

Is that on a small screen?
Comment 15 Matthias Clasen 2011-09-13 23:22:58 UTC
moderately small, yes: 1366x768
Comment 16 Cosimo Cecchi 2011-09-13 23:42:06 UTC
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.
Comment 17 Rui Matos 2011-09-16 03:20:00 UTC
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
Comment 18 Rui Matos 2011-09-16 03:20:45 UTC
Created attachment 196688 [details] [review]
dash: Fix resizing

Ugly fix.
Comment 19 Florian Müllner 2011-09-16 14:14:03 UTC
(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().
Comment 20 Florian Müllner 2011-09-20 16:49:45 UTC
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.
Comment 21 Florian Müllner 2011-09-20 16:50:24 UTC
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.
Comment 22 Florian Müllner 2011-09-20 16:50:35 UTC
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.
Comment 23 Florian Müllner 2011-09-20 16:54:37 UTC
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.
Comment 24 Florian Müllner 2011-09-22 16:16:15 UTC
*** Bug 658487 has been marked as a duplicate of this bug. ***
Comment 25 Matthias Clasen 2011-10-06 12:03:59 UTC
Patches work fine for me; haven't seen cutoff icons with them.
Comment 26 Ulrich Keller 2011-10-14 14:39:31 UTC
The patches in comments 20-22 solved the problem for me on Gnome 3.2.
Comment 27 Florian Müllner 2011-10-14 14:41:15 UTC
OK, so should we try to get this in for 3.2.1? Anyone up for review?
Comment 28 Matthias Clasen 2011-10-14 22:32:44 UTC
Yes, for getting this in. Unfortunately, I'm not a shell reviewer...
Comment 29 Rui Matos 2011-10-17 00:14:16 UTC
Review of attachment 197088 [details] [review]:

Looks good to me.

::: js/ui/dash.js
@@ +422,3 @@
         });
 
+

Spurious empty line.
Comment 30 Jasper St. Pierre (not reading bugmail) 2011-10-17 00:18:10 UTC
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".
Comment 31 Rui Matos 2011-10-17 00:46:27 UTC
Review of attachment 197089 [details] [review]:

Looks sound. It's actually easier to understand now!
Comment 32 Rui Matos 2011-10-17 01:03:06 UTC
Review of attachment 197090 [details] [review]:

And this looks pretty good too.
Comment 33 Rui Matos 2011-10-17 01:05:55 UTC
(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.
Comment 34 Florian Müllner 2011-10-17 13:49:58 UTC
(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?
Comment 35 Jasper St. Pierre (not reading bugmail) 2011-10-17 13:53:55 UTC
Perfect, thanks.
Comment 36 Florian Müllner 2011-10-17 14:15:16 UTC
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()