From e1384dd1596155704e277e8f353d2e80e10adea2 Mon Sep 17 00:00:00 2001 From: Pablo Andres Fuente Date: Tue, 18 Mar 2025 00:02:38 +0100 Subject: [PATCH] Fix `ScrollContainer` focus border issue Fixes #100176 Instead of using no clipping technique to draw the focus border because it causes problems if a `ScrollContainer` is nested in another `ScrollContainer`, now the focus border is drawn using an internal `PanelContainer`. --- editor/editor_inspector.cpp | 16 ---- editor/editor_inspector.h | 2 - editor/themes/editor_theme_manager.cpp | 8 +- scene/gui/scroll_container.cpp | 112 ++++++++++++++++++++----- scene/gui/scroll_container.h | 3 + 5 files changed, 98 insertions(+), 43 deletions(-) diff --git a/editor/editor_inspector.cpp b/editor/editor_inspector.cpp index 0fbb74939cc..54a25c62438 100644 --- a/editor/editor_inspector.cpp +++ b/editor/editor_inspector.cpp @@ -4956,12 +4956,6 @@ void EditorInspector::_clear_current_favorites() { update_tree(); } -void EditorInspector::_update_theme() { - updating_theme = true; - add_theme_style_override(SceneStringName(panel), get_theme_stylebox(SceneStringName(panel), SNAME("Tree"))); - updating_theme = false; -} - void EditorInspector::_notification(int p_what) { switch (p_what) { case NOTIFICATION_TRANSLATION_CHANGED: { @@ -4989,17 +4983,11 @@ void EditorInspector::_notification(int p_what) { ERR_FAIL_NULL(EditorFeatureProfileManager::get_singleton()); EditorFeatureProfileManager::get_singleton()->connect("current_feature_profile_changed", callable_mp(this, &EditorInspector::_feature_profile_changed)); set_process(is_visible_in_tree()); - get_parent()->connect(SceneStringName(theme_changed), callable_mp(this, &EditorInspector::_update_theme)); - _update_theme(); if (!is_sub_inspector()) { get_tree()->connect("node_removed", callable_mp(this, &EditorInspector::_node_removed)); } } break; - case NOTIFICATION_EXIT_TREE: { - get_parent()->disconnect(SceneStringName(theme_changed), callable_mp(this, &EditorInspector::_update_theme)); - } break; - case NOTIFICATION_PREDELETE: { if (!is_sub_inspector() && is_inside_tree()) { get_tree()->disconnect("node_removed", callable_mp(this, &EditorInspector::_node_removed)); @@ -5059,10 +5047,6 @@ void EditorInspector::_notification(int p_what) { } break; case EditorSettings::NOTIFICATION_EDITOR_SETTINGS_CHANGED: { - if (!is_sub_inspector() && EditorThemeManager::is_generated_theme_outdated()) { - _update_theme(); - } - if (use_settings_name_style && EditorSettings::get_singleton()->check_changed_settings_in_group("interface/editor/localize_settings")) { EditorPropertyNameProcessor::Style style = EditorPropertyNameProcessor::get_settings_style(); if (property_name_style != style) { diff --git a/editor/editor_inspector.h b/editor/editor_inspector.h index 14e2d816f12..6049d6dce1c 100644 --- a/editor/editor_inspector.h +++ b/editor/editor_inspector.h @@ -638,8 +638,6 @@ class EditorInspector : public ScrollContainer { void _set_property_favorited(const String &p_path, bool p_favorited); void _clear_current_favorites(); - void _update_theme(); - void _node_removed(Node *p_node); HashMap per_array_page; diff --git a/editor/themes/editor_theme_manager.cpp b/editor/themes/editor_theme_manager.cpp index 986a0469e74..d2e8d300d8e 100644 --- a/editor/themes/editor_theme_manager.cpp +++ b/editor/themes/editor_theme_manager.cpp @@ -1888,8 +1888,6 @@ void EditorThemeManager::_populate_editor_styles(const Ref &p_theme p_theme->set_stylebox("FocusViewport", EditorStringName(EditorStyles), style_widget_focus_viewport); Ref style_widget_scroll_container = p_config.button_style_focus->duplicate(); - // Make the focus outline appear to be flush with the buttons it's focusing, so not draw on top of the content. - style_widget_scroll_container->set_expand_margin_all(4); p_theme->set_stylebox("focus", "ScrollContainer", style_widget_scroll_container); // This stylebox is used in 3d and 2d viewports (no borders). @@ -2236,6 +2234,12 @@ void EditorThemeManager::_populate_editor_styles(const Ref &p_theme // Editor inspector. { + // Panel. + Ref editor_inspector_panel = p_config.tree_panel_style->duplicate(); + editor_inspector_panel->set_border_width_all(0); + editor_inspector_panel->set_content_margin_all(0); + p_theme->set_stylebox(SceneStringName(panel), "EditorInspector", editor_inspector_panel); + // Vertical separation between inspector categories and sections. p_theme->set_constant("v_separation", "EditorInspector", 0); diff --git a/scene/gui/scroll_container.cpp b/scene/gui/scroll_container.cpp index 00a2e8e01fd..02652547a83 100644 --- a/scene/gui/scroll_container.cpp +++ b/scene/gui/scroll_container.cpp @@ -31,6 +31,7 @@ #include "scroll_container.h" #include "core/config/project_settings.h" +#include "scene/gui/panel_container.h" #include "scene/main/window.h" #include "scene/theme/theme_db.h" @@ -44,7 +45,7 @@ Size2 ScrollContainer::get_minimum_size() const { if (!c) { continue; } - if (c == h_scroll || c == v_scroll) { + if (c == h_scroll || c == v_scroll || c == focus_panel) { continue; } @@ -72,7 +73,16 @@ Size2 ScrollContainer::get_minimum_size() const { } } - min_size += theme_cache.panel_style->get_minimum_size(); + Size2 panel_size = theme_cache.panel_style->get_minimum_size(); + min_size += panel_size; + if (draw_focus_border) { + Size2 focus_size = theme_cache.focus_style->get_minimum_size(); + // Only update the minimum size if the focus style's minimum size doesn't fit into the panel style's minimum size. + if (focus_size > panel_size) { + min_size += focus_size - panel_size; + } + } + return min_size; } @@ -258,21 +268,45 @@ void ScrollContainer::_update_scrollbar_position() { return; } + float right_margin = theme_cache.panel_style->get_margin(SIDE_RIGHT); + float left_margin = theme_cache.panel_style->get_margin(SIDE_LEFT); + float top_margin = theme_cache.panel_style->get_margin(SIDE_TOP); + float bottom_margin = theme_cache.panel_style->get_margin(SIDE_BOTTOM); + if (draw_focus_border) { + // Only update margins if the focus style's margins don't fit into the panel style's margins. + float focus_margin = theme_cache.focus_style->get_margin(SIDE_RIGHT); + if (focus_margin > right_margin) { + right_margin += focus_margin; + } + focus_margin = theme_cache.focus_style->get_margin(SIDE_LEFT); + if (focus_margin > left_margin) { + left_margin += focus_margin; + } + focus_margin = theme_cache.focus_style->get_margin(SIDE_TOP); + if (focus_margin > top_margin) { + top_margin += focus_margin; + } + focus_margin = theme_cache.focus_style->get_margin(SIDE_BOTTOM); + if (focus_margin > bottom_margin) { + bottom_margin += focus_margin; + } + } + Size2 hmin = h_scroll->is_visible() ? h_scroll->get_combined_minimum_size() : Size2(); Size2 vmin = v_scroll->is_visible() ? v_scroll->get_combined_minimum_size() : Size2(); - int lmar = is_layout_rtl() ? theme_cache.panel_style->get_margin(SIDE_RIGHT) : theme_cache.panel_style->get_margin(SIDE_LEFT); - int rmar = is_layout_rtl() ? theme_cache.panel_style->get_margin(SIDE_LEFT) : theme_cache.panel_style->get_margin(SIDE_RIGHT); + int lmar = is_layout_rtl() ? right_margin : left_margin; + int rmar = is_layout_rtl() ? left_margin : right_margin; h_scroll->set_anchor_and_offset(SIDE_LEFT, ANCHOR_BEGIN, lmar); h_scroll->set_anchor_and_offset(SIDE_RIGHT, ANCHOR_END, -rmar - vmin.width); - h_scroll->set_anchor_and_offset(SIDE_TOP, ANCHOR_END, -hmin.height - theme_cache.panel_style->get_margin(SIDE_BOTTOM)); - h_scroll->set_anchor_and_offset(SIDE_BOTTOM, ANCHOR_END, -theme_cache.panel_style->get_margin(SIDE_BOTTOM)); + h_scroll->set_anchor_and_offset(SIDE_TOP, ANCHOR_END, -hmin.height - bottom_margin); + h_scroll->set_anchor_and_offset(SIDE_BOTTOM, ANCHOR_END, -bottom_margin); v_scroll->set_anchor_and_offset(SIDE_LEFT, ANCHOR_END, -vmin.width - rmar); v_scroll->set_anchor_and_offset(SIDE_RIGHT, ANCHOR_END, -rmar); - v_scroll->set_anchor_and_offset(SIDE_TOP, ANCHOR_BEGIN, theme_cache.panel_style->get_margin(SIDE_TOP)); - v_scroll->set_anchor_and_offset(SIDE_BOTTOM, ANCHOR_END, -hmin.height - theme_cache.panel_style->get_margin(SIDE_BOTTOM)); + v_scroll->set_anchor_and_offset(SIDE_TOP, ANCHOR_BEGIN, top_margin); + v_scroll->set_anchor_and_offset(SIDE_BOTTOM, ANCHOR_END, -hmin.height - bottom_margin); _updating_scrollbars = false; } @@ -313,8 +347,22 @@ void ScrollContainer::_reposition_children() { Size2 size = get_size(); Point2 ofs; - size -= theme_cache.panel_style->get_minimum_size(); - ofs += theme_cache.panel_style->get_offset(); + Size2 panel_size = theme_cache.panel_style->get_minimum_size(); + Point2 panel_offset = theme_cache.panel_style->get_offset(); + size -= panel_size; + ofs += panel_offset; + if (draw_focus_border) { + // Only update the size and offset if focus style's doesn't fit into the panel style's. + Size2 focus_size = theme_cache.focus_style->get_minimum_size(); + if (focus_size > panel_size) { + size -= focus_size - panel_size; + } + Point2 focus_offset = theme_cache.focus_style->get_offset(); + if (focus_offset > panel_offset) { + ofs += focus_offset - panel_offset; + } + } + bool rtl = is_layout_rtl(); if (_is_h_scroll_visible() || horizontal_scroll_mode == SCROLL_MODE_RESERVE) { @@ -330,7 +378,7 @@ void ScrollContainer::_reposition_children() { if (!c) { continue; } - if (c == h_scroll || c == v_scroll) { + if (c == h_scroll || c == v_scroll || c == focus_panel) { continue; } Size2 minsize = c->get_combined_minimum_size(); @@ -350,6 +398,9 @@ void ScrollContainer::_reposition_children() { fit_child_in_rect(c, r); } + if (draw_focus_border) { + focus_panel->set_size(get_size()); + } queue_redraw(); } @@ -399,6 +450,7 @@ void ScrollContainer::_notification(int p_what) { if (p_what == NOTIFICATION_THEME_CHANGED) { scroll_border = get_theme_constant(SNAME("scroll_border"), SNAME("Tree")); scroll_speed = get_theme_constant(SNAME("scroll_speed"), SNAME("Tree")); + focus_panel->add_theme_style_override("panel", theme_cache.focus_style); } } break; @@ -415,15 +467,8 @@ void ScrollContainer::_notification(int p_what) { case NOTIFICATION_DRAW: { draw_style_box(theme_cache.panel_style, Rect2(Vector2(), get_size())); - if (draw_focus_border && (has_focus() || child_has_focus())) { - RID ci = get_canvas_item(); - RenderingServer::get_singleton()->canvas_item_add_clip_ignore(ci, true); - draw_style_box(theme_cache.focus_style, Rect2(Point2(), get_size())); - RenderingServer::get_singleton()->canvas_item_add_clip_ignore(ci, false); - focus_border_is_drawn = true; - } else { - focus_border_is_drawn = false; - } + focus_border_is_drawn = draw_focus_border && (has_focus() || child_has_focus()); + focus_panel->set_visible(focus_border_is_drawn); } break; case NOTIFICATION_DRAG_BEGIN: { @@ -535,7 +580,15 @@ void ScrollContainer::_notification(int p_what) { void ScrollContainer::update_scrollbars() { Size2 size = get_size(); - size -= theme_cache.panel_style->get_minimum_size(); + Size2 panel_size = theme_cache.panel_style->get_minimum_size(); + size -= panel_size; + if (draw_focus_border) { + Size2 focus_size = theme_cache.focus_style->get_minimum_size(); + // Only update the size if the focus style's minimum size doesn't fit into the panel style's minimum size. + if (focus_size > panel_size) { + size -= focus_size - panel_size; + } + } Size2 hmin = h_scroll->get_combined_minimum_size(); Size2 vmin = v_scroll->get_combined_minimum_size(); @@ -646,7 +699,7 @@ PackedStringArray ScrollContainer::get_configuration_warnings() const { if (!c) { continue; } - if (c == h_scroll || c == v_scroll) { + if (c == h_scroll || c == v_scroll || c == focus_panel) { continue; } @@ -736,7 +789,9 @@ void ScrollContainer::set_draw_focus_border(bool p_draw) { return; } draw_focus_border = p_draw; - queue_redraw(); + if (is_ready()) { + _reposition_children(); + } } bool ScrollContainer::get_draw_focus_border() { @@ -759,6 +814,17 @@ ScrollContainer::ScrollContainer() { add_child(v_scroll, false, INTERNAL_MODE_BACK); v_scroll->connect(SceneStringName(value_changed), callable_mp(this, &ScrollContainer::_scroll_moved)); + // We need to use a PanelContainer for the focus style instead of just drawing it directly with RenderingService + // due to a clippling issues. The Control that is being scrolled will be over the focus border because both will be + // drawn on the same CanvasItem. If we decide to ignore clipping, the focus border will be drawn even over other + // CanvasItems. + focus_panel = memnew(PanelContainer); + focus_panel->set_name("_focus"); + focus_panel->set_mouse_filter(MOUSE_FILTER_IGNORE); + focus_panel->set_focus_mode(FOCUS_NONE); + focus_panel->set_visible(draw_focus_border); + add_child(focus_panel, false, INTERNAL_MODE_BACK); + deadzone = GLOBAL_GET("gui/common/default_scroll_deadzone"); set_clip_contents(true); diff --git a/scene/gui/scroll_container.h b/scene/gui/scroll_container.h index 7f235270411..1970ed403fc 100644 --- a/scene/gui/scroll_container.h +++ b/scene/gui/scroll_container.h @@ -34,6 +34,8 @@ #include "scroll_bar.h" +class PanelContainer; + class ScrollContainer : public Container { GDCLASS(ScrollContainer, Container); @@ -49,6 +51,7 @@ public: private: HScrollBar *h_scroll = nullptr; VScrollBar *v_scroll = nullptr; + PanelContainer *focus_panel = nullptr; mutable Size2 largest_child_min_size; // The largest one among the min sizes of all available child controls.