Merge pull request godotengine#104317 from pafuent/fixing_wrong_focus_style_clipping_on_scroll_container

Fix `ScrollContainer` focus border issue

Author: Repiteo

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) {

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<StringName, int> per_array_page;

editor/themes/editor_theme_manager.cpp

@@ -1888,8 +1888,6 @@ void EditorThemeManager::_populate_editor_styles(const Ref<EditorTheme> &p_theme
 		p_theme->set_stylebox("FocusViewport", EditorStringName(EditorStyles), style_widget_focus_viewport);
 
 		Ref<StyleBoxFlat> 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).
@@ -2241,6 +2239,12 @@ void EditorThemeManager::_populate_editor_styles(const Ref<EditorTheme> &p_theme
 
 	// Editor inspector.
 	{
+		// Panel.
+		Ref<StyleBoxFlat> 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);
 

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: {
@@ -533,7 +578,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();
@@ -644,7 +697,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;
 		}
 
@@ -734,7 +787,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() {
@@ -757,6 +812,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);

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 @@ class ScrollContainer : public Container {
 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.