Godot: Button text does not respect Space setting of DynamicFont tres

Created on 10 May 2020  路  1Comment  路  Source: godotengine/godot

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.

Screenshot 2020-05-09 at 14 44 00

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.

Screenshot 2020-05-09 at 16 39 25

Steps to reproduce:

  1. Create a Button and a Label.
  2. Create a DynamicFont and change the Spacing to -20
  3. Assign the same DynamicFont file to Button and Label
  4. Observe the difference
bug gui

Most helpful comment


TL;DR:

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...


Character width from 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().

Related Code

Label stores the space width value in space_w here:

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'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.

Related Code

DynamicFont's get_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 Button calls get_string_size() (which calls get_char_size()) and the font's draw() a.k.a Font::draw() but also...

https://github.com/godotengine/godot/blob/6a0473bcc23c096ef9ee929632a209761c2668f6/scene/gui/button.cpp#L200-L228

...note that Font::draw() calls both get_char_size() (to check for clipping) and accumulates the result of the draw_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 in Font::draw() to hopefully avoid similar situations in future...

Other affected code

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:

Other handling of extra spacing

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).

https://github.com/godotengine/godot/blob/d7b85fbaa1fc438effe406c9d7f973749eb4e527/scene/resources/font.cpp#L572

(And, as I discovered, DynamicFont doesn't currently support kerning: #35730)

Solutions

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?)

Code to reproduce

Prerequisites:

  • A DynamicFont resource. (e.g. located at res://fonts/barlow-regular-64.tres)
  • The following script attached to a 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:

  • The assertion in _draw() will fail.

Red Herrings Encountered

From initial research, it appeared the issue may have been because Label & Button use two different ways of drawing their text:

But 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_space
    • Setters/getters/methods

      • set_spacing()/get_spacing()
    • Enums

      • SpacingType with SPACING_CHAR, SPACING_SPACE
    • C++ 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.]

>All comments


TL;DR:

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...


Character width from 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().

Related Code

Label stores the space width value in space_w here:

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'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.

Related Code

DynamicFont's get_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 Button calls get_string_size() (which calls get_char_size()) and the font's draw() a.k.a Font::draw() but also...

https://github.com/godotengine/godot/blob/6a0473bcc23c096ef9ee929632a209761c2668f6/scene/gui/button.cpp#L200-L228

...note that Font::draw() calls both get_char_size() (to check for clipping) and accumulates the result of the draw_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 in Font::draw() to hopefully avoid similar situations in future...

Other affected code

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:

Other handling of extra spacing

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).

https://github.com/godotengine/godot/blob/d7b85fbaa1fc438effe406c9d7f973749eb4e527/scene/resources/font.cpp#L572

(And, as I discovered, DynamicFont doesn't currently support kerning: #35730)

Solutions

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?)

Code to reproduce

Prerequisites:

  • A DynamicFont resource. (e.g. located at res://fonts/barlow-regular-64.tres)
  • The following script attached to a 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:

  • The assertion in _draw() will fail.

Red Herrings Encountered

From initial research, it appeared the issue may have been because Label & Button use two different ways of drawing their text:

But 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_space
    • Setters/getters/methods

      • set_spacing()/get_spacing()
    • Enums

      • SpacingType with SPACING_CHAR, SPACING_SPACE
    • C++ 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.]

Was this page helpful?
0 / 5 - 0 ratings