-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Conversation
@le717 Every time we update the fonts, we find some subtle differences that need to be fixed. How much testing have you done? Please list any noticeable differences -- both good and bad. |
Will check more later with some images, but off the top of my head I recall |
@redmunds I just looked at the differences, and they are very subtle. The biggest differences are slightly bolder (and at least in the Themes Settings dialog, slightly longer),headers a tad different letter spacing in places, taller I don't suppose there are some tests that rely on the font spacing to pass I can run to ensure nothing breaks? |
@le717 Thanks for the additional info. No, we don't have any tests to run for font changes -- just need lots of people using it. We're don't want to pull in a change this risky for this release (which we hope to be 1.0!), but we'll try to get it in after that. |
That's fine. 1.0 will be big enough as it is. |
This version of the font adds Cyrillic and Greek support. From what I've seen, the lack of adequate support for these locales has been a major source of complaints about Brackets in general from those communities. |
@busykai Unfortunately, Source Sans Pro is not the font used in the editor itself, but in the surrounding areas. But still, we should get this in :) |
@marcelgerber, oops. :) Thank you. Yeah, take it all back, I got confused. |
Yeah, but with this PR landed, the Cyrillic/Greek alphabet would look great in the non-editor area at least ;) |
forgive the odd question: where are the real fonts? The original Source Sans Pro is CFF/OpenType, so using the (lossy converted) .ttf versions, without the woff, eot and otf files (see #10680) misses out on getting the most out of the webfonts. |
@Pomax They're all included in the link at the top of this thread |
indeed, I mostly ask because the PR itself only seems to update only the .ttf files |
That's because that's all Brackets uses, the .ttf files (and yes, I have read that other issue). So when this PR was submitted, the .ttf files were all that needed updating. It seems now, though, this situation may change. |
@@ -30,7 +30,11 @@ | |||
/* SourceSansPro Regular */ | |||
@font-face { | |||
font-family: 'SourceSansPro'; | |||
src: url('fonts/SourceSansPro/SourceSansPro-Regular.ttf'); | |||
src: local('Source Sans Pro Regular'), | |||
url('fonts/SourceSansPro/SourceSansPro-Regular.eot?iefix') format('embedded-opentype'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, it's probably not needed, but I included it because we're still trying to see if method helps #10680.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ideally this goes the way of the dodo - if IE9< is not a concern, strip it as fast as possible =)
It might then also make sense to simply not bother with the .ttf files, since everything that isn't IE happily supports .otf (even Android 4.2! If a little broken, it hates GSUB tables for instance), and the latest IEs support WOFF just fine.
Old IE fonts removed, I would like a say from @peterflynn or someone on the Brackets team before removing the TTF fonts. I do have a question about the WOFF fonts. The Source Sans Pro download contains two subfolders, OTF and TTF, and the fonts inside end in
|
.ttf.woff fonts.otf.woff fontsI didn't realize I scrolled the code area, but it is unaffected by these changes (except for code hints, not shown), only the UI is. EDIT: Looking at it, the .otf.woff fonts look clearer and may fix some of the legibility/font rendering we got when we picked up the new version of CEF. I wonder if we'd get the same effect by switching to that font type of Source Code Pro... |
on a technical note when it comes to fonts that Adobe produces: all Adobe fonts are designed as CFF/OpenType fonts, with cubic Bezier outlines and CFF hinting (very powerful, lots of work). As such, Adobe only "makes" WOFF is a handy web format that can pack any opentype font (CFF or TrueType) as a mostly-gzipped-file, so the WOFF versions you see here are actually the The So where supported, |
@busykai @peterflynn @redmunds Do you think any more testing is required before this PR lands in master? can we think of any other scenarios which might break with this PR? |
@le717 did some testing but didn't specify which OS/Version. Each OS/version supported by Brackets should be checked. |
@redmunds Adding to the platform testing I think we should be testing this on HiDPI systems. |
@peterflynn Thanks for putting up the GIFs! The issue is much clearer now. From this it is very clear that the rendering of text is a little weird with the new fonts. Let me check on Win 7 and see if this is evident there as well. |
it's possible that with the otf.woff versions, the intended rules for minimal stem widths, alignment, and snapping are now actually being followed, leading to seeing "what the font is supposed to look like as intended by Adobe"... but it's probably worth filing on the Source Sans repository to ask what's up with this, too. If this is what they intended, the .ttf conversions might be improved in the future and the same problem will crop up even when upgrading from ttf to ttf. |
@nethip To be clear, I think the visual changes in the .ttf.woff screenshots are totally fine. If it looks similar on Win 7 & Mac, then that version of the font should be fine to merge. @Pomax I suspect it's not the intended appearance, since the aspect ratio of characters like "o" looks distinctly taller than raster examples of the font (e.g. this)... but I guess we'll see what they say in the other bug. |
@peterflynn based on the replies in the other bug, it looks like Adobe's call is to go off of the .otf versions as authoritative |
Sorry to run off and leave everyone hanging like that. I was off this weekend and I've spent the last two weeks preparing for it. I have been reading the messages as they came in, though. I'll try to jump back on this and address any concerns this week. :) |
Added to 1.4 (together with #11156). |
Agreed the font weight is heaver and makes the UI different, but once you use it for a few days, legibility improves beyond what I've seen since using Brackets (that was roughly around Sprint 35). There are a number are issues related to degrading font quality, and while most refer to Source Code Pro (and this PR edits Source Sans Pro), all font rendering quality has degraded because of CEF. That's why I chose the .otf.woff files: because it restores a lot of that missing quality. I did note the .otf.woff causes a font size issue in code hints, but literally that's the only issue I've seen (and I used this PR exclusively for a month of active coding). Everything else only looks better. |
@marcelgerber @peterflynn Since this is tagged for 1.4, what is the final call here, improved legibility or maintaining a consistent look? If the former, we'll have to make a quick fix to the font-size issue in Code Hints, but no trouble there. |
From my side, I was at no point against this change, so it's fine with me. But let's also hear what @peterflynn thinks. |
Moving this to 1.5 as this requires extensive testing. |
A new release of source san-s pro seems to be in the making. I have raised an issue adobe-fonts/source-sans#71 asking for the details. |
If the bold/italic fonts are improved in the new version (they might be already?) this issue could be solved: #11691 |
as a minor remark: this PR's been in the works for a year. If anything, it's probably time to just land it, and then file follow-ups for outstanding issues, or just close it as "yeah we're clearly never doing this". It shouldn't take over a year to decide to update the font family used =) |
But don't merge it yet. Although the .otf.woff fonts provide better legibility, since there was no agreement reached on switching to them, let me reset this to use .ttf fonts. Then you can merge it. :) |
adobe-fonts/source-sans/issues/71 says that their new font will be released this week. Could you check it out? |
The newest release of source sans is live @ https://github.com/adobe-fonts/source-sans-pro/releases |
Source Code Pro also got an update: https://github.com/adobe-fonts/source-code-pro/releases few months ago |
Also add local font-loading to help address in-browser concerns. https://github.com/adobe-fonts/source-sans-pro/releases/tag/2.020R-ro/1.075R-it
PR flattened and updated with latest TTF files. @Pomax I kept the |
Yes, we managed to get the latest source code pro font early(as at the time we helped the font team in beta testing). |
Superseded by #12663. |
For #8343.
TTF files from the latest release, namely, https://github.com/adobe-fonts/source-sans-pro/releases/tag/2.020R-ro/1.075R-it