Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions parley/src/layout/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ impl<B: Brush> LayoutData<B> {
}
for atom in slice.atoms_start() {
let character = &atom.characters()[0];
let whitespace = character.info.whitespace();
let boundary = character.info.boundary();
let style = &self.styles[character.style_index as usize];
let prev_text_wrap_mode = text_wrap_mode;
Expand All @@ -398,15 +399,31 @@ impl<B: Brush> LayoutData<B> {
let trailing_whitespace = whitespace_advance(prev_atom);
min_width = min_width.max(running_min_width - trailing_whitespace);
running_min_width = 0.0;
if boundary == Boundary::Mandatory {
max_width = max_width.max(running_max_width - trailing_whitespace);
running_max_width = 0.0;
}
}
running_min_width += atom.advance();
running_max_width += atom.advance();
if !is_rtl {
prev_atom = Some((character.info.whitespace(), atom.advance()));

// Handle `Whitespace::Newline` rather than relying on `Boundary::Mandatory`,
// because `Boundary::Mandatory` is only set on the character *following* a break,
// at which point it is too late too handle inline boxes between the line break
// and the following character.
//
// This function doesn't have special handling for CRLF because two linebreaks
// immediately following each are equivalent to one linebreak for the purpose of
// width calculation.
if whitespace == Whitespace::Newline {
let trailing_whitespace = whitespace_advance(prev_atom);
min_width = min_width.max(running_min_width - trailing_whitespace);
max_width = max_width.max(running_max_width - trailing_whitespace);
running_min_width = 0.0;
running_max_width = 0.0;
if !is_rtl {
prev_atom = None;
}
} else {
running_min_width += atom.advance();
running_max_width += atom.advance();
if !is_rtl {
prev_atom = Some((whitespace, atom.advance()));
}
}
}
let trailing_whitespace = whitespace_advance(prev_atom);
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
46 changes: 45 additions & 1 deletion parley_tests/tests/issues.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@

use crate::test_name;
use crate::util::TestEnv;
use parley::{Alignment, AlignmentOptions, PositionedLayoutItem, StyleProperty, TextWrapMode};
use parley::{
Alignment, AlignmentOptions, InlineBox, InlineBoxKind, PositionedLayoutItem, StyleProperty,
TextWrapMode,
};

/// Test that rendering RTL text doesn't affect subsequent LTR layouts.
/// See <https://github.com/linebender/parley/issues/489>.
Expand Down Expand Up @@ -114,3 +117,44 @@ Third line that ends with newlines\n\n";
env.with_name("max_context_with_mandatory_breaks")
.check_layout_snapshot(&layout);
}

/// Test that inline boxes directly following a mandatory line break all
/// contribute to the max content width of the line they end up on.
///
/// The mandatory break used to be detected via the boundary flag on the text
/// atom *after* the break, so inline boxes between the newline and that atom
/// were attributed to the line *before* the break, under-reporting the max
/// content width. In Blitz this caused inline-block links following a `<br>`
/// to wrap inside a shrink-to-fit container even though they fit on one line.
Comment on lines +123 to +128

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
///
/// The mandatory break used to be detected via the boundary flag on the text
/// atom *after* the break, so inline boxes between the newline and that atom
/// were attributed to the line *before* the break, under-reporting the max
/// content width. In Blitz this caused inline-block links following a `<br>`
/// to wrap inside a shrink-to-fit container even though they fit on one line.

#[test]
fn inline_boxes_after_newline_max_content_width() {
let mut env = TestEnv::new(test_name!(), None);

// A newline followed by three inline boxes separated by spaces.
let text = "\n ";
let mut builder = env.ranged_builder(text);
for (id, index) in [(0_u64, 1_usize), (1, 2), (2, 3)] {
builder.push_inline_box(InlineBox {
id,
kind: InlineBoxKind::InFlow,
index,
width: 14.0,
height: 30.0,
baseline: None,
});
}
let mut layout = builder.build(text);

let content_widths = layout.calculate_content_widths();

layout.break_all_lines(Some(content_widths.max));
layout.align(Alignment::Start, AlignmentOptions::default());
assert!(
layout.width() <= content_widths.max,
"Layout should never be wider than the max content width (width: {}, max: {})",
layout.width(),
content_widths.max
);
env.with_name("inline_boxes_after_newline")
.check_layout_snapshot(&layout);
}
Loading