Fix ItemList text trimming

[havi05]

This fixes #97438

The left content margin was probably calculated as an offset by the get_size() function in the line above, but I couldn't check that, because I haven't found the function. If this needs to be changed please tell me where to find it.

Video showing the resolved issue

https://github.com/user-attachments/assets/ad4a7355-5124-406f-b563-3b7a4ed2209d

[KoBeWi]

The minimal right margin is smaller than the left one. Shouldn't they be symmetrical? 🤔 EDIT: It's symmetrical when h_separation is 0. Looks like the left side uses half of the separation, the right one should too probably.

---

Also the autowrap margin with multiple columns seems to be the same as before:

https://github.com/user-attachments/assets/f7a022bc-095e-42e0-9726-21f3a86df2fb

But this can be considered a separate issue.

[havi05]

If the minimum right margin is the same size as the left margin, the text is cut off as it cannot be displayed completely in the center of the ItemList at this size. (see image)

Image

If you set the h_separation larger than the size of the ItemList, only the left size of the h_separation is taken into account and the text is displayed in full. Theoretically, if the h_separation is the size of the ItemList (or larger), it would fill the entire space and hide the text, leaving an empty item. (see video 1)

Therefore, I thought if there is enough space for the text, it should be displayed and it would be symmetrical if you enable the auto_width function (#93270). (see video 2)

Video 1

https://github.com/user-attachments/assets/ba83ac85-fc9d-4e88-acd3-21b1741b130f

Video 2

https://github.com/user-attachments/assets/f68b44dc-f95d-4e72-a234-700d8226f40e

[havi05]

~I think the autowrap margin should be considered as a separate issue.~ Is there a bug report already?

The autowrap margin takes the scroll bar into account, even if it is not visible. Here are the videos with and without the fix.

Before

https://github.com/user-attachments/assets/b9dae90d-9b9c-4541-b831-80fd45e90e21

After

https://github.com/user-attachments/assets/fa595e0f-1b7c-46f2-9041-e27f847555ec

[WhalesState]

It's symmetrical when h_separation is 0. Looks like the left side uses half of the separation, the right one should too probably.

This happens when adding panel->get_margin(SIDE_LEFT) to a position.x and subtracting panel->get_minimum_size().width from width instead of panel->get_margin(SIDE_RIGHT).

[havi05]

@WhalesState I don't think I quite understood your comment. Do you think it should stay the way I did it in the pr (and it's just the explanation to the problem), or should it be changed back to the previous state?

[WhalesState]

Seems like you already have fixed that, but i have found a regression.

• The icons are not aligned correctly when using top icons.

Already an existing issue:

• The h_separation is not clamped.
• The separation line is not drawn when aligning the icons on top.

https://github.com/user-attachments/assets/f9bacbe0-f137-4486-a09c-426dd93fadef

[havi05]

The icons are not aligned correctly when using top icons.

Fixed

The h_separation is not clamped.

I have added if statements so that v_separation and h_separation can no longer be negative. Is there a better way to disable negative numbers for Theme::DATA_TYPE_CONSTANT than checking each usage?

The separation line is not drawn when aligning the icons on top.

This was apparently intentionally removed. (https://github.com/godotengine/godot/pull/82236)

[WhalesState]

I have added if statements so that v_separation and h_separation can no longer be negative. Is there a better way to disable negative numbers for Theme::DATA_TYPE_CONSTANT than checking each usage?

I think we can't use property hints or setters for theme constants, you will have to clamp them everytime if the negative values will cause issues. you can use MAX(theme_cache.h_separation, 0), or maybe caching it int h_sep = MAX(theme_cache.h_separation, 0); if it will be used many times.

[havi05]

@KoBeWi What you think of my comment on the symmetry of the minimum margin? (https://github.com/godotengine/godot/pull/97439#issuecomment-2374384905) I think a new user could get confused that the text is truncated when the next entry to be separated is already on the next row.

[KoBeWi]

So I did an experiment to test the new behavior. I made a StyleBox with 0 content margins and borders equal to h_separation. This way it's easy to visualize the actual margins imposed by separation.

Here's old behavior:

https://github.com/user-attachments/assets/4adaf15c-43eb-4f77-b5fd-f16f405da1d2

It uses half separation on the left and full separation on the right.

Here's new behavior:

https://github.com/user-attachments/assets/bcc97cf6-5889-4c66-a0cb-5e01973905ff

Half separation on the left, right separation is ignored.

I think both are sort of inconsistent, but the new one is more friendly, as there is more space.

[havi05]

So it would be good for now, and we could open a discussion on better consistency if that is needed?

[KoBeWi]

Yeah, overall it's an improvement.

[havi05]

Should I rebase the branch or does this happen automatically when merging?

[KoBeWi]

Rebasing is only needed if there are conflicts, or sometimes when a PR is very old and wasn't recently updated.

[akien-mga]

Thanks! And congrats for your first merged Godot contribution :tada: