ColorPicker nodes move when a scene is opened

[TheSecondReal0]

Godot version

3.2.2, 3.3.4, 3.4.4, 3.5.rc6, 4.0a11 (and everything in between)

System information

Windows 10

Issue description

ColorPicker nodes will move when you open or switch to a scene containing them in the editor. The behavior is the same across many versions, only differing in what variables are actually being changed. On 3.0 versions the margins are increased while on 4.0 the latest 4.0 version (a11) position is increased.

The variable(s) increase by 4 each time. In 3.0 versions all margin values are increased by 4 while on 4.0 versions it adds 4 to all anchor offsets and the x and y of position.

The new values are also automatically saved to the .tscn file, meaning you have to manually undo them instead of just not saving the file.

Here's a video showcasing the bug on 3.4.4 (reproducing is the same on all versions listed)

colorpicker_bug_video.mp4

Steps to reproduce

The bug can be extremely reliably reproduced using the method shown in the above video (switching between multiple scene tabs). It also occurs sometimes when a scene is first loaded by the editor.

Important to note that the ColorPicker node does not have to be the scene root. It can be anywhere in the scene as long as its margins/position is not being controlled by another container.

Minimal reproduction project

colorpicker_bug_3.4.4.zip
colorpicker_bug_4.0.a11.zip

[kleonc]

Caused by #37088.

Combining from-theme-margin and the actual margin of the ColorPicker into the actual margin of the ColorPicker is a bad idea. I'd say applying from-theme-margin to the actual margin of the ColorPicker in any way is a bad idea. The actual margin of ColorPicker could be ignored/changed e.g. if ColorPicker would be a child of some Container. So if from-theme-margin is supposed to always be respected (not sure if it's designed/meant to be like that) then it can't be applied as an actual margin of the ColorPicker. In such case a potential solution would be to change ColorPicker from:

ColorPicker (extends BoxContainer) <- combined from-theme-margin and user-defined-margin mess
- internal child Controls

to something like:

ColorPicker (extends some Container respecting child margin?) <- user-defined margin
- internal BoxContainer <- from-theme-margin (always respected)
-- internal child Controls

[Rindbee]

In the current implementation, theme_override_constants/margin seems unnecessary.

godot/scene/gui/color_picker.cpp

Lines 78 to 83 in 8b059d4

	for (int i = 0; i < SLIDER_COUNT; i++) {
	labels[i]->set_custom_minimum_size(Size2(get_theme_constant(SNAME("label_width")), 0));
	set_offset((Side)i, get_offset((Side)i) + get_theme_constant(SNAME("margin")));
	}
	alpha_label->set_custom_minimum_size(Size2(get_theme_constant(SNAME("label_width")), 0));
	set_offset((Side)0, get_offset((Side)0) + get_theme_constant(SNAME("margin")));

Every time the theme changes, the offset will be reset, which is reflected in the change of position.