PistonDevelopers / conrod

An easy-to-use, 2D GUI library written entirely in Rust.
Other
3.35k stars 298 forks source link

[RFC] Layout in Conrod #1187

Open luleyleo opened 6 years ago

luleyleo commented 6 years ago

The problems with it

Currently, layout in Conrod is done using the two traits Positionable and Sizeable in combination with a couple of nested Enums to represent those.

I have three major issues with this approach:

First the unintuitive naming of some functions in the Positionable and Sizeable traits. Whenever I write a widget, I need the documentation of those traits on my second monitor to look up the functions names.

The next point is that this system sometimes lacks flexibility and you end up manually calculating position and size based on the rect argument in widget::UpdateArgs of the containing widget, the sizes of other widgets and also padding between widgets. This can quickly become reasonably complex and difficult to understand.

Last but not least there is the problem of not knowing the size of child widgets. It is a common use-case in GUI programming to have a parent container that wraps its children and is only as large as necessary to do so. But because Conrod handles widget creation and layout in one step, this is simply not possible. The only means for a widget to suggest a size are the default_width and default_height functions which are invoked before the children are set and thus have no clue about their size.

The alternatives

I've got two ideas to solve those problems:

Keeping the current system

This would require the least amount of work in both regards, refactoring Conrod and projects that depend on it.

The first issue can be solved by "simply" renaming some functions.

There are only two hard things in Computer Science: cache invalidation and naming things ― Phil Karlton

The second would probably remain (at least I can't come up with a solution right now). Perhaps by adding layout abstractions which move this code into its own space, like it is done with the Matrix.

And the third could be solved by doing layout related stuff in a separate step between setting the widgets and rendering them. When doing it this way there needs to be a replacement for the rect argument.

Adopting the Flexbox layout system

While being a completely new approach for Conrod it is the current standard on the web and a library developed by Facebook called Yoga handles layout for React native. This library is written in c++ respecting bindings to it, as it is used mostly in higher level languages. There is already a Rust binding. Due to being based on a c++ library it isn't quite idiomatic, but it should work just fine. The only real problem I have with it is its poor documentation, at least for the c++ core, but it's also relatively small. Using this library would thus mean that Conrod is no longer "written entirely in Rust".

The benefits we would gain with this approach are that most developers who have done at least some web development, which is often the case, are familiar with it and that it is, as the name suggests, flexible and at the same time pretty straightforward to use. Using an existing library which is supported by Facebook assures that it works correctly and is a lot less work to implement.

It also fits into Conrod's data model. We would simply store a widget's node alongside its state and provide a layout function for any widget builder where one can specify its flex attributes as an array of FlexStyles's. Having the layout in retained mode allows it to only calculate changes which should perform very well.

Another thing to keep in mind is floating widgets. I'm not yet sure how to approach those but I'm confident that there is a solution to this.

And then there is the question of how we should deal with widgets that don't have a parent specified. Currently, this doesn't really matter but with flex layout, the hierarchy is of crucial importance. We could implicitly treat them as root widgets that fill the window, or we warn about them and require an explicit .root() call on actual root widgets.

Braking things

Nevermind which approach we choose it will break pretty much all none primitive widgets.

When keeping the current system, it would break by renaming and absence of the rect parameter.

When choosing the Flex approach, there will be no Positionable and no Sizable trait anymore. As this would break pretty much everything, it will require rewriting the widgets altogether, keeping only the complex logic like text editing.

So both would be a major breaking change, but Flexbox even more.


This issue can be seen as an RFC; If someone has experienced other issues or has another idea or concerns, regarding layout, then please comment. When I find the time to do so, I'll give Yoga a shot and will try to implement it.

mitchmindtree commented 6 years ago

Thanks a lot for writing this up @Finnerale! I totally agree, I am also discontent with the current state of layout.

First off, I'll try to summarise the three major issues you mention.

  1. Unintuitive method names in the Positionable and Sizeable traits.
  2. Lack of flexibility w.r.t. Positionable and Sizeable traits.
  3. Calculating parent dimensions from children dimensions.

1.

Would you mind sharing a list of the culprit method names w.r.t. 1? If you have any suggestions for improvements upon each, that would also be greatly appreciated! Or perhaps is it more that you feel the behaviour of the trait methods are unintuitive "in general"?

2. and 3.

I suspect that 2. and 3. are closely related, in that 3. is easily the biggest issue I personally also run into w.r.t. layout flexibility.

The problem case that I personally run into the most is that widget foo often depends on widget bar for its position, while widget bar depends on widget foo for its size. The case you provide is a good example of this, e.g. a group of children may depend on their parent for their position, while the parent may depend on the accumulative size and padding of the children for its size. These constraints become difficult to express in conrod due to the requirement that each widget (including both their size and position) are instantiated one at a time.

One of the causes behind this "restriction" is that the Ui requires knowledge of each widget's absolute size and position by the time the call to Widget::set returns. I forget exactly why this is the case, but it is necessary for ui.rect_of(widget_id) to work and I think it might also be necessary for scrolling calculations, though I don't quite recall for certain! It's worth investigating this as if we can remove this requirement, we could simply let widgets describe positions in an entirely "relative" manner so that all absolute positioning is not evaluated until the end of the call to set_widgets. This would allow a parent to specify its dimensions as "the bounds of all child widgets + some padding" while still allowing us to position the children upon the parent. One condition of this would be that we could never allow a cycle to occur between relative positioning or cycling on a single dimension. E.g. we could not specify a paren't width relative to some child widget while at the same time specifying the child's width relative to the parent widget. What we could do however is allow for specifying a parent's height relative to its children, while specifying the width of the children relatively to the parent. I think the key is to ensure that the size and position along each dimension for each widget eventually propagates up through parents to some parent that has an absolute position and size along those dimensions (in most cases, the ui.window widget whose position is 0, 0 and size is the size of the window). Anyway, I'm diverging quite a lot with this paragraph, I'll get back to your suggestions!

Re FlexBox

I totally agree that adopting a flex-box style of layout could be a nicer and more intuitive way to go! I think I've also expressed this a few times in comments on other issues.

That said, I think binding to an external C++ library to solve this would probably be overkill and create a whole other suite of maintenance issues related to rust-bindgen and C++ which I'm not really willing to take on. A big part of the reason I started conrod was to have a pure-rust solution (bar OS-specific deps) so that building it "just works" using plain old cargo.

I think I much prefer your prior idea of creating some sort of over-arching helper widget along the lines of Matrix or List - something like a FlexList / FlowList / FlexBox / FlexArea or whatever you want to call it. This would allow the widget to do the tricky size/position calculations and provides "item"s (similar to how the List widget currently does) that can allow the user to specify unique widgets, while still automatically sizing and positioning them behind the scenes. This would also mean we don't have to break any of the existing API - just add a new widget.

I've been meaning to get around to this for quite a while, but have not had the time as I'm currently heavily focused on nannou. That said, nannou currently uses conrod for its GUI API so it's likely I'll get back to this at some point if no one beats me to it, I'm just not sure when exactly.

Btw, I am also interested in your idea of separating layout into a separate step, however I imagine this would require a pretty thorough overhaul of the whole layout system, would break lots of things and would generally require a lot more work. I think we can sort of emulate the benefits of this via a FlexBox widget, where the widget itself does the layout calculations before yielding "Item"s or "Element"s that the user can use to instantiate their custom widgets. I would look at the List widget implementation for inspiration. I think the List widget actually already allows for having a different "length" per item, but does not allow for "wrapping" onto a new row/column and also currently requires all widgets to be the same type (a restriction I've been meaning to remove).

I'm sorry I don't have an immediate, solid solution for you @Finnerale, but I hope this at least helps to clarify my thoughts on the topic a little! I really appreciate you taking the time to think this through and open a discussion about it :+1:

luleyleo commented 6 years ago

Problems with Positionable

After thinking a bit more about this I came to the conclusion that it's less a matter of naming than a matter of quantity. There are so many methods which are often just slightly different that it gets super difficult to remember more than a couple of them. But some specific issues:

A layout phase

Btw, I am also interested in your idea of separating layout into a separate step, however, I imagine this would require a pretty thorough overhaul of the whole layout system, would break lots of things and would generally require a lot more work.

First of: I don't think we can advance layout one bit without doing this. That aside it is (/seems) not that difficult (but certainly not easy).

I've implemented basic support for flexbox using yoga as a PoC and found that:

A widgets rect

One of the causes behind this "restriction" is that the Ui requires knowledge of each widget's absolute size and position by the time the call to Widget::set returns. I forget exactly why this is the case, but it is necessary for ui.rect_of(widget_id) to work and I think it might also be necessary for scrolling calculations, though I don't quite recall for certain!

While we can't use it while positioning and sizing widgets, we can use it for events and scrolling as those only need the rect of the layout that was rendered before.

Layout components

I think I much prefer your prior idea of creating some sort of over-arching helper widget along the lines of Matrix or List - something like a FlexList / FlowList / FlexBox / FlexArea or whatever you want to call it.

The only way to make this work (as far as I think) is by having layouts treated like widgets, setting them on the ui and calling them later in the layout phase.

his would also mean we don't have to break any of the existing API - just add a new widget.

It would break things too, as the rect parameter would disappear due to a separate layout phase which is impossible to avoid if we want to infer a parents size based on its children.

Some examples of what layout could look like

Those three systems are the ones that seem viable to me

Extending the current system

I'm not shure how to deal with that case: Three buttons which share their parents width evenly.

|------------------------|
||------||------||------||
|| Btn1 || Btn1 || Btn1 ||
||------||------||------||
|------------------------|

With the current system, I use the rect.w() of the parent and divide it by 3

widget::Button()::new()
    .w(rect.w() / 3.0)
    .set(ids.button1, ui);

New functions

w_of_parent(addition);
h_of_parent(addition);
// or, which would solve the previous problem:
w_of_parent(addition, multiplier);
h_of_parent(addition, multiplier);

These (especialy the second version) would be no problem for me with default values for parameter but

.w_of_parent(0.0)
// or
.w_of_parent(0.0, 1.0)

for the full width doesn't make me happy. This will also add complexity to the Sizeable trait but it should be ok.

Changed behavior

The layout will happen right before drawing and widgets will, by default, only grow as large as necessary to contain their children

Example

widget::Canvas::new()
    .top_left()
    .h(30.0)
    .w_of_parent(0.0)
    .set(ids.toolbar, ui);

TwoButtons::new()
    .parent(ids::Toolbar)
    .top_left()
    .set(ids.tb_item_1, ui);

TwoButtons::new()
    .parent(ids::Toolbar)
    .right(10.0)
    .set(ids.tb_item_2, ui);

widget::Canvas::new()
    // If the win_h is 200.0 -> 200.0 - 30.0 -> 170.0
    .h_of_parent(-30.0)
    .set(ids.content, ui);

widget::Text::new("Some Content")
    .parent(ids.content)
    .middle()
    .set(ids.label, ui);

/// Not real rust
widget TwoButtons {
    fn update(self, args, ui) -> Self::Event {
        self.padding(5.0);

        widget::Button::new()
            .parent(args.id)
            .label("First")
            .top_left()
            .set(ids.first, ui);

        widget::Button::new()
            .parent(args.id)
            .label("Second")
            .right(10.0)
            .set(ids.second, ui);
    }
}

Which would result in something like this:

|----------------------------------------------------|
||--------------------------------------------------||
|| |-----|  |------|    |-----| |------|            ||
|| |First|  |Second|    |First| |Second|            ||
|| |-----|  |------|    |-----| |------|            ||
||--------------------------------------------------||
||--------------------------------------------------||
||                                                  ||
||                                                  ||
||                                                  ||
||                                                  ||
||                   Some Content                   ||
||                                                  ||
||                                                  ||
||                                                  ||
||                                                  ||
||--------------------------------------------------||
|----------------------------------------------------|

The flex alternative

A standard implementation of flexbox. A possible addition would be spacing between the items, for some reason, it's not part of the specs. The hack around looks as following for a spacing of 5 px: Set each items margin to 5 px and the parents margin to -5 px. Alternatively, you can set a margin-left and margin-top for every child except for the last. The biggest benefit of this is probably that once one understands Flexbox (and there are many good learning resources) there is no need for anything else to do layout stuff. For the next two examples, I also renamed Canvas to Group as that name seems more appropriate.

widget::Group::new()
    .column()
    .stretch_items()
    .set(ids.root, ui);

widget::Group::new()
    .parent(ids.root)
    .row()
    .h(30.0)
    .set(ids.toolbar, ui);

TwoButtons::new()
    .parent(ids::toolbar)
    .set(ids.tb_item_1, ui);

TwoButtons::new()
    .parent(ids::toolbar)
    .margin_left(10.0)
    .set(ids.tb_item_2, ui);

widget::Group::new()
    .parent(ids.root)
    .grow(1)
    .justify_center()
    .align_center()
    .set(ids.content, ui);

widget::Text::new("Some Content")
    .parent(ids.content, ui)
    .set(ids.label, ui);

/// Not real rust
widget TwoButtons {
    fn update(self, args, ui) -> Self::Event {
        self.row().padding(5.0);

        widget::Button::new()
            .parent(args.id)
            .label("First")
            .set(ids.first, ui);

        widget::Button::new()
            .parent(args.id)
            .label("Second")
            .margin_left(10.0)
            .set(ids.second, ui);
    }
}

An idea using a more classical layout system

Any node has a layout in the form of a struct which implements a Layout trait

trait Layout {
    fn layout(&self, graph: Graph, widget: Id, children: &[Id])
}

Possible layouts would be

To avoid heap allocation for every widget's layout we could have a LayoutVariant enum like

enum LayoutVariant {
    Stack(Stack),
    Linear(Linerar),
    ...
    Flex(Flex),
    Custom(Box<Layout + 'static>),
}

I would, however, prefer that LayoutVariant is named just Layout but I can't come up with a good name for the Layout trait then.

The Widget trait has a method called layout to specify one:

fn layout(mut self, layout: LayoutVariant) -> Self {
    self.common_mut().layout = layout;
    self
}
// In use:
widget.layout(Linear::new().some_calls().into());

or

fn layout<L>(mut self, layout: L) -> Self
where
    L: Layout + 'static,
    L: Into<LayoutVariant>,
{
    self.common_mut().layout = layout.into();
    self
}

which would take care of the into() call for you.

In practice something like this:

widget::Group::new()
    .layout(Linear::new().grow(ids.content))
    .set(ids.root, ui);

widget::Group::new()
    .parent(ids.root)
    .layout(Linear::new().row().spacing(10.0))
    // This could also be moved to the layout's builder
    // but I can't imagine a layout where children dont
    // need a way to specify their size
    .h(30.0)
    .set(ids.toolbar, ui);

TwoButtons::new()
    .parent(ids::toolbar)
    .set(ids.tb_item_1, ui);

TwoButtons::new()
    .parent(ids::toolbar)
    .set(ids.tb_item_2, ui);

widget::Group::new()
    .parent(ids.root)
    // This could be the default
    .layout(Stack::new().centered())
    .set(ids.content, ui);

widget::Text::new("Some Content")
    .parent(ids.content, ui)
    .set(ids.label, ui);

/// Not real rust
widget TwoButtons {
    fn layout(&self, &state) -> LayoutVariant {
        Linear::new().row().spacing(10.0).into()
    }

    fn update(self, args, ui) -> Self::Event {
        // The alternative to a `Widget::Layout` function
        self.layout(Linear::new().row().spacing(10.0));

        widget::Button::new()
            .parent(args.id)
            .label("First")
            .set(ids.first, ui);

        widget::Button::new()
            .parent(args.id)
            .label("Second")
            .set(ids.second, ui);
    }
}
dvc94ch commented 5 years ago

Can't any layout engine be plugged in by setting wh and xy explicitly? I kept getting a WouldCycle error so that's what I started doing. The question is if the layout part can't be moved to an optional crate

milkowski commented 5 years ago

There is Rust native flexbox implementation called Stretch https://vislyhq.github.io/stretch/ No need do use any C++ externals nor NIH reimplementations.