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

ItemList has clipped items, unwanted extra spacing and inconsistent handling of mouse clicks depending on spacing parameters #88217

Open
davthedev opened this issue Feb 11, 2024 · 5 comments

Comments

@davthedev
Copy link
Contributor

davthedev commented Feb 11, 2024

Tested versions

  • Reproducible in 4.3 dev [4e990cd]
  • Reproducible in 4.2.1 stable
  • Found the issue in 3.0 branch too

System information

Godot v4.2.1.stable - Ubuntu 22.04.3 LTS 22.04 - X11 - Vulkan (Mobile) - integrated Intel(R) Iris(R) Plus Graphics (ICL GT2) () - Intel(R) Core(TM) i7-1065G7 CPU @ 1.30GHz (8 Threads)

Issue description

Depending on the constants set to adjust items spacing on the ItemList control, there are quirks in the rendering and pointer event handling that cause:

  • Inconsistency between the area used to detect mouse events on each item, and the draw area of the selection rectangle.
  • Unwanted extra spacing at the bottom, or the first/last item hover/selection graphic being clipped off
  • Actual spacing is double the value set in pixels on the corresponding property

I have played a bit, had a look in the source code and attempted to describe how it works. Those are quite a bunch of issues grouped together, for the reason that the root cause is the item positioning algorithm. Below is how it looks when the vertical spacing is set to 10px.

image

Mouse events inconsistency

The mouse detection area for each item is smaller than the perceived selection rectangle. If horizontal (for multiple columns) or vertical spacing is more than zero, all items appear to be touching each other, due to the way the selection / hover rectangle is rendered, but there is a gap between items where clicks do not have any effects. (also reported on #88095)

From the drawing above, the green (B) rectangle on each item is the mouse events target.
The blue (C) rectangle is where the selection/hover background is drawn on the corresponding item.

Measurement inconsistency

In the above example, vertical spacing is set to 10px, but the result is 20px more vertical space in comparison to when the same spacing is set to 0. The algorithm ends up adding twice the desired spacing.

Extra spacing at the bottom

image

In this example, the more vertical spacing between items, the larger the gap at the bottom of the list when fully scrolled at the end. Only applies to single-columns lists with scrolling. No issue if vertical spacing is zero.

The extra spacing appears to be half the vertical spacing value, and behaves like being the vertical spacing value regarding mouse events.

Cut-off selection rectangle at the top of single-column list

image

The first item has its selection rectangle cut-off. It is even more visible with larger values for vertical spacing. No cutoff occurs if spacing is 0. This is due to a compensation by the algorithm to attempt having some visual consistency with the baseline of the first row text.

Cut-off selection rectangle on all sides of multiple column lists

image

image

The cutoff effect of the selection rectangle is visible at both the top and bottom of the list when set to multiple columns with icons. Note that the rounding of the corner is missing to notice the cutoff.
Leftmost example has no spacing.

Icon margin setting causes text to appear outside the item rectangle

image

image

On the second screenshot, we can see how the previous row text is hidden by the selection. Also notice that the text is clipped beyond the last row.
Leftmost example has no spacing.

Refactoring proposal

Due to the presence of multiple issues related by the positioning algorithm, I have a proposal to reimplement the rendering of those items and solve all those bugs at once. I have a nearly working prototype. Needs some discussion as it may cause some little breaking changes.

godotengine/godot-proposals#9076

Steps to reproduce

Open the reproduction project and run the scene. You can play with the window size; layout is anchored to it.

Select items in the different lists to view the issues with spacing and some cut-off elements.

Minimal reproduction project (MRP)

itemlist-issue.zip

@Calinou
Copy link
Member

Calinou commented Feb 12, 2024

@davthedev
Copy link
Contributor Author

Sure, there is a possibility to fix the deadzone and the bottom gap with small changes.

The current algorithm computes item positions, uses this reference for the mouse events detection, then the drawing pass grows the box to "close" the gap. It would require bringing the second growth pass to the box sizing calculation.

I propose to kill three birds with one stone by directly reimplementing the list layout algorithm (see mentioned proposal link), instead of doing separate fixes that will be undone in case the list layout algorithm is redone later. Which solution would make more sense?

@Calinou
Copy link
Member

Calinou commented Feb 13, 2024

I propose to kill three birds with one stone by directly reimplementing the list layout algorithm (see mentioned proposal link), instead of doing separate fixes that will be undone in case the list layout algorithm is redone later. Which solution would make more sense?

A big refactor is unlikely to be merged in time for 4.3, which is why I'd prefer a fix for the current editor theme issue to land first.

@davthedev
Copy link
Contributor Author

Bugfix for the click deadzone issue only: #88347

@davthedev
Copy link
Contributor Author

Bugfix for the following in #88577

  • Clipping of selection indicator at the top or bottom of the list
  • Gap at the bottom of the list in rows mode when the end of the scrollbar is reached
  • In grid display, labels were not properly centered, a little offset to the side was happening
  • Separators overlapping some items by a pixel or two beyond the border
  • Inconsistent scale of vertical separation, which is double in comparison to horizontal separation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants