Closed Bug 946987 Opened 11 years ago Closed 11 years ago

[Australis] curved tabs looks ugly when scaled up (hi-dpi)

Categories

(Firefox :: Theme, defect)

All
Windows 8
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 31
Tracking Status
firefox29 + fixed
firefox30 + verified
firefox31 --- verified

People

(Reporter: smaug, Assigned: MattN)

References

(Blocks 2 open bugs)

Details

(Keywords: regression, Whiteboard: [Australis:P3])

Attachments

(5 files, 7 obsolete files)

Attached image ugly_tab.png (obsolete) (deleted) —
For some reason curved tabs look rather ugly on Win8. They look fine on Win7 (and on Linux)
Component: General → Theme
Summary: [Australis] curved tabs looks ugly on Windows 8 → [Australis] curved tabs looks ugly when scaled up (hi-dpi)
We're planning on working on HiDPI support for our UI on Windows after Australis.
Hardware: x86_64 → All
Whiteboard: [Australis:P4]
(In reply to Matthew N. [:MattN] from comment #1) > We're planning on working on HiDPI support for our UI on Windows after > Australis. Indeed, but I have to agree with Olli that this looks pretty terrible. Why does this look OK on OS X hidpi, and isn't there "just" some CSS we can port to make the tabs look less affrontingly awful? :-)
I'm upgrading this to P3- to at least get this on the radar for this week.
Whiteboard: [Australis:P4] → [Australis:P3-]
Attached image up-to-date ugly tab (deleted) —
I guess the tab a bit less ugly these days, now that we use right colors, but it still looks very unfinished.
If someone can create some high-dpi version of these images, we can fix this. In metro we're doing 1.0x, 1.4x, and 1.8x. chrome://browser/skin/tabbrowser/tab-background-start.png Who is the ux lead for desktop currently?
(In reply to :Gijs Kruitbosch from comment #2) > (In reply to Matthew N. [:MattN] from comment #1) > > We're planning on working on HiDPI support for our UI on Windows after > > Australis. > > Indeed, but I have to agree with Olli that this looks pretty terrible. Why > does this look OK on OS X hidpi, and isn't there "just" some CSS we can port > to make the tabs look less affrontingly awful? :-) Because someone took the time to generate 2.0x images for osx - http://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/jar.mn#246
(In reply to Jim Mathies [:jimm] from comment #6) > If someone can create some high-dpi version of these images, we can fix > this. In metro we're doing 1.0x, 1.4x, and 1.8x. > > chrome://browser/skin/tabbrowser/tab-background-start.png > > Who is the ux lead for desktop currently? I think shorlander is the person to talk to here.
Flags: needinfo?(shorlander)
Attached file check your dots per px (deleted) —
We should probably morph this bug to a more general "update images resources for high-dpi on Windows" unless something like that already exists? I can take the time to put together a patch now that metrofx is out the door.
(In reply to Jim Mathies [:jimm] from comment #10) > > We should probably morph this bug to a more general "update images resources > for high-dpi on Windows" unless something like that already exists? > > I can take the time to put together a patch now that metrofx is out the door. Updating all our images is significantly more work and isn't an Australis regression. There is a separate bug for it (bug 878288). I think we should focus this bug to be about the tabstrip. If we have time to fix other icons we can do that, but there are too many more pressing issues to fix for Australis right now.
Blocks: 878288
wfm, if someone can post the images, I can put together a patch.
Attached file windows-hidpi-tab-assets-01.zip (obsolete) (deleted) —
Windows Tab Assets for 1.25x, 1.5x and 2x. The 2x obviously scaled fine. For 1.25 and 1.5 I had to round up. Not sure what will happen at those stops; the CSS might need further tweaks.
Flags: needinfo?(shorlander)
Comment on attachment 8385386 [details] windows-hidpi-tab-assets-01.zip Tested these, don't think they will work as is. Going to need to tweak them first.
Attachment #8385386 - Attachment is obsolete: true
I don't think we want to end up with four versions of these images. Would scaling down the 2x images for 1.25x & Co. look better than scaling up the 1x images?
(In reply to Dão Gottwald [:dao] from comment #15) > I don't think we want to end up with four versions of these images. Would > scaling down the 2x images for 1.25x & Co. look better than scaling up the > 1x images? I tried that. It didn't look great.
I also wonder how scaling down would affect our TART numbers...
(In reply to Stephen Horlander [:shorlander] from comment #16) > (In reply to Dão Gottwald [:dao] from comment #15) > > I don't think we want to end up with four versions of these images. Would > > scaling down the 2x images for 1.25x & Co. look better than scaling up the > > 1x images? > > I tried that. It didn't look great. Does it look better than upscaling? :) (In reply to Mike Conley (:mconley) from comment #17) > I also wonder how scaling down would affect our TART numbers... Probably not worse than the upscaling we're doing today. Also, do we have any idea whether some scaling factors might be more common than others?
(In reply to Dão Gottwald [:dao] from comment #18) > (In reply to Stephen Horlander [:shorlander] from comment #16) > > (In reply to Dão Gottwald [:dao] from comment #15) > > > I don't think we want to end up with four versions of these images. Would > > > scaling down the 2x images for 1.25x & Co. look better than scaling up the > > > 1x images? > > > > I tried that. It didn't look great. > > Does it look better than upscaling? :) Well, yes ;) Here is what it looks like scaled to 150%. Top is upscaled with the current 1x image, second is downscaled with the 2x image and the bottom is a properly sized 1.5x image. http://cl.ly/image/440T2g1d2M2V Downscaling does look nicer than upscaling but still fuzzy. The specifically sized image looks a lot better.
Attached patch patch (obsolete) (deleted) — Splinter Review
Assignee: nobody → jmathies
Attachment #8386364 - Flags: review?(dao)
Attached patch Make Start and End of Tab 32x32 (obsolete) (deleted) — Splinter Review
(In reply to Jim Mathies [:jimm] from comment #21) > Created attachment 8386364 [details] [diff] [review] > patch I checked your patch and it has the same problems I was running into. Namely rounding errors causing gaps and overlap between the start, middle and end of the tab. I think before we can cleanly support 100%, 125%, 150% and 200% stops the base image will have to evenly multiply by those stops. Currently the start and end elements are 31 x 30. This patch changes tab start and end to 32 x 32. This also even scaling. On OS X it does have a bug that I can't figure out. Increasing the tab height and the negative margin on the nav-bar breaks in Customize Mode, with lightweight themes and in fullscreen. Not sure where we are doing those calculations though.
Attachment #8386436 - Flags: feedback?(MattN+bmo)
Attached file windows-hidpi-tab-assets-02.zip (obsolete) (deleted) —
New 32x32 base images.
Attachment #8386364 - Flags: review?(dao)
Wouldn't using an SVG as the primary asset ease the adoption of different pixel densities here? I'm well aware of SVG not being "the magic bullet" (as in "one size fits all"), but should be easier to customize (or is it?) to acting pixel density on launch/runtime. As it was mentioned in the sister bug 878288, comment 7 even performance shouldn't degrade too much, due to improved runtime caching of SVG assets (bug 764299), but we could always generate and cache these images on (first) launch, if runtime caching turns out to be less performant than ideal. The gains would, then be obvious, not having to re-generate (and store) all the assets for the myriad of different densities. Might this work? I'm not sure what are the exact tweaks needed for specific densities, is it possible that these could be automated/algorithmized?
(In reply to Szmozsánszky István [:flaki] from comment #24) > Wouldn't using an SVG as the primary asset ease the adoption of different > pixel densities here? I'm well aware of SVG not being "the magic bullet" (as > in "one size fits all"), but should be easier to customize (or is it?) to > acting pixel density on launch/runtime. > > As it was mentioned in the sister bug 878288, comment 7 even performance > shouldn't degrade too much, due to improved runtime caching of SVG assets > (bug 764299), but we could always generate and cache these images on (first) > launch, if runtime caching turns out to be less performant than ideal. If I recall correctly we were using SVG for the image and not just the clip-path during on of the iterations. I think it caused a performance hit though. Maybe something to try again with the improvements to SVG, but not at this stage. > The gains would, then be obvious, not having to re-generate (and store) all > the assets for the myriad of different densities. Might this work? I'm not > sure what are the exact tweaks needed for specific densities, is it possible > that these could be automated/algorithmized? If the base SVG image is designed correctly it should just scale correctly. Would have to try it though to be sure because rounding issues and pixel snapping are often issues when trying to scale SVG.
(In reply to Stephen Horlander [:shorlander] from comment #22) > Currently the start and end elements are 31 x 30. This patch changes tab > start and end to 32 x 32. This also even scaling. On OS X it does have a bug > that I can't figure out. Increasing the tab height and the negative margin > on the nav-bar breaks in Customize Mode, with lightweight themes and in > fullscreen. Not sure where we are doing those calculations though. Change 1px to 2px at https://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/browser.css?rev=cc1fdefca201&mark=2773#2759 There will be a new problem with the new customize mode outlines though. I'll run my screenshot tool to try find problems and look at the patch some more.
Blocks: win-hidpi
No longer blocks: 878288
Depends on: 983761
Attached patch MattN WIP v.1 (obsolete) (deleted) — Splinter Review
shorlander: the new Windows curves don't connect as nicely to the nav-bar border (at the bottom of the curve). * Current 1.0x: http://grab.by/vaEA * 32x32 1.25x: http://grab.by/vaEG * 32x32 2.0x: http://grab.by/vaEy On 1.25x it's very noticeable that the highlight on the nav-bar is twice the thickness as the highlight at the bottom of the tab curve. You can see the difference in the images alone. The 1.25x curve goes to the edge of the image whereas it doesn't with @1x and @2x. Stephen, can you fix the new images so that the highlight lines up better on the bottom? It's easiest to see on black themes.
Flags: needinfo?(shorlander)
Attachment #8343383 - Attachment is obsolete: true
Assignee: jmathies → shorlander
Status: NEW → ASSIGNED
Attachment #8386436 - Flags: feedback?(MattN+bmo) → feedback+
Attached patch Folded WIP with new path (obsolete) (deleted) — Splinter Review
Updated images. Just folded all the various WIP patches into one.
Attachment #8386436 - Attachment is obsolete: true
Attachment #8386437 - Attachment is obsolete: true
Flags: needinfo?(shorlander)
Flags: needinfo?(MattN+bmo)
This will be prioritized by the desktop team along with the rest of the Australis follow-up work. The desktop team will not fix this in 29 or 30. As such, dropping the tracking flags.
Flags: needinfo?(MattN+bmo)
Keywords: regression
There was some confusion by a few of us in our earlier conversation about this bug. MattN is on this and it is targeted at 29. Restoring the tracking flags that I removed.
Attachment #8386364 - Attachment is obsolete: true
Attachment #8391774 - Attachment is obsolete: true
Attachment #8397531 - Attachment description: 946987_add-hidpi-tabs-WIP.patch → Folded WIP with new path
Depends on: 990384
No longer blocks: 990218
Attachment #8397531 - Attachment is obsolete: true
Whiteboard: [Australis:P3-] → [Australis:P3]
Matthew, are you still targeting 29 for this bug? Thanks
Flags: needinfo?(MattN+bmo)
(In reply to Sylvestre Ledru [:sylvestre] from comment #34) > Matthew, are you still targeting 29 for this bug? Thanks We're going to make a decision by Monday as to whether we'll do anything for 29. -- This patch definitely makes the stroke crisper although it still makes the connection problem on the bottom of the curve look somewhat worse than the blurry connection IMO. This patch is much lower risk though so if people think this is better than m-c then I'm fine with taking the patch. Screenshots of stock m-c and with this patch applied are here (207 MB zip file): https://drive.google.com/file/d/0B8j8iLTl0K0UOUhwQXU3ekVzenc/edit?usp=sharing
Assignee: shorlander → MattN+bmo
Attachment #8405754 - Flags: ui-review?(shorlander)
Attachment #8405754 - Flags: ui-review?(madhava)
Attachment #8405754 - Flags: ui-review?(dolske)
Flags: needinfo?(MattN+bmo)
OK. Do you know at what time? I am going to launch the go to build for beta 8 Monday. If it does not make it for beta 8, it is going to be too late for 29.
MattN, by any chance are there tryserver builds for the approach 2?
(In reply to Olli Pettay [:smaug] from comment #38) > MattN, by any chance are there tryserver builds for the approach 2? I just happened to push them: https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mozilla@noorenberghe.ca-d5ca06b23110/
(In reply to Sylvestre Ledru [:sylvestre] from comment #37) > OK. Do you know at what time? I am going to launch the go to build for beta > 8 Monday. If it does not make it for beta 8, it is going to be too late for > 29. What time would you like it landed by? Decision makers are in Eastern and Pacific timezones.
Before 11PM PDT would be great!
Comment on attachment 8405754 [details] [diff] [review] Approach 2 - v.1 Downscale @2x images for 1.25dppx and higher with the existing tab size I went thought the pile of screenshots Matt generated for this patch. I found of number of small glitches, but they're all present in the screenshots of m-c today. So this patch is improving the tabs, but I don't see any significant visual regressions. We'll come back post-29 to improve Windows hidpi further.
Attachment #8405754 - Flags: ui-review?(shorlander)
Attachment #8405754 - Flags: ui-review?(madhava)
Attachment #8405754 - Flags: ui-review?(dolske)
Attachment #8405754 - Flags: ui-review+
Comment on attachment 8405754 [details] [diff] [review] Approach 2 - v.1 Downscale @2x images for 1.25dppx and higher with the existing tab size [Approval Request Comment] Bug caused by (feature/regressing bug #): Initial implementation of Australis tabs (bug 738491) User impact if declined: Users with >1dppx Testing completed (on m-c, etc.): Automated screenshot collection reviewed by dolske, myself, and others. Risk to taking this patch (and alternatives if risky): Low risk image addition and copying some CSS from OS X to use the images with 1.25 dppx and higher. String or IDL/UUID changes made by this patch: None
Attachment #8405754 - Flags: review?(mconley)
Attachment #8405754 - Flags: approval-mozilla-beta?
Attachment #8405754 - Flags: approval-mozilla-aurora?
Comment on attachment 8405754 [details] [diff] [review] Approach 2 - v.1 Downscale @2x images for 1.25dppx and higher with the existing tab size Review of attachment 8405754 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine to me. The placement of the block is a little odd - if you wanted to move it closer to the tabs.inc.css include, that might make more sense. Thanks Matt! ::: browser/themes/windows/browser.css @@ +2887,5 @@ > } > > /* End private browsing indicators */ > > +@media (min-resolution: 1.25dppx) { Is there a particular reason why this block is here? If not, it might make more sense to put this nearer to where tabs.inc.css is included.
Attachment #8405754 - Flags: review?(mconley) → review+
(In reply to Mike Conley (:mconley) from comment #44) > Comment on attachment 8405754 [details] [diff] [review] > > Is there a particular reason why this block is here? If not, it might make > more sense to put this nearer to where tabs.inc.css is included. Nope, I think that was just like that from another patch and I didn't notice. Fixed. https://hg.mozilla.org/integration/fx-team/rev/5c1bbf1f4820
Flags: in-testsuite-
Attachment #8405754 - Flags: feedback?
Attachment #8405754 - Flags: approval-mozilla-beta?
Attachment #8405754 - Flags: approval-mozilla-beta+
Attachment #8405754 - Flags: approval-mozilla-aurora?
Attachment #8405754 - Flags: approval-mozilla-aurora+
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Attached image Ugly curved tabs (deleted) —
Running from inbound which includes this patch and I still see ugly curved tabs. My dpi is 120 (125%) on Windows 8.1.1. Built from https://hg.mozilla.org/integration/mozilla-inbound/rev/2e83d29d7f6b Mozilla/5.0 (Windows NT 6.3; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0
btw... I've been using this snippet of css code to make the tabs flow nicely into the border: #nav-bar { border-top: 2px solid hsl(210, 6.1%, 71.1%) !important; background-clip: padding-box;}
The problem I think is the use of images, and this makes the tabs not scale well, can we not achieve the same results by replacing the images with something that will work better, such as a mix of CSS gradients, and a border radius?
(In reply to Matthew Bonner from comment #52) > The problem I think is the use of images, and this makes the tabs not scale > well, can we not achieve the same results by replacing the images with > something that will work better, such as a mix of CSS gradients, and a > border radius? I would be surprised if this kind of curve were doable in pure CSS gradients + border radius without having to have many boxes. Even if it were, we did extensive performance testing on these (and still do automated performance testing on them) and how they affect the animation. I would be (pleasantly) surprised if there were a way we could avoid images and maintain performance.
(In reply to Gary [:streetwolf] from comment #49) > Created attachment 8406126 [details] > Ugly curved tabs That's expected with this patch. Due to time constraints for shipping Firefox 29, we can't make it perfect for all situations. Bug 995733 tracks followup work.
Comment on attachment 8405754 [details] [diff] [review] Approach 2 - v.1 Downscale @2x images for 1.25dppx and higher with the existing tab size Big fingers. Removing the unwanted feedback flag.
Attachment #8405754 - Flags: feedback?
Flags: in-qa-testsuite?
Keywords: verifyme
Tested on a Microsoft Surface Pro 2 device running Windows 8.1 64bit using Firefox 30, build ID: 20140605174243 and Firefox 31 beta 5, build ID: 20140626181429. Marking this issue verified as further work is tracked in bug 995733.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Flags: in-qa-testsuite?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: