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

Handling 0.0 Flex params #1674

Closed
arthmis opened this issue Mar 25, 2021 · 4 comments
Closed

Handling 0.0 Flex params #1674

arthmis opened this issue Mar 25, 2021 · 4 comments
Labels
enhancement adds or requests a new feature widget concerns a particular widget

Comments

@arthmis
Copy link
Collaborator

arthmis commented Mar 25, 2021

The docs don't warn against or protect against using 0.0 as a Flex parameter or mixing negative and positive flex parameters.

  • Using 0.0 as a Flex parameter causes the BoxConstraints passed into the flex children to be infinite.
  • If there are multiple flex children then their params all have to be negative or all positive.
  • If different children have a mix of positive and negative flex params then the sizes are incorrect.

Using 0.0:

    let button = Label::new("Hello").center();
    let container = Container::new(button);
    let child = SizedBox::new(container)
        .width(300.)
        .height(300.)
        .expand()
        .center();
    Flex::column().with_flex_child(child, 0.0)

These are the warnings for using 0.0

Mar 25 11:08:28.826  WARN layout: druid::box_constraints: Infinite minimum height constraint passed to Container:
Mar 25 11:08:28.827  WARN layout: druid::box_constraints: Infinite minimum height constraint passed to Align:
Mar 25 11:08:28.828  WARN layout: druid::core: Widget `druid::widget::align::Align<()>` has an infinite height.
Mar 25 11:08:28.829  WARN layout: druid::widget::sized_box: SizedBox is returning an infinite height.
Mar 25 11:08:28.829  WARN layout: druid::core: Widget `druid::widget::sized_box::SizedBox<()>` has an infinite height.
Mar 25 11:08:28.829  WARN layout: druid::widget::align: Align widget's child has an infinite height.
Mar 25 11:08:28.830  WARN layout: druid::core: Widget `druid::widget::align::Align<()>` has an infinite height.
Mar 25 11:08:28.830  WARN layout: druid::widget::flex: A non-Flex child has an infinite height.

Mixing negative and positive params like this will give incorrect sizes:

    let button = Label::new("Hello").center();
    let container = Container::new(button);
    let child = SizedBox::new(container)
        .width(300.)
        .height(300.)
        .expand()
        .center();
     let second_child = SizedBox::empty()
         .width(500.)
         .height(400.)
         .background(Color::WHITE);
    Flex::column().with_flex_child(child, 1.0)
     .with_flex_child(second_child, -2.0)

This is the warning output:

Mar 25 11:07:55.656  WARN layout: druid::box_constraints: Bad BoxConstraints passed to Align:
Mar 25 11:07:55.656  WARN layout: druid::box_constraints: BoxConstraints { min: 0.0W×0.0H, max: 584.0W×-561.0H }
Mar 25 11:07:55.657  WARN layout: druid::box_constraints: Bad BoxConstraints passed to SizedBox:
Mar 25 11:07:55.657  WARN layout: druid::box_constraints: BoxConstraints { min: 0.0W×0.0H, max: 584.0W×-561.0H }
Mar 25 11:07:55.657  WARN layout: druid::box_constraints: Bad BoxConstraints passed to Container:
Mar 25 11:07:55.658  WARN layout: druid::box_constraints: BoxConstraints { min: 584.0W×-561.0H, max: 584.0W×-561.0H }
[druid\src\widget\flex.rs:678] child_bc = BoxConstraints {
    min: 0.0W×0.0H,
    max: 584.0W×-561.0H,
}
[druid\src\widget\flex.rs:674] desired_major = 1122.0
[druid\src\widget\flex.rs:674] actual_major = 1122.0
[druid\src\widget\flex.rs:678] child_bc = BoxConstraints {
    min: 0.0W×0.0H,
    max: 584.0W×1122.0H,
}
[druid\src\widget\flex.rs:678] child_size = 500.0W×400.0H

I think flex values should be forced to be greater than 0.0.

@cmyr
Copy link
Member

cmyr commented Mar 25, 2021

Agreed, I think either an assert or a warning + replacing with 1.0 would both be reasonable here. Since these params tend to be static and not computed I think an assert (or at least a debug assert) is reasonable? I would be happy for a PR for this.

@cmyr cmyr added enhancement adds or requests a new feature widget concerns a particular widget labels Mar 25, 2021
@arthmis
Copy link
Collaborator Author

arthmis commented Mar 31, 2021

Would it be ok to do debug assert, so that it crashes in debug mode, but when it's release it will warn and replace with 1.0?

@cmyr
Copy link
Member

cmyr commented Mar 31, 2021

@arthmis sure, that works for me.

@arthmis
Copy link
Collaborator Author

arthmis commented Apr 7, 2021

closed with #1691

@arthmis arthmis closed this as completed Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement adds or requests a new feature widget concerns a particular widget
Projects
None yet
Development

No branches or pull requests

2 participants