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

Label + [autowrap] in VBoxContainer changes its size at startup (like line_spacing) #74052

Closed
FlankeR-46 opened this issue Feb 27, 2023 · 7 comments · Fixed by #77651 or #78009
Closed

Comments

@FlankeR-46
Copy link

Godot version

v4 RC3 - RC6

System information

Windows 10

Issue description

If a Label with any autowrap enabled is in the VBoxContainer, then depending on the type of autowrap, the Label size changes along the Y axis when it is first displayed (or the project is launched). As if at this moment the line_spacing parameter increases. If you change line_spacing through the code after the program is launched (the project file is attached), then everything immediately falls into place, and in all the Labels of this container.
I understand that this is not a great problem and it can be circumvented with one line of code, however, I spent several hours trying to figure out why my Popup window does not take the correct size at the time of the first call, and why everything falls into place after the second call. It turns out I had a VBoxContainer with Labels there, which behaved like this.
PS: changing the size of the main window, apparently, leads to a forced redrawing of the Labels, and also everything falls into place.
label_test

Steps to reproduce

image
The autowrap parameter for Labels should be different from OFF

Minimal reproduction project

The project is attached. By clicking on the buttons or changing the screen size, you can see this issue.
_labels_test.zip

@FlankeR-46
Copy link
Author

PPS: After downloading the project, you will see that the Y-dimensions of the VBoxContainers have changed in the same way as when you first started it. But if you change the size with your hands, they immediately take the right size.
image

@Rindbee
Copy link
Contributor

Rindbee commented Apr 24, 2023

godot/scene/gui/label.cpp

Lines 618 to 620 in f178cad

if (dirty || font_dirty || lines_dirty) {
const_cast<Label *>(this)->_shape();
}

int width = (get_size().width - style->get_minimum_size().width);

godot/scene/gui/label.cpp

Lines 155 to 158 in f178cad

if (xl_text.length() == 0) {
minsize = Size2(1, get_line_height());
return;
}

min_size.height = MAX(min_size.height, font->get_height(font_size) + font->get_spacing(TextServer::SPACING_TOP) + font->get_spacing(TextServer::SPACING_BOTTOM));

These auto/smart options will cause issues when calculating get_minimum_size(), height and width are dependent.

@hsandt
Copy link
Contributor

hsandt commented May 2, 2023

Confirmed with other containers such as the Panel Container, which makes the extra size obvious thanks to the background.
EDIT 2: confirmed even with non-container Control nodes, see EDIT 2 at the bottom

In addition, the number of words seems to matter in some cases, and reopening the scene can show yet another container size in the editor.

Hierarchy:
image

Appearance in editor with any number of words, no wrap, shows expected result:
image

Runtime is same. Closing and reopening the scene gives same result.

2 words, Arbitrary wrap gives the same result in editor, but at runtime, box is bigger vertically:
image

Closing and reopening the scene shows a panel even bigger:
image

If you play from here, the runtime will still be as before, not as big.

2 words, Word wrap shows expected result (note I closed and reopened scene to refresh the buggy view from Arbitrary autowrap)

3 words, Word wrap surprisingly triggers the bug again:
image

(the length of each word doesn't matter, only the count; as if each word was adding a ghost line)

Closing and reopening the scene will also show a bigger box, although not as big as arbitrary:
image

From here, every extra word will increase the box size after closing and reopening scene, as if adding a ghost line. However, runtime box won't get bigger.

Word (Smart) wrap is similar, but it triggers the bug at runtime even for 1 word, and after reopen scene, the box tends to be bigger.


EDIT: In addition, adding a newline with actual text, or more words until it autowraps to at least a second line, will fix the issue. So it really occurs for one line. This was already noticed in #75402

EDIT 2: it seems that even when parented to a non-container Control node, the bug happens. Setting the vertical alignment to Center, or better, Bottom, will make it obvious. It's just that it's harder to see without a background.

Or you can use the close/reopen scene trick and click on the node to see the rectangle:

Hierarchy:
image

Editor preview:
image

Editor preview after reopen scene:
image

Runtime:
image

@Rindbee
Copy link
Contributor

Rindbee commented May 27, 2023

After Control::_update_minimum_size() is called, data.last_minimum_size should be the latest get_combined_minimum_size(), but not for the cases here.

godot/scene/gui/control.cpp

Lines 1585 to 1592 in 2210111

Size2 minsize = get_combined_minimum_size();
data.updating_last_minimum_size = false;
if (minsize != data.last_minimum_size) {
data.last_minimum_size = minsize;
_size_changed();
emit_signal(SceneStringNames::get_singleton()->minimum_size_changed);
}

After the minimum_size_changed signal is emitted, the BoxContainer resort child nodes will cause the get_combined_minimum_size() of Lable to change. That is, data.last_minimum_size is out of date at this time.

@Rindbee
Copy link
Contributor

Rindbee commented Jun 1, 2023

If you observe the changes of various values, it is easier to find the cause of this issue.

godot/scene/gui/control.cpp

Lines 1588 to 1592 in 2210111

if (minsize != data.last_minimum_size) {
data.last_minimum_size = minsize;
_size_changed();
emit_signal(SceneStringNames::get_singleton()->minimum_size_changed);
}

notification(NOTIFICATION_RESIZED);

Add print code before and after comparison and before notification.

print_line(data.last_minimum_size, data.minimum_size_cache, data.minimum_size_valid, data.size_cache, this, "before/after comparison", minsize);

Compile, then use the compiled build to run the following test project via the command line.

$ godot  --path path/to/project

label-test.zip (please import the project in advance)

Output:

(0, 0) (0, 0) true (0, 0) Control:<Control#26541556875> before comparison (0, 0)
(0, 0) (0, 0) true (0, 0) Control:<Control#26541556875> after comparison (0, 0)
(0, 0) (0, 0) true (956, 1033) Control:<Control#26541556875> before NOTIFICATION_RESIZED
(0, 0) (1, 26) false (116, 26) VBoxContainer:<VBoxContainer#26575111309> before comparison (1, 26)
(1, 26) (1, 26) true (116, 26) VBoxContainer:<VBoxContainer#26575111309> after comparison (1, 26)
(0, 0) (1, 26) true (0, 0) Label:<Label#26608665747> before comparison (1, 26)
(1, 26) (1, 26) true (1, 26) Label:<Label#26608665747> before NOTIFICATION_RESIZED
[_on_minimum_size_changed] get_combined_minimum_size(): (1, 26) size: (1, 26)
(1, 26) (1, 26) true (1, 26) Label:<Label#26608665747> after comparison (1, 26)
(1, 26) (1, 52) false (1, 52) Label:<Label#26608665747> before NOTIFICATION_RESIZED
(1, 26) (1, 78) false (116, 78) VBoxContainer:<VBoxContainer#26575111309> before NOTIFICATION_RESIZED
(1, 26) (1, 78) true (1, 78) Label:<Label#26608665747> before NOTIFICATION_RESIZED
(0, 0) (0, 0) true (956, 1033) Control:<Control#26541556875> before comparison (0, 0)
(0, 0) (0, 0) true (956, 1033) Control:<Control#26541556875> after comparison (0, 0)
(1, 26) (1, 78) true (116, 78) VBoxContainer:<VBoxContainer#26575111309> before comparison (1, 78)
(1, 78) (1, 78) true (116, 78) VBoxContainer:<VBoxContainer#26575111309> after comparison (1, 78)
(1, 26) (1, 78) true (116, 78) Label:<Label#26608665747> before NOTIFICATION_RESIZED
(1, 78) (1, 78) true (116, 78) VBoxContainer:<VBoxContainer#26575111309> before comparison (1, 78)
(1, 78) (1, 78) true (116, 78) VBoxContainer:<VBoxContainer#26575111309> after comparison (1, 78)
(1, 26) (1, 26) false (116, 78) Label:<Label#26608665747> before comparison (1, 26)
(1, 26) (1, 26) false (116, 78) Label:<Label#26608665747> after comparison (1, 26)

For comparison, the output in #77651:

(0, 0) (0, 0) true (0, 0) Control:<Control#26541556875> before comparison (0, 0)
(0, 0) (0, 0) true (0, 0) Control:<Control#26541556875> after comparison (0, 0)
(0, 0) (1, 26) true (116, 26) VBoxContainer:<VBoxContainer#26575111309> before comparison (1, 26)
(1, 26) (1, 26) true (116, 26) VBoxContainer:<VBoxContainer#26575111309> after comparison (1, 26)
(0, 0) (1, 26) true (0, 0) Label:<Label#26608665747> before comparison (1, 26)
(1, 26) (1, 26) true (1, 26) Label:<Label#26608665747> before NOTIFICATION_RESIZED
[_on_minimum_size_changed] get_combined_minimum_size(): (1, 26) size: (1, 26)
(1, 26) (1, 26) true (1, 26) Label:<Label#26608665747> after comparison (1, 26)
(0, 0) (0, 0) true (956, 1033) Control:<Control#26541556875> before NOTIFICATION_RESIZED
(0, 0) (0, 0) true (956, 1033) Control:<Control#26541556875> before comparison (0, 0)
(0, 0) (0, 0) true (956, 1033) Control:<Control#26541556875> after comparison (0, 0)
(1, 26) (1, 26) true (116, 26) VBoxContainer:<VBoxContainer#26575111309> before comparison (1, 26)
(1, 26) (1, 26) true (116, 26) VBoxContainer:<VBoxContainer#26575111309> after comparison (1, 26)
(1, 26) (1, 26) true (116, 26) Label:<Label#26608665747> before NOTIFICATION_RESIZED
(1, 26) (1, 26) true (116, 26) VBoxContainer:<VBoxContainer#26575111309> before comparison (1, 26)
(1, 26) (1, 26) true (116, 26) VBoxContainer:<VBoxContainer#26575111309> after comparison (1, 26)
(1, 26) (1, 26) true (116, 26) Label:<Label#26608665747> before comparison (1, 26)
(1, 26) (1, 26) true (116, 26) Label:<Label#26608665747> after comparison (1, 26)

@Rindbee
Copy link
Contributor

Rindbee commented Jun 12, 2023

If the value changes are described in the order of time, the changes are shown in the following table, this is the value change during two consecutive _update_minimum_size() calls of the child control:

time data.last_minimum_size data.minimum_size_cache explanation
Ti V0 => Vi (changed) Vi in _update_minimum_size() (child)
... Vi ... ...
Tj Vi Vj ...
... Vi ... ...
Tk Vi Vk in _update_minimum_size() (parent)
... Vi ... ...
Tm Vi Vm ...
... Vi ... ...
Tn Vi => Vi (no changed) Vm => Vi in _update_minimum_size() (child)

There are two key points:

  1. Not sure which value is used when the parent container updates the minimum size;
  2. At Tn, the minimum_size_changed signal is not emitted because the values are equal (data.last_minimum_size == data.minimum_size_cache). So the parent container is not updating its minimum size with the correct child minimum size again.

This issue happened earlier, #78009 ensured the correct size early by _size_changed().

My suggestion is to emit the minimum_size_changed signal at time Tn since data.minimum_size_cache ever changed.

I'm worried about something that might be similar, but that happens at other times, since _size_changed() isn't a public method after all, or maybe I'm overly worried.

@YuriSizov
Copy link
Contributor

YuriSizov commented Jun 12, 2023

since _size_changed() isn't a public method after all

I considered making it accessible publicly, so other classes could request a check on the actual size when they expect there might be a change. But I couldn't really come up with a case where some control would cause changes in its actual size but not its minimum size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment