Fix popup_centered not centered when size changed

[yvie-k]

When popup_centered was called while the size of the popup has changed (e.g. Label text got changed), it was calculating the position based on the old size.

This commit forces the size to be recalculated.

[akien-mga]

Sorry for the late review. It makes sense to force calculating the size IMO, but the reason I haven't merged this yet is that I'm not sure show() is the best way to trigger that, as it will already be called by popup() its call to show_modal().

popup() also already has some logic to fix the size, taking into account the provided bounds, so overall the proposed solution seems like a workaround for something that should likely be handled directly in popup().

[yvie-k]

Thx for reviewing.

The reason I used show()is that I couldn't find the actual function where the size is being calculated.

Do you know where the actual function is located? I'd be happy to contribute a better fix for this

[yvie-k]

I've taken another look into the codebase.

The _fix_size() logic only checks if the popup is outside the screen and adjusts it accordingly.

I don't think it's a good idea to attempt a fix in _fix_size(), because that would need to center the new expanded size around the current center of the popup. This would differ from the current behaviour.

I think doing a size recalculation before the calculations in popup_centered and popup_centered_ratio is the best way of handling that.

Another idea I just had is to do the popup first and then adjust the position of the popup.

[yvie-k]

After experimenting a bit, I realized that the size of a Label is only updated when it becomes visible.

Therefore calculating the new position after doing the popup doesn't work.

[akien-mga]

#19334 was supposed to have fixed this issue and similar ones, but this PR (and #29760) are evidence that it's still not working everywhere. CC @guilhermefelipecgs

[guilhermefelipecgs]

I didn't understood how to reproduce this. I created a popup with a label and before calling popup_centered I change the label size and the size of popup always opens correct for me.

@Dragoncraft89 Could you attach a demo project showing this behavior please?

[guilhermefelipecgs]

@Dragoncraft89 Never mind. I discovery how to reproduce this behavior.

[guilhermefelipecgs]

This problems happens because size is cached in Control node. So, to force an update, you need to find which function are not triggering minimum_size_changed (e.g. Label::set_text) and replace get_size to get_combined_minimum_size in Popup's methods.

Here is how I would fix popup_centered and Label::set_text:

diff --git a/scene/gui/label.cpp b/scene/gui/label.cpp
index e3e9368a1..584de3cb1 100644
--- a/scene/gui/label.cpp
+++ b/scene/gui/label.cpp
@@ -541,6 +541,8 @@ void Label::set_text(const String &p_string) {
        word_cache_dirty = true;
        if (percent_visible < 1)
                visible_chars = get_total_character_count() * percent_visible;
+
+       minimum_size_changed();
        update();
 }
 
diff --git a/scene/gui/popup.cpp b/scene/gui/popup.cpp
index fdb1b65f7..6ceaeb905 100644
--- a/scene/gui/popup.cpp
+++ b/scene/gui/popup.cpp
@@ -138,7 +138,7 @@ void Popup::popup_centered(const Size2 &p_size) {
 
        Rect2 rect;
        Size2 window_size = get_viewport_rect().size;
-       rect.size = p_size == Size2() ? get_size() : p_size;
+       rect.size = p_size == Size2() ? get_combined_minimum_size() : p_size;
        rect.position = ((window_size - rect.size) / 2.0).floor();
 
        popup(rect);

That's how I'm testing Label::set_text:

func _on_Button_pressed():
	$ConfirmationDialog/Label2.text = "12345678901234567890123456789012345678901234567890"
	$ConfirmationDialog.popup_centered()

The way this PR was made (using show()) doesn't fix the code above. (at least not for me)

[akien-mga]

Superseded by #30053.

Thanks for the contribution nevertheless, as it helped find out the better fix!