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.

Sign up for GitHub

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

Fix PopupPanel and PopupMenu menu styles #96518

Merged
merged 1 commit into from
Oct 24, 2024
Merged

Fix PopupPanel and PopupMenu menu styles #96518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion editor/plugins/control_editor_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,6 @@ ControlEditorPopupButton::ControlEditorPopupButton() {
set_focus_mode(FOCUS_NONE);

popup_panel = memnew(PopupPanel);
popup_panel->set_theme_type_variation("ControlEditorPopupPanel");
add_child(popup_panel);
popup_panel->connect("about_to_popup", callable_mp(this, &ControlEditorPopupButton::_popup_visibility_changed).bind(true));
popup_panel->connect("popup_hide", callable_mp(this, &ControlEditorPopupButton::_popup_visibility_changed).bind(false));
Expand Down
46 changes: 17 additions & 29 deletions editor/themes/editor_theme_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,16 @@ void EditorThemeManager::_create_shared_styles(const Ref<EditorTheme> &p_theme,
// in 4.0, and even if it was, it may not always work in practice (e.g. running with compositing disabled).
p_config.popup_style->set_corner_radius_all(0);

p_config.popup_border_style = p_config.popup_style->duplicate();
p_config.popup_border_style->set_content_margin_all(MAX(Math::round(EDSCALE), p_config.border_width) + 2 + (p_config.base_margin * 1.5) * EDSCALE);
// Always display a border for popups like PopupMenus so they can be distinguished from their background.
p_config.popup_border_style->set_border_width_all(MAX(Math::round(EDSCALE), p_config.border_width));
if (p_config.draw_extra_borders) {
p_config.popup_border_style->set_border_color(p_config.extra_border_color_2);
} else {
p_config.popup_border_style->set_border_color(p_config.dark_color_2);
}

p_config.window_style = p_config.popup_style->duplicate();
p_config.window_style->set_border_color(p_config.base_color);
p_config.window_style->set_border_width(SIDE_TOP, 24 * EDSCALE);
Expand Down Expand Up @@ -707,7 +717,7 @@ void EditorThemeManager::_populate_standard_styles(const Ref<EditorTheme> &p_the
}

// PopupPanel
p_theme->set_stylebox(SceneStringName(panel), "PopupPanel", p_config.popup_style);
p_theme->set_stylebox(SceneStringName(panel), "PopupPanel", p_config.popup_border_style);
}

// Buttons.
Expand Down Expand Up @@ -1310,18 +1320,11 @@ void EditorThemeManager::_populate_standard_styles(const Ref<EditorTheme> &p_the

// PopupMenu.
{
Ref<StyleBoxFlat> style_popup_menu = p_config.popup_style->duplicate();
Ref<StyleBoxFlat> style_popup_menu = p_config.popup_border_style->duplicate();
// Use 1 pixel for the sides, since if 0 is used, the highlight of hovered items is drawn
// on top of the popup border. This causes a 'gap' in the panel border when an item is highlighted,
// and it looks weird. 1px solves this.
style_popup_menu->set_content_margin_individual(EDSCALE, 2 * EDSCALE, EDSCALE, 2 * EDSCALE);
// Always display a border for PopupMenus so they can be distinguished from their background.
style_popup_menu->set_border_width_all(EDSCALE);
if (p_config.draw_extra_borders) {
style_popup_menu->set_border_color(p_config.extra_border_color_2);
} else {
style_popup_menu->set_border_color(p_config.dark_color_2);
}
style_popup_menu->set_content_margin_individual(Math::round(EDSCALE), 2 * EDSCALE, Math::round(EDSCALE), 2 * EDSCALE);
p_theme->set_stylebox(SceneStringName(panel), "PopupMenu", style_popup_menu);

Ref<StyleBoxFlat> style_menu_hover = p_config.button_style_hover->duplicate();
Expand All @@ -1331,17 +1334,17 @@ void EditorThemeManager::_populate_standard_styles(const Ref<EditorTheme> &p_the

Ref<StyleBoxLine> style_popup_separator(memnew(StyleBoxLine));
style_popup_separator->set_color(p_config.separator_color);
style_popup_separator->set_grow_begin(p_config.popup_margin - MAX(Math::round(EDSCALE), p_config.border_width));
style_popup_separator->set_grow_end(p_config.popup_margin - MAX(Math::round(EDSCALE), p_config.border_width));
style_popup_separator->set_grow_begin(Math::round(EDSCALE) - MAX(Math::round(EDSCALE), p_config.border_width));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks sus. There is EDSCALE on both sides, so this has high chance of being 0. Though not sure what's the expected value here.

style_popup_separator->set_grow_end(Math::round(EDSCALE) - MAX(Math::round(EDSCALE), p_config.border_width));
style_popup_separator->set_thickness(MAX(Math::round(EDSCALE), p_config.border_width));

Ref<StyleBoxLine> style_popup_labeled_separator_left(memnew(StyleBoxLine));
style_popup_labeled_separator_left->set_grow_begin(p_config.popup_margin - MAX(Math::round(EDSCALE), p_config.border_width));
style_popup_labeled_separator_left->set_grow_begin(Math::round(EDSCALE) - MAX(Math::round(EDSCALE), p_config.border_width));
style_popup_labeled_separator_left->set_color(p_config.separator_color);
style_popup_labeled_separator_left->set_thickness(MAX(Math::round(EDSCALE), p_config.border_width));

Ref<StyleBoxLine> style_popup_labeled_separator_right(memnew(StyleBoxLine));
style_popup_labeled_separator_right->set_grow_end(p_config.popup_margin - MAX(Math::round(EDSCALE), p_config.border_width));
style_popup_labeled_separator_right->set_grow_end(Math::round(EDSCALE) - MAX(Math::round(EDSCALE), p_config.border_width));
style_popup_labeled_separator_right->set_color(p_config.separator_color);
style_popup_labeled_separator_right->set_thickness(MAX(Math::round(EDSCALE), p_config.border_width));

Expand Down Expand Up @@ -2114,21 +2117,6 @@ void EditorThemeManager::_populate_editor_styles(const Ref<EditorTheme> &p_theme

// EditorValidationPanel.
p_theme->set_stylebox(SceneStringName(panel), "EditorValidationPanel", p_config.tree_panel_style);

// ControlEditor.
{
p_theme->set_type_variation("ControlEditorPopupPanel", "PopupPanel");

Ref<StyleBoxFlat> control_editor_popup_style = p_config.popup_style->duplicate();
control_editor_popup_style->set_shadow_size(0);
control_editor_popup_style->set_content_margin(SIDE_LEFT, p_config.base_margin * EDSCALE);
control_editor_popup_style->set_content_margin(SIDE_TOP, p_config.base_margin * EDSCALE);
control_editor_popup_style->set_content_margin(SIDE_RIGHT, p_config.base_margin * EDSCALE);
control_editor_popup_style->set_content_margin(SIDE_BOTTOM, p_config.base_margin * EDSCALE);
control_editor_popup_style->set_border_width_all(0);

p_theme->set_stylebox(SceneStringName(panel), "ControlEditorPopupPanel", control_editor_popup_style);
}
}

// Editor inspector.
Expand Down
1 change: 1 addition & 0 deletions editor/themes/editor_theme_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ class EditorThemeManager {
Ref<StyleBoxFlat> button_style_hover;

Ref<StyleBoxFlat> popup_style;
Ref<StyleBoxFlat> popup_border_style;
Ref<StyleBoxFlat> window_style;
Ref<StyleBoxFlat> dialog_style;
Ref<StyleBoxFlat> panel_container_style;
Expand Down
7 changes: 1 addition & 6 deletions editor/window_wrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,6 @@ void ScreenSelect::_notification(int p_what) {
} break;
case NOTIFICATION_THEME_CHANGED: {
set_icon(get_editor_theme_icon("MakeFloating"));
popup_background->add_theme_style_override(SceneStringName(panel), get_theme_stylebox("PanelForeground", EditorStringName(EditorStyles)));

const real_t popup_height = real_t(get_theme_font_size(SceneStringName(font_size))) * 2.0;
popup->set_min_size(Size2(0, popup_height * 3));
Expand Down Expand Up @@ -454,14 +453,10 @@ ScreenSelect::ScreenSelect() {
// Create the popup.
const Size2 borders = Size2(4, 4) * EDSCALE;

popup = memnew(Popup);
popup = memnew(PopupPanel);
popup->connect("popup_hide", callable_mp(static_cast<BaseButton *>(this), &ScreenSelect::set_pressed).bind(false));
add_child(popup);

popup_background = memnew(Panel);
popup_background->set_anchors_and_offsets_preset(PRESET_FULL_RECT);
popup->add_child(popup_background);

MarginContainer *popup_root = memnew(MarginContainer);
popup_root->add_theme_constant_override("margin_right", borders.width);
popup_root->add_theme_constant_override("margin_top", borders.height);
Expand Down
1 change: 0 additions & 1 deletion editor/window_wrapper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ class ScreenSelect : public Button {
GDCLASS(ScreenSelect, Button);

Popup *popup = nullptr;
Panel *popup_background = nullptr;
HBoxContainer *screen_list = nullptr;

void _build_advanced_menu();
Expand Down
Loading