godot icon indicating copy to clipboard operation
godot copied to clipboard

Implement a LayoutContainer

Open groud opened this issue 1 year ago • 13 comments

This implements a new LayoutContainer. This container aims at placing a single Control child, with a set of presets that are basically the same as the default Control anchors presets. This can replace both CenterContainer and MarginContainer.

Edit:

https://github.com/godotengine/godot/assets/6093119/5b081f34-efbd-46d0-ad14-1de50d893f4c

In terms of properties, you have:

  • A "relative margins" preset (center, top center, top wide, etc.), with an option "custom" option. If set to "custom", the underlying relative margins values can be set manually.
  • Margins values: I used margins instead of offsets as this new container has now a minimum size. And unlike with the usual "anchors and offset" positioning, it makes little sense for a LayoutContainer to have child controls outside of its own area. So I believe using "margins" makes more sense here.
  • Theme margins values: additional margins (in pixels) that can use the theme variables. This basically ensure compatibility with the MarginContainer node (which thus could be removed and replaced by a LayoutContainer node)
Original post:

https://github.com/godotengine/godot/assets/6093119/d126a5a9-7ac2-44a3-9ef6-85c51937ca53

In terms of properties, you have:

  • An anchors preset (center, top center, top wide, etc.), with an option "custom" option. If set to "custom", the underlying anchors values can be set manually.
  • Margins values: I used margins instead of offsets as this new container has now a minimum size. And unlike with the usual "anchors and offset" positioning, it makes little sense for a LayoutContainer to have child controls outside of its own area. So I believe using "margins" makes more sense here.
  • Theme margins values: additional margins that can use the theme variables. This basically ensure compatibility with the MarginContainer node (which thus could be removed and replaced by a LayoutContainer node)

This implements parts of https://github.com/godotengine/godot-proposals/issues/4994

Edit: For the "custom" preset mode, I wonder if we should rename "anchors" to, like, "relative margins" or something, and like, invert the value for right and bottom side. Basically, "relative margins" set as (0,0,0,0) would mean "full rect", while right now, "full rect" means anchors being (0,0,1,1).

groud avatar May 31 '24 14:05 groud

This makes a lot of sense to me

JekSun97 avatar May 31 '24 19:05 JekSun97

I've updated the PR. I replaced anchors by "relative margins". It seems easier to understand IMO. I've also wrote the class's documentations.

groud avatar Jun 04 '24 08:06 groud

I really like this. While you can do mostly the same thing with just anchors, I think relative margins makes a lot more sense and would make UI work much easier. Shame it won't make it into 4.3

With a bit of testing, I noticed the centering isn't working properly though. I used the same Label on a CenteringContainer and a centered LayoutContainer. The label was offset from the center of the screen.

Screenshot_24_06_04_2

popcar2 avatar Jun 04 '24 12:06 popcar2

I really like this. While you can do mostly the same thing with just anchors, I think relative margins makes a lot more sense and would make UI work much easier. Shame it won't make it into 4.3

Yeah. What kind of prevented doing the same thing with Control is the fact a Control node can go outside the boundaries of its parent, which is not the case for Containers. That is also why I kind of wanted to removed anchors & offsets from Control, they are quite confusing compared to margins.

With a bit of testing, I noticed the centering isn't working properly though. I used the same Label on a CenteringContainer and a centered LayoutContainer. The label was offset from the center of the screen.

Oh yeah, good catch. I pushed a new version now, it should be fixed.

groud avatar Jun 04 '24 14:06 groud

Full Rect seems to have broken with the update. It now sets the size of the child to 0 rather than fill the container. Also, I got weird behavior when I set a custom minimum size and try using custom margins, it goes out of bounds.

https://github.com/godotengine/godot/assets/16920817/6a35472b-7c76-4789-aaef-d6727965d954

popcar2 avatar Jun 05 '24 10:06 popcar2

The Margins should be called Offsets, don't repeat the same mistake 🙃 Why is Custom preset clamped to 0-1?

KoBeWi avatar Jun 09 '24 18:06 KoBeWi

The Margins should be called Offsets, don't repeat the same mistake 🙃 Why is Custom preset clamped to 0-1?

No, this is on purpose. I made this LayoutContainer not allowing a child outside of its boundaries, and thus clamping things to 0-1. This otherwise would make minimum size calculation not work, which I think is a bigger problem.

Also, as a consequence, margins make sense, and are also a lot simpler to grasp than anchors/offset. At least that's a feedback I often got about the anchors/offset system.

groud avatar Jun 09 '24 18:06 groud

Well, we got feedback about Margins being confusing too, because they aren't really margins. E.g. on Center layout it becomes offset.

EDIT: Wide layouts aren't wide.

KoBeWi avatar Jun 09 '24 19:06 KoBeWi

Well, we got feedback about Margins being confusing too, because they aren't really margins. E.g. on Center layout it becomes offset.

No, not with this container at least. The margins keep their values as they are not modified if the underlying node cannot fit within the "relative margins". So they don't really "become offsets". I think this is a lot less confusing for most people.

groud avatar Jun 09 '24 19:06 groud

+1 that one should be called "Offsets." Especially considering that the 'extra margins' are linear, while the 'margins' are pixel sizes.

souplamp avatar Jun 20 '24 19:06 souplamp

+1 that one should be called "Offsets." Especially considering that the 'extra margins' are linear, while the 'margins' are pixel sizes.

Offsets are called "offsets" because they are always the same direction as the axis (positive X means the side -left or right- is pushed right, and negative X means the side is pushed left). Here "margins " make more sense as they act as actual margins (always positive values, on the left side of the controls it means that the child's side is pushed right, and on the right side, it is pushed left). Offsets and margins are different things, and here, I used margins, not offsets.

groud avatar Jun 28 '24 09:06 groud

Here "margins " make more sense as they act as actual margins (always positive values, on the left side of the controls it means that the child's side is pushed right, and on the right side, it is pushed left).

In that case, could the linear margins be called 'relative margins' and pixel margins 'absolute margins'? Similar to CSS.

souplamp avatar Jun 28 '24 17:06 souplamp

In that case, could the linear margins be called 'relative margins' and pixel margins 'absolute margins'? Similar to CSS.

Relative margins are already called that way (the properties are custom_relative_margin_<side>. The other ones are just "margins", but well, I can call them "absolute margins" if people prefer it that way.

On the other hand, custom_relative_margin_<side> are not visible if a preset is selected, so maybe having "absolute margine" being visible without any "relative margins" might be a bit weird IMO.

groud avatar Jun 28 '24 19:06 groud