googlefonts / opensans

Open Sans font
Other
212 stars 31 forks source link

Condensed added to master (PR includes notes) #3

Closed laerm0 closed 6 years ago

laerm0 commented 6 years ago

I have added the condensed weights to the master Glyphs file.

This is not ready to go yet. Some things still need work.

Finally, I didn't know what the best way was to track what @meirsadan does. Meir, do you want to just fix anything that needs fixing in the Hebrew or do you want to pass notes to me? Also, I dunno the mechanics of github in this way: should @davelab6 accept this PR as an intermediate thing and then Meir can fork off that and make changes, or should we keep this PR in limbo and Meir can make his changes to a fork of my fork which I PR and add to this PR?

Finally, the things I want to fix I am putting on hold anyway because I am going to do the italics. Meir, feel free to work on the source.

meirsadan commented 6 years ago

Hey, thanks for adding me! I just skimmed through the glyphs file and would take a longer look over the weekend. Also, I'd be happy to know how this interpolation was made? Is it an automatic interpolation/extrapolation process of condensing the typeface or has it been extracted from an existing source? I think I remember there already being a condensed Open Sans Hebrew?

laerm0 commented 6 years ago

Hi Meir –

The Hebrew in here is the Open Sans Hebrew you're thinking of: I merged it in last fall. The three interpolated weights were done in Glyphs (using the Pablo distribution) from the two weights I had TTF source for.

meirsadan commented 6 years ago

Hey,

Sorry for taking this long to report back! I've reviewed the condensed style.

I'm not sure how the condensed style is generated/interpolated? I know Yanek Iontef worked on the design but I'm not sure if the condensed was also done by him. I've done a mostly technical review here, as a design review will require more time...

Technical issues that need attention:

I should point out that the early access version of Open Sans Hebrew Condensed has well set diacritic positions and the geresh and gershayim glyphs are present. Maybe you already are using it as such, but I think that version is a great point of reference.

I also used Pablo's excellent Font Testing Page which I made a Hebrew section for, to review the files.

Please ask if any more details are needed on any of the issues, or to take a look at any specific detail that you think needs attention. :>

laerm0 commented 6 years ago

Hi @meirsadan –

Thank you again for the excellent feedback. I was working on other aspects of this project but today I have fixed the things you note. I still wonder about the qof with dagesh in the bold condensed: as you say, it is very close. Is it okay for the dagesh to come above of the left stem, or should it align to the top of that stem?§ If yes, is it better to make a smaller dagesh or a wider counter?

Thanks again.

§ like so:

screen region 2018-04-29 at 15 01 00
meirsadan commented 6 years ago

Hey, I think it's perfectly fine. A lot of the times the dagesh has to be nudged to different places depending on the counter. The issue is with what happens at small sizes, which I guess will be resolved by hinting?

laerm0 commented 6 years ago

Great, thank you.

Yes, hinting will be a solution for the small sizes. Cross your fingers. ;)

On Sun, Apr 29, 2018 at 3:21 PM, Meir Sadan notifications@github.com wrote:

Hey, I think it's perfectly fine. A lot of the times the dagesh has to be nudged to different places depending on the counter. The issue is with what happens at small sizes, which I guess will be resolved by hinting?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/googlefonts/opensans/pull/3#issuecomment-385251142, or mute the thread https://github.com/notifications/unsubscribe-auth/AKR-AZhZ_t0VbIJ1WogyGFO7fHhrO3m7ks5ttb5mgaJpZM4S1LWR .

-- Micah Stupak-Hahn Measure + Fit Typographic Engineering micah@measureandfit.co 405-MRFONTS

davelab6 commented 6 years ago

@laerm0 what is needed to get this merged? :)

laerm0 commented 6 years ago

Hints as good as the original, pretty much.

davelab6 commented 6 years ago

I suggest to merge it now, then :)

laerm0 commented 6 years ago

Hm. I see no conflicts on my side, so I dunno what the error is.

I pushed VTT hinted variable fonts to my fork today.

davelab6 commented 6 years ago

In https://github.com/googlefonts/opensans/pull/3/commits I see only 4 commits.

I think all your work is on master branch, so this can just be closed?

laerm0 commented 6 years ago

Oh, that makes sense, yes I've been working on my master branch.

On Thu, Jun 21, 2018, 16:43 Dave Crossland notifications@github.com wrote:

In https://github.com/googlefonts/opensans/pull/3/commits I see only 4 commits.

I think all your work is on master branch, so this can just be closed?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/googlefonts/opensans/pull/3#issuecomment-399237203, or mute the thread https://github.com/notifications/unsubscribe-auth/AKR-AT33LG2kNmk6tIavLwdWScSqytIMks5t_AVggaJpZM4S1LWR .

davelab6 commented 6 years ago

Thanks again to @meirsadan for your insights!