-
-
Notifications
You must be signed in to change notification settings - Fork 21.1k
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
Add sub-pixel glyph positioning support. #57877
Conversation
fa7b77f
to
14f78fd
Compare
Disclaimer: I am not a contributor to Godot (yet), so please don't consider this comment as authoritative in any way. I just want to leave my opinion as a Godot user who is subscribed to godotengine/godot-proposals#1258. I applaud your effort in implementing something new, and especially the level of clarity and detail in your demonstration and explanation. This was very easy to understand and does add something to consider as an improvement to the engine. That being said, this does not appeal to me personally. I wouldn't be enabling it in my projects because it introduces no visual improvement in my personal opinion while introducing a performance detriment. Even if someone does understand it and see a benefit, it does not resolve the issue in godotengine/godot-proposals#1258, while simultaneously introducing additional complexity in configuration. Without the same level of clarity as is present here in this MR also present in the documentation, the related option may be difficult to understand for users. My personal opinion is that LCD subpixel font rendering is the ultimate goal, and anything short of that does not solve the problem, as the text still looks relatively blurry. |
Could you take a screenshot of the project manager and editor with and without this feature enabled (at 100% display scale)? |
14f78fd
to
8fb7c6b
Compare
I have added
This might sound scary, but font cache is not large in the first place, for example real memory impact for the editor: with Chinese localization, and after browsing multiple help pages and scripts to rasterize as much glyph as possible additional 3 MB of RAM were used (2.4 → 5.6), with English localization it's about 0.75 MB increase. No performance impact is noticeable. |
On the feature itself: I believe this is useful, especially in games because using LCD antialiasing in games is challenging (since you need to know the background color for every area that displays text). In contrast, sub-pixel glyph positioning doesn't require custom blending functions or knowing the background color in advance. Does this PR have a benefit when hinting is set to Light or Normal?
My only concern is that new glyphs may cause stuttering during gameplay, especially on mobile where fonts are often displayed at large sizes. I suppose enabling ASCII prerendering by default would alleviate this. |
Hinting has a similar effect on the sub-pixel positioned font as it has on the normal font (not sure if it's better with or without hinting, I guess it's the matter of preferences).
It's always possible to pre-rendering glyphs (will rasterize all variants). With the large font sizes, it's probably less useful anyway. Unlikely it will cause any issues with ASCII text, for fonts with large amount of glyph (like CJK) it probably can, since it already was an issue on mobile, but it's disabled by default. |
It might be worth enabling this in the editor by default (either Half or Quarter), since I doubt most users will know how to improve font rendering in the editor. The performance impact is probably unnoticeable in the editor. In projects, I'm not sure if this should be enabled by default. In projects, larger font sizes are generally used (at least 16), so the difference will be barely noticeable. If we also enable ASCII prerendering by default, performance should be fine during gameplay. On the other hand, if we leave this disabled by default, then very few people will bother checking the settings and enabling this feature. Sub-pixel glyph positioning isn't something most people are aware of… including me until yesterday 🙂 Edit: Since the benefits of sub-pixel glyph positioned are largely linked to the font size, maybe we could rework the settings as follows:
Footnotes |
8fb7c6b
to
dbc4d4d
Compare
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.
Looks good.
doc/classes/FontData.xml
Outdated
@@ -600,5 +600,8 @@ | |||
<member name="style_name" type="String" setter="set_font_style_name" getter="get_font_style_name" default=""""> | |||
Font style name. | |||
</member> | |||
<member name="subpixel_positioning" type="int" setter="set_subpixel_positioning" getter="get_subpixel_positioning" enum="TextServer.SubpixelPositioning" default="1"> | |||
Font glyph sub-pixel positioning mode. |
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.
Could we document why you would use this? Basically that it trades performance for sharper rendering, especially for smaller font sizes, and may not have much effect for large font sizes.
We could also recommend the AUTO setting and point to the project setting.
dbc4d4d
to
c5d1f32
Compare
c5d1f32
to
2919957
Compare
Thanks! |
Now that an Auto option is available, we should probably see if there's a still point in exposing the editor setting. To me, it seems there's little reason to change it from the default, as the Auto setting makes a good job at only using subpixel positioning when it makes a noticeable difference. |
If you ever find yourself making a subjective argument for what value a setting should be set to, that's when you know the setting needs to be exposed, as others may disagree. |
Note: This has nothing to do with LCD sub-pixel rendering, it's a completely unrelated technique.
Instead of rasterizing each glyph vector outline once, it's rasterized multiple times, offset by a fraction of a pixel.
Pros:
Cons:
Rendering example (half and quarter modes looks almost identically, but there are slight differences visible on the second screenshot):
5x zoomed part of example (red arrow shows one of the differences between half and quarter modes).
Animated difference between half and quarter modes:
Related proposal - godotengine/godot-proposals#1258 (probably described issue is mostly caused by hinting not fully applied without sub-pixel positioning, but there's also LCD sub-pixel rendering discussion in the same proposal).