Skip to content

Make font hinting user-configurable.#78830

Merged
GuardianDll merged 10 commits intoCleverRaven:masterfrom
CLIDragon:fix-font-rendering
Jan 5, 2025
Merged

Make font hinting user-configurable.#78830
GuardianDll merged 10 commits intoCleverRaven:masterfrom
CLIDragon:fix-font-rendering

Conversation

@CLIDragon
Copy link
Copy Markdown
Contributor

@CLIDragon CLIDragon commented Dec 29, 2024 •
edited
Loading

Summary

Interface "Make font-hinting user-configurable"

Purpose of change

Fixes the slight blurriness of Terminus on ImGui windows which was caused by not hinting in bitmap mode.

fixes #73120

Describe the solution

The font is loaded through the same fontdata.json file that was always used.

I'm not really happy with the clarity of the error messages, but I am not sure how to make them clearer. I don't think it's possible to use JsonWithPath.throw_error() because if it occurs before the first typeface is loaded the game will crash before load.

Describe alternatives you've considered

None.

Testing

Loaded the game. Checked that error messages were displayed appropriately for incorrect sizes.

Additional context

Current
image

Previous
image

@github-actions github-actions Bot added Info / User Interface Game - player communication, menus, etc. [JSON] Changes (can be) made in JSON [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions labels Dec 29, 2024
@github-actions github-actions Bot added the astyled astyled PR, label is assigned by github actions label Dec 29, 2024
moxian
moxian reviewed Dec 29, 2024
View reviewed changes
Comment thread data/fontdata.json Outdated
@Procyonae
Copy link
Copy Markdown
Member

Procyonae commented Dec 29, 2024

This could do with some temporary migration for the default fonts otherwise noone keeping their config on update will get the changes unless I'm misunderstanding

@github-actions github-actions Bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Dec 29, 2024
@Kilvoctu
Copy link
Copy Markdown
Contributor

Kilvoctu commented Dec 30, 2024 •
edited
Loading

Pulled and tested this, and yeah, seems like anyone with an existing config/fonts.json won't see the changes in this update. I agree some more user-facing documentation would be preferred.

Before, with Terminus
Screenshot 2024-12-29 175319

After I deleted my config/fonts.json and let it rebuild
Screenshot 2024-12-29 175337

Before, using a custom font FiraCode
Screenshot 2024-12-29 180455

Same custom font, hinting bitmap
Screenshot 2024-12-29 174915

The screenshots don't really do it justice; it does look much better in game

db48x
db48x approved these changes Dec 30, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@db48x db48x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I like this change, but I have one or two suggestions.

Comment thread src/font_loader.h Outdated
Comment thread src/cata_imgui.cpp Outdated
@github-actions github-actions Bot removed the astyled astyled PR, label is assigned by github actions label Dec 31, 2024
@github-actions github-actions Bot added <Documentation> Design documents, internal info, guides and help. [Markdown] Markdown issues and PRs astyled astyled PR, label is assigned by github actions labels Dec 31, 2024
@CLIDragon
Copy link
Copy Markdown
Contributor Author

CLIDragon commented Dec 31, 2024 •
edited
Loading

This could do with some temporary migration for the default fonts otherwise noone keeping their config on update will get the changes unless I'm misunderstanding

I've changed it so that config/fonts.json is automatically updated to the new format. Additionally, if the font is written as a single string terminus hinting will default to bitmap and Roboto-Medium hinting will default to Auto.

@CLIDragon
Make font hinting user-configurable.
48d6286 
* Configure default hinting for Terminus and Roboto to better values.
* Document font options in docs/FONT_OPTIONS.md
* Fix the slight blurriness of Terminus on ImGui which was caused by
  not hinting in bitmap mode.
@github-actions github-actions Bot removed the BasicBuildPassed This PR builds correctly, label assigned by github actions label Dec 31, 2024
db48x
db48x suggested changes Dec 31, 2024
View reviewed changes
Comment thread src/font_loader.cpp
Comment thread src/font_loader.cpp
Comment thread src/font_loader.cpp Outdated
@db48x
Copy link
Copy Markdown
Contributor

db48x commented Dec 31, 2024

I expanded the documentation and made some other small tweaks. I sent them to you in CLIDragon#2.

@github-actions github-actions Bot removed the astyled astyled PR, label is assigned by github actions label Jan 1, 2025
@github-actions github-actions Bot added the astyled astyled PR, label is assigned by github actions label Jan 1, 2025
@db48x
Copy link
Copy Markdown
Contributor

db48x commented Jan 1, 2025

Sent you another tweak in CLIDragon#3.

@github-actions github-actions Bot added BasicBuildPassed This PR builds correctly, label assigned by github actions and removed BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jan 1, 2025
@CLIDragon CLIDragon closed this Jan 1, 2025
@CLIDragon CLIDragon reopened this Jan 1, 2025
@github-actions github-actions Bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 1, 2025
@CLIDragon CLIDragon closed this Jan 2, 2025
@CLIDragon CLIDragon reopened this Jan 2, 2025
@CLIDragon CLIDragon closed this Jan 3, 2025
@CLIDragon CLIDragon reopened this Jan 3, 2025
@CLIDragon CLIDragon closed this Jan 3, 2025
@CLIDragon CLIDragon reopened this Jan 3, 2025
@CLIDragon
Copy link
Copy Markdown
Contributor Author

CLIDragon commented Jan 4, 2025

Failing on vehicle_split_test. Should be ok to merge.

@GuardianDll GuardianDll merged commit 530b632 into CleverRaven:master Jan 5, 2025
@GuardianDll GuardianDll mentioned this pull request Jan 5, 2025
Closed
@CLIDragon CLIDragon mentioned this pull request Jan 6, 2025
Closed
@CLIDragon CLIDragon deleted the fix-font-rendering branch January 6, 2025 04:23
@db48x
Copy link
Copy Markdown
Contributor

db48x commented Jan 13, 2025 •
edited
Loading

Just a preview:

Screencast.From.2025-01-13.01-35-21.mp4

@henry-thomson henry-thomson mentioned this pull request May 23, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@db48x db48x db48x requested changes

@moxian moxian moxian left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` <Documentation> Design documents, internal info, guides and help. Info / User Interface Game - player communication, menus, etc. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Imgui rendered fonts do not look as crisp as in the other menus with the software renderer

6 participants

@CLIDragon @Procyonae @Kilvoctu @db48x @moxian @GuardianDll