Skip to content
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

Use Lato on MacOS and Windows #580

Merged
merged 1 commit into from
Feb 15, 2019

Conversation

baconpaul
Copy link
Collaborator

As described in #214, we want to use the OFL-licensed font
Lato for all components in Surge with the constraint that we
want to ship the font with the DLL and not require it to be installed
on an end users system.

To do this cross platforms, introduce a new C++ file
"RuntimeFont.h" which defines Surge::GUI::initializeRuntimeFontFromDistribution
and implement that in src/mac/RuntimeFontMac.cpp, Windows, etc.
This PR implements for Mac and Win and neither implements
nor references the symbol in Linux.

This change implements that in macOS by doing the following

1: Introducing the font to resources/fonts
2: Moving the fonts to the bundle with scripts/macOS/package*
3: Moving the font initiation for minifont and patchfont to
runtime rather than surge dll load time (but only do this
for fonts which USE_RUNTIME_LOADED_FONTS, which right now
is only MAC. Once all platforms are done, that switch will
disappear)
4: Write the MacOS specific CoreText code to load a font from
memory into the system

This change implements that in Windows by doing the following

1: Including the font in the .rc file and subsequent res bundle
as a binary
2: Load the font and place it in a well known temp file
3: Load that temp file into the font registry using AddFontResource
4: Following the subsequent standard cfont implementation.

@baconpaul
Copy link
Collaborator Author

@jsakkine a review greatly appreciated

@itsmedavep or @esaruoho if you want to build and test this too that would be lovely

No rush. Nothing here should conflict with anything.

Oh and: Linux was too hard. I gave up. vstgui makes the API too far away.


//#include <commctrl.h>

#if MAC || WINDOWS
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be done in premake5.lua, not here. Is use the standard for us for feature flags?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the way we staged zoom also. Have a HOST_SUPPORTS_ZOOM (I could do HOST_SUPPORTS_RUNTIME_FONT) and put it in the code; when we got the last host, remove the #ifdef from the code. I could move it to premake but that seems both more icky and less clear to the reader of the code. Mind if I leave it? (Happy to change the name of the variable. USES_RUNTIME_FONTS? HAS_RUNTIME_FONT_SUPPORT? What do you think?)

@jarkkojs
Copy link
Collaborator

OK, I'm ready with my review.

@baconpaul
Copy link
Collaborator Author

Thank you. My guess is I will look at this tomorrow maybe Wednesday.

Sorry for using the idiomatic language “plumb through”! Kinda means attach from one end to another like when you are running pipes through a house. I’ll clean it all up.

Appreciated!

As described in surge-synthesizer#214, we want to use the OFL-licensed font
Lato for all components in Surge with the constraint that we
want to ship the font with the DLL and not require it to be installed
on an end users system.

To do this cross platforms, introduce a new C++ file
"RuntimeFont.h" which defines Surge::GUI::initializeRuntimeFontFromDistribution
and implement that in src/mac/RuntimeFontMac.cpp, Windows, etc.
This PR implements for Mac and Win and neither implements
nor references the symbol in Linux.

This change implements that in macOS by doing the following

1: Introducing the font to resources/fonts
2: Moving the fonts to the bundle with scripts/macOS/package*
3: Moving the font initiation for minifont and patchfont to
   runtime rather than surge dll load time (but only do this
   for fonts which USE_RUNTIME_LOADED_FONTS, which right now
   is only MAC. Once all platforms are done, that switch will
   disappear)
4: Write the MacOS specific CoreText code to load a font from
   memory into the system

This change implements that in Windows by doing the following

1: Including the font in the .rc file and subsequent res bundle
as a binary
2: Load the font and place it in a well known temp file
3: Load that temp file into the font registry using AddFontResource
4: Following the subsequent standard cfont implementation.
@baconpaul
Copy link
Collaborator Author

HI @jsakkine

I rewrote the comments based on your feedback and renamed the function

I didn't change the feature guard - that's the same pattern we used in zoom and I'd sooner stick with it. And I am happy to rename those weirdly named global variables but would sooner that was not intermixed with this function change. Once this is approved I can make that mechanical change, test the builds, and the push it at the same time I push this, but with a separate commit and PR.

Please let me know what you think. Thanks!

@baconpaul baconpaul merged commit dde17b5 into surge-synthesizer:master Feb 15, 2019
@baconpaul baconpaul deleted the lato-font-take-2 branch February 15, 2019 14:43
baconpaul added a commit to baconpaul/surge that referenced this pull request Feb 21, 2019
In a review of surge-synthesizer#580 @jsakkine pointed out that the font global
names were not clear as to their purpose and unlike everything
else, prefixed with "surge_". This commit renames the 3 global
fonts to more descriptive names.
baconpaul added a commit that referenced this pull request Feb 23, 2019
In a review of #580 @jsakkine pointed out that the font global
names were not clear as to their purpose and unlike everything
else, prefixed with "surge_". This commit renames the 3 global
fonts to more descriptive names.
baconpaul added a commit to baconpaul/surge that referenced this pull request Jul 10, 2019
As described in surge-synthesizer#214, we want to use the OFL-licensed font
Lato for all components in Surge with the constraint that we
want to ship the font with the DLL and not require it to be installed
on an end users system.

To do this cross platforms, introduce a new C++ file
"RuntimeFont.h" which defines Surge::GUI::initializeRuntimeFontFromDistribution
and implement that in src/mac/RuntimeFontMac.cpp, Windows, etc.
This PR implements for Mac and Win and neither implements
nor references the symbol in Linux.

This change implements that in macOS by doing the following

1: Introducing the font to resources/fonts
2: Moving the fonts to the bundle with scripts/macOS/package*
3: Moving the font initiation for minifont and patchfont to
   runtime rather than surge dll load time (but only do this
   for fonts which USE_RUNTIME_LOADED_FONTS, which right now
   is only MAC. Once all platforms are done, that switch will
   disappear)
4: Write the MacOS specific CoreText code to load a font from
   memory into the system

This change implements that in Windows by doing the following

1: Including the font in the .rc file and subsequent res bundle
as a binary
2: Load the font and place it in a well known temp file
3: Load that temp file into the font registry using AddFontResource
4: Following the subsequent standard cfont implementation.

Closes surge-synthesizer#214 
Former-commit-id: 4e13f3971cc7030d0a2c87e5a8a83ceacfe8a463 [formerly dde17b5]
Former-commit-id: ec2872a4ad6a52979691dc4774f5833fd3d9e63e
Former-commit-id: 357261ae57bff1c95e12518c8ef058c4037f2c95
baconpaul added a commit to baconpaul/surge that referenced this pull request Jul 10, 2019
In a review of surge-synthesizer#580 @jsakkine pointed out that the font global
names were not clear as to their purpose and unlike everything
else, prefixed with "surge_". This commit renames the 3 global
fonts to more descriptive names.


Former-commit-id: bd014fabe73a1e954e0aca13b11adfff187c75ff [formerly 3705d7f]
Former-commit-id: 3f1f49d1ef12c4b0bac87979b618dbaa547fc569
Former-commit-id: 8484174dc6ee402f0260663b03eb85753a0c99b7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants