New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Reddit Sans #6812
Comments
Thanks @stephenhutchings. I briefly looked at all the source files and everything looks good. I tagged this for review for onboarding to Google Fonts in Q4. We will review this submission in more detail soon. |
Hello @stephenhutchings. This project has been accepted for onboarding to Google Fonts and we are moving forward with the onboarding process this week with Reddit Sans as a high priority project. I left some issues in the project's issue tracker. These are things that have come up in the QA process and need to be fixed before onboarding the fonts. If for some reason these changes don't make sense in the upstream project's repository, Google Fonts can maintain a separate fork that meets all our QA requirements, but Ideally the project at github.com/reddit/redditsans would be as close to the version on Google Fonts as possible. The main, most important issues that need to be addressed before onboarding are:
Also, do you have plans to release this as a variable font at some point? If so, would it make sense to combine Reddit Sans and Reddit Sans Condensed into one variable font with a weight and width axis? I'm working on an updated version of the source files that makes the vertical metrics consistent across the family now, and can make a PR to your upstream repository with that file if you like, but I understand you will probably want to review the vertical metrics issue yourself and might have some opinions on how it is handled. Is there a good reason why the vertical metrics are now consistent across the family? Let me know. Thanks! |
Thanks @eliheuer. Appreciate the detailed comments. It won't be a problem addressing all of the issues you have raised in the main project, so there is no need to maintain a separate fork. I should be able to resolve all of those issues this week. With respect to inconsistent vertical metrics, that seems to be an oversight rather than intentional. A variable font is a possibility with separate width and weight axes. However, there are currently no italics in the condensed width so it would be limited to the roman fonts. Nothing is planned at the moment, but I'll discuss this further with the Reddit team. If we do go down that route, what effect does that have on the Google Fonts side of things? |
I've released a new version (v1.012) that addresses the bulk of issues that you've highlighted. There are two remaining issues related to copyright. If these are non-critical, Reddit's preference is to leave the copyright as is. |
We merged a first version to see what it looks like in sandbox and speed up the onboarding process of the font, but it is not complying with our requirements so we would be waiting for an update :) We would need:
|
Thanks for the update @RosaWagner. If @eliheuer is happy to set up the gftools build step that would be much appreciated. I can make the other changes to the repo structure next week. |
Hey @RosaWagner @eliheuer ! Following up here. Stephen, our font designer made the necessary updates to Reddit Sans (v1.012) outlined here. Question regarding this copyright convo. We'd like to keep the copyright attribution as "Reddit, Inc.". We have a CLA with Google in place. I wanted to check in and make sure this suffices here? I'll note that we will be updating the copyright URL to link out to the Reddit Sans repo, per your team's suggestion. Other than these items, is there anything else needed from us to move forward? TY! |
Checking in here! Is there anything else needed from our team here? Thank you! @RosaWagner @eliheuer |
@tr000y Hello, I will look into the copyright issue. The main problem is that the fonts need to be built with the Google Fonts build system described here: https://googlefonts.github.io/gf-guide/build.html But the fonts currently use I will try to get a fork working with the Google Fonts build system as an example if possible. |
@eliheuer I made a bit of progress in the build step on the gf-onboarding branch. This required some changes to the source files, and I need to do a review to ensure the built fonts don't vary significantly from the current production fonts. I'm not sure it's possible to get the build working without changing the source files, but if you have any insight here that would be great. |
@eliheuer, as told in the meetings: we don't need to support the
|
I don't think @eliheuer handled a case of building multiple families in one repo with intermediate layers etc before, so I allowed myself to step in to show a way to handle that case. I arranged the sources in a fork to be able to build with gftools builder, but I didn't go into details for the QA. Also, I set up an "ideal" scenario which might differ from the "desired" output (explanations below). link: https://github.com/RosaWagner/redditsans/tree/gf-onboarding What I did in the sources
What to do about: the range of the weight axis is larger than the proposed set of instances.
copyright The blocker for this project Here again there are several possibilities:
random stuff I noted but didn't fix
@stephenhutchings, once you've chose you preferred scenarios, @eliheuer can take over. |
@RosaWagner @eliheuer Thanks so much for outlining all of this! Our preference for 1. range of weight axis and 2. the glyphsLib issue is option two in both scenarios. For both issues, is this something GF would handle, or will we need to update on our end? |
Thanks @tr000y, to make it clear you choose:
We can provide the bash script that exports the range you prefer, but you would have to add the intermediate layers everywhere (it might require a bit of scripting so it doesn’t take ages). Do you want to do that from the sources I already set up? |
I made the bash script and created a PR to @eliheuer's fork so he can take over (eliheuer/redditsans#1). If you feel the emergency to fix the rest of the todo listed in the PR, I can also PR the current state of the branch to your branch. Edit: issue with ots sanitizer filled in fonttools/fonttools#3350 |
@RosaWagner please file an issue in fonttools about the instancer and will take a look, thanks |
Marc is Mister STAT Table. :-) |
cc @m4rc1e, but I think he is on vacation until the end of the year, I can do my best to look into it in his absence. |
It can be workaround with patching a stat table without width axis with gen-stat at the end of the process using the bash script already there |
I'm currently looking into this issue. Will report back or fix. |
The STAT table for the Condensed family is being generated by the googlefonts axisregistry. I need to do some investigating to see whether it is complaint with the MS spec. For the time being, we could just declare a STAT table in the sanscondensed.yaml yaml file? I'll happily do this.
The Condensed Axis Record is being added because it found a Condensed token in the name table. It most likely used nameID 1 or 16. Perhaps we should only be discovering name record token using NameID 2 or 17. |
@m4rc1e except if I missed something, when I tested that, it still added the width axis. I explained more in details the problem in this issue: googlefonts/gftools#801. But I tested a lot of stuff recently with different projects, so I might have confused something, does it work when you do it? |
Defining a stat table in the condensed yaml file works for me. I don't get a Condesed axis. PR submitted, reddit/redditsans#13 |
Hello @tr000y and @stephenhutchings! Here you can find my review for Reddit Sans (I'll start the Reddit Mono today). Feel free to ask if you have any questions! / Global comment Version: In the version exported I checked, it’s the 1.012, should be bumped to 1.014 (to avoid any confusion with that PR: #6850 and with the version on the main branch of your repo -which is not the last one if I well understood-) / Fontbakery report remarks Fails: What can be improved to remove WARNS:
(To check in every fonts)
Check math signs have the same width Italic: same remarks + weird behaviour with the Caron (looks a bit far in the regular master) Screen.Recording.2024-01-19.at.14.41.48.movFontBakery reportfontbakery version: 0.10.9 [2] Experimental checks⚠ WARN: Shapes languages in all GF glyphsets. (com.google.fonts/check/glyphsets/shape_languages)
[code: warning-language-shaping] ⚠ WARN: Shapes languages in all GF glyphsets. (com.google.fonts/check/glyphsets/shape_languages)
[code: warning-language-shaping] [1] Family checksℹ INFO: Check axis ordering on the STAT table. (com.google.fonts/check/STAT/axis_order)
[20] RedditSans-Italic[wght].ttf🔥 FAIL: Copyright notices match canonical pattern in fonts (com.google.fonts/check/font_copyright)
🔥 FAIL: Check variable font instances (com.google.fonts/check/fvar_instances)
⚠ WARN: Checking OS/2 achVendID. (com.google.fonts/check/vendor_id)
⚠ WARN: Check for codepoints not covered by METADATA subsets. (com.google.fonts/check/metadata/unreachable_subsetting)
Use -F or --full-lists to disable shortening of long lists. Or you can add the above codepoints to one of the subsets supported by the font: ⚠ WARN: License URL matches License text on name table? (com.google.fonts/check/name/license_url)
⚠ WARN: Are there caret positions declared for every ligature? (com.google.fonts/check/ligature_carets)
⚠ WARN: Is there kerning info for non-ligated sequences? (com.google.fonts/check/kerning_for_non_ligated_sequences)
⚠ WARN: Ensure fonts have ScriptLangTags declared on the 'meta' table. (com.google.fonts/check/meta/script_lang_tags)
⚠ WARN: Check font contains no unreachable glyphs (com.google.fonts/check/unreachable_glyphs)
Use -F or --full-lists to disable shortening of long lists. ⚠ WARN: Does the font contain a soft hyphen? (com.google.fonts/check/soft_hyphen)
⚠ WARN: Check math signs have the same width. (com.google.fonts/check/math_signs_width)
Width = 1044: Width = 1051: Width = 1090: Width = 1056: Width = 1014: Width = 1042: Width = 1137: Width = 1016: ⚠ WARN: Are there any misaligned on-curve points? (com.google.fonts/check/outline_alignment_miss)
Use -F or --full-lists to disable shortening of long lists. [code: found-misalignments] ⚠ WARN: Ensure soft_dotted characters lose their dot when combined with marks that replace the dot. (com.google.fonts/check/soft_dotted)
The dot of soft dotted characters should disappear in other cases, for example: į̆ į̇ į̈ į̉ į̊ į̋ į̏ į̑ į̒ į̛̀ į̛́ į̛̂ į̛̃ į̛̄ į̛̆ į̛̇ į̛̈ į̛̉ į̛̊ į̛̋ Your font fully covers the following languages that require the soft-dotted feature: Ebira (Latn, 2,200,000 speakers), Ma’di (Latn, 584,000 speakers), Dutch (Latn, 31,709,104 speakers), Lithuanian (Latn, 2,357,094 speakers), Igbo (Latn, 27,823,640 speakers), Navajo (Latn, 166,319 speakers). Your font does not cover the following languages that require the soft-dotted feature: Basaa (Latn, 332,940 speakers), Avokaya (Latn, 100,000 speakers), Ejagham (Latn, 120,000 speakers), Koonzime (Latn, 40,000 speakers), Ukrainian (Cyrl, 29,273,587 speakers), Aghem (Latn, 38,843 speakers), Kom (Latn, 360,685 speakers), Dan (Latn, 1,099,244 speakers), Nateni (Latn, 100,000 speakers), Lugbara (Latn, 2,200,000 speakers), Belarusian (Cyrl, 10,064,517 speakers). [code: soft-dotted] ℹ INFO: Show hinting filesize impact. (com.google.fonts/check/hinting_impact)
ℹ INFO: Font has old ttfautohint applied? (com.google.fonts/check/old_ttfautohint)
ℹ INFO: EPAR table present in font? (com.google.fonts/check/epar)
ℹ INFO: Is the Grid-fitting and Scan-conversion Procedure ('gasp') table set to optimize rendering? (com.google.fonts/check/gasp)
PPM <= 65535: ℹ INFO: Check for font-v versioning. (com.google.fonts/check/fontv)
ℹ INFO: Font contains all required tables? (com.google.fonts/check/required_tables)
ℹ INFO: List all superfamily filepaths (com.google.fonts/check/superfamily/list)
[21] RedditSans[wght].ttf🔥 FAIL: Copyright notices match canonical pattern in fonts (com.google.fonts/check/font_copyright)
🔥 FAIL: Check variable font instances (com.google.fonts/check/fvar_instances)
⚠ WARN: Checking OS/2 achVendID. (com.google.fonts/check/vendor_id)
⚠ WARN: Check for codepoints not covered by METADATA subsets. (com.google.fonts/check/metadata/unreachable_subsetting)
Use -F or --full-lists to disable shortening of long lists. Or you can add the above codepoints to one of the subsets supported by the font: ⚠ WARN: License URL matches License text on name table? (com.google.fonts/check/name/license_url)
⚠ WARN: Are there caret positions declared for every ligature? (com.google.fonts/check/ligature_carets)
⚠ WARN: Is there kerning info for non-ligated sequences? (com.google.fonts/check/kerning_for_non_ligated_sequences)
⚠ WARN: Ensure fonts have ScriptLangTags declared on the 'meta' table. (com.google.fonts/check/meta/script_lang_tags)
⚠ WARN: Check font contains no unreachable glyphs (com.google.fonts/check/unreachable_glyphs)
Use -F or --full-lists to disable shortening of long lists. ⚠ WARN: Does the font contain a soft hyphen? (com.google.fonts/check/soft_hyphen)
⚠ WARN: Detect any interpolation issues in the font. (com.google.fonts/check/interpolation_issues)
⚠ WARN: Check math signs have the same width. (com.google.fonts/check/math_signs_width)
Width = 1052: Width = 1010: Width = 984: Width = 1098: Width = 992: ⚠ WARN: Are there any misaligned on-curve points? (com.google.fonts/check/outline_alignment_miss)
Use -F or --full-lists to disable shortening of long lists. [code: found-misalignments] ⚠ WARN: Ensure soft_dotted characters lose their dot when combined with marks that replace the dot. (com.google.fonts/check/soft_dotted)
The dot of soft dotted characters should disappear in other cases, for example: į̆ į̇ į̈ į̉ į̊ į̋ į̏ į̑ į̒ į̛̀ į̛́ į̛̂ į̛̃ į̛̄ į̛̆ į̛̇ į̛̈ į̛̉ į̛̊ į̛̋ Your font fully covers the following languages that require the soft-dotted feature: Ebira (Latn, 2,200,000 speakers), Ma’di (Latn, 584,000 speakers), Dutch (Latn, 31,709,104 speakers), Lithuanian (Latn, 2,357,094 speakers), Igbo (Latn, 27,823,640 speakers), Navajo (Latn, 166,319 speakers). Your font does not cover the following languages that require the soft-dotted feature: Basaa (Latn, 332,940 speakers), Avokaya (Latn, 100,000 speakers), Ejagham (Latn, 120,000 speakers), Koonzime (Latn, 40,000 speakers), Ukrainian (Cyrl, 29,273,587 speakers), Aghem (Latn, 38,843 speakers), Kom (Latn, 360,685 speakers), Dan (Latn, 1,099,244 speakers), Nateni (Latn, 100,000 speakers), Lugbara (Latn, 2,200,000 speakers), Belarusian (Cyrl, 10,064,517 speakers). [code: soft-dotted] ℹ INFO: Show hinting filesize impact. (com.google.fonts/check/hinting_impact)
ℹ INFO: Font has old ttfautohint applied? (com.google.fonts/check/old_ttfautohint)
ℹ INFO: EPAR table present in font? (com.google.fonts/check/epar)
ℹ INFO: Is the Grid-fitting and Scan-conversion Procedure ('gasp') table set to optimize rendering? (com.google.fonts/check/gasp)
PPM <= 65535: ℹ INFO: Check for font-v versioning. (com.google.fonts/check/fontv)
ℹ INFO: Font contains all required tables? (com.google.fonts/check/required_tables)
ℹ INFO: List all superfamily filepaths (com.google.fonts/check/superfamily/list)
Summary
Note: The following loglevels were omitted in this report:
|
For Reddit Mono: -> Same remarks as for Reddit Sans for license url and soft hyphen circumflex doesn't look well aligned Screen.Recording.2024-01-19.at.16.00.13.movThe y's dot is a bit too much on the right: FontBakery reportfontbakery version: 0.10.9 [1] Experimental checks⚠ WARN: Shapes languages in all GF glyphsets. (com.google.fonts/check/glyphsets/shape_languages)
[code: warning-language-shaping] [1] Family checksℹ INFO: Check axis ordering on the STAT table. (com.google.fonts/check/STAT/axis_order)
[18] RedditMono[wght].ttf🔥 FAIL: Copyright notices match canonical pattern in fonts (com.google.fonts/check/font_copyright)
🔥 FAIL: Check variable font instances (com.google.fonts/check/fvar_instances)
⚠ WARN: Checking OS/2 achVendID. (com.google.fonts/check/vendor_id)
⚠ WARN: Check for codepoints not covered by METADATA subsets. (com.google.fonts/check/metadata/unreachable_subsetting)
Use -F or --full-lists to disable shortening of long lists. Or you can add the above codepoints to one of the subsets supported by the font: ⚠ WARN: License URL matches License text on name table? (com.google.fonts/check/name/license_url)
⚠ WARN: Ensure fonts have ScriptLangTags declared on the 'meta' table. (com.google.fonts/check/meta/script_lang_tags)
⚠ WARN: Check font contains no unreachable glyphs (com.google.fonts/check/unreachable_glyphs)
Use -F or --full-lists to disable shortening of long lists. ⚠ WARN: Does the font contain a soft hyphen? (com.google.fonts/check/soft_hyphen)
⚠ WARN: Check accent of Lcaron, dcaron, lcaron, tcaron (derived from com.google.fonts/check/alt_caron) (com.google.fonts/check/alt_caron)
⚠ WARN: Checking correctness of monospaced metadata. (com.google.fonts/check/monospace)
⚠ WARN: Are there any misaligned on-curve points? (com.google.fonts/check/outline_alignment_miss)
Use -F or --full-lists to disable shortening of long lists. [code: found-misalignments] ℹ INFO: Show hinting filesize impact. (com.google.fonts/check/hinting_impact)
ℹ INFO: Font has old ttfautohint applied? (com.google.fonts/check/old_ttfautohint)
ℹ INFO: EPAR table present in font? (com.google.fonts/check/epar)
ℹ INFO: Is the Grid-fitting and Scan-conversion Procedure ('gasp') table set to optimize rendering? (com.google.fonts/check/gasp)
PPM <= 65535: ℹ INFO: Check for font-v versioning. (com.google.fonts/check/fontv)
ℹ INFO: Font contains all required tables? (com.google.fonts/check/required_tables)
ℹ INFO: List all superfamily filepaths (com.google.fonts/check/superfamily/list)
Summary
Note: The following loglevels were omitted in this report:
|
Hello @stephenhutchings, Cheers! |
Hi @emmamarichal. I am looking at these issues, although I have limited time due to other work. Hopefully I'll make some headway in the next few days. |
@emmamarichal I believe the latest fonts on the gf-onboarding branch address most issues you raised above, except for one.
The position of the ogonek component on "ę" seems to be correct in the source files and the non-variable font exports. I am not sure why the variable fonts use a different position. Any help here would be appreciated. |
@stephenhutchings Yes sorry, for the ogonek, I think I looked at the .ttf and it's normal to have this kind of issue in a font editor. But all looks great otherwise. Thank you! You can merge this branch to the main one so I can start the onboarding process. |
@emmamarichal It's the end of the day for me here but I'll look at getting this merged into master this week. Let's move forward with onboarding the condensed without italics. Marc's changes to the |
Ok, since I didn't know what was the decision about it, I didn't review the condensed version yet. Let me do it today, I'll add a comment here with my review :) |
Thanks @emmamarichal I will look at getting these fixed as soon as possible. |
@emmamarichal These issues are now fixed and merged onto the main branch, with release tag |
Hey @emmamarichal, following up here. Is anything else needed on our end to move forward? Ty! |
Ah yes sorry, I was quite busy last week! Will take a look this week and see if I can do a PR to the GF repo :) |
Font Project Git Repo URL:
https://github.com/reddit/redditsans
Super short description of the Font Family:
Reddit Sans is a humanist sans-serif designed for Reddit. Reddit Sans is complemented by Reddit Sans Condensed and Reddit Mono, which are also contained in the same repository. Fonts can be previewed on the microsite.
Requirements:
Google Fonts will publish only fonts that match its requirements. Please familiarize yourself with the complete documentation in the Google Fonts Guide (GF-Guide) and ensure your font project complies with them before submitting the font family. You can also use the Google Fonts Project Template, which will help you create a repository that follows the needed structure and includes build requirements.
By filling this issue, you can confirm the project meets the requirements (by ticking the cases or putting x between the square brackets in text mode):
Image:
Attach here a pic or a screenshot of the font. One image is enough, it can be a few letters, to give a quick overview.
The text was updated successfully, but these errors were encountered: