Regression in editor buttons clipping last letter under certain circumstances

[GuyUnger]

Tested versions

happens in v4.3.dev4
does not happen in 4.3.dev3, 4.3.dev2 or 4.3.dev1 (i tested)

System information

Windows 11 - 4.3.dev4.official [df78c06]

Issue description

Using the theme by passivestar https://github.com/passivestar/godot-minimal-theme with the editor on 75% scaling the last letters in the top buttons (2d, 3d, script, assetlib) get hidden on hover.
Apparently this was an issue before ( #85449 ) under the same circumstances ( #88066 ) and was solved, probably with #87335

#86378 might have re-introduced this issue but not certain

Steps to reproduce

use the theme by passivestar ( https://github.com/passivestar/godot-minimal-theme ) and set scaling to 75%

Minimal reproduction project (MRP)

this happens in editor with any project

[passivestar]

Also makes the icon shift but I assume that's because of the missing letter

Screen.Recording.mp4

[akien-mga]

Related to #88652 (comment), CC @RobProductions @Calinou

[Rindbee]

godot/editor/themes/editor_theme_manager.cpp

Lines 2448 to 2454 in f2045ba

	const String custom_theme_path = EDITOR_GET("interface/theme/custom_theme");
	if (!custom_theme_path.is_empty()) {
	Ref<Theme> custom_theme = ResourceLoader::load(custom_theme_path);
	if (custom_theme.is_valid()) {
	theme->merge_with(custom_theme);
	}
	}

EDSCALE was not considered when merging themes. It's also not taken into account when importing the current editor theme.

[RobProductions]

I believe this comes from the "Base spacing" variable which as you can see here breaks the stylebox when it's set to < 4:

Capture2024-03-04.21-09-19.mov

It does seem to be limited to external themes though (or at least the one linked here which I tried) and thankfully doesn't break on default Godot UI:

Capture2024-03-04.21-12-03.mov

From what I can tell Editor Scale has no bearing on it as both videos were recorded with 100% scale, but I could be wrong. The hover stylebox margins did change in #88652 as you can see here:

https://github.com/godotengine/godot/pull/88652/files

Though that was meant to fix the "draw extra borders" margin that should've been passed down to that. Could be breaking because the new "MainScreenButton" variation needs the same margin treatment as the flat buttons but I believe when last I tested that it also appeared cut off, hence why it has different margins from the menu_transparent_style that it stems from. It remained that way because I couldn't find any case that breaks it, but with external themes it seems there is an issue.

Took a while to assemble all that because I did a fresh compilation of Godot on a new machine but now that I'm able to reproduce it I can try testing out some fixes, just might take a bit of trial and error. I don't know exactly what's wrong yet but hopefully that narrows down the search, apologies for not catching it earlier!

[Rindbee]

It is mainly caused by the use of different minimum size styleboxes for the same button in different statuses.

In this MRP, the minimum sizes of styleboxes are different in hover and normal status.

Since MainScreenButton lacks the stylebox used in hover status, it will fall back to Button.

In addition, interface/editor/display_scale will not affect custom themes (#89122 (comment)), custom theme items are usually values assuming scale is 100%. Therefore, this issue will occur when interface/editor/display_scale is not 100% and a custom theme that is not fully covered is used.

Finally, I'm confused by the code that calls set_content_margin() with the results of get_margin() and get_border_width().

godot/editor/themes/editor_theme_manager.cpp

Lines 1822 to 1824 in a07dd0d

	style_flat_button->set_content_margin((Side)i, p_config.button_style->get_margin((Side)i) + p_config.button_style->get_border_width((Side)i));
	style_flat_button_hover->set_content_margin((Side)i, p_config.button_style->get_margin((Side)i) + p_config.button_style->get_border_width((Side)i));
	style_flat_button_pressed->set_content_margin((Side)i, p_config.button_style->get_margin((Side)i) + p_config.button_style->get_border_width((Side)i));

godot/scene/resources/style_box.cpp

Lines 78 to 86 in a07dd0d

	float StyleBox::get_margin(Side p_side) const {
	ERR_FAIL_INDEX_V((int)p_side, 4, 0.0);
	if (content_margin[p_side] < 0) {
	return get_style_margin(p_side);
	} else {
	return content_margin[p_side];
	}
	}

godot/scene/resources/style_box_flat.cpp

Lines 35 to 38 in a07dd0d

	float StyleBoxFlat::get_style_margin(Side p_side) const {
	ERR_FAIL_INDEX_V((int)p_side, 4, 0.0);
	return border_width[p_side];
	}

godot/scene/resources/style_box_flat.cpp

Lines 82 to 85 in a07dd0d

	int StyleBoxFlat::get_border_width(Side p_side) const {
	ERR_FAIL_INDEX_V((int)p_side, 4, 0);
	return border_width[p_side];
	}

If it wants to use content_margin[p_side] here, why not use get_content_margin() instead.

[RobProductions]

It is mainly caused by the use of different minimum size styleboxes for the same button in different statuses.
Since MainScreenButton lacks the stylebox used in hover status, it will fall back to Button.

Good catch! I defined the hover stylebox in the theme variation with the correct margins (same fix as what I showed above in #88652 ) and that seemed to solve the problem:

Capture2024-03-06.21-45-59.mov

In addition, interface/editor/display_scale will not affect custom themes (#89122 (comment)), custom theme items are usually values assuming scale is 100%. Therefore, this issue will occur when interface/editor/display_scale is not 100% and a custom theme that is not fully covered is used.

You're right, I do see an issue at 75% scale on the custom theme:

But I believe it's only because internally it sets the margin value based on EDSCALE as you can see here:

godot/editor/themes/editor_theme_manager.cpp

Line 129 in a07dd0d

style->set_content_margin_individual((p_left + p_margin_left) * EDSCALE, (p_top + p_margin_top) * EDSCALE, (p_right + p_margin_right) * EDSCALE, (p_bottom + p_margin_bottom) * EDSCALE);

Hence the issue still stems from margins and correcting them as shown in the video allows it to stay constrained on all scale values too, so as far as I can tell this should resolve it for now.

If it wants to use content_margin[p_side] here, why not use get_content_margin() instead.

The code I wrote that you see there currently mirrors an existing pattern that was there previously which I guess was a little more convoluted than it needed to be, but I tested it and you're right, it will work with get_content_margin() too! I updated those lines to use it, thanks for pointing that out!

There are just two things I need to figure out before I submit my PR, one is why the hover highlights on the main buttons seem to lose its rounded edge and another is cleaning up the bottom bar pressed style which also shows clipping at times. The bottom bar issue is puzzling because it seems that assigning a stylebox override is what causes these margins to get messed up. Perhaps the custom theme wipes that out somehow and it just needs its own theme variation like the main screen buttons, but when I tried that it still cuts off. Will update when I learn more and I think like I'm pretty close to having this ready now :)