850918 - Handle 200% DPI for Australis tabs

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Matthew N. [:MattN]]

Attachment: Screenshot with 200% font size on Windows 7

(Quoting Dão Gottwald [:dao] from bug 738491 comment #140)
> Comment on attachment 722700 [details] [diff] [review]
> The handling of increased font sizes seems to have regressed here. Can you
> make sure to stretch the selected tab's images vertically? (This appears to
> be already working for background tabs.) Also, increasing font size seems to
> stretch the new tab button some but not enough, leaving a gap at the bottom.

Comment 1

[Matthew N. [:MattN]]

To get to 200% DPI in Win7:
Right-click on desktop > Screen resolution > Set custom text size (DPI) [in right nav.] > 200% > Apply > Log off

To change font sizes on Win7 (without logging off):
Right-click on desktop > Personalization > Window Color > Advanced appearance settings… and you will get the old customization dialog where you can change the dialog text to 20pt (for example)

Comment 2

[Matthew N. [:MattN]]

Some problems to look out for:
1) Selected tab components (start, middle, & end) are scaled 
2) Background tab components are scaled
3) New tab button height matches the selected and background tabs
4) New tab button image is centred.

Comment 3

[:Gijs (he/him)]

(In reply to Matthew N. [:MattN] from comment #1)
> To get to 200% DPI in Win7:
> Right-click on desktop > Screen resolution > Set custom text size (DPI) [in
> right nav.] > 200% > Apply > Log off
> 
> To change font sizes on Win7 (without logging off):
> Right-click on desktop > Personalization > Window Color > Advanced
> appearance settings… and you will get the old customization dialog where you
> can change the dialog text to 20pt (for example)

FWIW, if I use the first method for 200% DPI, everything works just fine.

With the second method, I get something akin to the screenshot posted. Looking into this.

Comment 4

[:Gijs (he/him)]

Looks like this is due to the paths which we use to clip the selected tab's background-fill. These are defined using pixels and SVG's userSpaceOnUse unit definition. I can look at converting this but I'm not sure how easy it'll be.

Comment 5

[:Gijs (he/him)]

Attachment: Patch

This sets a background-size for the middle tab, and adjusts the clip paths used to draw the background of the selected tab.

The new tab button's hover effect gets cut off on the bottom if you increase the font-size. This is to do with the clip path of that button, but I'm tweaking that in bug 823237 anyway and I'd like to do it just once. I'll check the image centering when that's done, too. :-)

Comment 6

[Matthew N. [:MattN]]

Comment on attachment 743678 [details] [diff] [review]
Patch

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

::: browser/base/content/browser.xul
@@ +1196,5 @@
>    </vbox>
>  
>    <svg:svg height="0">
> +    <svg:clipPath id="tab-curve-clip-path-start" clipPathUnits="objectBoundingBox">
> +      <svg:path d="M 0.9677,0.0645 C 0.5499,0.0796 0.53145,0.3998 0.4876,0.5484 0.4371,0.8186 0.3809,0.9912 0.0487,0.9924 L 0,1 l 1.03,0 0,-0.9355 z"/>

The width of the clip-path is 30px while the height is 31px.  It seems like you used 31px as the width since the first x-coordinate should be 1 (=30/30).

Comment 7

[Matthew N. [:MattN]]

(Quoting Matthew N. [:MattN] from comment #6)
> The width of the clip-path is 30px while the height is 31px.  It seems like
> you used 31px as the width since the first x-coordinate should be 1 (=30/30).

To be clear, the start and end should be 30px wide, not 31px.

Comment 8

[:Gijs (he/him)]

(In reply to Matthew N. [:MattN] from comment #7)
> (Quoting Matthew N. [:MattN] from comment #6)
> > The width of the clip-path is 30px while the height is 31px.  It seems like
> > you used 31px as the width since the first x-coordinate should be 1 (=30/30).
> 
> To be clear, the start and end should be 30px wide, not 31px.

They are both de facto 31px at the moment. The H 31 instruction draws a horizontal line 31 pixels to the right.

Comment 9

[:Gijs (he/him)]

Attachment: Patch

Use 30px for the width instead.

This still needed some tweaking at the end. I'm not entirely sure why. In any case, here's an easy comparison:

http://jsbin.com/imahep/4/edit

The last control point of the curve (-0.30,0.44) was originally (0.4333,0.4194), id est (-13,13) divided by 30 and 31 respectively. Without this change, the fill overlays the stroke on the bottom extreme edges of the tab.

I'll post an updated screenshot at 200% DPI in a second, and verify linux straight after.

Comment 10

[:Gijs (he/him)]

Attachment: Screenshot at 200% DPI

The extra margin above the tabs is because the app button becomes too big. I didn't fix that because we're getting rid of the app button anyway.

Comment 11

[:Gijs (he/him)]

Testing on linux, everything looks fine there too, AFAICT.

Comment 12

[Matthew N. [:MattN]]

Comment on attachment 744082 [details] [diff] [review]
Patch

r+ with that last point put back to its scaled value. That is a separate issue that requires shifting the whole tab down 1 px so it overlaps 2px along with fixing the height and is being addressed in bug 858089.

Comment 13

[:Gijs (he/him)]

(In reply to Matthew N. [:MattN] from comment #12)
> Comment on attachment 744082 [details] [diff] [review]
> Patch
> 
> r+ with that last point put back to its scaled value. That is a separate
> issue that requires shifting the whole tab down 1 px so it overlaps 2px
> along with fixing the height and is being addressed in bug 858089.

OK. However, my comment missed a - sign and was unclear. The curves are symmetric, and so this point on the left one should have a negative horizontal coordinate. I've gone ahead and made this change as I think it's obvious this is what we meant to do here, both from looking at the context, and at my patch, and at the history (ie the user-space path that was there before). :-)

http://hg.mozilla.org/projects/ux/rev/a74ed4b3c5ba

Comment 14

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/7208d3921d65
https://hg.mozilla.org/mozilla-central/rev/a74ed4b3c5ba