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

Refactor Font configuration and import UI, and Font resources. #62108

Merged
merged 1 commit into from
Jul 7, 2022

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Jun 16, 2022

Supersede #61406, Supersede #61473, Supersede #62065

  • Merges all import time properties of Font and FontData into single Font resource. Including fallback font list and pre-rendered glyphs (different sets of pre-rendered glyphs can be selected for different sizes/variations).
  • Adds FontVariation subclass to access OpenType variations, embolden/slant, font collection face index, OpenType features, and extra spacing and allow fallback override in runtime.

New font resources structure:
Screenshot 2022-07-06 at 14 04 28

FontVariation screenshot:
Screenshot 2022-06-16 at 16 27 58

@sairam4123
Copy link

Can font size be changed for per control node?

@bruvzg
Copy link
Member Author

bruvzg commented Jun 17, 2022

Can font size be changed for per control node?

It's already possible with the Theme properties and not changed by this PR.

@bruvzg bruvzg marked this pull request as ready for review June 17, 2022 10:18
@bruvzg bruvzg requested review from a team as code owners June 17, 2022 10:18
@bruvzg bruvzg force-pushed the font_config_v3 branch 2 times, most recently from ea64688 to cb56389 Compare June 23, 2022 19:37
reduz
reduz previously requested changes Jul 6, 2022
Copy link
Member

@reduz reduz left a comment

Choose a reason for hiding this comment

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

It looks good to me, but I have the feeling that using FontBase all across the codebase probably is a bit unnecesary since we don't do this with other resource types (like Texture2D, AudioStream, Mesh, etc).

Maybe we should do

FontBase -> Font (abstract)
Font -> FontFile or VectorFont or maybe just ScalableFont (which is the technical name for OTF/TTF/etc).

@bruvzg
Copy link
Member Author

bruvzg commented Jul 6, 2022

FontBase -> Font (abstract)
Font -> FontFile or VectorFont or maybe just ScalableFont (which is the technical name for OTF/TTF/etc).

OK, I'll change it to Font and FontFile (the same class is used for bitmap fonts, so it's not always scalable).

@reduz
Copy link
Member

reduz commented Jul 6, 2022

@bruvzg Alright, sorry for the change request and great job!, realized we were no longer using resources with "Base" naming in 4.0 too late so my mistake.

@bruvzg
Copy link
Member Author

bruvzg commented Jul 6, 2022

I'll change it to Font and FontFile

Done.

@akien-mga akien-mga dismissed reduz’s stale review July 7, 2022 10:22

Requested changes done, and @reduz approved the follow-up #62139 so it should be good to go.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks great!

@akien-mga akien-mga merged commit 1e553e3 into godotengine:master Jul 7, 2022
@akien-mga
Copy link
Member

Thanks!

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.

4 participants