Closed
Bug 1337856
Opened 8 years ago
Closed 7 years ago
Installation assets in Windows are low dpi
Categories
(Firefox :: Installer, defect, P1)
Tracking
()
VERIFIED
FIXED
Firefox 58
Tracking | Status | |
---|---|---|
firefox56 | --- | unaffected |
firefox57 | --- | verified |
firefox58 | --- | verified |
People
(Reporter: sole, Assigned: mhowell)
References
Details
(Whiteboard: [fce-active-legacy])
Attachments
(10 files)
(deleted),
image/png
|
Details | |
(deleted),
image/jpeg
|
Details | |
(deleted),
image/jpeg
|
Details | |
(deleted),
image/jpeg
|
Details | |
(deleted),
text/x-review-board-request
|
agashlin
:
review+
ritu
:
approval-mozilla-beta+
|
Details |
(deleted),
text/x-review-board-request
|
agashlin
:
review+
ritu
:
approval-mozilla-beta+
|
Details |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
application/x-zip-compressed
|
Details | |
(deleted),
application/x-zip-compressed
|
Details |
I just installed Nightly on a Windows 10 laptop. The graphics assets displayed while the installer is running are low dpi, which unfortunately makes the process look quite unpolished when running on this high resolution laptop where everything else is very sharp looking.
Reporter | ||
Updated•8 years ago
|
OS: Unspecified → Windows 10
Hardware: Unspecified → x86
Assignee | ||
Updated•7 years ago
|
Priority: -- → P3
Comment 2•7 years ago
|
||
What images do we need to fix this? bgstub.bmp is the only image I see in the branding directory and it's low res.
Updated•7 years ago
|
Flags: needinfo?(mhowell)
Assignee | ||
Comment 3•7 years ago
|
||
That's the only image involved (for each branding).
There's a bigger issue though, which is how to get that image actually displayed; ideally I'd like to include just a high res image and scale it down for low res displays, but the scaling that's used in there right now makes that look hideous. And including both images would be too much file size. So I need to come up with something clever before we can make any use of a higher res image.
Flags: needinfo?(mhowell)
Comment 5•7 years ago
|
||
I've noticed this issue too. Lines are not smooth in the background and the logo is not scaled nicely. See attachment which illustrates this behavior. Probably you could use vector images?
Thanks.
Updated•7 years ago
|
Whiteboard: [fce-active]
Assignee | ||
Comment 9•7 years ago
|
||
We keep getting more reports of this. It needs to get fixed.
status-firefox57:
--- → affected
status-firefox58:
--- → affected
OS: Windows 10 → Windows
Priority: P3 → P1
Hardware: x86 → All
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → mhowell
Status: NEW → ASSIGNED
Assignee | ||
Comment 10•7 years ago
|
||
Stephen, I think I finally have a way to fix this, but I need high-res versions of the stub installer background image for nightly, dev edition, and release; there's one in bug 1365998 that I can use for the unofficial branding, but the others there are out of date. I seem to remember you've offered to provide those before, are you still who I should be asking?
Flags: needinfo?(shorlander)
Comment 11•7 years ago
|
||
Flags: needinfo?(shorlander)
Comment 12•7 years ago
|
||
Comment 13•7 years ago
|
||
Assignee | ||
Comment 14•7 years ago
|
||
Thanks @shorlander, those are perfect.
I'm going to verify that all of these images look nice under different DPI settings with the new scaling method I'm trying to use, and I'll have patches up here for review once that's done.
Assignee | ||
Comment 15•7 years ago
|
||
The above testing didn't go quite perfectly. The 2x images look amazing on a hi-DPI display, but have lighter text on the "Nightly" and "Developer Edition" labels, and that text shows aliasing when scaled down by half for a display not in a hi-DPI mode.
Fortunately, since we'll be able to use JPEG's now, we can include both the 1x and 2x image in the stub installer build without affecting the file size (in fact the file size still ends up dropping about 45K) and select the correct one to use based on the system DPI. So that's my new plan.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 18•7 years ago
|
||
We want to make sure things look good at 150% on Windows (since that's the recommended settings). If we can, we should explicitly choose 1x or 2x on 150% rather than trying to do scaling.
Assignee | ||
Comment 19•7 years ago
|
||
Well, we have to either scale the 1x image up or the 2x image down for 150%. Scaling down the 2x seems to look better to me at 150%, so that's what the above patch does.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8918450 [details]
Bug 1337856 Part 1 - Use a better up/down-scaling method for the stub installer background.
https://reviewboard.mozilla.org/r/189290/#review195100
Attachment #8918450 -
Flags: review?(agashlin) → review+
Comment 23•7 years ago
|
||
mozreview-review |
Comment on attachment 8918451 [details]
Bug 1337856 Part 2 - Swap out stub installer background images with JPEG's, and add high-res versions.
https://reviewboard.mozilla.org/r/189292/#review195102
Attachment #8918451 -
Flags: review?(agashlin) → review+
Comment 24•7 years ago
|
||
Pushed by mhowell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f1ded70b5b84
Part 1 - Use a better up/down-scaling method for the stub installer background. r=agashlin
https://hg.mozilla.org/integration/autoland/rev/e34bf13e7042
Part 2 - Swap out stub installer background images with JPEG's, and add high-res versions. r=agashlin
Comment 25•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f1ded70b5b84
https://hg.mozilla.org/mozilla-central/rev/e34bf13e7042
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Updated•7 years ago
|
Comment 26•7 years ago
|
||
57 is not on nightly anymore.
Comment 27•7 years ago
|
||
We should seriously consider this for 57. We're putting a lot of energy into the stub installer for 57 and it looks really crappy on hires Windows.
Assignee | ||
Comment 28•7 years ago
|
||
For reference, this is what the fixed version of attachment 8904796 [details] looks like; the difference in quality of the wordmark is huge.
Comment 29•7 years ago
|
||
Indeed but do we have the same issue with release?
Assignee | ||
Comment 30•7 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #29)
> Indeed but do we have the same issue with release?
Yes.
On the left is the current beta stub at 150% scaling. On the right is one I just built with the release branding and this patch.
I actually didn't think it would be that bad, I'm kind of surprised; I thought a lot of the problem was due to the lighter weight of the type on e.g. the word "Nightly" but this is just as bad, and the logo looks a lot worse over the lighter background.
Comment 31•7 years ago
|
||
(Can we reconsider the 57 wontfix, then? IMO we should strongly consider 57 uplift here. This bug gives a bad first impression to users who we anticipate/hope will be coming back to Firefox to try 57 -- so if this patch is OK from a riskiness perspective, I think we should try to take it.)
Flags: needinfo?(sledru)
Comment 32•7 years ago
|
||
If this is impacting release, we probably want to take it.
Leave the final call to Ritu as it is late here.
Flags: needinfo?(sledru) → needinfo?(rkothari)
Hi Matt, this looks like a safe/low risk change that we can uplift to 57. I am assuming this is also a recent regression in 57. Please nominate a patch for uplift to Beta57. Thanks!
status-firefox56:
--- → unaffected
Flags: needinfo?(rkothari) → needinfo?(mhowell)
Assignee | ||
Comment 34•7 years ago
|
||
Thanks, Ritu. The patches that are on here apply cleanly to beta for me, so I'll put in the request on those.
This isn't a recent regression at all I'm afraid, it's been present in one form or another since the stub installer has been around, but became more noticeable as of 55.
Flags: needinfo?(mhowell)
Assignee | ||
Comment 35•7 years ago
|
||
Comment on attachment 8918450 [details]
Bug 1337856 Part 1 - Use a better up/down-scaling method for the stub installer background.
Approval Request Comment
[Feature/Bug causing the regression]:
This bug has been present since the stub installer has been around, but was made worse by bug 1365998.
[User impact if declined]:
See comment 30.
[Is this code covered by automated tests?]:
No
[Has the fix been verified in Nightly?]:
I've verified it manually myself; I checked how all the backgrounds look on multiple display scaling settings.
[Needs manual test from QE? If yes, steps to reproduce]:
No, I've done manual testing already.
[List of other uplifts needed for the feature/fix]:
No other bugs, just both parts on this one.
[Is the change risky?]:
No
[Why is the change risky/not risky?]:
It's a small, targeted patch that doesn't affect the installation process itself, and I've tested it manually.
[String changes made/needed]:
None
Attachment #8918450 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 36•7 years ago
|
||
Comment on attachment 8918451 [details]
Bug 1337856 Part 2 - Swap out stub installer background images with JPEG's, and add high-res versions.
Approval Request Comment
See above comment for other patch.
Attachment #8918451 -
Flags: approval-mozilla-beta?
Comment on attachment 8918450 [details]
Bug 1337856 Part 1 - Use a better up/down-scaling method for the stub installer background.
Thanks Matt. I agree, this feels like a must fix to make 57 more compelling/awesome, Beta57+
Attachment #8918450 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8918451 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
Comment 38•7 years ago
|
||
bugherder uplift |
Comment 39•7 years ago
|
||
I've tested on Windows 7 x64 and Windows 10 x64 (on laptop) using Firefox 57 Beta 13 and latest Nightly 58.0a1, DPI 125%, 150%, 175%/200% and I have the following mentions:
- on Windows 7 x64 - all the icons, background images, installation assets and the text of Stub Installer look good
- on Windows 10 (laptop) - the background images and the text of Stub Installer look good, but the Title and the Taskbar icons are low dpi (on 150% and 175%). Also, I noticed that the Title Bar is a few pixels shorter than the rest of the Stub window (see "TitleBar.ing"). Please see attachment "issueswin10.png".
Any thoughts?
Flags: needinfo?(mhowell)
Assignee | ||
Comment 40•7 years ago
|
||
Were you logging out of Windows after changing the scaling setting? Not doing that is the only way I can reproduce either issue. There's not really any way around needing to do that just because of how the relevant Windows API's work, which is why the display settings dialog recommends it after the scaling is changed.
Flags: needinfo?(mhowell)
Comment 41•7 years ago
|
||
Yes, you are right: I'm not logging out of Windows after changing the scaling setting. I've tested again and I've performed log out after changing the scaling setting: the Title and the Taskbar icons look better now, but I consider that the icons still are a bit low dpi. What do you think?
Please see the attachment "logout-after-changing-scaling.zip".
Assignee | ||
Comment 42•7 years ago
|
||
Hmm. I see what you're talking about, but I can't reproduce it. It looks like Explorer is selecting the wrong size icon to display for some reason, but I'm not sure what we can do about that. What happens if you turn off small taskbar buttons, is it still broken?
Also, this is something that should already have been working okay prior to this patch and that should not have been affected by this patch, so it might be better to file a separate bug.
Comment 43•7 years ago
|
||
(In reply to Matt Howell [:mhowell] from comment #42)
> Hmm. I see what you're talking about, but I can't reproduce it. It looks
> like Explorer is selecting the wrong size icon to display for some reason,
> but I'm not sure what we can do about that. What happens if you turn off
> small taskbar buttons, is it still broken?
>
> Also, this is something that should already have been working okay prior to
> this patch and that should not have been affected by this patch, so it might
> be better to file a separate bug.
I logged bug 1417928.
Marking this issue as VERIFIED FIXED.
Updated•7 years ago
|
Whiteboard: [fce-active] → [fce-active-legacy]
You need to log in
before you can comment on or make changes to this bug.
Description
•