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

Fixed-size layout overflows with measure_function. #732

Open
SoulSharer opened this issue Oct 24, 2024 · 3 comments
Open

Fixed-size layout overflows with measure_function. #732

SoulSharer opened this issue Oct 24, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@SoulSharer
Copy link

SoulSharer commented Oct 24, 2024

taffy version

02453b2 and fae17c2 (v0.6.1)

Platform

Rust

What you did

I took examples/measure.rs and changed:

  • Available space on compute_layout_with_measure() to be fixed 512x512.
  • Changed image size to be 600x720, so that aspect ratio could cause overflow of the layout.
  • Added flex_shrink: 1 to children Styles in hopes it would fix calculation.
  • Root node size fills available space with percent(1.0).

Modified measure.rs follows:

mod common {
    pub mod image;
    pub mod text;
}
use common::image::{image_measure_function, ImageContext};
use common::text::{text_measure_function, FontMetrics, TextContext, WritingMode, LOREM_IPSUM};
use taffy::prelude::*;

enum NodeContext {
    Text(TextContext),
    Image(ImageContext),
}

fn measure_function(
    known_dimensions: taffy::geometry::Size<Option<f32>>,
    available_space: taffy::geometry::Size<taffy::style::AvailableSpace>,
    node_context: Option<&mut NodeContext>,
    font_metrics: &FontMetrics,
) -> Size<f32> {
    if let Size { width: Some(width), height: Some(height) } = known_dimensions {
        return Size { width, height };
    }

    match node_context {
        None => Size::ZERO,
        Some(NodeContext::Text(text_context)) => {
            text_measure_function(known_dimensions, available_space, &*text_context, font_metrics)
        }
        Some(NodeContext::Image(image_context)) => image_measure_function(known_dimensions, image_context),
    }
}

fn main() -> Result<(), taffy::TaffyError> {
    let mut taffy: TaffyTree<NodeContext> = TaffyTree::new();

    let font_metrics = FontMetrics { char_width: 10.0, char_height: 10.0 };

    let text_node = taffy.new_leaf_with_context(
        Style {
            flex_shrink: 1.0,
            ..Default::default()
        },
        NodeContext::Text(TextContext { text_content: LOREM_IPSUM.into(), writing_mode: WritingMode::Horizontal }),
    )?;

    let image_node = taffy
        .new_leaf_with_context(Style {
            flex_shrink: 1.0,
            ..Default::default()
        }, NodeContext::Image(ImageContext { width: 600.0, height: 720.0 }))?;

    let root = taffy.new_with_children(
        Style {
            display: Display::Flex,
            flex_direction: FlexDirection::Column,
            size: percent(1.0),
            ..Default::default()
        },
        &[text_node, image_node],
    )?;

    // Compute layout and print result
    taffy.compute_layout_with_measure(
        root,
        Size { width: taffy::AvailableSpace::Definite(512.0), height: taffy::AvailableSpace::Definite(512.0) },
        // Note: this closure is a FnMut closure and can be used to borrow external context for the duration of layout
        // For example, you may wish to borrow a global font registry and pass it into your text measuring function
        |known_dimensions, available_space, _node_id, node_context, _style| {
            measure_function(known_dimensions, available_space, node_context, &font_metrics)
        },
    )?;
    taffy.print_tree(root);

    Ok(())
}

Result is:

TREE
└──  FLEX COL [x: 0    y: 0    w: 512  h: 512  content_w: 600  content_h: 810  border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967299))
    ├──  LEAF [x: 0    y: 0    w: 512  h: 90   content_w: 512  content_h: 90   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967297))
    └──  LEAF [x: 0    y: 90   w: 512  h: 614  content_w: 600  content_h: 720  border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967298))

Second leaf height overflows available space.

What went wrong

  • what were you expecting?
    Leaf children should fit into available space.

  • what actually happened?
    Second leaf overflows layout.

Additional information

I've tried different style configurations, including flex-basis, but all seems to come down to a measure_function which does not receive full information from a parent to properly size image content that would fit the layout.

[examples\common\image.rs:12:5] known_dimensions = Size {
    width: Some(
        512.0,
    ),
    height: None,
}
[examples\common\image.rs:12:5] known_dimensions = Size {
    width: Some(
        512.0,
    ),
    height: None,
}
[examples\common\image.rs:12:5] known_dimensions = Size {
    width: None,
    height: Some(
        614.4,
    ),
}
[examples\common\image.rs:12:5] known_dimensions = Size {
    width: None,
    height: None,
}
@SoulSharer SoulSharer added the bug Something isn't working label Oct 24, 2024
@nicoburns
Copy link
Collaborator

This seems like a legitimate bug. I think the root of the problem here is that we need to treat "replaced" content such as images specially (how exactly, I'm not quite sure). You might be able to work around it by returning a zero size when the MinContent size is requested.

@SoulSharer
Copy link
Author

SoulSharer commented Oct 26, 2024

Appreciate a timely response!

I tried returning Size::ZERO when AvailableSpace::MIN_CONTENT, it now fits perfectly but the whole point of keeping aspect ratio of an image with measure_function is sadly gone (layout bounds are now stretched).

@juancampa
Copy link

FWIW we're also being affected by the same issue. In our case, it's a "row" flex and it's overflowing to the right. We're also measuring text, instead of an image, but otherwise it's exactly the same.

This is a long standing issue btw, I know because we were seeing it on taffy 0.4.3 and I upgraded yesterday to see if it was fixed but no luck.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants