Godot version:
3.2.1 Stable
OS/device including version:
Mac OS Catalina 10.15.2
Issue description:
It seems that text on Buttons do not respect the Space setting of my DynamicFont. Screenshot shows my button (top) and label (bottom). The label is correct.

The space is set to -20 on the TRES file. I'm 100% sure I'm using the same TRES for Button and Label text, because I only have one. Other properties, such as Size and Outline Size are working fine.

Steps to reproduce:
Somewhat ironically, the reason Label displays the text correctly appears to be because Label doesn't draw spaces. :D
And so coincidentally Label avoids the issue that DynamicFont's draw_char() implementation does not account for the extra space character spacing.
But Button is not so lucky...
draw_char() vs get_char_size()Because Label doesn't draw spaces it never uses the width value returned for the space character from draw_char(), instead it uses the width value returned from get_char_size().
Label stores the space width value in space_w here:
And later uses it in a number of places, the key one being here where it modifies the x offset of the next rendered word:
The word splitting/space character removal appears around here:
https://github.com/godotengine/godot/blob/058a0afdeca83145d58a95c426dd01216c397ea9/scene/gui/label.cpp#L419-L458
And while DynamicFont's get_char_size() implementation does account for the extra space character spacing, its draw_char() implementation does not account for the extra space character spacing.
DynamicFont's get_char_size() accounts for extra space character spacing here:
But its draw_char() only adds the extra character spacing to the width returned from FreeType, at the end of this line:
This underlying issue appears to have existed since the original commit that added the spacing functionality: https://github.com/godotengine/godot/commit/af6ef01c692311410c084b0bf4f3fe2f4d46786d#diff-fce1ec7b33979481c1af96f2d6be6686R787
And Button calls get_string_size() (which calls get_char_size()) and the font's draw() a.k.a Font::draw() but also...
...note that Font::draw() calls both get_char_size() (to check for clipping) and accumulates the result of the draw_char() call:
So, in addition to fixing the issue in DynamicFont::draw_char(), it might be helpful to at least add an assert that the two methods are equal in Font::draw() to hopefully avoid similar situations in future...
This means that any code that uses DynamicFont's draw_char() or draw() method to draw space characters or strings containing space characters will render the text incorrectly.
This also affects features like text alignment or clipping if the code uses width value(s) from get_string_size()/get_char_size() with width value(s) returned from draw_char() in calculations.
From some non-comprehensive tests:
LineEdit -- Bizarrely, didn't seem affected, until I modified the alignment to be centered--whereupon it displayed the non-placeholder text correctly in the running app but incorrectly in the editor! However when I entered placeholder text it centered correctly in editor & app. And...then when I removed the placeholder the original text displayed correctly in the editor too--so who knows what that means...
(After further investigation I suspect that LineEdit probably doesn't like negative character widths...which is probably fair enough. :) )
RichTextLabel -- seems to handle the extra space spacing correctly in both bbcode & non-bbcode modes.
ItemList -- doesn't handle the extra space spacing correctly. It keeps the original spacing and also clips the last characters of my test text.
CanvasItem.draw_string() -- doesn't handle the extra space spacing correctly.
Here are the related searches:
draw_char search -- if the code stores the return value it is probably affected.
As mentioned above, DynamicFont's draw_char() implementation does account for extra character spacing, in addition to the width value returned from FreeType.
As a comparison, BitmapFont's draw_char() implementation actually returns the result of get_char_size()--but BitmapFont doesn't support extra character/space spacing anyway (so this implementation detail is probably kerning related).
(And, as I discovered, DynamicFont doesn't currently support kerning: #35730)
This suggests that some potential solutions:
Modify DynamicFont's draw_char() implementation to return the result of get_char_size() also although that might be considered "inefficient".
Modify DynamicFont's draw_char() implementation to add to the returned width if the character is a space.
But this duplicates the handling code and runs the risk that a similar situation might occur in future (or already occurs) if get_char_size() calculates anything differently to draw_char() (e.g. kerning? hinting? outlines?)
Prerequisites:
DynamicFont resource. (e.g. located at res://fonts/barlow-regular-64.tres)Control in a scene.extends Control
var the_font: DynamicFont = preload("res://fonts/barlow-regular-64.tres")
func _ready():
self.the_font.set_spacing(DynamicFont.SPACING_SPACE, -20)
func _draw():
assert(the_font.get_string_size(" ").x == self.draw_char(the_font, Vector2(0,0), " ", ""))
Result:
_draw() will fail.From initial research, it appeared the issue may have been because Label & Button use two different ways of drawing their text:
Label uses draw_char() method of FontDrawer: https://github.com/godotengine/godot/blob/6a0473bcc23c096ef9ee929632a209761c2668f6/scene/gui/label.cpp#L278Button uses draw() method of Font directly: https://github.com/godotengine/godot/blob/6a0473bcc23c096ef9ee929632a209761c2668f6/scene/gui/button.cpp#L228But this difference was not the cause of the issue with extra space character spacing.
There are a number of names given to spacing related properties/methods/constants/enums which complicates finding all the references in the code base (e.g. for character/space spacing):
Properties
extra_spacing_char, extra_spacing_spaceSetters/getters/methods
set_spacing()/get_spacing()Enums
SpacingType with SPACING_CHAR, SPACING_SPACEC++ code
spacing_char, spacing_space
space_w, space_width
' ' :D
[WIP: Intend to add some other info & code references but since "draft" issue comments aren't supported am posting this first.]
Most helpful comment
TL;DR:
Somewhat ironically, the reason
Labeldisplays the text correctly appears to be becauseLabeldoesn't draw spaces. :DAnd so coincidentally
Labelavoids the issue thatDynamicFont'sdraw_char()implementation does not account for the extra space character spacing.But
Buttonis not so lucky...Character width from
draw_char()vsget_char_size()Because
Labeldoesn't draw spaces it never uses the width value returned for the space character fromdraw_char(), instead it uses the width value returned fromget_char_size().Related Code
Labelstores the space width value inspace_where:https://github.com/godotengine/godot/blob/058a0afdeca83145d58a95c426dd01216c397ea9/scene/gui/label.cpp#L114
And later uses it in a number of places, the key one being here where it modifies the x offset of the next rendered word:
https://github.com/godotengine/godot/blob/058a0afdeca83145d58a95c426dd01216c397ea9/scene/gui/label.cpp#L234-L241
The word splitting/space character removal appears around here:
https://github.com/godotengine/godot/blob/058a0afdeca83145d58a95c426dd01216c397ea9/scene/gui/label.cpp#L419-L458
And while
DynamicFont'sget_char_size()implementation does account for the extra space character spacing, itsdraw_char()implementation does not account for the extra space character spacing.Related Code
DynamicFont'sget_char_size()accounts for extra space character spacing here:https://github.com/godotengine/godot/blob/d7b85fbaa1fc438effe406c9d7f973749eb4e527/scene/resources/dynamic_font.cpp#L858-L859
But its
draw_char()only adds the extra character spacing to the width returned from FreeType, at the end of this line:https://github.com/godotengine/godot/blob/d7b85fbaa1fc438effe406c9d7f973749eb4e527/scene/resources/dynamic_font.cpp#L886
This underlying issue appears to have existed since the original commit that added the spacing functionality: https://github.com/godotengine/godot/commit/af6ef01c692311410c084b0bf4f3fe2f4d46786d#diff-fce1ec7b33979481c1af96f2d6be6686R787
And
Buttoncallsget_string_size()(which callsget_char_size()) and the font'sdraw()a.k.aFont::draw()but also...https://github.com/godotengine/godot/blob/6a0473bcc23c096ef9ee929632a209761c2668f6/scene/gui/button.cpp#L200-L228
...note that
Font::draw()calls bothget_char_size()(to check for clipping) and accumulates the result of thedraw_char()call:https://github.com/godotengine/godot/blob/d7b85fbaa1fc438effe406c9d7f973749eb4e527/scene/resources/font.cpp#L69-L74
So, in addition to fixing the issue in
DynamicFont::draw_char(), it might be helpful to at least add an assert that the two methods are equal inFont::draw()to hopefully avoid similar situations in future...Other affected code
This means that any code that uses
DynamicFont'sdraw_char()ordraw()method to draw space characters or strings containing space characters will render the text incorrectly.This also affects features like text alignment or clipping if the code uses width value(s) from
get_string_size()/get_char_size()with width value(s) returned fromdraw_char()in calculations.From some non-comprehensive tests:
LineEdit-- Bizarrely, didn't seem affected, until I modified the alignment to be centered--whereupon it displayed the non-placeholder text correctly in the running app but incorrectly in the editor! However when I entered placeholder text it centered correctly in editor & app. And...then when I removed the placeholder the original text displayed correctly in the editor too--so who knows what that means...(After further investigation I suspect that
LineEditprobably doesn't like negative character widths...which is probably fair enough. :) )RichTextLabel-- seems to handle the extra space spacing correctly in both bbcode & non-bbcode modes.ItemList-- doesn't handle the extra space spacing correctly. It keeps the original spacing and also clips the last characters of my test text.CanvasItem.draw_string()-- doesn't handle the extra space spacing correctly.Here are the related searches:
get_char_sizesearchdraw_charsearch -- if the code stores the return value it is probably affected.Other handling of extra spacing
As mentioned above,
DynamicFont'sdraw_char()implementation does account for extra character spacing, in addition to the width value returned from FreeType.As a comparison,
BitmapFont'sdraw_char()implementation actually returns the result ofget_char_size()--butBitmapFontdoesn't support extra character/space spacing anyway (so this implementation detail is probably kerning related).https://github.com/godotengine/godot/blob/d7b85fbaa1fc438effe406c9d7f973749eb4e527/scene/resources/font.cpp#L572
(And, as I discovered,
DynamicFontdoesn't currently support kerning: #35730)Solutions
This suggests that some potential solutions:
Modify
DynamicFont'sdraw_char()implementation to return the result ofget_char_size()also although that might be considered "inefficient".Modify
DynamicFont'sdraw_char()implementation to add to the returned width if the character is a space.But this duplicates the handling code and runs the risk that a similar situation might occur in future (or already occurs) if
get_char_size()calculates anything differently todraw_char()(e.g. kerning? hinting? outlines?)Code to reproduce
Prerequisites:
DynamicFontresource. (e.g. located atres://fonts/barlow-regular-64.tres)Controlin a scene.Result:
_draw()will fail.Red Herrings Encountered
From initial research, it appeared the issue may have been because
Label&Buttonuse two different ways of drawing their text:Labelusesdraw_char()method ofFontDrawer: https://github.com/godotengine/godot/blob/6a0473bcc23c096ef9ee929632a209761c2668f6/scene/gui/label.cpp#L278Buttonusesdraw()method ofFontdirectly: https://github.com/godotengine/godot/blob/6a0473bcc23c096ef9ee929632a209761c2668f6/scene/gui/button.cpp#L228But this difference was not the cause of the issue with extra space character spacing.
Other code base observations
There are a number of names given to spacing related properties/methods/constants/enums which complicates finding all the references in the code base (e.g. for character/space spacing):
Properties
extra_spacing_char,extra_spacing_spaceSetters/getters/methods
set_spacing()/get_spacing()Enums
SpacingTypewithSPACING_CHAR,SPACING_SPACEC++ code
spacing_char,spacing_spacespace_w,space_width' ':D[WIP: Intend to add some other info & code references but since "draft" issue comments aren't supported am posting this first.]