796435 - Installing a web app from Marketplace uses incorrect icon size

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia, defect, P1)

Description

[GH to BZ]

[GitHub issue by jds2501 on 2012-07-26T02:52:50Z, https://github.com/mozilla-b2g/gaia/issues/2840]
## Build

Device: Otoro
Date: 7/25/2012

## Steps

1. Go to the marketplace app
2. Login
3. Install Jauntly or Calculator
4. Go to homescreen and find those apps

## Expected

The icons for the apps should not be pixelated and be scaled down correctly.

## Actual

The icons for the apps look pixelated and not scaled down correctly.

Comment 1

[GH to BZ]

[GitHub comment by jds2501 on 2012-07-26T02:54:29Z]
cc @cleemoz - Marketplace is intending to make 128 x 128 a required icon size to be specified in the manifest, so this has to look "good' on Firefox OS. Otherwise, many of the apps are likely going to not look too good if you install the app from marketplace with a 128 x 128 icon.

This is also the icon size I was going to suggest to you over email for twitter given that this is now a requirement, but we better get this issue resolved.

Comment 2

[GH to BZ]

[GitHub comment by tonychung on 2012-07-26T04:54:38Z]
@jds2501, can you add a screenshot?    to do it on a otoro device, you can hold Home+ Power button, then launch DDMS and copy the file from /sdcard/DCIM/screenshots

Comment 3

[GH to BZ]

[GitHub comment by jds2501 on 2012-07-26T17:01:57Z]
http://tinypic.com/r/29zqvdx/6

Comment 4

[GH to BZ]

[GitHub comment by autonome on 2012-07-26T23:08:39Z]
@patrykdesign what's the icon policy for this? scale down or use a placeholder?

Comment 5

[GH to BZ]

[GitHub comment by cleemoz on 2012-07-27T19:58:37Z]
@patrykdesign: can you comment on 128x128 icons?  Does this sound right to you?

Comment 6

[GH to BZ]

[GitHub comment by cvan on 2012-07-27T20:27:22Z]
I had difficulty finding the stylesheet for the homescreen icons, but `background-size: contain` should do the trick

Comment 7

[GH to BZ]

[GitHub comment by patrykdesign on 2012-07-31T14:21:16Z]
@cvan 
On device the icons sizes we proposed to marketplace are:
40px - ldpi
54px - mdpi (otoro)
80px - hdpi
108px -xhdpi

So the icon should scale down to 54px if its too big. Then programmatically a drop shadow will added to the icon when installed.

Comment 8

[GH to BZ]

[GitHub comment by jds2501 on 2012-07-31T14:24:27Z]
For context to this discussion, desktop and android explicitly call out support for icons sizes of:

* 16 x 16
* 32 x 32
* 48 x 48
* 64 x 64
* 128 x 128
* 256 x 256

Comment 9

[GH to BZ]

[GitHub comment by autonome on 2012-07-31T15:18:25Z]
This would nice, but not going to hold the release on it. If there's an abundance of evidence showing that marketplace apps will refuse to read the docs and provide the correct icon size, then re-nominate.

Comment 10

[GH to BZ]

[GitHub comment by jds2501 on 2012-07-31T15:26:55Z]
@autonome On the contrary, typically most apps on marketplace are making use of 128 x 128 icons, so this definitely a typical use case. Marketplace is also still requiring this for apps that are listed on Mozilla Marketplace. This still a valid nom.

Comment 11

[GH to BZ]

[GitHub comment by jds2501 on 2012-07-31T15:28:17Z]
@patrykdesign I also haven't seen use of those icon sizes for apps on marketplace (if any actually). @cvan Have you seen any?

Comment 12

[GH to BZ]

[GitHub comment by jds2501 on 2012-07-31T15:31:30Z]
@autonome Our docs don't indicate anything about the icon sizes @patrykdesign cites. See my comment above about what is cited in https://developer.mozilla.org/en/Apps/Manifest.

Comment 13

[GH to BZ]

[GitHub comment by cleemoz on 2012-07-31T15:58:08Z]
Can we block on the app submission of Marketplace to manage this?  

We won't be able to do much for sites that are 'added to the Home Screen'.

Comment 14

[GH to BZ]

[GitHub comment by cleemoz on 2012-07-31T15:58:24Z]
@patrykdesign: can you sync with the Marketplace team on this?

Comment 15

[GH to BZ]

[GitHub comment by jcarpenter on 2012-07-31T16:06:37Z]
@jds2501 @patrykdesign By way of reminder, we discussed this in mid June email thread, "B2G Icon Size". It looks like it dropped off at that point without a conclusion. Patryk and myself also discussed it with Ragavan, Chris Lee & Bryan Clark in "App manifest icon size" thread (late June), again without firm conclusion. So we need to resurrect and resolve this :)

Comment 16

[GH to BZ]

[GitHub comment by jds2501 on 2012-07-31T17:02:31Z]
@cvan Just analyzed our current apps on marketplace to find our stats on icon sizes used. See below:

{'24px': 7,
 '1024px': 1,
 '48px': 196,
 '16px': 202,
 '32px': 32,
 '57px': 2,
 '56px': 1,
 '50px': 1,
 '40px': 1,
 '114px': 3,
 '64px': 30,
 '72px': 4,
 '128px': 247,
 '512px': 4,
 '256px': 12,
 '96px': 6}

Comment 17

[GH to BZ]

[GitHub comment by patrykdesign on 2012-08-01T14:39:56Z]
@jds2501 @jcarpenter @cleemoz @autonome 
So there was a thread a few months back (w. Ragavan, Bryan and a few others) where were decided on the sizing. To summarize.

The way we plan on displaying the icons is as follows:
+ The homescreen would programmatically assembly the ICON + Drop Shadow, the reason for is greater consistency since we won't have the same scrutinizing as iOS.
+ For the launch device the overall icon canvas size (with the drop shadow) will be 64x64 pixels, therefore the ICON has be smaller otherwise, we will need to programmatically shrink the icon (usually ruining its quality).

If you look at android icons, they have plenty of negative space at each size. Our icons would have a similar effect. The icon sizes I specified are as follows:
40x40px icon, maps onto a 48x48px canvas - ldpi
54x54px icon maps onto a 64x64px canvas - mdpi (otoro)
80x80px icon maps onto a 96x96px canvas - hdpi
108x108px icon maps onto a 128x128px canvas -xhdpi

I hope this makes more sense.

Comment 18

[GH to BZ]

[GitHub comment by jds2501 on 2012-08-01T14:56:47Z]
So to reword so I understand in my own words, for say a 128 x 128 icon, programmatically, we would do the following:

Scale the icon size down to 54px x 54px and add negative space around the icon. Negative space + 54px x 54px original icon = 64 x 64 icon to be used.

Is this correct?

So let's take Jauntly for example (that's a common test case that's been used on desktop and android for sanity testing that's reliable) - Why does it look pixelated in that case? Here's a direct link to the icons:

* http://getjauntly.com/icons/jauntly-16x16.png
* http://getjauntly.com/icons/jauntly-32x32.png
* http://getjauntly.com/icons/jauntly-64x64.png
* http://getjauntly.com/icons/jauntly-128x128.png
* http://getjauntly.com/icons/jauntly-256x256.png

I should probably also ask this question (which may affect the icon quality) - When there are multiple icon sizes specified in a manifest, which icon is used by default? Desktop I know uses the larger icon by default always, not entirely sure what Android does.

Comment 19

[GH to BZ]

[GitHub comment by patrykdesign on 2012-08-01T15:07:36Z]
@jds2501 yes you are correct. 
@vingtetun can comment better why the icons may appear pixelated, he owns the homscreen development. I suspect is a poor scaling algorithm.

Comment 20

[GH to BZ]

[GitHub comment by crdlc on 2012-08-01T15:48:52Z]
In current implementation as far as I know, when there are multiple icon sizes we do

* device width < 480 the smaller (Otoro)
* device width >= 480 the bigger. 

https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/appmanager.js#L205

Why those icons are pixelated? I dont't know, how is the definition of the icon sizes for these apps please?

Currently we scale to 60x60 all icons programmatically

https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/style/grid.css#L107

@VictoriaGerchinhoren, @rafaelrebolleda

Comment 21

[GH to BZ]

[GitHub comment by crdlc on 2012-08-01T15:54:08Z]
sorry for my ignorance, what is Drop Shadow? @patrykdesign

Comment 22

[GH to BZ]

[GitHub comment by patrykdesign on 2012-08-01T17:35:03Z]
@crdlc http://cl.ly/image/0t0B3r3l3C2N

Comment 23

[GH to BZ]

[GitHub comment by crdlc on 2012-08-01T17:41:40Z]
umm I thought that, thanks!! As far as I know Andreas said to us in a meeting in Barcelona that shadows have a lot of problems of performance with the transitions so that we shouldn't use them. So the designers painted the icons with the drop shadow. I don't know how this issue is currently, @vingtetun do you remember this, shadow & transitions = bad performance?

Comment 24

[GH to BZ]

[GitHub comment by patrykdesign on 2012-08-01T18:26:17Z]
@crdlc CSS drop shadows have performance issues. Ours would likely be PNG if its a performance issue.
I think if we can't do CSS drop shadows. We'll provide a generic shadow PNG to apply to every icon from marketplace. Perhaps for the native apps, we'd integrate them into the icon.

Comment 25

[GH to BZ]

[GitHub comment by jds2501 on 2012-08-01T18:55:12Z]
I don't understand why this was marked as not a blocker for basecamp. Can someone please clarify?

Comment 26

[GH to BZ]

[GitHub comment by jcarpenter on 2012-08-01T19:14:16Z]
> I don't understand why this was marked as not a blocker for basecamp. Can someone please clarify?

IIRC from the triage meeting, we hummed and hawed and eventually deemed non-blocking because it is understood to be an aesthetic issue, and does not effect impede functionality. Mind you, we haven't seen it with our own eyes yet. In your opinion is the issue so bad that we should call it a blocker?

FWIW, I want to solve this before release, regardless of status.

Comment 27

[GH to BZ]

[GitHub comment by tonychung on 2012-08-01T19:17:16Z]
@jds2501 attached a screenshot in one of his comments:  http://tinypic.com/r/29zqvdx/6.   Its fugly enough if you stack it next to another optimized App, you'd want to question what's wrong with my phone.  

imagine a whole page filled with these blurry icons.

Comment 28

[GH to BZ]

[GitHub comment by jcarpenter on 2012-08-01T19:18:46Z]
http://cdn01.cdn.gofugyourself.com/wp-content/uploads/2009/03/71241_dawson-crying.edt.JPG

BLOCKER

Comment 29

[GH to BZ]

[GitHub comment by tonychung on 2012-08-01T19:19:46Z]
lol.  can we use that as a default icon to replace apps that have the pixelated icons?

Comment 30

[GH to BZ]

[GitHub comment by cleemoz on 2012-08-03T08:45:30Z]
What's the recommended path forward?

Comment 31

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Some of the comments did not moved successfully ...

https://github.com/mozilla-b2g/gaia/issues/2840

We still need someone fixing home screen on icon size selection.

Comment 32

[Tim Guan-tin Chien [:timdream] (please needinfo)]

CCing akeybl

Comment 33

[Alex Keybl [:akeybl]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #32)
> CCing akeybl

Apologies for the cut off comments here. We believe this is the only affected bug. Let's live with it for now, unless you feel strongly otherwise.

Comment 34

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

The screenshot in comment #3 looks like a 16x16 icon (favicon?) got scaled up, not anything bigger scaled down.  My guess is that the wrong source icon size is being used for the home screen -- perhaps only if an exact size match isn't found or something.

Comment 35

[Christopher Van Wiemeersch [:cvan]]

(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #34)
> The screenshot in comment #3 looks like a 16x16 icon (favicon?) got scaled
> up, not anything bigger scaled down.  My guess is that the wrong source icon
> size is being used for the home screen -- perhaps only if an exact size
> match isn't found or something.

Yeah, that appears to be the case:

https://github.com/mozilla-b2g/gaia/issues/2840#issuecomment-8704831
https://github.com/mozilla-b2g/gaia/issues/2840#issuecomment-8716604

Comment 36

[Lisa Brewster [:adora]]

The screenshot from comment #3 was taken several months ago.  Here's what this issue looks like today (I think it's the same root cause, anyway):

http://adora.io/screens/tiny-icons-20121002-114100.jpg

This is a pretty crappy experience, especially if we have bigger icons that we're just not using.

Comment 37

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Yeah, that looks like the same issue -- 16x16 icons that, in the current case, aren't being scaled up where they were before.  Either way it's the wrong size being used.

Comment 38

[Robert Kaiser]

The current algorithm actually always uses the smallest available icon if the screen is less than 480px wide and the largest available icon if the screen is wider. We should at least, as a first step, use the largest available icon that is smaller than the target size (64 for small screens and 128 for large ones?).

Comment 39

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

That makes no sense -- we should use the next biggest size from the target and scale down, since that's bound to look better than scaling up!

Comment 40

[Robert Kaiser]

Well, my "at least" in comment #38 was assuming we do no scaling at all, like we're doing right now, I was looking for the minimum thing we can do to be better. Of course, when we do scaling, we want to downscale instead of upscale - but right now we're not even selecting an icon if it's already our target size and there's smaller/larger ones available.

That said, the code that selects the size to use right now is at https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/appmanager.js#L233 - as you see, smaller than 480px width takes the last (smallest) icon, others take the first (largest) one.

Comment 41

[Patryk Adamczyk [:patryk] UX]

We need to scale down to a specific size to match with the homescreen icons. 
Also our application icons do not include the drop shadow, its drawn programmatically. I would suggest we do the same with these, is there a spec drawn out for icon submission? If not I can help create one.

After doing some tests here is the size determined to match best:
52x52 is the ideal icon size for icons without a drop shadow.

Comment 42

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Robert Kaiser (:[e-mail]) from comment #40)
> That said, the code that selects the size to use right now is at
> https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/
> appmanager.js#L233 - as you see, smaller than 480px width takes the last
> (smallest) icon, others take the first (largest) one.

That's what I have been saying for like 2 weeks ago and I have not see any moment since then.

Dietrich, it seems that the assignee info got lost during the Github to Bz migration. Can you put it back and notify the assignee about this item on his dish?

Comment 43

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Attachment: patch v1

Comment 44

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 667884 [details] [diff] [review]
patch v1

r+ with nit addressed. 
Please test again after nit applied (don't make the mistake I made in another bug :-/)

>--- a/apps/homescreen/js/appmanager.js
>+++ b/apps/homescreen/js/appmanager.js
>@@ -224,13 +224,26 @@ var Applications = (function() {
>-  function getIconSize(sizes) {
>-    return sizes[(deviceWidth < 480) ? sizes.length - 1 : 0];
>+  function getPreferSize(icons, prefer) {

prefer -> preferredSize

>+      if (size > prefer) {
>+        result = size < result ? size : result;
>+      }

if (size >= preferredSize && size < result)
  result = size;

>+    });
>+    return result === Number.MAX_VALUE ? max : result;

// If there is an icon matching the preferred size, we return the result,
// if there isn't, we will return the maximum available size.
return (result === Number.MAX_VALUE) ? max : result;

(add a comment and parentheses for readability)


>@@ -255,16 +268,12 @@ var Applications = (function() {
>-    var icon = icons[getIconSize(sizes)];
>+    var PREFER_ICON_SIZE = 54;

PREFERRED_ICON_SIZE

>+    var icon = icons[getPreferSize(icons, PREFER_ICON_SIZE)];
>     if ((icon.indexOf('data:') !== 0) &&
>         (icon.indexOf('http://') !== 0) &&
>         (icon.indexOf('https://') !== 0)) {
>-- 
>1.7.9.5
>

Comment 45

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Yuren, your fix did not include the part in canvas that would resize the icon with arbitrarily size into the 54px box. Is that not needed?

Comment 46

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Attachment: patch v2

no, it's not needed fix, because resize work is done by page.js => generateShadow

Comment 47

[Tim Guan-tin Chien [:timdream] (please needinfo)]

https://github.com/mozilla-b2g/gaia/commit/1889fe5eee213e71fb2899389ccbefe509eba410

Merged. thanks.

During an offline comment we have also found the the real icon size for Otoro is 64px, so the patch had been changed accordingly.

Comment 48

[Jason Smith [:jsmith]]

Verified on 10/11 build.