iced-rs / iced

A cross-platform GUI library for Rust, inspired by Elm
https://iced.rs
MIT License
24.67k stars 1.15k forks source link

Flex logic doesn't properly allocate space between `Length::Shrink` children that need to shrink #1727

Open ids1024 opened 1 year ago

ids1024 commented 1 year ago

Is there an existing issue for this?

Is this issue related to iced?

What happened?

In this example, with a window less than 1000px tall, only the first image is displayed, filling the height.

But if Shrink is changed to Fill, the height is distributed equally.

It seems Length::Shrink::fill_factor() returns 0, and the flex logic doesn't seem to handle that properly.

use iced::{widget, Application};

fn solid_color_image(n: u32, color: [u8; 3]) -> iced::widget::image::Handle {
    let pixels: Vec<_> = (0..n * n)
        .flat_map(|_| [color[0], color[1], color[2], 255])
        .collect();
    iced::widget::image::Handle::from_pixels(n, n, pixels)
}

#[derive(Debug)]
enum Message {}

#[derive(Default)]
struct App {}

impl iced::Application for App {
    type Message = Message;
    type Executor = iced::executor::Default;
    type Flags = ();
    type Theme = iced::Theme;

    fn new(_flags: ()) -> (Self, iced::Command<Message>) {
        (Self::default(), iced::Command::none())
    }

    fn title(&self) -> String {
        "Test".to_string()
    }

    fn update(&mut self, _message: Message) -> iced::Command<Message> {
        iced::Command::none()
    }

    fn view(&self) -> iced::Element<Message> {
        let length = iced::Length::Shrink;
        widget::column![
            widget::image(solid_color_image(1000, [255, 0, 0])).height(length),
            widget::image(solid_color_image(1000, [0, 255, 0])).height(length),
            widget::image(solid_color_image(1000, [0, 0, 255])).height(length),
        ]
        .into()
    }

    fn subscription(&self) -> iced::Subscription<Message> {
        iced::Subscription::none()
    }
}

fn main() {
    App::run(iced::Settings::default()).unwrap();
}

What is the expected behavior?

Presumably it shouldn't be the case that widgets do shrink with Length::Fill but not with Length::Shrink. (If not, the names aren't quite appropriate.)

In this particular case Shrink should behave the same way as Fill, but the exact right logic may be subtle since there could be children with different types of Length, of varying sizes.

Length::Shrink is documented as /// Fill the least amount of space. The flex logic here might make more sense if this really was the minimum space the widget can take up, but this clearly doesn't match how images work (since they can shrink arbitrarily).

Version

master

Operative System

Linux

Do you have any log output?

No response

nicoburns commented 1 year ago

Length::Shrink is documented as /// Fill the least amount of space

This definitely seems not to match the implementation. Shrink is really "determine size based on intrinsic size of the content". IMO this is a sensible sizing mode to have (although as you've noticed doesn't work too well for images where the native size may not be that relevant), but it would be better named as Content, MinContent, HugContents or similar.

hecrj commented 1 year ago

Yes, you are correct. I think we should rename it to FitContent.