Created attachment 145222 [details] [review]
[AppSwitcher] Scale app icons down to fit on screen

Fix comment.

Comment on attachment 145222 [details] [review]
[AppSwitcher] Scale app icons down to fit on screen

We want to use fixed icon sizes, to assure pretty rendering when drawing non-SVG icons. bug 590563 said:

 * All app icons must have the same width and height.  Select the largest value
from following list of icon size such that the application icons will fit onto
one row:
   [96, 64, 48, 32, 22]

Created attachment 146199 [details] [review]
[AppSwitcher] Scale app icons down to fit on screen

Select the best fitting size from [96, 64, 48, 32, 22]

Comment on attachment 146199 [details] [review]
[AppSwitcher] Scale app icons down to fit on screen

>+        // Decide when to start scaling down, 18 is the padding used for the switcher list

You should read that out of the parsed CSS rather than hardcoding it here; look at the computation of arrowHeight in AppSwitcher's _allocate().

>+        let maxIcons = Math.floor((global.screen_width - 18 * 2) / ((POPUP_APPICON_SIZE + POPUP_LIST_SPACING * 2) * 2));

The math here is wrong. With N icons there will be N POPUP_APPICON_SIZEs but only N-1 POPUP_LIST_SPACINGs. Not sure what either of the final "* 2"s is for.

>+                let iconScale = POPUP_APPICON_SIZE * (maxIcons / apps.length);

This is also wrong, because the math isn't taking into account the fact that the icons will change size but the padding won't.

Don't compute it in terms of scaling. Just loop over iconSizes until you find a size that fits. Eg:

    let maxWidth = global.screen_width - totalPadding;
    for (let i = 0; i < iconSizes.length; i++) {
        let widthAtSize = apps.length * iconSizes[i] + (apps.length - 1) * POPUP_LIST_SPACING;
        if (widthAtSize <= maxWidth)
            ...

Also, we probably don't want the switcher to go all the way from 0 to screen_width, so subtract another totalPadding from maxWidth to leave a margin there.

Created attachment 147716 [details] [review]
[AppSwitcher] Scale app icons down to fit on screen

Fixed up patch

Comment on attachment 147716 [details] [review]
[AppSwitcher] Scale app icons down to fit on screen

I get:

(mutter:7499): St-WARNING **: st_widget_get_theme_node called on a widget not in a stage

and then it measures the switcher incorrectly, extending it a bit off the screen before switching down to the next size.

Hardcoding totalPadding based on what it *should* be according to the CSS does not work either... it seems that the icon boxes are getting larger allocations than we'd expect.

Maybe AppSwitcher needs to override SwitcherList's GenericContainer implementations? Perhaps we shouldn't create the icons until right before they're drawn? Not totally sure what the best fix is.

Created attachment 154866 [details] [review]
[AppSwitcher] Scale app icons down to fit on screen

OK, here is a new approach that should 

1) Not show any such warnings and 2) does not make any assumptions about how big the switcher would be.

It uses a "try and error" method which looks a bit odd at first, but is the only way I can think of (the amount of changes are small and it works just fine).

Created attachment 154868 [details] [review]
[AppSwitcher] Scale app icons down to fit on screen

Small fix.

Created attachment 155065 [details] [review]
[AppSwitcher] Scale app icons down to fit on screen

*) Rebase against patch in 597983 and add a dependency on that bug.

this has been merged with 597983 now right?

(In reply to comment #10)
> this has been merged with 597983 now right?

Yes

*** This bug has been marked as a duplicate of bug 597983 ***