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

View font does not fallback to default if missing #4566

Closed
jonpalmisc opened this issue Aug 16, 2023 · 3 comments
Closed

View font does not fallback to default if missing #4566

jonpalmisc opened this issue Aug 16, 2023 · 3 comments
Assignees
Labels
Component: UI Issue needs changes to the user interface Effort: Low Issue should take < 1 week Impact: Low Issue is a papercut or has a good, supported workaround Type: Bug Issue is a non-crashing bug with repro steps UI: Fonts Issues with fonts and font rendering
Milestone

Comments

@jonpalmisc
Copy link
Contributor

If you configure your view font to a font that isn't installed (or in my case, copy your settings to a new machine without the configured font), the view font becomes the default system font and comedy ensues. The font should fall back to the default mono font if the configured font is missing.

image

Binary Ninja 3.4.4271 (Stable)

@fuzyll fuzyll added Type: Bug Issue is a non-crashing bug with repro steps Component: UI Issue needs changes to the user interface Impact: Low Issue is a papercut or has a good, supported workaround Effort: Low Issue should take < 1 week UI: Fonts Issues with fonts and font rendering labels Aug 16, 2023
@fuzyll fuzyll closed this as completed Aug 28, 2023
@fuzyll
Copy link
Contributor

fuzyll commented Aug 28, 2023

I believe I've got this taken care of in the upcoming 3.5.4481-dev build that will hopefully make it through CI in the next ~30 minutes. This is currently implemented by looking for an exact match to what's in user settings and, if it doesn't exist, substituting our default as a fallback.

It's possible there's an edge-case that this doesn't cover, but I tested that it worked when putting nonsense in as font names in settings, so feel free to re-open if there's a situation I've missed.

@fuzyll
Copy link
Contributor

fuzyll commented Aug 28, 2023

Reopening this as it appears I did introduce a problem where certain fonts can no longer be set as per the issue above.

@fuzyll fuzyll reopened this Aug 28, 2023
@fuzyll fuzyll closed this as completed Aug 29, 2023
@fuzyll
Copy link
Contributor

fuzyll commented Aug 29, 2023

So, as far as I'm aware, the way you're supposed to do this in Qt is by calling exactMatch on the QFont. My understanding is that this will check the underlying font database to see if a font that matches exists. If it doesn't, it'll return the 'closest' font that matches the "style hint" that's been set.

I attempted to fix this by using exactMatch to detect the failing case and falling back to our default fonts if they don't. Unfortunately, exactMatch appears to just return false for everything. I missed this because I didn't adequately test for the case where we want to replace the default font and it's a valid font. I believe this is the Qt bug I'm running into, which is closed despite multiple people reporting having the same issue for the past year and a half and across both Qt5 and Qt6.

I've reimplemented this by just setting the "style hint" for the monospaced font so that it at least chooses a monospaced replacement. This isn't as nice as choosing the default font as a fallback, but it at least lets me close this issue as the view font should look appropriate now.

Should be resolved again in builds >= 3.5.4496-dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: UI Issue needs changes to the user interface Effort: Low Issue should take < 1 week Impact: Low Issue is a papercut or has a good, supported workaround Type: Bug Issue is a non-crashing bug with repro steps UI: Fonts Issues with fonts and font rendering
Projects
None yet
Development

No branches or pull requests

2 participants