Closed Bug 1224732 Opened 9 years ago Closed 8 years ago

[HiDPI] tab separators are sometimes too thick

Categories

(Firefox :: Theme, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 51
Tracking Status
firefox51 --- verified

People

(Reporter: arni2033, Assigned: dao)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

>>> Info: Win7_64, Nightly 45, 32bit, ID 20151113030248, new profile <<< STR_1: 1. Set DPI -> 120% : A) Set that DPI level -> 120% in your OS [OR] B) Set layout.css.devPixelsPerPx -> 1.2 2. Open new firefox window, open 6 tabs in it. 3. Pin the first 5 tabs, click the 6th tab (to make it active). STR_2: 1. Set DPI -> 125% : A) Set that DPI level -> 125% in your OS [OR] B) Set layout.css.devPixelsPerPx -> 1.25 2. Open new firefox window, open 6 tabs in it. 3. Pin the first 5 tabs, drag the last pinned tab to the beginning of tabs toolbar. 4. Wait 1-2 seconds. Result: [screenshot 1] Pinned tab separators are sometimes too thick. Bug 1189212 isn't fixed Expectations: Pinned tab separators should have normal width Note: How could one _EVER_ hope to fix that on all possible HiDPI levels by changing border-width?! The root of the problem is wrong border scaling on zoom / transform:scale() (see bug 477157 / bug 890383). Well, there IS a way to avoid fixing that bug: the devs could handle all possible DPI levels separately, setting appropriate border-width in each case. But that sounds really bad.
By the way, non-pinned tabs expose the same issue. ~Many thanks for supporting DPI in firefox UI~
I don't think these are implemented with border-width, are they? The patch in bug 1189212 made them be width: 1px pseudo-elements, and I think that is still the case. Am I missing something?
Oh, you're totally right, I'm sorry. Those borders arent "border-width"s anymore. But I still think that borders scaling is involved since #toolbar-menubar, #TabsToolbar, #nav-bar and #PersonalToolbar have width == 1138.33px on DPI == 120%. Screenshot: http://ssmaker.ru/cc3dd23f.png It's most likely caused by border-width somewhere.
(In reply to arni2033 from comment #3) > Oh, you're totally right, I'm sorry. Those borders arent "border-width"s > anymore. > But I still think that borders scaling is involved since #toolbar-menubar, > #TabsToolbar, #nav-bar and #PersonalToolbar have width == 1138.33px on DPI > == 120%. Screenshot: http://ssmaker.ru/cc3dd23f.png I'm confused. The 1138.33px value maps the device pixels via the 1.2 DPI, which means that the total device pixel size was 1138.3333 * 1.2 = 1366 device pixels, which almost precisely matches the size of your screenshot. Seems expected to me? Obviously that doesn't fix the width of these separators. dholbert, can you help elucidate what is going on here and what we could do to fix? If nothing else, I'm tempted to create a CSS variable set to "1px" and then set it to 1 / DPI from JS, considering we don't have device pixel units in CSS. That might do a better job of guaranteeing the right size.
Flags: needinfo?(dholbert)
Despite a "width: 1px" declaration, an element can actually end up painting a 2px-wide background if it's positioned on a fractional device-pixel-value, due to pixel-snapping. I think this is basically how it's supposed to work, though I forget the details, so it's possible this might be indicative of a bug somehow. See this testcase, which demonstrates this happening (no need to adjust any prefs) -- a few of the black bars here are wider than the rest. (and if you apply full-page zoom, it changes which ones are wider, since the zoom-factor impacts the device-pixel position of these elements).
Flags: needinfo?(dholbert)
Sorry, disregard my "no need to adjust any prefs" in prev. comment -- you do need to adjust prefs and/or apply some full-page zoom to see thicker bars with that testcase.
Expanding on how pixel-snapping comes into play here: - The separator here is 1 CSS pixel wide, so it's really 1.2 device pixels wide, if you've set layout.css.devPixelsPerPx to 1.2 (per STR here. - Suppose you have a separator element that's positioned such that its left edge is at e.g. device-pixel-position 0.4px. So, its right edge is at device-pixel-position 1.6px. - We can't start drawing from device-pixel-position 0.4px, so we pixel-snap -- we round each edge to the nearest pixel boundary. In this case, that means the left edge gets rounded down to position 0.0, and the right edge gets rounded up to position 2.0. - So, this separator is drawn as being 2px wide. In contrast, if it were positioned at a device-pixel position of e.g. 0.0 or 0.1 or 0.9, then both edges would get rounded the same way. But for intermediate fractional values, we round the left & right edges in opposite directions. To avoid this problem, our frontend code needs to avoid positioning these elements at fractional device-pixel positions (hard, probably), or make sure these separator-elements are a whole number of device-pixels wide (which it sounds like Gijs is already considering per comment 4).
Attachment #8688168 - Attachment description: testcase with 1px-wide element & various margin-left values → testcase with 1px-wide element & various margin-left values (view with full-page zoom or custom layout.css.devPixelsPerPx) (demonstrates pixel-snapping making some bars wider)
Flags: needinfo?(gijskruitbosch+bugs)
Has STR: --- → yes
Blocks: win-hidpi
Attached patch patch (deleted) — Splinter Review
same fix as for bug 1162441
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Comment on attachment 8787777 [details] [diff] [review] patch Review of attachment 8787777 [details] [diff] [review]: ----------------------------------------------------------------- I literally was writing this exact patch just now.
Attachment #8787777 - Flags: review+
Flags: needinfo?(gijskruitbosch+bugs)
Actually, sorry, that's a lie, I picked border-right. Not like it makes a difference, though...
Summary: [HiDPI] Pinned tab separators are sometimes too thick → [HiDPI] tab separators are sometimes too thick
Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b4dd17b3ddcf Draw tab separators with a border rather than background-image. r=gijs
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Depends on: 1300410
Depends on: 1300734
I have reproduced this bug with Nightly 45.0a1 (2015-11-13) on Windows 8.1 (64 Bit!). This bug's fix is verified on Latest Nightly 51.0a1. Build ID : 20160908030434 User Agent : Mozilla/5.0 (Windows NT 6.3; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0 [bugday-20160907]
Thanks!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: