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

One-line Labels with word wrapping add a lot of vertical space when including white spaces #75402

Closed
ghost opened this issue Mar 27, 2023 · 10 comments

Comments

@ghost
Copy link

ghost commented Mar 27, 2023

Godot version

4.0.1

System information

Windows 10

Issue description

When having a VBoxContainer contain a Label (ProblematicLabel) with translated text and word wrapping, but the translated text is only one line, plus having an HBoxContainer after ProblematicLabel with two labels with translated texts (doesn't matter if they have word wrapping or not), a lot of vertical space is added to ProblematicLabel.

If you download the minimal reproduction project and run it, you'll see what I mean, and if you then change the TEST_TRANSLATION to TWOLINE_TRANSLATION, you'll see that when ProblematicLabel has two lines or more, the vertical space isn't added.

This was also a problem in Godot 4.0.0 and the Release Candidates I tried.

Steps to reproduce

It's very specific, at least the case I found, so it would be very hard to explain properly, check the minimal reproduction project.

Minimal reproduction project

LabelIssue.zip

@Calinou Calinou changed the title [Godot 4.0.1] One line lable translations with word wrapping add a lot of vertical space. One-line Label translations with word wrapping add a lot of vertical space Mar 27, 2023
@ghost
Copy link
Author

ghost commented Apr 6, 2023

Hello again, I've been testing and found some other examples.

If you remove all blank spaces from the translated text, " ", when using word wrapping mode, the vertical space won't be added, however, it will still be added when using arbitrary wrapping mode.

Also, when using word wrapping mode, it seems like the more blank spaces you add to the sentence, the bigger the added vertical space is, at least until a certain point.

Edit: I forgot to add, I tried printing the translated text to see if maybe the translation server was adding some unwanted characters, but it isn't, I also tried removing the HBoxContainer and added a ColorRect, and it still happened, so the HBoxContainer doesn't have to do with the problem. Nor the translation server, so I can only guess it has to do with the word wrapping.

@ghost
Copy link
Author

ghost commented Apr 6, 2023

Another find, it seems like the translation has nothing to do with it, having the label be a one liner with two spaces causes the same effect, so the translation part became irrelevant. I could swear I tried not using translation and it working fine before? But maybe I just didn't add the spaces when I tried it.

It definitely has to do with how the TextServer handles spaces when autowrapping is on, I tried poking around the source code to see if I could see the problem but since there's no comments I really can't find anything. It's something that will take me a while to figure out.

@ghost ghost changed the title One-line Label translations with word wrapping add a lot of vertical space One-line Labels with word wrapping add a lot of vertical space when including spaces Apr 6, 2023
@ghost ghost changed the title One-line Labels with word wrapping add a lot of vertical space when including spaces One-line Labels with word wrapping add a lot of vertical space when including white spaces Apr 6, 2023
@ghost
Copy link
Author

ghost commented Apr 11, 2023

Alright, I've been snooping around the source code again, and I found out that the "ascent" attribute used to calculate a label minsize, looks like the culprit, although I'm not entirely sure.

I tried commenting the ascent attribute in the text size formula
AscentCommented

and the vertical space disappeared, however, as expected, the position of all the texts is then messed up.
NoAscent

This is what it looks like, when the ascent attribute is used:
YesAscent

So, what does this mean? I have no clue. I tried figuring out how this is generated exacly, and it seems like it comes either from a glyph offset???? or the ascender attribute from the font? Which I have no idea how both of them are calculated and this code is a labyrinth without any comments. I would appreciate if someone that has worked in text server before can help me out or something. This seems like a very important issue to fix, considering it affects any one line labels with white spaces.

I will however continue to look around to see if I can somehow figure it out.

@ghost
Copy link
Author

ghost commented Apr 12, 2023

Another update, after playing around with ascent I didn't find anything conclusive, or anything that seemed wrong to me, so I went back to the line breaks that happen when word wrapping is active.

I wanted to check what was happening with the width of the label when it's _shape() function was being called and some weird things are going on, at least I think so.

Let's check what happens to the width when there are no white spaces:

WidthWithoutSpaces

I will assume that the multiple calls to _shape() is normal, and as the calls go on, the width end up being the actual width that it's supposed to be, this being 500, the width you can see in the editor.

Now let's take a look at what happens when there are spaces:

WidthWithSpaces

It's basically the same thing, however, right before the final call that gets the width right, there's a second call that gets width 1 again. And that second call only happens to the label with white spaces, since the ones that only have "Yes" and "No", get the same amount of calls.
I don't know if this is normal behavior, just wanted to point it out.

I then went to see what happened with the number of lines when having white spaces or not. But before going into that, I'll show where I'm getting the number from. It's from this function in the _shape() function of Label:

LinesLoop

In here, each line comes in 2, marking the first character of the line, and the last character of the line. So if line_breaks.size() is "2", that means there's only one line, if it's "4", it means there are two lines, etc.

Without white spaces:

NumberOfLinesNoSpaces

As expected, the size of line_breaks is "2", so there's only 1 line.

With white spaces:

NumberOfLinesWithSpaces

As you can see, the size of line_breaks becomes "6", which means there are three lines detected, which fits with the amount of vertical space being added if you check the pictures of my last comment.
And once again, even though we see that the last call correctly sets it back to "2", meaning only one line. There is a weird second call to _shape() right before that one.

So from this I kinda suspect that the vertical space is being maintained from that weird second to last call, and after the last one that correctly sets the lines, there's no "redraw" happening or whatever, which maybe means that this is only a visual bug from a lack of drawing the last update? It seems to be something along those lines, specially when considering what happens when a label with two lines is drawn:

TwoLineLineBreakSize

As we can see, at the end, there is a second _shape() call that correctly sets the number of lines to two. This is missing from the one line example.

Edit: Also, the size of line_breaks in this example go up to "22" (11 lines), which is the number of "words" separated by the ten white spaces present in the sentence, which seems to be consistent with the one line label behavior.

So I think it may come down to a lack of a final update of the label.

I'll try to see if I can find where this is happening, or rather, not happening.

@ghost
Copy link
Author

ghost commented Apr 14, 2023

Wellp, I'm convinced now that it has to do with a last update not occuring, because I've debugged and the final lines calculated are correct, the only difference is that a two or more lined label gets a last _shape() call from a resize function I don't remember the specific name of now.

As for why commenting the ascent variable fixed it, I believe that was just because it was getting rid of the ascent space of the extra lines and that's all, so it looked like it fixes it, but not really.

But yeah, if you set the custom minimum width of the label to the correct one before executing, the vertical space isn't added.

This pretty much tells me that what is happening is that the VBoxContainer is probably getting the minimum size from shape when the "minimum width" is 0 or 1, which leads to the height being that of "3" lines, so the VBoxContainer height is then set to that, and as a last step, it then sets it's own width to be the correct one, however, in the case of one line labels, this width set is not triggering the last _shape() call like it does for multi-line labels.

I'm on the verge of giving up. I feel like this is the closest I can get to the solution.

@ghost
Copy link
Author

ghost commented Apr 14, 2023

Yes, okay, 100% confirmed.
When I add a _shape() call to the resize notification, it works flawlesly.

newshape

However, I'm not sure this is the correct way of fixing it.

@Rindbee
Copy link
Contributor

Rindbee commented Apr 24, 2023

It seems that line_spacing is taken into account even when it's a single line, I'm not sure if this is expected.

0

The issue is related to autowrap_mode = AUTOWRAP_WORD, similar to #74052.

@ghost
Copy link
Author

ghost commented Apr 24, 2023

It seems that line_spacing is taken into account even when it's a single line, I'm not sure if this is expected.

That doesn't seem to happen in Godot 3, but don't know if it's a wanted change they did. In any case, seems to be an unrelated issue, as the issue I exposed never used any line_spacing, and it wouldn't make sense to be fixed with a call to _shape() on resize.

But seeing as how weird the GUI code is, who knows if it has something to do with that.

The issue 74052 is also fixed with the added _shape() call and as far as I know, also never used line_spacing if I recall correctly from when I tried it.

@Rindbee
Copy link
Contributor

Rindbee commented Apr 24, 2023

In the Label with the text "TEST_TRANSLATION", if the autowrap_mode is set to AUTOWRAP_OFF, the issue disappears.

So it's a duplicate of #74052.

@Calinou
Copy link
Member

Calinou commented May 24, 2023

@Calinou Calinou closed this as not planned Won't fix, can't repro, duplicate, stale May 24, 2023
@Calinou Calinou removed this from the 4.1 milestone May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants