From 7884d632812996ef26184412846c533276cb4cc0 Mon Sep 17 00:00:00 2001 From: Mounir Tohami <53877170+WhalesState@users.noreply.github.com> Date: Fri, 23 Feb 2024 15:51:47 +0200 Subject: [PATCH] Fix PopupMenu doesn't respect it's ScrollContainer's margins --- editor/editor_command_palette.cpp | 1 + editor/editor_layouts_dialog.cpp | 1 + editor/export/project_export.cpp | 1 + editor/gui/editor_object_selector.cpp | 1 + .../plugins/animation_tree_editor_plugin.cpp | 1 + editor/plugins/font_config_plugin.cpp | 1 + editor/plugins/text_shader_editor.h | 1 + scene/gui/color_picker.cpp | 1 + scene/gui/popup_menu.cpp | 134 ++++++++---------- scene/gui/popup_menu.h | 5 +- 10 files changed, 68 insertions(+), 79 deletions(-) diff --git a/editor/editor_command_palette.cpp b/editor/editor_command_palette.cpp index d740641f570..b963330fdc0 100644 --- a/editor/editor_command_palette.cpp +++ b/editor/editor_command_palette.cpp @@ -36,6 +36,7 @@ #include "editor/gui/editor_toaster.h" #include "editor/themes/editor_scale.h" #include "scene/gui/control.h" +#include "scene/gui/margin_container.h" #include "scene/gui/tree.h" EditorCommandPalette *EditorCommandPalette::singleton = nullptr; diff --git a/editor/editor_layouts_dialog.cpp b/editor/editor_layouts_dialog.cpp index bd9a4ea1bff..db6fa2c0359 100644 --- a/editor/editor_layouts_dialog.cpp +++ b/editor/editor_layouts_dialog.cpp @@ -37,6 +37,7 @@ #include "editor/themes/editor_scale.h" #include "scene/gui/item_list.h" #include "scene/gui/line_edit.h" +#include "scene/gui/margin_container.h" void EditorLayoutsDialog::_line_gui_input(const Ref &p_event) { Ref k = p_event; diff --git a/editor/export/project_export.cpp b/editor/export/project_export.cpp index 7088d4e2abb..223900e8b30 100644 --- a/editor/export/project_export.cpp +++ b/editor/export/project_export.cpp @@ -45,6 +45,7 @@ #include "scene/gui/check_button.h" #include "scene/gui/item_list.h" #include "scene/gui/link_button.h" +#include "scene/gui/margin_container.h" #include "scene/gui/menu_button.h" #include "scene/gui/option_button.h" #include "scene/gui/popup_menu.h" diff --git a/editor/gui/editor_object_selector.cpp b/editor/gui/editor_object_selector.cpp index fd46788c738..862ecc0a68f 100644 --- a/editor/gui/editor_object_selector.cpp +++ b/editor/gui/editor_object_selector.cpp @@ -35,6 +35,7 @@ #include "editor/editor_string_names.h" #include "editor/multi_node_edit.h" #include "editor/themes/editor_scale.h" +#include "scene/gui/margin_container.h" Size2 EditorObjectSelector::get_minimum_size() const { Ref font = get_theme_font(SNAME("font")); diff --git a/editor/plugins/animation_tree_editor_plugin.cpp b/editor/plugins/animation_tree_editor_plugin.cpp index 7269395baf7..a2bbf313ca6 100644 --- a/editor/plugins/animation_tree_editor_plugin.cpp +++ b/editor/plugins/animation_tree_editor_plugin.cpp @@ -45,6 +45,7 @@ #include "scene/animation/animation_blend_tree.h" #include "scene/animation/animation_player.h" #include "scene/gui/button.h" +#include "scene/gui/margin_container.h" #include "scene/gui/menu_button.h" #include "scene/gui/panel.h" #include "scene/gui/scroll_container.h" diff --git a/editor/plugins/font_config_plugin.cpp b/editor/plugins/font_config_plugin.cpp index b71a11e166b..f9b5e280c9b 100644 --- a/editor/plugins/font_config_plugin.cpp +++ b/editor/plugins/font_config_plugin.cpp @@ -33,6 +33,7 @@ #include "editor/editor_settings.h" #include "editor/import/dynamic_font_import_settings.h" #include "editor/themes/editor_scale.h" +#include "scene/gui/margin_container.h" /*************************************************************************/ /* EditorPropertyFontMetaObject */ diff --git a/editor/plugins/text_shader_editor.h b/editor/plugins/text_shader_editor.h index a1b45d1b2e7..73d7de98e4c 100644 --- a/editor/plugins/text_shader_editor.h +++ b/editor/plugins/text_shader_editor.h @@ -32,6 +32,7 @@ #define TEXT_SHADER_EDITOR_H #include "editor/code_editor.h" +#include "scene/gui/margin_container.h" #include "scene/gui/menu_button.h" #include "scene/gui/panel_container.h" #include "scene/gui/rich_text_label.h" diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index 9d245959169..cdb091cb32d 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -36,6 +36,7 @@ #include "core/os/keyboard.h" #include "core/os/os.h" #include "scene/gui/color_mode.h" +#include "scene/gui/margin_container.h" #include "scene/resources/image_texture.h" #include "scene/resources/style_box_flat.h" #include "scene/resources/style_box_texture.h" diff --git a/scene/gui/popup_menu.cpp b/scene/gui/popup_menu.cpp index e69f7137986..42100bf9faf 100644 --- a/scene/gui/popup_menu.cpp +++ b/scene/gui/popup_menu.cpp @@ -213,8 +213,8 @@ Size2 PopupMenu::_get_item_icon_size(int p_idx) const { } Size2 PopupMenu::_get_contents_minimum_size() const { - Size2 minsize = theme_cache.panel_style->get_minimum_size(); // Accounts for margin in the margin container - minsize.x += scroll_container->get_v_scroll_bar()->get_size().width * 2; // Adds a buffer so that the scrollbar does not render over the top of content + Size2 minsize = theme_cache.panel_style->get_minimum_size(); + minsize.width += scroll_container->get_v_scroll_bar()->get_size().width; float max_w = 0.0; float icon_w = 0.0; @@ -304,16 +304,11 @@ int PopupMenu::_get_items_total_height() const { } int PopupMenu::_get_mouse_over(const Point2 &p_over) const { - if (p_over.x < 0 || p_over.x >= get_size().width) { + if (p_over.x < 0 || p_over.x >= get_size().width || p_over.y < theme_cache.panel_style->get_margin(Side::SIDE_TOP)) { return -1; } - // Accounts for margin in the margin container - Point2 ofs = theme_cache.panel_style->get_offset(); - - if (ofs.y > p_over.y) { - return -1; - } + Point2 ofs; for (int i = 0; i < items.size(); i++) { ofs.y += theme_cache.v_separation; @@ -570,32 +565,39 @@ void PopupMenu::_input_from_window_internal(const Ref &p_event) { if (scroll_container->get_v_scroll_bar()->is_visible_in_tree()) { if (is_layout_rtl()) { item_clickable_area.position.x += scroll_container->get_v_scroll_bar()->get_size().width; - } else { - item_clickable_area.size.width -= scroll_container->get_v_scroll_bar()->get_size().width; } + item_clickable_area.size.width -= scroll_container->get_v_scroll_bar()->get_size().width; } Ref b = p_event; if (b.is_valid()) { - if (!item_clickable_area.has_point(b->get_position())) { - return; - } - MouseButton button_idx = b->get_button_index(); - if (!b->is_pressed()) { - // Activate the item on release of either the left mouse button or - // any mouse button held down when the popup was opened. - // This allows for opening the popup and triggering an action in a single mouse click. - if (button_idx == MouseButton::LEFT || initial_button_mask.has_flag(mouse_button_to_mask(button_idx))) { + // Activate the item on release of either the left mouse button or + // any mouse button held down when the popup was opened. + // This allows for opening the popup and triggering an action in a single mouse click. + if (button_idx == MouseButton::LEFT || initial_button_mask.has_flag(mouse_button_to_mask(button_idx))) { + if (b->is_pressed()) { + is_scrolling = is_layout_rtl() ? b->get_position().x < item_clickable_area.position.x : b->get_position().x > item_clickable_area.size.width; + + if (!item_clickable_area.has_point(b->get_position())) { + return; + } + _mouse_over_update(b->get_position()); + } else { + if (is_scrolling) { + is_scrolling = false; + return; + } bool was_during_grabbed_click = during_grabbed_click; during_grabbed_click = false; initial_button_mask.clear(); + if (!item_clickable_area.has_point(b->get_position())) { + return; + } // Disable clicks under a time threshold to avoid selection right when opening the popup. - uint64_t now = OS::get_singleton()->get_ticks_msec(); - uint64_t diff = now - popup_time_msec; - if (diff < 150) { + if (was_during_grabbed_click && OS::get_singleton()->get_ticks_msec() - popup_time_msec < 150) { return; } @@ -638,25 +640,7 @@ void PopupMenu::_input_from_window_internal(const Ref &p_event) { if (!item_clickable_area.has_point(m->get_position())) { return; } - - int over = _get_mouse_over(m->get_position()); - int id = (over < 0 || items[over].separator || items[over].disabled) ? -1 : (items[over].id >= 0 ? items[over].id : over); - - if (id < 0) { - mouse_over = -1; - control->queue_redraw(); - return; - } - - if (items[over].submenu && submenu_over != over) { - submenu_over = over; - submenu_timer->start(); - } - - if (over != mouse_over) { - mouse_over = over; - control->queue_redraw(); - } + _mouse_over_update(m->get_position()); } Ref k = p_event; @@ -700,14 +684,31 @@ void PopupMenu::_input_from_window_internal(const Ref &p_event) { } } +void PopupMenu::_mouse_over_update(const Point2 &p_over) { + int over = _get_mouse_over(p_over); + int id = (over < 0 || items[over].separator || items[over].disabled) ? -1 : (items[over].id >= 0 ? items[over].id : over); + + if (id < 0) { + mouse_over = -1; + control->queue_redraw(); + return; + } + + if (!is_scrolling && items[over].submenu && submenu_over != over) { + submenu_over = over; + submenu_timer->start(); + } + + if (over != mouse_over) { + mouse_over = over; + control->queue_redraw(); + } +} + void PopupMenu::_draw_items() { control->set_custom_minimum_size(Size2(0, _get_items_total_height())); RID ci = control->get_canvas_item(); - Size2 margin_size; - margin_size.width = margin_container->get_margin_size(SIDE_LEFT) + margin_container->get_margin_size(SIDE_RIGHT); - margin_size.height = margin_container->get_margin_size(SIDE_TOP) + margin_container->get_margin_size(SIDE_BOTTOM); - // Space between the item content and the sides of popup menu. bool rtl = control->is_layout_rtl(); // In Item::checkable_type enum order (less the non-checkable member), with disabled repeated at the end. @@ -720,8 +721,7 @@ void PopupMenu::_draw_items() { submenu = theme_cache.submenu; } - float scroll_width = scroll_container->get_v_scroll_bar()->is_visible_in_tree() ? scroll_container->get_v_scroll_bar()->get_size().width : 0; - float display_width = control->get_size().width - scroll_width; + float display_width = control->get_size().width; // Find the widest icon and whether any items have a checkbox, and store the offsets for each. float icon_ofs = 0.0; @@ -765,11 +765,7 @@ void PopupMenu::_draw_items() { float h = _get_item_height(i); if (i == mouse_over) { - if (rtl) { - theme_cache.hover_style->draw(ci, Rect2(item_ofs + Point2(scroll_width, -theme_cache.v_separation / 2), Size2(display_width, h + theme_cache.v_separation))); - } else { - theme_cache.hover_style->draw(ci, Rect2(item_ofs + Point2(0, -theme_cache.v_separation / 2), Size2(display_width, h + theme_cache.v_separation))); - } + theme_cache.hover_style->draw(ci, Rect2(item_ofs + Point2(0, -theme_cache.v_separation / 2), Size2(display_width, h + theme_cache.v_separation))); } String text = items[i].xl_text; @@ -851,7 +847,7 @@ void PopupMenu::_draw_items() { // Submenu arrow on right hand side. if (items[i].submenu) { if (rtl) { - submenu->draw(ci, Point2(scroll_width + theme_cache.panel_style->get_margin(SIDE_LEFT) + theme_cache.item_end_padding, item_ofs.y + Math::floor(h - submenu->get_height()) / 2), icon_color); + submenu->draw(ci, Point2(theme_cache.panel_style->get_margin(SIDE_LEFT) + theme_cache.item_end_padding, item_ofs.y + Math::floor(h - submenu->get_height()) / 2), icon_color); } else { submenu->draw(ci, Point2(display_width - theme_cache.panel_style->get_margin(SIDE_RIGHT) - submenu->get_width() - theme_cache.item_end_padding, item_ofs.y + Math::floor(h - submenu->get_height()) / 2), icon_color); } @@ -888,7 +884,7 @@ void PopupMenu::_draw_items() { // Accelerator / Shortcut if (items[i].accel != Key::NONE || (items[i].shortcut.is_valid() && items[i].shortcut->has_valid_event())) { if (rtl) { - item_ofs.x = scroll_width + theme_cache.panel_style->get_margin(SIDE_LEFT) + theme_cache.item_end_padding; + item_ofs.x = theme_cache.panel_style->get_margin(SIDE_LEFT) + theme_cache.item_end_padding; } else { item_ofs.x = display_width - theme_cache.panel_style->get_margin(SIDE_RIGHT) - items[i].accel_text_buf->get_size().x - theme_cache.item_end_padding; } @@ -907,11 +903,6 @@ void PopupMenu::_draw_items() { } } -void PopupMenu::_draw_background() { - RID ci2 = margin_container->get_canvas_item(); - theme_cache.panel_style->draw(ci2, Rect2(Point2(), margin_container->get_size())); -} - void PopupMenu::_minimum_lifetime_timeout() { close_allowed = true; // If the mouse still isn't in this popup after timer expires, close. @@ -1023,7 +1014,11 @@ void PopupMenu::_notification(int p_what) { } } break; - case NOTIFICATION_THEME_CHANGED: + case NOTIFICATION_THEME_CHANGED: { + scroll_container->add_theme_style_override("panel", theme_cache.panel_style); + + [[fallthrough]]; + } case Control::NOTIFICATION_LAYOUT_DIRECTION_CHANGED: case NOTIFICATION_TRANSLATION_CHANGED: { DisplayServer *ds = DisplayServer::get_singleton(); @@ -1169,14 +1164,6 @@ void PopupMenu::_notification(int p_what) { if (!is_embedded()) { set_process_internal(true); } - - // Set margin on the margin container - margin_container->begin_bulk_theme_override(); - margin_container->add_theme_constant_override("margin_left", theme_cache.panel_style->get_margin(Side::SIDE_LEFT)); - margin_container->add_theme_constant_override("margin_top", theme_cache.panel_style->get_margin(Side::SIDE_TOP)); - margin_container->add_theme_constant_override("margin_right", theme_cache.panel_style->get_margin(Side::SIDE_RIGHT)); - margin_container->add_theme_constant_override("margin_bottom", theme_cache.panel_style->get_margin(Side::SIDE_BOTTOM)); - margin_container->end_bulk_theme_override(); } } break; } @@ -2800,16 +2787,11 @@ void PopupMenu::popup(const Rect2i &p_bounds) { } PopupMenu::PopupMenu() { - // Margin Container - margin_container = memnew(MarginContainer); - margin_container->set_anchors_and_offsets_preset(Control::PRESET_FULL_RECT); - add_child(margin_container, false, INTERNAL_MODE_FRONT); - margin_container->connect("draw", callable_mp(this, &PopupMenu::_draw_background)); - // Scroll Container scroll_container = memnew(ScrollContainer); + scroll_container->set_anchors_and_offsets_preset(Control::PRESET_FULL_RECT); scroll_container->set_clip_contents(true); - margin_container->add_child(scroll_container); + add_child(scroll_container, false, INTERNAL_MODE_FRONT); // The control which will display the items control = memnew(Control); diff --git a/scene/gui/popup_menu.h b/scene/gui/popup_menu.h index 9f4d28e95d9..492084b43ff 100644 --- a/scene/gui/popup_menu.h +++ b/scene/gui/popup_menu.h @@ -32,7 +32,6 @@ #define POPUP_MENU_H #include "core/input/shortcut.h" -#include "scene/gui/margin_container.h" #include "scene/gui/popup.h" #include "scene/gui/scroll_container.h" #include "scene/property_list_helper.h" @@ -110,10 +109,12 @@ class PopupMenu : public Popup { Vector items; BitField initial_button_mask; bool during_grabbed_click = false; + bool is_scrolling = false; int mouse_over = -1; int submenu_over = -1; String _get_accel_text(const Item &p_item) const; int _get_mouse_over(const Point2 &p_over) const; + void _mouse_over_update(const Point2 &p_over); virtual Size2 _get_contents_minimum_size() const override; int _get_item_height(int p_idx) const; @@ -142,7 +143,6 @@ class PopupMenu : public Popup { uint64_t search_time_msec = 0; String search_string = ""; - MarginContainer *margin_container = nullptr; ScrollContainer *scroll_container = nullptr; Control *control = nullptr; @@ -195,7 +195,6 @@ class PopupMenu : public Popup { } theme_cache; void _draw_items(); - void _draw_background(); void _minimum_lifetime_timeout(); void _close_pressed();