On HiDPI we use the display font for rendering UI text instead of the text font. This looks bad.

The information about what font we should use for rendering is already contained in the NSFont object that we create in gfxMacPlatformFontList::LookupSystemFont. It would be nice if we could forward that information along somehow, instead of taking our own pick based on the device pixel font size.

This isn't so much the wrong font as it's that because of the way we handle DPI, font tracking isn't applied correctly. This happens also under 10.10, so it's not limited to 10.11. Both the .Helvetica Neue DeskInterface fonts and the new San Francisco font family used for UI contain 'trak' tables which apply tracking (i.e. scaling the advance slightly depending upon the size). So it's not the family selection that's the problem so much as it's the lack of application of the tracking info, since we're selecting one size and scaling it down. Totally half-assed guess as to a solution here but I think if we select the font at the "right" size (i.e. not scaled to DPI), then apply a *font* transform, I think we may be able to work around this. Not sure how we make that work in the code for handling DPI but I'm guessing Jonathan does.

Note that I think this probably means any font containing a 'trak' table also needs to be shaped with CoreText rather than harfbuzz.

Iterate over a range of font sizes for the system UI font, normalizing the size by scaling down the transform on the graphics context.

Default drawing of 8.0pt text in red, with other font sizes scaled to match.

The word 'localhost' in the URL bar under Safari/Nightly under 10.10. Note how much more cramped the Nightly version is.

(In reply to John Daggett (:jtd) from comment #5) > Note that I think this probably means any font containing a 'trak' table > also needs to be shaped with CoreText rather than harfbuzz. Yes, harfbuzz doesn't currently know about 'trak'; it's an AAT-specific table. So we probably want to add this to the heuristics at http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxMacPlatformFontList.mm#181 (Though in practice I wouldn't be surprised if those fonts are already going through CoreText due to presence of a 'morx' table?)

(In reply to Jonathan Kew (:jfkthame) from comment #9) > (Though in practice I wouldn't be surprised if those fonts are already going > through CoreText due to presence of a 'morx' table?) The .Helvetica Neue DeskInterface UI fonts under OSX 10.10 has morx/trak tables but the San Francisco UI family on OSX 10.11 has GSUB/GPOS tables along with bsln/trak tables but no morx table. Hmmm...

I'm not convinced that respecting trak information will solve this problem on 10.11. I'm willing to believe it will on 10.10, though. Experimentally, I set kTextDisplayCrossover to 40.0 in my build, and as a result, fonts in the address bar and in context menus looked pixel-perfect identical to Safari. I think this suggests that we don't need to do anything about font tracking in order to fix the problem on 10.11.

Here's a pretty hacky workaround, which fixes the bug as filed (but not what it says at the moment).

Comment on attachment 8680611 [details] [diff] [review] 10.11 workaround Wait, this doesn't work at all. Not sure what I was seeing.

Comment on attachment 8680611 [details] [diff] [review] 10.11 workaround Review of attachment 8680611 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/src/nsFontMetrics.cpp @@ +139,5 @@ > gfxFontStyle style(aFont.style, > aFont.weight, > aFont.stretch, > gfxFloat(aFont.size) / mP2A, > + gfxFloat(aFont.size) / p2aIgnoreHiDPI, This doesn't look right; I think what you'd want here would be something more like gfxFloat(aFont.size) / mozilla::AppUnitsPerCSSPixel() to get a size that can usefully be compared to kTextDisplayCrossover, ignoring device resolution (and page zoom).

You're right, that was all I was missing.

FWIW, I'm not keen on the idea of adding an extra "size" field to gfxFontStyle just for this; it seems like we should be able to find some other way to make that information available only when it's needed, without having to add it to every gfxFontStyle we use. But I haven't looked at how all this hangs together, to see how feasible that really is... (Also, if we do need a field or variable like this, whether in gfxFontStyle or separately, I'd be inclined to go for a name such as cssPixelSize rather than sizeIgnoringHiDPI. The point is that gfx fonts generally work in terms of device pixel size, but in this particular case it's the resolution-independent CSS px size that you want to compare to the threshold.)

(In reply to Markus Stange [:mstange] from comment #16) > Created attachment 8680710 [details] [diff] [review] > 10.11 workaround Um, does this actually work?!? It will pick up the right family but it's not going to pick up the right tracking I think. For that I think we need to be creating a font that is explicitly the CSS pixel size rather than the highDPI size that will be scaled down via a global transform.

Yes it does. It looks like the 10.11 text font doesn't require tracking to be applied, or doesn't have any tracking information that makes a difference depending on the font size. It won't fix the problem on 10.10 of course.

(In reply to Jonathan Kew (:jfkthame) from comment #17) > FWIW, I'm not keen on the idea of adding an extra "size" field to > gfxFontStyle just for this; it seems like we should be able to find some > other way to make that information available only when it's needed, without > having to add it to every gfxFontStyle we use. But I haven't looked at how > all this hangs together, to see how feasible that really is... I think I agree here. Maybe the best thing would be to push a dev2cssPixel conversion factor down into gfxFontGroup and pass that into FindFamily within gfxFontGroup::AddPlatformFont. I think this will be needed to deal with optically sized families like Sitka on Windows.

Add the ratio of devPixelSize to cssPixelSize into gfxFontGroup. Then pass it into FindFamily when selecting font families within BuildFontList. In the system font case, select the actual family based on the cssPixel size of the font.

Comment on attachment 8688823 [details] [diff] [review] patch, pass through devPixel-to-cssPixel ratio to gfxFontGroup and FindFamily Review of attachment 8688823 [details] [diff] [review]: ----------------------------------------------------------------- I suppose this is OK. It seems kinda messy, but given that I didn't like adding it to the gfxFontStyle either, I don't have a better idea for now.