godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.14k stars 94 forks source link

Add a `rect_max_size` property to Control nodes #1802

Closed MattUV closed 2 years ago

MattUV commented 3 years ago

Describe the project you are working on: I'm working on an English language training software for French ATCs (it mostly uses Control nodes and is not a game). Based on radio exchanges and their transcripts, ATCs must solve some English exercises, designed by a teacher within the software.

Describe the problem or limitation you are having in your project: My software requires users to read lots of texts. On larger screens, it is not comfortable to have the texts take the whole screen width. Hence I have to manually adapt the maximum width of my nodes if they get too large. It is working, but as I use many Control objects, it stutters a lot when resizing.

Describe the feature / enhancement and how it helps to overcome the problem or limitation: I propose to add a rect_max_size on control nodes that would work like rect_min_size. It would be transparent to the user, and help with complex UI by gaining a lot of time.

Without this feature sans**

With this feature avec

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams: When a Control node "tries" to be bigger than what is defined in rect_max_size, it cannot be enlarged anymore. It would be the same behavior as rect_min_size, but inversed.

If this enhancement will not be used often, can it be worked around with a few lines of script?: It can be worked around by adding a MarginContainer (SpanLimiter) and use this piece of code

func _on_SpanLimiter_resized():
    _limit_span()

func _limit_span():
    if SpanLimiter.rect_size.x > MAX_SPAN:
        var half_margin = (SpanLimiter.rect_size.x - MAX_SPAN) / 2

        SpanLimiter.set("custom_constants/margin_left", half_margin)
        SpanLimiter.set("custom_constants/margin_right", half_margin)

    else:
        SpanLimiter.set("custom_constants/margin_left", 5)
        SpanLimiter.set("custom_constants/margin_right", 5)

However, using it in many scenes is not convenient. Plus, it forces to add a MarginContainer and modify the Nodes hierarchy only for that feature. It is also not really efficient (it stutters and shows black bars when resizing)

Is there a reason why this should be core and not an add-on in the asset library?: It looks a lot like rect_min_size, and seems more natural to be integrated.

Killfrra commented 3 years ago

Wrote my own container to constrain the size of the node and align it horizontally and vertically inside the container. Perhaps it will be useful to someone. I think something like this should be added to the editor

tool
class_name SpanLimiter
extends Container

enum Align { LEFT, CENTER, RIGHT }
enum VAlign { TOP, CENTER, BOTTOM }

const MAX_FLOAT := 10000000 #INF
# BUG: When saving scene with x or y of Vector2 set to inf, game crashes with
# E _parse_node_tag: res://scene.tscn:${line with "Vector2( inf, inf )"} - Parse Error: Expected float in constructor
#  <C++ Source>  scene/resources/resource_format_text.cpp:278 @ _parse_node_tag()
# Tested with Godot 3.3.2

export var rect_max_size: Vector2 = Vector2(MAX_FLOAT, MAX_FLOAT) setget set_custom_maximum_size
func set_custom_maximum_size(to):
    rect_max_size = to
    fit_child()

export(Align) var align = Align.CENTER setget set_align
func set_align(to):
    align = to
    fit_child()

export(VAlign) var valign = VAlign.CENTER setget set_valign
func set_valign(to):
    valign = to
    fit_child()

func _get_configuration_warning():
    if get_child_count() != 1:
        return "This node is designed for use with a single child"
    return ""

func _notification(what):
    if what == NOTIFICATION_RESIZED:
        fit_child()

func fit_child():

    if get_child_count() == 0:
        return

    var position = Vector2.ZERO
    var size = rect_size
    match align:
        Align.CENTER:
            position.x = max(0, (rect_size.x - rect_max_size.x) / 2)
        Align.RIGHT:
            position.x = max(0, rect_size.x - rect_max_size.x)

    match valign:
        VAlign.CENTER:
            position.y = max(0, (rect_size.y - rect_max_size.y) / 2)
        VAlign.BOTTOM:
            position.y = max(0, rect_size.y - rect_max_size.y)

    size.x = min(rect_size.x, rect_max_size.x)
    size.y = min(rect_size.y, rect_max_size.y)

    fit_child_in_rect(get_child(0), Rect2(position, size))

func _get_minimum_size():
    if get_child_count() != 0:
        return Vector2(
            max(rect_min_size.x, get_child(0).rect_min_size.x),
            max(rect_min_size.y, get_child(0).rect_min_size.y)
        )
    else:
        return rect_min_size
Nukiloco commented 2 years ago

I'm going to also vote for this feature. There is many places in my UI that I need to have a way of setting a max size. I just end up using a lot of control nodes just to make it happen which can get tedious at some points. Sometimes it can even take a few minutes just to try to limit the size of my UI. Please add this feature!

markdibarry commented 2 years ago

It is really confusing why max-size is not a feature. This is a feature that would come in handy extensively in UI. It's also very difficult to workaround this limitation when the only containers that honor size flags for children also grow beyond their set size when the children do. Compounded with the missing "child node added" signal, this makes it incredibly difficult to make even a simple autowrap label to behave like the following:

https://user-images.githubusercontent.com/21325943/147025003-634a6537-0788-46a9-94f1-d4bc205ce109.mp4

To achieve this, Godot requires setting up multiple scripts, multiple resized signals, forcing children to call parent's methods, and special cases for when a child is added in-editor vs in-game, or when a textbox already is contained within a scene. This is very brittle and convoluted with a tree looking like so: image

Without these workarounds, autowrap behaves like so: image

90% of this code would be eliminated with a max-width field. Just my 2 cents!

MattUV commented 2 years ago

Hello, like @Killfrra , i also wrote a custom container node, during the Godot add-on jam.

I made it a plugin that you can download here: https://github.com/MattUV/godot-MaxSizeContainer

I'll upload it to the asset library soon.

seppoday commented 2 years ago

Hello, like @Killfrra , i also wrote a custom container node, during the Godot add-on jam.

I made it a plugin that you can download here: https://github.com/MattUV/godot-MaxSizeContainer

I'll upload it to the asset library soon.

Great addon. Native solution would be very nice tho :(

Calinou commented 2 years ago

Implementing this in core would also allow replacing https://github.com/godotengine/godot/pull/31588 (which is kind of hacky) by a cleaner solution.

YuriSizov commented 2 years ago

Great addon. Native solution would be very nice tho :(

A native solution would make the control system very complex and introduce caveats. The plugin works on this assumption:

Make sure max_size is bigger than the minimum possible size of the child.

The native solution cannot make such assumptions. So it either needs a very detailed resolution policy which value should be considered first, or it cannot be done at all. And even if we had a resolution policy, there is no guarantee we can make it work in a natural manner for every use case or even most use cases.

I initially supported this proposal, but for it to work we would have to make the overall layout system way more complex and way less predictable. I don't think that the benefits outweigh these downsides. I think we can only consider a custom container that would work like that, instead of adding a property to every control and affecting the whole system.

mordae commented 2 years ago

The native solution cannot make such assumptions. So it either needs a very detailed resolution policy which value should be considered first, or it cannot be done at all. And even if we had a resolution policy, there is no guarantee we can make it work in a natural manner for every use case or even most use cases.

CSS policy on min-/max-width: https://www.w3.org/TR/CSS21/visudet.html#min-max-widths

It says explicitly:

The spec is clear in the behavior but it might not be what the author expects

I think that this is what you are concerned about, but I personally believe that a good solution is better than nothing. Especially since there is such a strong precedent in CSS many are already familiar with.

@YuriSizov: Please reconsider.

YuriSizov commented 2 years ago

@mordae Like I've said, it makes sense to me as a dedicated container rather than a built-in property of all controls.

CSS is a bag of hacks, and "The spec is clear in the behavior but it might not be what the author expects" is not a statement that bodes well. Godot's positioning system is in no way similar to the block or flex model, so we can't really make decisions on "Well, they did it".

I am also not a sole judge, so I don't see a reason to plead for me to reconsider. I think I bring up valid concerns.

sosasees commented 2 years ago

after thinking about it for a bit, i support maximum size being exclusive to a MaxSizeContainer node: because minimum size is useful everywhere, but maximum size is situational.

MattUV commented 2 years ago

I think that many comments above made clear that having a built-in max size property isn't a good idea after all. Keeping it as a plugin container is better.

I'll close the the issue now. Thank you everyone !

sosasees commented 2 years ago

Implementing this in core would also allow replacing godotengine/godot#31588 (which is kind of hacky) by a cleaner solution.

how often is max size useful in the actual Godot editor?

is it just this one instance or are there more?

if max size is useful often enough, the editor itself would benefit from the addition of MaxSizeContainer as a built-in node.

barrrettt commented 1 year ago

Ultrawire main menu. image I need limits 😆 I vote for a dedicated container "Limiter". image

vonagam commented 1 year ago

Strange, I did not get what was the real blocker here. The only concrete question presented was how to impartially resolve conflicts when max_size is smaller then min_size and vice versa.

And I don't see a problem here - the last set value beats previously set value: if max_size is being set to a smaller value than already set min_size then min_size gets reduced to match max one. It is the same as with anchor_left and anchor_right for example.

Am I missing something?

YuriSizov commented 1 year ago

The only concrete question presented was how to impartially resolve conflicts when max_size is smaller then min_size and vice versa.

@vonagam

That's not what the main problem was. Minimum size is not a fixed value, it's not just a property the user sets. It's a computed size, that accounts for the minimum size desired by the user, and the actual minimum size required by sizes of the node's children. That minimum size is not directly controlled by the node, or by the user for that matter, so it will conflict with this potential max size property at runtime, where the node's max size could be smaller than the minimum possible size of all of its children (plus stylebox and various spacing properties).

There is no clear way to resolve this without breaking the expectation: either min sizes of the children are not respected, or the max size of the parent isn't.

As a dedicated container node it can be implemented with the documentation explaining how these conflicts are resolved and which expectations are going to be broken (probably, max size won't be enforced in that case). But as a property of all controls affecting size resolution for everything, it will make the entire system less intuitive and less straightforward.

barrrettt commented 1 year ago

I think the goal is to maintain a limit and keep the flexibility in the children.

I think it could be done with a dedicated control to limit the width and / or height. I managed to do it with a script solution i paste it to give some idea. Could you do something like that in a dedicated node?

using Godot;
using System;

/*  
    This script need 1 parent (this) and 3 childs:
    The parent changes the ratios of the children
    to keep the middle child at maximum horizontal size
*/

public partial class Limiter_gui : Control{

    //max size x of center child
    [Export] public float limitX = 2024;

    //the 3 childs
    private Control margin_l;
    private Control content_main;
    private Control margin_r;

    //maximize signal not work, need this:
    [Export] public double checktime = 0.5f;
    private double time;
    private double lastCheck;
    private Window.ModeEnum mode;

    public override void _Ready() { 
        margin_l = GetNode<Control>("margin_l");
        content_main = GetNode<Control>("center");
        margin_r = GetNode<Control>("margin_r");

        //connect event with function, to resize sprites to screen size
        GetTree().Root.SizeChanged += changeSize;
        changeSize();

        //actual mode
        mode = GetTree().Root.Mode;
    }

    public override void _Process(double delta) {
        time += delta;
        if (lastCheck - time > checktime) return; //premature optimization

        Window.ModeEnum modenow = GetTree().Root.Mode;

        //if mode change -> recalc sizes
        if (mode != modenow) {
            changeSize();
            mode = modenow;
            lastCheck = time;
        }
    }

    private void changeSize() {
        float total = Size.x;
        float rest,center;

        if (total > limitX) {
            rest = total - limitX;
            center = limitX;

        } else {
            rest = limitX - total;
            center = total;
        }
        margin_l.SizeFlagsStretchRatio = rest / 2;
        content_main.SizeFlagsStretchRatio = center;
        margin_r.SizeFlagsStretchRatio = rest / 2;
    }
}

the scene

image

This way I can achieve a flexible behavior of the main panel. This panel reacts to changes in the aspect ratio of the Root Viewport.

image

image

It would be interesting to have a native way to do this?

Calinou commented 1 year ago

This way I can achieve a flexible behavior of the main panel. This panel reacts to changes in the aspect ratio of the Root Viewport.

You should be able to achieve this with an AspectRatioContainer and some scripting to dynamically change the aspect ratio at runtime when the viewport is resized.

PS: Check the Multiple resolutions demo, it shows how to clamp the GUI to 16:9 on ultrawide aspect ratios.

LEEROY-3 commented 11 months ago

I do not understand why this is such an overcomplicated discussion.

There is no clear way to resolve this without breaking the expectation: either min sizes of the children are not respected, or the max size of the parent isn't.

Would child's minimum size being greater than parent's maximum size not simply cause an overflow? The child's "extra" size in this case will not contribute to layout. That's the standard behavior of most markup and layout frameworks I've used.

To directly answer your question: Yes, the parent's max-size property will be respected over the child's min-size. @YuriSizov

Somebody please elaborate.

YuriSizov commented 11 months ago

@LEEROY-3 That's not how containers work. They rely on children sizes, and this is not a situation with a clear resolution. An overflow is only one option, and it may not even work because every container's logic depends on being able to fit children.

And it's not like an overflow is a situation that we want to have. It's just a fallback behavior, but at that point the UI is no longer usable.

LEEROY-3 commented 11 months ago

@YuriSizov this is exactly how containers should work from user perspective. This is also a situation with a pretty clear resolution, again, from user perspective. Overflow is generally accepted behavior, even if not the best one, and the technical issues of implementing it should be discussed further instead of closing the issue after two opinions, despite votes for this feature majorly outweighing votes against.

It's impossible due to the logical constraints of this world to respect both the parent and the child when two constraints are present, which is exactly why most markup frameworks, as I said, respect the parent during a conflict like this. Here it's the user's job to ensure that no such conflict occurs.

The importance of providing UI designers with means of easily and quickly constructing user interfaces outweigh the potential design flaws. No matter how "hacky" CSS is in your opinion and how much leaving out the max-size property makes sense from system design perspective, the former has been successfully used for decades and powers most of the internet content.

This issue should either be re-opened for further technical discussion or a new issue proposing for a "size-container", similar to what Unreal Engine has in it's UI toolkit that respects it's min and max size over it's children, opened.

YuriSizov commented 11 months ago

@LEEROY-3 It was closed by the author based on the discussion. This is a normal conclusion for a proposal, even if something is popular as an idea. That said, the issue is open for discussion, and we are having it right now. So feel free to offer your opinion on the technical side of things and how limitations of Godot's UI system can be resolved to support max size on all controls.

I also suggested this can be done as a dedicated container, which would alleviate most of the problems. Or rather would concentrate all the problems under the roof of one control node instead of spreading it across the whole GUI system.

The importance of providing UI designers with means of easily and quickly constructing user interfaces outweigh the potential design flaws.

That's not how you design and build things. You cannot just ignore problems because you really really want something to happen. There are already bugs in the system related to sizing because of the desire to add convenient features like aspect ratio containers without proper thought into how it will fit. And users face them and we cannot easily fix them without uprooting everything to make it work.

What good is a feature if we cannot guarantee it will work properly in every situation and you have to learn to wait for hidden bugs and unexpected behaviors instead of just using it?

As for CSS being a bag of hacks, well, it's something that everyone in webdev knows. It's why it's an impossible task to create a new browser engine these days. Vendors have been implementing crutches on top of crutches to address problems with the box model, with positioning, to add flex model on top. It's a mess, and we cannot just look at what they do and replicate it. It's not a straightforward nor a simple system. It's a very complex and complicated mess.

We don't want this kind of complexity, even if it means that the feature set is going to be smaller. We want a system that is predictable and easy to understand. Both for users and maintainers. So it's an important aspect to discuss and be aware of.

LEEROY-3 commented 11 months ago

So at the very least, we agree about the dedicated control node.

Implementation of size constraint container would potentially solve the aspect ratio problem as well. In this case, this control will explicitly respect it's own constraints over it's children. If we go this route, we will have a clear reference of such element as something similar has already been implemented by Epic for UE.

As far as I can see, this would be the most predictable solution to this problem. I will create a dedicated issue describing this solution asap.

conde2 commented 7 months ago

I agree with this idea: image

Is there any workaround to limit container size, without using code?

PinkSerenity commented 7 months ago

And it's not like an overflow is a situation that we want to have. It's just a fallback behavior, but at that point the UI is no longer usable.

The issue is that the only one solution for dealing with intentional overflows, clip_contents (which is used by ScrollContainer), is static and can't properly resize and expand as possible if the content grows in size. Sure, you could code that in, but I think at least for ScrollContainer, which is made for the purpose of overflows, adding dynamic behavior and amax_size setting is reasonable.

Preferably, each axis should be toggleable, and if scrolling is disabled, content should be clipped instead. This means that ScrollContainer needs to override the default clip_contents behavior, which will remain static for everything else to only make minimal changes for compatibility.

To completely avoid any issues, this behavior could instead be added to a new Container called OverflowContainer, with ScrollContainer being marked as "deprecated". With the next big version change, OverflowContainer could replace ScrollContainer completely and take on its name.

While this could also be an Asset, I believe that dynamic overflow handling is important enough to be a built-in feature.

This would solve proposal #2119 and issue #38575. Additionally, it would provide users with an easier way to setup complicated UIs like horizontally expanding accordion menus or overlays that expand up to a maximum size before becoming scrollable.

KoB-Kirito commented 6 months ago

Maybe it helps to give another real world problem this would solve: https://forum.godotengine.org/t/auto-resizing-richtextlabel-with-minimum-and-maximum-size/46051

The hacking required to get a chat bubble to work like that justifies max_size imho. A dedicated container wouldn't help at all for how wrap works on Label and RichTextLabel.

shanayyy commented 3 months ago

Guys we really should look at SwiftUI's layout philosophy, it's simple and effective. In comparison I think Godot's GUI layout is too complicated. This is an intro: https://www.hackingwithswift.com/books/ios-swiftui/how-layout-works-in-swiftui And this is an online demo: https://www.swiftuifieldguide.com/layout/layout/