-
-
Notifications
You must be signed in to change notification settings - Fork 842
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
Fix: Allow changing size of default OpenTTD font. #12641
Conversation
Size configuration for default font was ignored as a different code path to load the font was followed. Resolved by removing this additional path and conditionally selecting the default font.
{ | ||
const FontCacheSubSetting *settings = GetFontCacheSubSetting(fs); | ||
if (!settings->font.empty()) return settings->font; | ||
if (_fcsettings.prefer_sprite) return {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the prefer sprite flag override any font name having been set? Wouldn't have a font name set here break the prefer sprites checkbox?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer sprite only affects default font, if you set a font name the checkbox should be disabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is how it works today. I didn't know that.
I am not sure about disabling the checkbox though. That seems to be unrelated to this review though.
Oh dear, this breaks the default font size on 1x interface scale, due to "try to determine a good height based on the minimal height" code in each font cache implementation. FS_SMALL becomes 8px and FS_NORMAL becomes 12px, instead of 6px and 10px. |
I guess I don't understand. Are 6px/10px not good sizes or is the code that determines what the minimum height is giving inflated sizes back? |
Motivation / Problem
Size configuration for default font was ignored as a different code path to load the font was followed.
Size of default font could only be changed after manually setting the configured font to the default font name.
Description
Resolved by removing this additional path and conditionally selecting the default font.
This allows the normal font size selection to occur, and reduces some special-casing for the default font.
Limitations
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.