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.

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

[4.3 - regression] Inspector scrolls up on changes everytime #91287

Closed
viksl opened this issue Apr 28, 2024 · 5 comments · Fixed by #92108
Closed

[4.3 - regression] Inspector scrolls up on changes everytime #91287

viksl opened this issue Apr 28, 2024 · 5 comments · Fixed by #92108

Comments

@viksl
Copy link
Contributor

viksl commented Apr 28, 2024

Tested versions

Reproducible: 4.3[3b18061]
Not reproducible: 4.2.2 stable

System information

Windows 11 - Vulkan - Nvidia RTX 4070 - intel i5 13600KF

Issue description

When you change a poperty in Inspector when scrolled down after the change the inspector automatically scrolls up.
Very annoying when trying to work.
Video: https://youtu.be/ej8usyqF0JI

Steps to reproduce

  1. Add HBoxContainer to your scene.
  2. Open several sections so you can scroll down.
  3. Scroll down to Mouse section.
  4. Change Filter to anything which si different to what is already in.
  5. Observe how the inspector scrolls up.

Minimal reproduction project (MRP)

Not needed, just follow the steps.

@matheusmdx
Copy link
Contributor

Bisecting points to #78960 as the culprit

image

@akien-mga
Copy link
Member

CC @kleonc

@kleonc
Copy link
Member

kleonc commented May 2, 2024

The issue seems to be that when the inspector tree is being rebuilt after changing the property value the relevant property editor is being refocused before the parent containers layout the child controls properly. And ScrollContainer (which EditorInspector extends) is connected to Viewport.gui_focus_changed signal and applies its follow-focus logic on such signal.

Specifically here the other_rect is incorrect / it's at the top of ScrollContainer's child VBoxContainer (hence it's scrolled to the top):

void ScrollContainer::_gui_focus_changed(Control *p_control) {
if (follow_focus && is_ancestor_of(p_control)) {
ensure_control_visible(p_control);
}
}
void ScrollContainer::ensure_control_visible(Control *p_control) {
ERR_FAIL_COND_MSG(!is_ancestor_of(p_control), "Must be an ancestor of the control.");
Rect2 global_rect = get_global_rect();
Rect2 other_rect = p_control->get_global_rect();
float right_margin = v_scroll->is_visible() ? v_scroll->get_size().x : 0.0f;
float bottom_margin = h_scroll->is_visible() ? h_scroll->get_size().y : 0.0f;
Vector2 diff = Vector2(MAX(MIN(other_rect.position.x, global_rect.position.x), other_rect.position.x + other_rect.size.x - global_rect.size.x + (!is_layout_rtl() ? right_margin : 0.0f)),
MAX(MIN(other_rect.position.y, global_rect.position.y), other_rect.position.y + other_rect.size.y - global_rect.size.y + bottom_margin));
set_h_scroll(get_h_scroll() + (diff.x - global_rect.position.x));
set_v_scroll(get_v_scroll() + (diff.y - global_rect.position.y));
}


For:
ETFXK6pKTW

here's some debug printing:
Godot_v4 3-dev6_win64_GkNy3fqbBh

Plugin outputting the above.

@tool
extends EditorPlugin

var inspector: EditorInspector

func _enter_tree() -> void:
	inspector = get_editor_interface().get_inspector()
	for property_signal: Signal in [
		inspector.property_deleted,
		inspector.property_edited,
		inspector.property_keyed,
		inspector.property_selected,
		inspector.property_toggled,
	]:
		property_signal.connect(
			func(property, skip_0 = null, skip_1 = null):
				var signal_name := property_signal.get_name()
				on_property_signal(property, signal_name)
				#on_property_signal.call_deferred(property, signal_name + "(DEFERRED)")
		)
	inspector.get_viewport().gui_focus_changed.connect(on_focus_changed)
	
func _exit_tree() -> void:
	pass

func on_focus_changed(control: Control) -> void:
	print()
	print("-".repeat(120))
	print("%s(%s)" % ["on_focus_changed", control])
	if inspector.is_ancestor_of(control):
		print_subtree(inspector)
	print("-".repeat(120))
	

var last_property: String
var last_property_editor: EditorProperty = null
func on_property_signal(property: String, property_signal: String) -> void:
	last_property = property
	last_property_editor = find_property_editor(property)
	print()
	print("-".repeat(120))
	print("%s(%s)" % [property_signal, property])
	print_subtree(inspector)
	print("-".repeat(120))

func find_property_editor(property: String) -> EditorProperty:
	var to_check := [inspector]
	while to_check.size() > 0:
		var current: Node = to_check.pop_back()
		if current is EditorProperty and current.get_edited_property() == property:
			return current
		to_check.append_array(current.get_children())
	return null

func print_subtree(node: Node, indent_level: int = 0) -> void:
	var indent := " ".repeat(indent_level)
	var node_info := "%s" % [node]
	if node is Control:
		node_info += " rect=%s global_rect=%s" % [
			colored(node.get_rect(), "yellow"),
			colored(node.get_global_rect(), "green"),
		]
	if last_property_editor:
		if node == last_property_editor:
			print_rich("%s%s" % [indent, colored(node_info, "magenta")])
		elif node.is_ancestor_of(last_property_editor) or last_property_editor.is_ancestor_of(node):
			print_rich("%s%s" % [indent, colored(node_info, "pink")])
		#else:
			#print_rich("%s%s" % [indent, node_info])
	for child in node.get_children():
		print_subtree(child, indent_level + 1)

func colored(what: Variant, color_string: String) -> String:
	return "[color=%s]%s[/color]" % [color_string, str(what)]


Not sure how exactly this should be fixed.

@muratpeker
Copy link

Same happens when changing material properties under Ubuntu 22.04 with 4.3 dev6

@akien-mga
Copy link
Member

I can confirm the issue too, it's very annoying when editing a ParticleProcessMaterial, every time a property is edited that implies an update (to e.g. surface other properties when enabling a feature), it scrolls back up.

CC @KoBeWi @Calinou if you have any idea to help @kleonc figure out a solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Immediate Blocker
Development

Successfully merging a pull request may close this issue.

7 participants