JuliaGraphics / FreeTypeAbstraction.jl

A Julian abstraction layer over FreeType.jl
Other
25 stars 20 forks source link

add cache mechanism #76

Open t-bltg opened 1 year ago

t-bltg commented 1 year ago

Fix https://github.com/JuliaGraphics/FreeTypeAbstraction.jl/issues/67.

codecov[bot] commented 1 year ago

Codecov Report

Base: 94.62% // Head: 95.31% // Increases project coverage by +0.69% :tada:

Coverage data is based on head (0864e78) compared to base (b7d6d65). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #76 +/- ## ========================================== + Coverage 94.62% 95.31% +0.69% ========================================== Files 6 6 Lines 316 320 +4 ========================================== + Hits 299 305 +6 + Misses 17 15 -2 ``` | [Impacted Files](https://codecov.io/gh/JuliaGraphics/FreeTypeAbstraction.jl/pull/76?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGraphics) | Coverage Δ | | |---|---|---| | [src/FreeTypeAbstraction.jl](https://codecov.io/gh/JuliaGraphics/FreeTypeAbstraction.jl/pull/76/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGraphics#diff-c3JjL0ZyZWVUeXBlQWJzdHJhY3Rpb24uamw=) | `100.00% <100.00%> (ø)` | | | [src/findfonts.jl](https://codecov.io/gh/JuliaGraphics/FreeTypeAbstraction.jl/pull/76/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGraphics#diff-c3JjL2ZpbmRmb250cy5qbA==) | `89.06% <100.00%> (-4.59%)` | :arrow_down: | | [src/types.jl](https://codecov.io/gh/JuliaGraphics/FreeTypeAbstraction.jl/pull/76/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGraphics#diff-c3JjL3R5cGVzLmps) | `94.28% <100.00%> (+0.05%)` | :arrow_up: | | [src/precompile.jl](https://codecov.io/gh/JuliaGraphics/FreeTypeAbstraction.jl/pull/76/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGraphics#diff-c3JjL3ByZWNvbXBpbGUuamw=) | `100.00% <0.00%> (+100.00%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGraphics). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaGraphics)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

jkrumbiegel commented 1 year ago

How would saving these path strings during compilation play with relocation? Seems to me that it would break if you'd move a sysimage with baked in paths somewhere else. I guess fonts are difficult in that way as they could be in different places on different systems.

For Makie, we really only care about not triggering font search in the default case, correct? In that case, we should look into caching Makie's default fonts in a relocatable way. That's only a couple fonts, mostly the four Makie TeX Gyre Heros fonts and a few fallbacks. If those fonts are already known at load, we wouldn't get latencies until the user specifies a new font for the first time.

t-bltg commented 1 year ago

How would saving these path strings during compilation play with relocation

In this PR in FTA, the cache is invalidated on __init__ so there is no impact on relocation. However, this could be a problem in Makie in https://github.com/MakieOrg/Makie.jl/pull/2518 yes.

For Makie, we really only care about not triggering font search in the default case

There are a few options here, but fully caching the font might not be feasible (because of the FT handles opened). What I aim at least for https://github.com/JuliaGraphics/FreeTypeAbstraction.jl/issues/67#issuecomment-1364043802 is trying to cache a few font paths for defaults as done here avoiding re-running the costly search and trial + best score approach of findfont at load time.

jkrumbiegel commented 1 year ago

I did mean caching the path, yes. Just in some relocatable way.

t-bltg commented 1 year ago

I've added a failsafe in Makie.

For now, if the path is non-existent, just fallback to using FTA.findfont for finding & loading the font. Trying to truly relocate the path seems a bit excessive and error prone.