Add Click Margin Theme Constant for BaseButton

[Kazox61]

Closes godotengine/godot-proposals#4338

This adds a Hit Margin Theme Constant for BaseButton.
The only way I found to implement this was by overriding the has_point for the BaseButton. This is needed since the BaseButton uses gui_input for detecting input and gui_input is only be called when has_point is true.

base_button_hit_margin.mp4

[Kazox61]

Is there any chance we could bring this into 4.8?

[YeldhamDev]

Could you rebase this to retrigger the artifact generation?

[Kazox61]

I would like to get feedback if the implementation to override the method has_point is a good idea?

[YeldhamDev]

Sorry, I will need you to rebase one more time to include a CI fix that was just merged.

[YeldhamDev]

Two issues:

• It should be documented that overriding _has_point() basically disables this feature.
• I'm not sure about the current name. I think something like click_margin would be more clear as to what it does.

[Kazox61]

It should be documented that overriding _has_point() basically disables this feature.

Not sure if this is needed. Overriding has_point on a button disables the inside check anyways. But I can add it to the theme property.

[Kazox61]

I'm not sure about the current name. I think something like click_margin would be more clear as to what it does.

But the margin also affects the hover and oder stuff so I'm not a fan of click_margin either.

[YeldhamDev]

Not sure if this is needed. Overriding has_point on a button disables the inside check anyways. But I can add it to the theme property.

You're right, maybe it's too much.

But the margin also affects the hover and oder stuff so I'm not a fan of click_margin either.

TextureButton uses click_mask as a property, that is also used to indicate if it's hovered. It doesn't need to fully encompass what it's used for, it just needs to make things clearer.

[Calinou]

Tested locally, it works as expected. Code looks good to me.

Some feedback:

• It's possible to use negative hit margins, which makes the button harder to hit with the mouse (clicking on the button's edges won't press it).

  • With large negative values, the rect size can become negative, causing this error to be spammed:

ERROR: Rect2 size is negative, this is not supported. Use Rect2.abs() to get a Rect2 with a positive size.
   at: has_point (./core/math/rect2.h:185)

I would clamp the final rect size to 0 to avoid this.

• To improve usability, the editor could use a default hit margin of Math::round(2 * EDSCALE) pixels on desktop platforms, and Math::round(4 * EDSCALE) pixels on the Android editor (for touchscreens).

  • The slight margin on desktop platforms is designed to help when moving the mouse quickly and clicking on various buttons, so you don't need to be as precise. It's subtle, but should help a lot already.

[Kazox61]

I would clamp the final rect size to 0 to avoid this.

I think it's better to use the has_areainstead of clamp

I also added the click_margin in the classic_theme and modern_theme. Not sure if this is the right place and the correct implementation.

[YeldhamDev]

Seems good to me, what about you, @Calinou?

[Calinou]

Works great now 🙂

The click margin in the editor is subtle, but it should help avoid a lot of missed clicks in the long run, particularly between aligned buttons (like the filter category icons in the Output panel).

While testing this, I noticed it's not possible to override theme items on an entire abstract class in the Theme editor with either of the ways to add new theme items:

click_margin doesn't appear in search results and can't be overriden at all using the Manage Items... dialog. It can be overridden using the sidebar still (the first method), but only on BaseButton subclasses (Button, LinkButton, TextureButton).

In the long term, we should base the Android click margin based on the input method used to perform the input, but this can be implemented in a future PR (likely by introducing click_margin_touch and keeping the current constant for mouse usage only). This depends on #110893 being implemented first, so that it can distinguish touch inputs from mouse inputs.

This constant could also be implemented in LineEdit and TextEdit in future PRs, as it would also help improve usability for those controls.

[Kazox61]

This depends on #110893 being implemented first, so that it can distinguish touch inputs from mouse inputs.

The linked PR is already merged?

This constant could also be implemented in LineEdit and TextEdit in future PRs, as it would also help improve usability for those controls.

A lot of Control Nodes would benefit from it.

[Repiteo]

Thanks!