From f97f042a746ed14f2915c5b46de29d582140c70f Mon Sep 17 00:00:00 2001 From: William Salmon Date: Sat, 23 Oct 2021 15:19:41 +0100 Subject: [PATCH 1/4] Fix Fill options for non Fixed width options --- druid/src/widget/image.rs | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/druid/src/widget/image.rs b/druid/src/widget/image.rs index 656c4438f1..9ba71065e3 100644 --- a/druid/src/widget/image.rs +++ b/druid/src/widget/image.rs @@ -198,19 +198,14 @@ impl Widget for Image { bc.debug_check("Image"); // If either the width or height is constrained calculate a value so that the image fits - // in the size exactly. If it is unconstrained by both width and height take the size of - // the image. + // in the size exactly. If it is unconstrained by both width and height then use the fill + // strategy to determine the size. let max = bc.max(); let image_size = self.image_size(); - let size = if bc.is_width_bounded() && !bc.is_height_bounded() { - let ratio = max.width / image_size.width; - Size::new(max.width, ratio * image_size.height) - } else if bc.is_height_bounded() && !bc.is_width_bounded() { - let ratio = max.height / image_size.height; - Size::new(ratio * image_size.width, max.height) - } else { - bc.constrain(image_size) - }; + let affine = self.fill.affine_to_fill(max, image_size).as_coeffs(); + // The first and forth elements of the affine are the x and y scale factor. + // So just multiply them by the original size to get the ideal area based on `self.fill`. + let size = Size::new(affine[0] * image_size.width, affine[3] * image_size.height); trace!("Computed size: {}", size); size } @@ -367,20 +362,18 @@ mod tests { // A middle row of 600 pixels is 100 padding 200 black, 200 white and then 100 padding. let expecting: Vec = [ - vec![41, 41, 41, 255].repeat(100), vec![255, 255, 255, 255].repeat(200), vec![0, 0, 0, 255].repeat(200), - vec![41, 41, 41, 255].repeat(100), + vec![41, 41, 41, 255].repeat(200), ] .concat(); assert_eq!(raw_pixels[199 * 600 * 4..200 * 600 * 4], expecting[..]); // The final row of 600 pixels is 100 padding 200 black, 200 white and then 100 padding. let expecting: Vec = [ - vec![41, 41, 41, 255].repeat(100), vec![0, 0, 0, 255].repeat(200), vec![255, 255, 255, 255].repeat(200), - vec![41, 41, 41, 255].repeat(100), + vec![41, 41, 41, 255].repeat(200), ] .concat(); assert_eq!(raw_pixels[399 * 600 * 4..400 * 600 * 4], expecting[..]); From f5df3bea409a7bb29997143522542516d06e2c40 Mon Sep 17 00:00:00 2001 From: William Salmon Date: Sat, 23 Oct 2021 18:04:09 +0100 Subject: [PATCH 2/4] Constrain to max bounds --- druid/src/widget/image.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/druid/src/widget/image.rs b/druid/src/widget/image.rs index 9ba71065e3..59ec65e04d 100644 --- a/druid/src/widget/image.rs +++ b/druid/src/widget/image.rs @@ -205,7 +205,13 @@ impl Widget for Image { let affine = self.fill.affine_to_fill(max, image_size).as_coeffs(); // The first and forth elements of the affine are the x and y scale factor. // So just multiply them by the original size to get the ideal area based on `self.fill`. - let size = Size::new(affine[0] * image_size.width, affine[3] * image_size.height); + let mut width = affine[0] * image_size.width; + let mut height = affine[3] * image_size.height; + // We are using the image scale properties so if one dimension + // would be over scaled to keep AR fixed then we just clip back to the `bc.max()` + width = width.min(max.width); + height = height.min(max.height); + let size = Size::new(width, height); trace!("Computed size: {}", size); size } From bd96450e6a889a2660efdc9f62d53844c72f6503 Mon Sep 17 00:00:00 2001 From: William Salmon Date: Mon, 1 Nov 2021 20:51:53 +0000 Subject: [PATCH 3/4] Set layout tests to use FillStrat::Fill to expose inf fill issue --- druid/src/widget/image.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/druid/src/widget/image.rs b/druid/src/widget/image.rs index 59ec65e04d..9d86fd9eaa 100644 --- a/druid/src/widget/image.rs +++ b/druid/src/widget/image.rs @@ -438,7 +438,7 @@ mod tests { ); let image_widget = - Scroll::new(Container::new(Image::new(image_data)).with_id(id_1)).vertical(); + Scroll::new(Container::new(Image::new(image_data).fill_mode(FillStrat::Fill)).with_id(id_1)).vertical(); Harness::create_simple(true, image_widget, |harness| { harness.send_initial_events(); @@ -466,7 +466,7 @@ mod tests { ); let image_widget = - Scroll::new(Container::new(Image::new(image_data)).with_id(id_1)).horizontal(); + Scroll::new(Container::new(Image::new(image_data).fill_mode(FillStrat::Fill)).with_id(id_1)).horizontal(); Harness::create_simple(true, image_widget, |harness| { harness.send_initial_events(); From e69fa2a91f789c201bc8d6f446b44d1b97d67a79 Mon Sep 17 00:00:00 2001 From: William Salmon Date: Mon, 1 Nov 2021 20:49:56 +0000 Subject: [PATCH 4/4] Catch edge cases were the image widget would request inf sized layout --- druid/src/widget/image.rs | 43 +++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/druid/src/widget/image.rs b/druid/src/widget/image.rs index 9d86fd9eaa..78eaffc801 100644 --- a/druid/src/widget/image.rs +++ b/druid/src/widget/image.rs @@ -197,12 +197,34 @@ impl Widget for Image { ) -> Size { bc.debug_check("Image"); - // If either the width or height is constrained calculate a value so that the image fits - // in the size exactly. If it is unconstrained by both width and height then use the fill - // strategy to determine the size. + // Let the fill strat determine the size of the widget as a function of the bc.max() + // and the original image size. But catch cases were they would return INFINITY + // and try to make a sensible area as a function of the fill strat. let max = bc.max(); let image_size = self.image_size(); - let affine = self.fill.affine_to_fill(max, image_size).as_coeffs(); + let mut max_width = bc.max().width; + let mut max_height = bc.max().height; + let mut fill_strat = self.fill; + if max_width == f64::INFINITY && max_height == f64::INFINITY { + // If we are in a scroll box for width and height then fall back to + // original image size + max_height = image_size.height; + max_width = image_size.width; + } else if max_width == f64::INFINITY || max_height == f64::INFINITY { + // If we are in a scroll box for width or heigh but not both + // then give self.fill a box the size of the scroll box and let it decide how to + // fill it. Unless that would result in requesting INFINITY in which case fall back + // to contain. + fill_strat = match fill_strat { + FillStrat::FitWidth if max_width == f64::INFINITY => FillStrat::Contain, + FillStrat::FitHeight if max_height == f64::INFINITY => FillStrat::Contain, + FillStrat::Cover | FillStrat::Fill => FillStrat::Contain, + _ => fill_strat, + } + } + let affine = fill_strat + .affine_to_fill(Size::new(max_width, max_height), image_size) + .as_coeffs(); // The first and forth elements of the affine are the x and y scale factor. // So just multiply them by the original size to get the ideal area based on `self.fill`. let mut width = affine[0] * image_size.width; @@ -211,6 +233,7 @@ impl Widget for Image { // would be over scaled to keep AR fixed then we just clip back to the `bc.max()` width = width.min(max.width); height = height.min(max.height); + let size = Size::new(width, height); trace!("Computed size: {}", size); size @@ -437,8 +460,10 @@ mod tests { 2, ); - let image_widget = - Scroll::new(Container::new(Image::new(image_data).fill_mode(FillStrat::Fill)).with_id(id_1)).vertical(); + let image_widget = Scroll::new( + Container::new(Image::new(image_data).fill_mode(FillStrat::Fill)).with_id(id_1), + ) + .vertical(); Harness::create_simple(true, image_widget, |harness| { harness.send_initial_events(); @@ -465,8 +490,10 @@ mod tests { 2, ); - let image_widget = - Scroll::new(Container::new(Image::new(image_data).fill_mode(FillStrat::Fill)).with_id(id_1)).horizontal(); + let image_widget = Scroll::new( + Container::new(Image::new(image_data).fill_mode(FillStrat::Fill)).with_id(id_1), + ) + .horizontal(); Harness::create_simple(true, image_widget, |harness| { harness.send_initial_events();