8000 ass_fontselect: add LIBASS_FORCE_FONTCONFIG env var by rcombs · Pull Request #788 · libass/libass · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

ass_fontselect: add LIBASS_FORCE_FONTCONFIG env var #788

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rcombs
Copy link
Member
@rcombs rcombs commented Jun 14, 2024

This simplifies testing fontconfig when on a platform with another preferred font provider.

Also improves docs for ASS_DefaultFontProvider.

rcombs added 2 commits June 13, 2024 21:09
This simplifies testing fontconfig when on a platform with another preferred
font provider.
@rcombs rcombs requested review from astiob and TheOneric June 14, 2024 04:13
@astiob
Copy link
Member
astiob commented Jun 14, 2024

Not a fan. What are you using for testing? mpv already provides --sub-font-provider=fontconfig; our test utility could do the same (although I’ve never found this necessary myself; you can just recompile with other providers disabled).

@rcombs
Copy link
Member Author
rcombs commented Jun 14, 2024

This comes up for me when using the test utility (and especially make check); particularly when working on fontselect code, it's much quicker and easier than recompiling every time I want to test both the fontconfig path and the native one. I considered adding this to each of our internal tools (test, profile, fuzz…), but it'd have required a bunch of code duplication to achieve the same result, and IMO this is reasonably within the bounds of what auto can mean.

@TheOneric
Copy link
Member

I’m kinda indifferent to (not) adding an env var for this.

At least the singling out of fontconfig seems a bit odd though. CoreText and DirectWrite can only occur on a single platform each and already are the highest prio provider there, so admittedly no AUTO nudge for them is needed (atm). Perhaps nudging AUTO to always select no system provider at all and using one common LIBASS_FONTPROVIDER_AUTO env var might make sense?

@astiob
Copy link
Member
astiob commented Jun 15, 2024

Perhaps nudging AUTO to always select no system provider at all and using one common LIBASS_FONTPROVIDER_AUTO env var might make sense?

I don’t follow; could you rephrase that?

rcombs’s idea is that on systems that have both Fontconfig and another provider, this would let us more easily test the non-default Fontconfig, which is normally overridden by the other provider.

@TheOneric
Copy link
Member

rcombs’s idea is that on systems that have both Fontconfig and another provider, this would let us more easily test the non-default Fontconfig, which is normally overridden by the other provider.

Despite being named FORCE_FONTCONFIG the env var only takes effect if AUTODETECT was im- or explicitly set by the API user. If fontconfig support was not built-in, no system font provider gets selected. So the env var effectively nudges, or considering it can’t fallback to another provider “restricts”, autodetection into a particular direction.

Currently fontconfig is the only system font provider existing on (some) platforms as the non-default choice. But we might also want to test for things working correctly with no system font provider at all (e.g. nothing crashes when no fallback can be found) or (albeit it seems unlikely atm) we might gain more multip-platform providers or additional platform-specific providers for an already existing platform (e.g. a GDI-only fontprovider).
Having a LIBASS_FORCE_XXX env var for each of them raises questions about which takes priority if multiple are set and seems like a more messy interface. Thus the suggestion to use one shared LIBASS_FONTPROVIDER_AUTO env var to select the default provider or none at all for AUTODETECTION

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0