1359881 - Update icon in hamburger menu looks grainy on low-res displays

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect, P2)

Description

[Andreas Tolfsen ❲:ato❳]

Attachment: lowresgrainy.png

The application update icon that appears when a new Firefox version is available, looks crisp and beautiful on hi-res screens, but grainy on low-res 1:1 displays.

See the attached screenshots.

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: highrescrisp.png

Comment 2

[Robert Strong (they/them - no direct email)]

Bram, can you help with creating an image to fix this?

Comment 3

[Bram Pitoyo [:bram]]

The low-res (@1x) image is here:
https://cl.ly/3Z2A1O2p3R0i

But perhaps we should use an SVG that can be freely scaled?
https://cl.ly/0c0d353y160M

Comment 4

[Alex Thayer [:alexical] (she/her)]

I think the grainyness mentioned is just due to the 1px border rasterization. Bram, would you mind reviewing this image where I'm using a 2px border instead? (This would only be for 1:1 displays.)

https://d2ppvlu71ri8gs.cloudfront.net/items/0x0I072m2V2B1h282J1W/newbadge.PNG

Comment 5

[Bram Pitoyo [:bram]]

(In reply to Doug Thayer [:dthayer] from comment #4)
> I think the grainyness mentioned is just due to the 1px border
> rasterization. Bram, would you mind reviewing this image where I'm using a
> 2px border instead? (This would only be for 1:1 displays.)
> 
> https://d2ppvlu71ri8gs.cloudfront.net/items/0x0I072m2V2B1h282J1W/newbadge.PNG

Thanks for posting the example, Doug. It looks good to me!

Comment 6

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1359881 - Increase border width of update badge

This should help resolve the grainyness that was due to the
rasterization of a curved 1px border.

Review commit: https://reviewboard.mozilla.org/r/137832/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/137832/

Comment 7

[:Gijs (he/him)]

Comment on attachment 8866192 [details]
Bug 1359881 - Increase border width of update badge

https://reviewboard.mozilla.org/r/137832/#review141058

::: browser/themes/shared/customizableui/panelUI.inc.css:119
(Diff revision 1)
> -  border: 1px solid -moz-dialog;
> +  border: 2px solid -moz-dialog;
>    /* "!important" is necessary to override the rule in toolbarbutton.css */
> -  margin: -9px 0 0 !important;
> +  margin: -8px 0 0 !important;

So, I'm a little confused. I don't have hardware to test this off-hand so I'm just going to comment and leave the r? for now, and hopefully you can clarify and/or next week when I'm home I can look at it myself.

Basically, you're increasing the border, and decreasing the negative top margin. As I understand it, that moves the top of the border (and therefore the image) *down* by 1px, and because the border is bigger, the green bit of the image moves down 2px. I kind of assume this is my morning sleepy brain missing something, but that doesn't seem right?

The other thing I'm confused about is that this is XUL, and so the border "eats into" the width/height, which is staying the same, which will cause the image we're using to be warped, which will not make things better on lo-res devices... right? I guess I'm missing something else here, maybe? :-)

Comment 8

[Alex Thayer [:alexical] (she/her)]

Comment on attachment 8866192 [details]
Bug 1359881 - Increase border width of update badge

https://reviewboard.mozilla.org/r/137832/#review141058

> So, I'm a little confused. I don't have hardware to test this off-hand so I'm just going to comment and leave the r? for now, and hopefully you can clarify and/or next week when I'm home I can look at it myself.
> 
> Basically, you're increasing the border, and decreasing the negative top margin. As I understand it, that moves the top of the border (and therefore the image) *down* by 1px, and because the border is bigger, the green bit of the image moves down 2px. I kind of assume this is my morning sleepy brain missing something, but that doesn't seem right?
> 
> The other thing I'm confused about is that this is XUL, and so the border "eats into" the width/height, which is staying the same, which will cause the image we're using to be warped, which will not make things better on lo-res devices... right? I guess I'm missing something else here, maybe? :-)

Thanks for the quick feedback, Gijs! :)

First, I included an image in the bug, which I am realizing I should link here in case you haven't seen it: https://d2ppvlu71ri8gs.cloudfront.net/items/0x0I072m2V2B1h282J1W/newbadge.PNG I asked Bram to review it for some of the reasons you mentioned and he gave a thumbs up.

The added border width will in fact eat into the badge's space rather than increasing it, but it does not cause the image to change size. I'm not sure if there are conditions under which images _would_ change size, but this is not one of them. The reason I am increasing the border without increasing the min-width or min-height is that doing so makes the badge too big over all, and it ends up either eating into the second pancake of the menu icon, or sitting too high and looking off-balance.

Regarding why I moved the badge down a pixel - the added space makes the badge appear set out a bit more from the pancake menu icon. Moving it down a bit makes it feel more nestled into it again. It's a bit subtle and maybe I'm crazy but it felt like the right direction to go.

Comment 9

[:Gijs (he/him)]

Comment on attachment 8866192 [details]
Bug 1359881 - Increase border width of update badge

https://reviewboard.mozilla.org/r/137832/#review141338

Thanks for the clarifications! In that case, this is good to go.

FWIW, I *think* we force-set the width and height of (all) badges elsewhere... I wonder if we can make the badge itself look better on lodpi if we adjust the image for this (IIRC it's some bizarre size like 12x13px - but maybe there's other code that's already working around that) ?

Anyway, that doesn't need to block this bug.

Comment 10

[Robert Strong (they/them - no direct email)]

Pushed via autoland

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ddcfea98fb34
Increase border width of update badge r=Gijs

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/ddcfea98fb34