Closed Bug 1153306 Opened 7 years ago Closed 7 years ago

The Loop/Hello icon is blurry in HiDPI mode on Windows

Categories

(Hello (Loop) :: Client, defect, P3)

All
Windows 8.1
defect

Tracking

(firefox40 verified, firefox41 verified)

VERIFIED FIXED
mozilla41
Tracking Status
firefox40 --- verified
firefox41 --- verified

People

(Reporter: jaws, Assigned: jaws)

References

Details

(Whiteboard: [UX bug])

Attachments

(3 files)

STR:
Go to about:config and set layout.css.devPixelsPerPx to 2.0
Look at Hello icon on the toolbar

ER:
Just as crisp as the other icons

AR:
Blurry
Component: Theme → Client
Product: Firefox → Loop
Version: Trunk → unspecified
Shell, can we get this prioritized for Firefox 40? Bug 1147702 is going to ship with Firefox 40.
Flags: needinfo?(sescalante)
In total, we'll want:
/browser/themes/windows/loop/menuPanel@2x.png
/browser/themes/windows/loop/toolbar-inverted@2x.png
/browser/themes/windows/loop/toolbar@2x.png
The download indicator button is also blurry.
It just needs a small fix on browser.css, because toolbar@2x.png already exists.

So will any mozillaian guys see it?


#downloads-indicator-icon {
  background: -moz-image-rect(url("chrome://browser/skin/Toolbar@2x.png"),
                              0, 396, 36, 360) center no-repeat;
  background-size: 18px;
  min-width: 18px;
  min-height: 18px;
}
#downloads-button[attention] > #downloads-indicator-anchor > #downloads-indicator-icon {
  background-image: -moz-image-rect(url("chrome://browser/skin/Toolbar@2x.png"), 36, 396, 72, 360);
  background-size: 18px;
}

#downloads-button:not([counter]) > #downloads-indicator-anchor > #downloads-indicator-progress-area > #downloads-indicator-counter {
  background: -moz-image-rect(url("chrome://browser/skin/Toolbar@2x.png"),
                              0, 396, 36, 360) center no-repeat;
  background-size: 12px;
}
Hi Michael - can we get the assets that jared lists in Comment 2
Rank: 35
Flags: needinfo?(sescalante)
Flags: needinfo?(mmaslaney)
Flags: firefox-backlog+
Priority: -- → P3
Whiteboard: [UX bug]
(In reply to Fushan Wen from comment #3)

Just wanted to note that this was fixed in bug 1153529.

We are still waiting on the Hello graphics from Michael though. I'll send him an email.
I see that if Hello icon is moved to customization additional tools and features the icon is blurry as well without changing 'layout.css.devPixelsPerPx' that has value '-1' by default. I`m testing on Surface Pro 2 and Windows 7 64-bit 125 dpi. 

Happens only in Nightly and it`s a regression. 

m-i:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=223975f99ca3&tochange=8225a3b75df6 

Is this bug covering this behavior as well or I can go ahead and log a new issue?
Flags: needinfo?(jaws)
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #6)
> Happens only in Nightly and it`s a regression. 

I'm not sure how this is a regression. It may be more noticeable now that the other icons are crisp in the Customize Mode, but this should have been looking blurry since the beginning of Hello. 

> Is this bug covering this behavior as well or I can go ahead and log a new
> issue?

This bug is covering this behavior.
Flags: needinfo?(jaws)
Attached file loop-icons.zip
Flags: needinfo?(mmaslaney) → needinfo?(gijskruitbosch+bugs)
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Flags: needinfo?(gijskruitbosch+bugs)
Attached patch PatchSplinter Review
Attachment #8616947 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8616947 [details] [diff] [review]
Patch

Review of attachment 8616947 [details] [diff] [review]:
-----------------------------------------------------------------

Please revert the changes to the non-2x versions, as they seem to all have grown in size with no difference in the actual images. It probably wouldn't hurt chucking the 2x files through an image minimizer as well, though I've not checked for certain. See bug 1062969 for earlier work. (We really should be automizing this... though I'm not sure how that would work...)

::: browser/themes/shared/menupanel.inc.css
@@ +383,5 @@
> +
> +  #loop-button[cui-areatype="menu-panel"] > .toolbarbutton-badge-container > .toolbarbutton-icon,
> +  toolbarpaletteitem[place="palette"] > #loop-button > .toolbarbutton-badge-container > .toolbarbutton-icon {
> +    width: 32px;
> +    height: 32px;

This is new. Over in bug 1165679 we discussed fixing the rule in panelUIOverlay.css, and then (I think it was) Dão (who) added a plain width/height in it in another bug. But all the badge-containers are in there, and not the icons, and I bet that changing that will cause "issues" because of the margin that we also set.

I don't really understand why this is still necessary considering the container is already set to 32px. I'm assuming you tested it and found it necessary?

I think we should tidy these rules up a bit because right now we set width all over the place and the rules make little sense to me anymore. Can you file a followup bug for this?

::: browser/themes/windows/jar.mn
@@ +635,5 @@
>  % override chrome://browser/skin/Toolbar@2x.png                       chrome://browser/skin/Toolbar-aero@2x.png                         os=WINNT osversion=6.1
>  % override chrome://browser/skin/loop/menuPanel.png                   chrome://browser/skin/loop/menuPanel-aero.png                     os=WINNT osversion=6
>  % override chrome://browser/skin/loop/menuPanel.png                   chrome://browser/skin/loop/menuPanel-aero.png                     os=WINNT osversion=6.1
> +% override chrome://browser/skin/loop/menuPanel@2x.png                chrome://browser/skin/loop/menuPanel-aero@2x.png                     os=WINNT osversion=6
> +% override chrome://browser/skin/loop/menuPanel@2x.png                chrome://browser/skin/loop/menuPanel-aero@2x.png                     os=WINNT osversion=6.1

You need to add similar overrides for the toolbar version...

@@ +641,5 @@
>  % override chrome://browser/skin/Toolbar.png                          chrome://browser/skin/Toolbar-aero.png                            os=WINNT osversion=6
>  % override chrome://browser/skin/Toolbar.png                          chrome://browser/skin/Toolbar-aero.png                            os=WINNT osversion=6.1
>  % override chrome://browser/skin/Toolbar.png                          chrome://browser/skin/Toolbar-XP.png                              os=WINNT osversion<6
>  % override chrome://browser/skin/loop/toolbar.png                     chrome://browser/skin/loop/toolbar-aero.png                       os=WINNT osversion=6
>  % override chrome://browser/skin/loop/toolbar.png                     chrome://browser/skin/loop/toolbar-aero.png                       os=WINNT osversion=6.1

... here, I think? Both for aero and XP (!)
Attachment #8616947 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to :Gijs Kruitbosch from comment #10)
> Please revert the changes to the non-2x versions, as they seem to all have
> grown in size with no difference in the actual images. It probably wouldn't
> hurt chucking the 2x files through an image minimizer as well, though I've
> not checked for certain.

I checked with PNGCrush and all of the @2x images were already optimized. I've reverted the 1x images.

> ::: browser/themes/shared/menupanel.inc.css
> @@ +383,5 @@
> > +
> > +  #loop-button[cui-areatype="menu-panel"] > .toolbarbutton-badge-container > .toolbarbutton-icon,
> > +  toolbarpaletteitem[place="palette"] > #loop-button > .toolbarbutton-badge-container > .toolbarbutton-icon {
> > +    width: 32px;
> > +    height: 32px;
> 
> I don't really understand why this is still necessary considering the
> container is already set to 32px. I'm assuming you tested it and found it
> necessary?

Without this rule, the icon has a box model with width:64px, height:32px when in the menupanel. The width and height seem to be coming from the -moz-image-region. I can't figure out why the height is reduced but the width not. 
 
> I think we should tidy these rules up a bit because right now we set width
> all over the place and the rules make little sense to me anymore. Can you
> file a followup bug for this?

Filed bug 1173401.

> ::: browser/themes/windows/jar.mn
> You need to add similar overrides for the t

Good catch. Thanks!
https://hg.mozilla.org/mozilla-central/rev/646bc6779d35
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Attached patch Patch for 40Splinter Review
Approval Request Comment
[Feature/regressing bug #]: Windows 10 HiDPI
[User impact if declined]: HiDPI icons on Windows are blurry
[Describe test coverage new/current, TreeHerder]: on mozilla-central for much of 41-nightly
[Risks and why]: none expected 
[String/UUID change made/needed]: none

https://hg.mozilla.org/try/pushloghtml?changeset=71084a9edf1e
Attachment #8627208 - Flags: approval-mozilla-beta?
Comment on attachment 8627208 [details] [diff] [review]
Patch for 40

On m-c for 3 weeks already. Visual fix in support of Windows 10. Beta+
Attachment #8627208 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
Confirming the fix on Windows 10 64-bit using:
- latest Aurora, build ID: 20150706004011
- Firefox 40 beta 2, build ID: 20150706172413.
Status: RESOLVED → VERIFIED
QA Contact: cornel.ionce
You need to log in before you can comment on or make changes to this bug.