Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Update to latest Source Sans Pro #8985

Closed
wants to merge 1 commit into from
Closed

Update to latest Source Sans Pro #8985

wants to merge 1 commit into from

Conversation

le717
Copy link
Contributor

@le717 le717 commented Sep 5, 2014

@redmunds
Copy link
Contributor

@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.

@le717
Copy link
Contributor Author

le717 commented Sep 24, 2014

Will check more later with some images, but off the top of my head I recall tgetting shorter, the dot in i moving vertically, a shorter stem on a, and better looking bold text in dialog titles.

@le717
Copy link
Contributor Author

le717 commented Sep 30, 2014

@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 ts (I was wrong previously), and the dots on is and js being pulled closer to the letter. I've posted my review images and grayscale-diffs here for you to review. If for anything, the reasons cited in the issue (Greek and Cyrillic fixes, and possible help with Russian translation) should be enough to update them.

I don't suppose there are some tests that rely on the font spacing to pass I can run to ensure nothing breaks?

@redmunds
Copy link
Contributor

@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.

@le717
Copy link
Contributor Author

le717 commented Oct 14, 2014

That's fine. 1.0 will be big enough as it is.

@busykai
Copy link
Contributor

busykai commented Jan 3, 2015

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.

@marcelgerber
Copy link
Contributor

@busykai Unfortunately, Source Sans Pro is not the font used in the editor itself, but in the surrounding areas.
Source Code Pro is the font being used in the editor and apparently, even the latest version doesn't support the Cyrillic/Greek alphabet.

But still, we should get this in :)

@busykai
Copy link
Contributor

busykai commented Jan 3, 2015

@marcelgerber, oops. :) Thank you. Yeah, take it all back, I got confused.

@marcelgerber
Copy link
Contributor

Yeah, but with this PR landed, the Cyrillic/Greek alphabet would look great in the non-editor area at least ;)

@Pomax
Copy link

Pomax commented Mar 3, 2015

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.

@peterflynn
Copy link
Member

@Pomax They're all included in the link at the top of this thread

@Pomax
Copy link

Pomax commented Mar 4, 2015

indeed, I mostly ask because the PR itself only seems to update only the .ttf files

@le717
Copy link
Contributor Author

le717 commented Mar 4, 2015

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'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we actually need this, AFAICT this only targets IE<9, and as Brackets' in-browser variant is thought to be for modern browsers only (at least that's what I know), I don't think we need this line.

cc @humphd @Pomax

Copy link
Contributor Author

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.

Copy link

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.

@le717
Copy link
Contributor Author

le717 commented Mar 5, 2015

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|otf.woff, and I am not sure which set I am supposed to use. I've used the .ttf.woff files in this PR as the .otf.woff files are largely different. I'll try to get some screenshots.

  • TODO: Perform these same changes for Source Code Pro

@le717
Copy link
Contributor Author

le717 commented Mar 5, 2015

.ttf.woff fonts

ttf-woff-fonts

.otf.woff fonts

otf-woff-fonts

I 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...

@Pomax
Copy link

Pomax commented Mar 6, 2015

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" .otf fonts, never .ttf fonts.

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 .otf versions, wrapped.

The .ttf versions in the packs are included purely for compatibility reasons (older microsoft engines will only take ttf/eot) but are to be avoided whenever possible: TrueType conversions are always lossy, and have less accurate outlines that CFF/OpenType fonts (cubic Bezier curves have to be approximated with quadratic curves instead), as well as much less hinting (TrueType has nowhere near as detailed a mechanism for hinting as CFF does).

So where supported, otf (including WOFF wrapped otf) will look vastly better. Thankfully, that's pretty much "everywhere" these days.

@nethip
Copy link
Contributor

nethip commented Mar 11, 2015

@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?

@redmunds
Copy link
Contributor

@le717 did some testing but didn't specify which OS/Version. Each OS/version supported by Brackets should be checked.

@nethip
Copy link
Contributor

nethip commented Mar 13, 2015

@redmunds Adding to the platform testing I think we should be testing this on HiDPI systems.

@peterflynn
Copy link
Member

The latest version of this PR (using .otf.woff) differs much more from master than the earlier version did (using .ttf.woff). And I don't think it's a difference for the better: characters look vertically stretched in the .otf.woff version, like they have the wrong aspect ratio (and the font weight also seems heavier).

Here's an animation comparing master to .ttf.woff:
ttf-comparison

Here's master compared to .otf.woff:
otf-comparison

The first change seems much less drastic.

Note: I tested this on Windows 8. We should do similar comparisons on Windows 7 and Mac OS to make sure it looks acceptable there too.

@nethip
Copy link
Contributor

nethip commented Mar 16, 2015

@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.

@Pomax
Copy link

Pomax commented Mar 16, 2015

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.

@peterflynn
Copy link
Member

@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.

@Pomax
Copy link

Pomax commented Mar 17, 2015

@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

@le717
Copy link
Contributor Author

le717 commented Mar 30, 2015

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. :)

@marcelgerber
Copy link
Contributor

Added to 1.4 (together with #11156).

@le717
Copy link
Contributor Author

le717 commented May 25, 2015

The taller proportions of the .otf.woff versions definitely match the existing Mac 1.2 appearance much more closely than the Win 1.2 appearance, but the weight is still significantly heaver than any shipping Brackets build today:

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.

@le717
Copy link
Contributor Author

le717 commented Jun 12, 2015

@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.

@marcelgerber
Copy link
Contributor

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.

@nethip
Copy link
Contributor

nethip commented Jun 25, 2015

Moving this to 1.5 as this requires extensive testing.

@nethip nethip modified the milestones: Release 1.5, Release 1.4 Jun 25, 2015
@abose
Copy link
Contributor

abose commented Sep 11, 2015

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 a newer relase is there, it would be better to pick that up.
Pushing this change to next release due to this.

@abose abose modified the milestones: Release 1.6, Release 1.5 Sep 11, 2015
@petetnt
Copy link
Collaborator

petetnt commented Sep 14, 2015

If the bold/italic fonts are improved in the new version (they might be already?) this issue could be solved: #11691

@Pomax
Copy link

Pomax commented Sep 14, 2015

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 =)

@le717
Copy link
Contributor Author

le717 commented Sep 14, 2015

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. :)

@abose
Copy link
Contributor

abose commented Sep 15, 2015

adobe-fonts/source-sans/issues/71 says that their new font will be released this week. Could you check it out?

@abose
Copy link
Contributor

abose commented Sep 16, 2015

The newest release of source sans is live @ https://github.com/adobe-fonts/source-sans-pro/releases

@petetnt
Copy link
Collaborator

petetnt commented Sep 16, 2015

Source Code Pro also got an update: https://github.com/adobe-fonts/source-code-pro/releases few months ago

@le717
Copy link
Contributor Author

le717 commented Sep 18, 2015

@abose Will pull in the latest fonts.

@petetnt Funny, we just updated those in #11301, but that was merged on June 30 but the newest release was July 15. @abose Is that the same release you added (but managed to get them early) or am I missing something?

@le717
Copy link
Contributor Author

le717 commented Sep 18, 2015

PR flattened and updated with latest TTF files.

@Pomax I kept the local() calls to help with Brackets in-browser. I still kinda wish the .otf.woff could have been used (at least on Windows they provide better text clarity), but so that happens. :(

@marcelgerber
Copy link
Contributor

@petetnt Funny, we just updated those in #11301, but that was merged on June 30 but the newest release was July 15. @abose Is that the same release you added (but managed to get them early) or am I missing something?

Yes, this is the version we now use.

@abose
Copy link
Contributor

abose commented Sep 19, 2015

Yes, we managed to get the latest source code pro font early(as at the time we helped the font team in beta testing).
But we did not get source sans fonts at the time.

@marcelgerber
Copy link
Contributor

Superseded by #12663.
Closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants