PopupMenu visual issues in 3.6 RC1

[Alternalo]

Tested versions

• Reproducible in 3.6 RC1
• Not reproducible in 3.5.2 Stable

System information

Windows 10

Issue description

Simply adding a PopupMenu to the scene with some items will result in this look in 3.5.2:

Doing the same thing in 3.6 RC1 will make it look like this

And applying separators on items in 3.6 will make it look like this

Steps to reproduce

• Create a new UI scene
• Add a PopupMenu node to the scene
• Add items to the Popupmenu node
• Make the node visible, so you can see the issue

Minimal reproduction project (MRP)

PopupIssue.zip

[lawnjelly]

Haven't bisected, but likely related to:
#66711
@rsubtil

[akien-mga]

Haven't bisected, but likely related to: #66711 @rsubtil

Indeed, I've confirmed that reverting that PR (and the follow-ups which fixed regressions from that PR) would solve this issue. Branch with all relevant changes reverted: https://github.com/akien-mga/godot/commits/revert-66711/

That's one option, but a heavy handed one at this stage, it would be good to at least get the issue debugged a bit to see if the problem is a fundamental one that will require e.g. breaking the behavior again, or an oversight that can be patched easily.

[rsubtil]

I can have a quick look in the coming days, but if the issue is not simple, I don't think I'll have the availability to fix it. I'll leave any findings if that's the case, maybe someone can pick up from there.

[timothyqiu]

Moving the four add_constant_override() calls to NOTIFICATION_VISIBILITY_CHANGED (the same as the original #41640) seems to solve the issue.

godot/scene/gui/popup_menu.cpp

Lines 646 to 655 in 67d4b73

	case NOTIFICATION_POST_POPUP: {
	initial_button_mask = Input::get_singleton()->get_mouse_button_mask();
	during_grabbed_click = (bool)initial_button_mask;
	// Set margin on the margin container
	Ref<StyleBox> panel_style = get_stylebox("panel");
	margin_container->add_constant_override("margin_top", panel_style->get_margin(Margin::MARGIN_TOP));
	margin_container->add_constant_override("margin_bottom", panel_style->get_margin(Margin::MARGIN_BOTTOM));
	margin_container->add_constant_override("margin_left", panel_style->get_margin(Margin::MARGIN_LEFT));
	margin_container->add_constant_override("margin_right", panel_style->get_margin(Margin::MARGIN_RIGHT));
	} break;

Note that there is also a later PR that removes the need for a separate MarginContainer on the master branch.

[lawnjelly]

It looks like the fix #87462 itself may introduce more regressions.
This illustrates the pitfalls of cherry picking from 4.x in areas such as GUI, because of the entangled dependencies.

I guess we must also consider @akien-mga 's test approach of reverting all the popup menu related PRs to 3.5 state.

[lawnjelly]

Fixed by #96557 .