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 BaseDirs for locating font directories #82

Merged
merged 3 commits into from
Dec 10, 2024

Conversation

tecosaur
Copy link
Contributor

@tecosaur tecosaur commented Dec 30, 2023

Identifying the correct platform-specific font directories can be done mostly correctly most of the time just with a hardcoded list of places to look (particularly on Apple systems). However, on Windows and XDG-following (i.e. Linux and friends) systems the user and system font directories can end up in other places, and accommodating these edge cases takes some effort.

This effort has already been put in with the BaseDirs package, which is a small zero-dependency package whose entire purpose is to find the correct directories for different types of content on various platforms. So, we can remove the current font-folder-finding code entirely and just call BaseDirs.fonts() for a reduction in code here and an improvement in compatibility.

Copy link

codecov bot commented Dec 30, 2023

Codecov Report

Attention: Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 94.55%. Comparing base (7630a9d) to head (9807a13).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
src/FreeTypeAbstraction.jl 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #82      +/-   ##
==========================================
- Coverage   95.06%   94.55%   -0.52%     
==========================================
  Files           6        6              
  Lines         324      312      -12     
==========================================
- Hits          308      295      -13     
- Misses         16       17       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tecosaur
Copy link
Contributor Author

It seems like codecov is complaining about the line @static if Sys.isunix() && !Sys.isapple() not being tested (I don't think it can be hit at runtime while @static applies).

Identifying the correct platform-specific font directories can be done
mostly correctly most of the time just with a hardcoded list of places
to look (particularly on Apple systems). However, on Windows and
XDG-following (i.e. Linux and friends) systems the user and system font
directories can end up in other places, and accommodating these edge
cases takes some effort.

This effort has already been put in with the BaseDirs package, which is
a small zero-dependency package whose entire purpose is to find the
correct directories for different types of content on various platforms.
So, we can remove the current font-folder-finding code entirely and just
call BaseDirs.fonts() for a reduction in code here and an improvement in
compatibility.
@tecosaur
Copy link
Contributor Author

tecosaur commented Oct 4, 2024

Just rebased and benchmarked latency (with Julia 1.10):

julia> @time using FreeTypeAbstraction # current release
  0.459916 seconds (858.54 k allocations: 87.634 MiB, 6.14% gc time, 1.33% compilation time)
  
julia> @time using FreeTypeAbstraction # this PR
  0.463937 seconds (878.13 k allocations: 90.774 MiB, 3.77% gc time, 1.26% compilation time)

Seems like this makes a negligible difference to loading time 🙂

@SimonDanisch SimonDanisch merged commit a018db8 into JuliaGraphics:master Dec 10, 2024
9 of 11 checks passed
@SimonDanisch
Copy link
Member

Thank you :)

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