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

Non-windows: Link fontawesome into executable. #442

Merged
merged 4 commits into from
Nov 2, 2022

Conversation

goeiecool9999
Copy link
Collaborator

@goeiecool9999 goeiecool9999 commented Nov 1, 2022

On linux we don't have access to resource files so we have to resort to converting files to C source code.
For fontawesome this would result in a 1MB C source file from a 160KB .ttf file. Clearly not acceptable.
This is the next best thing. Short assembly file that exposes symbols to the .ttf file data.
I'm creating this pull request to trigger CI to check if mac supports it too. If not I'll make it linux only.
Confirmed working on macOS and Linux.

@goeiecool9999 goeiecool9999 marked this pull request as ready for review November 1, 2022 23:02
@Tachi107
Copy link
Contributor

Tachi107 commented Nov 2, 2022

Could you please add an option to avoid embedding them? This would be undesirable in distribution packages and possibly also Flatpak

Edit: also, I think it would be necessary to see if FontAwesome's license allows embedding, and under which conditions.

@goeiecool9999
Copy link
Collaborator Author

goeiecool9999 commented Nov 2, 2022

Could you please add an option to avoid embedding them? This would be undesirable in distribution packages and possibly also Flatpak

Edit: also, I think it would be necessary to see if FontAwesome's license allows embedding, and under which conditions.

Cemu already embeds it in the windows .exe so I assume they sorted out the licensing. Initially I was gonna try to figure out fontconfig but then I thought it might cause issues if the system font is different from what cemu is expecting so I went the route of embedding it. Speaking for myself here I wouldn't mind a package including a 160kb font in an executable. Are you anticipating issues for packages/flatpack other than size and data duplication?

@Tachi107
Copy link
Contributor

Tachi107 commented Nov 2, 2022 via email

@Exzap
Copy link
Member

Exzap commented Nov 2, 2022

Could you please add an option to avoid embedding them? This would be undesirable in distribution packages and possibly also Flatpak

We don't officially support distro packaging so this isn't something we should take into account. Embedding works fine on every platform and there are no licensing issues with fontawesome. Flatpak also has no issues with this

@goeiecool9999
Copy link
Collaborator Author

goeiecool9999 commented Nov 2, 2022

Yes, distributions generally try to avoid embedding stuff into the executable for various reasons/policies, and as far as I understand flatpak is similar in this regard.

I think we've settled the issue. Cemu will not officially support loading fonts available on the system.
Nevertheless here you have a patch that should work for your purposes.
goeiecool9999@57f33be
I forgot to add an explicit linking statement with fontconfig but that should be easily fixed.

@Tachi107
Copy link
Contributor

Tachi107 commented Nov 2, 2022

We don't officially support distro packaging so this isn't something we should take into account

Cemu also didn't have to account for vcpkg-less builds, but in the end that turned out to be needed for Flatpak 🙃

Of course you don't need to add the option, it's just nice if you do.

@Exzap Exzap merged commit dfa7774 into cemu-project:main Nov 2, 2022
@goeiecool9999
Copy link
Collaborator Author

Just in case you still want to use the patch to use fontconfig
goeiecool9999@cbbef62
This is an updated version I messed something up in the cmake file

@Tachi107
Copy link
Contributor

Tachi107 commented Nov 3, 2022

Thanks! It will surely be useful to somebody :)

@goeiecool9999 goeiecool9999 deleted the embedfontawesome branch November 4, 2022 18:34
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.

3 participants