Skip to content

Commit

Permalink
Obey white-space when intrinsically sizing an IFC
Browse files Browse the repository at this point in the history
The old logic was assuming that all whitespace was a break opportunity,
and that no newlines would be preserved.

Note that text shaping considers the advance of a newline to be the same
as a space. This was problematic because if we have a segment with a
preserved space and newline, only the advance of the space should
contrinute to the size of the block container. Therefore, I'm changing
the breaker logic in other to have newline characters in their own
segment.

Then glyph_run_is_whitespace_ending_with_preserved_newline can just be
renamed to glyph_run_is_preserved_newline.

This patch is still not perfect because it doesn't check allow_wrap(),
so `nowrap` is treated like `normal`, and `pre-wrap` like `pre`.
  • Loading branch information
Loirooriol committed Mar 14, 2024
1 parent 0e78c81 commit a8339e0
Show file tree
Hide file tree
Showing 24 changed files with 70 additions and 71 deletions.
46 changes: 29 additions & 17 deletions components/gfx/text/text_run.rs
Expand Up @@ -225,6 +225,16 @@ impl<'a> TextRun {

let breaker = breaker.as_mut().unwrap();

let mut push_range = |range: &std::ops::Range<usize>, options: &ShapingOptions| {
glyphs.push(GlyphRun {
glyph_store: font.shape_text(&text[range.clone()], &options),
range: Range::new(
ByteIndex(range.start as isize),
ByteIndex(range.len() as isize),
),
});
};

while !finished {
let (idx, _is_hard_break) = breaker.next(text);
if idx == text.len() {
Expand All @@ -240,9 +250,9 @@ impl<'a> TextRun {

// Split off any trailing whitespace into a separate glyph run.
let mut whitespace = slice.end..slice.end;
if let Some((i, _)) = word
.char_indices()
.rev()
let mut rev_char_indices = word.char_indices().rev().peekable();
let ends_with_newline = rev_char_indices.peek().map_or(false, |&(_, c)| c == '\n');
if let Some((i, _)) = rev_char_indices
.take_while(|&(_, c)| char_is_whitespace(c))
.last()
{
Expand All @@ -254,26 +264,28 @@ impl<'a> TextRun {
continue;
}
if !slice.is_empty() {
glyphs.push(GlyphRun {
glyph_store: font.shape_text(&text[slice.clone()], options),
range: Range::new(
ByteIndex(slice.start as isize),
ByteIndex(slice.len() as isize),
),
});
push_range(&slice, options);
}
if !whitespace.is_empty() {
let mut options = *options;
options
.flags
.insert(ShapingFlags::IS_WHITESPACE_SHAPING_FLAG);
glyphs.push(GlyphRun {
glyph_store: font.shape_text(&text[whitespace.clone()], &options),
range: Range::new(
ByteIndex(whitespace.start as isize),
ByteIndex(whitespace.len() as isize),
),
});

// The breaker breaks after every newline, so either there is none,
// or there is exactly one at the very end. In the latter case,
// split it into a different run. That's because shaping considers
// a newline to have the same advance as a space, but during layout
// we want to treat the newline as having no advance.
if ends_with_newline {
whitespace.end -= 1;
if !whitespace.is_empty() {
push_range(&whitespace, &options);
}
whitespace.start = whitespace.end;
whitespace.end += 1;
}
push_range(&whitespace, &options);
}
slice.start = whitespace.end;
}
Expand Down
42 changes: 23 additions & 19 deletions components/layout_2020/flow/inline.rs
Expand Up @@ -2248,8 +2248,8 @@ struct ContentSizesComputation<'a> {
current_line: ContentSizes,
/// Size for whitepsace pending to be added to this line.
pending_whitespace: Au,
/// Whether or not this IFC has seen any non-whitespace content.
had_non_whitespace_content_yet: bool,
/// Whether or not this IFC has seen any content, excluding discarded whitespace.
had_content_yet: bool,
/// Stack of ending padding, margin, and border to add to the length
/// when an inline box finishes.
ending_inline_pbm_stack: Vec<Length>,
Expand Down Expand Up @@ -2304,31 +2304,35 @@ impl<'a> ContentSizesComputation<'a> {
for run in segment.runs.iter() {
let advance = run.glyph_store.total_advance();

if !run.glyph_store.is_whitespace() {
self.had_non_whitespace_content_yet = true;
self.current_line.min_content += advance;
self.current_line.max_content += self.pending_whitespace + advance;
self.pending_whitespace = Au::zero();
} else {
if run.glyph_store.is_whitespace() {
// If this run is a forced line break, we *must* break the line
// and start measuring from the inline origin once more.
if text_run.glyph_run_is_whitespace_ending_with_preserved_newline(run) {
self.had_non_whitespace_content_yet = true;
if text_run.glyph_run_is_preserved_newline(run) {
self.had_content_yet = true;
self.forced_line_break();
self.current_line = ContentSizes::zero();
continue;
}

// Discard any leading whitespace in the IFC. This will always be trimmed.
if !self.had_non_whitespace_content_yet {
let white_space =
text_run.parent_style.get_inherited_text().white_space;
// TODO: need to handle white_space.allow_wrap() too.
if !white_space.preserve_spaces() {
// Discard any leading whitespace in the IFC. This will always be trimmed.
if self.had_content_yet {
// Wait to take into account other whitespace until we see more content.
// Whitespace at the end of the IFC will always be trimmed.
self.line_break_opportunity();
self.pending_whitespace += advance;
}
continue;
}

// Wait to take into account other whitespace until we see more content.
// Whitespace at the end of the IFC will always be trimmed.
self.line_break_opportunity();
self.pending_whitespace += advance;
}

self.had_content_yet = true;
self.current_line.min_content += advance;
self.current_line.max_content += self.pending_whitespace + advance;
self.pending_whitespace = Au::zero();
}
}
},
Expand All @@ -2341,7 +2345,7 @@ impl<'a> ContentSizesComputation<'a> {
self.current_line.min_content += self.pending_whitespace + outer.min_content;
self.current_line.max_content += self.pending_whitespace + outer.max_content;
self.pending_whitespace = Au::zero();
self.had_non_whitespace_content_yet = true;
self.had_content_yet = true;
},
_ => {},
});
Expand Down Expand Up @@ -2380,7 +2384,7 @@ impl<'a> ContentSizesComputation<'a> {
paragraph: ContentSizes::zero(),
current_line: ContentSizes::zero(),
pending_whitespace: Au::zero(),
had_non_whitespace_content_yet: false,
had_content_yet: false,
ending_inline_pbm_stack: Vec::new(),
}
.traverse(inline_formatting_context)
Expand Down
11 changes: 4 additions & 7 deletions components/layout_2020/flow/text_run.rs
Expand Up @@ -150,7 +150,7 @@ impl TextRunSegment {
// If this whitespace forces a line break, queue up a hard line break the next time we
// see any content. We don't line break immediately, because we'd like to finish processing
// any ongoing inline boxes before ending the line.
if text_run.glyph_run_is_whitespace_ending_with_preserved_newline(run) {
if text_run.glyph_run_is_preserved_newline(run) {
ifc.defer_forced_line_break();
continue;
}
Expand Down Expand Up @@ -426,11 +426,8 @@ impl TextRun {
self.prevent_soft_wrap_opportunity_at_end;
}

pub(super) fn glyph_run_is_whitespace_ending_with_preserved_newline(
&self,
run: &GlyphRun,
) -> bool {
if !run.glyph_store.is_whitespace() {
pub(super) fn glyph_run_is_preserved_newline(&self, run: &GlyphRun) -> bool {
if !run.glyph_store.is_whitespace() || run.range.length() != ByteIndex(1) {
return false;
}
if !self
Expand All @@ -442,7 +439,7 @@ impl TextRun {
return false;
}

let last_byte = self.text.as_bytes().get(run.range.end().to_usize() - 1);
let last_byte = self.text.as_bytes().get(run.range.begin().to_usize());
last_byte == Some(&b'\n')
}
}
Expand Down
2 changes: 0 additions & 2 deletions tests/wpt/meta/css/CSS2/generated-content/content-175.xht.ini

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

2 changes: 0 additions & 2 deletions tests/wpt/meta/css/CSS2/text/white-space-pre-001.xht.ini

This file was deleted.

2 changes: 0 additions & 2 deletions tests/wpt/meta/css/css-lists/inline-list.html.ini

This file was deleted.

@@ -0,0 +1,2 @@
[tab-size-integer-001.html]
expected: FAIL
@@ -0,0 +1,2 @@
[tab-size-integer-002.html]
expected: FAIL
@@ -0,0 +1,2 @@
[tab-size-integer-003.html]
expected: FAIL
@@ -0,0 +1,2 @@
[tab-size-length-001.html]
expected: FAIL
@@ -0,0 +1,2 @@
[tab-size-length-002.html]
expected: FAIL
@@ -0,0 +1,2 @@
[tab-size-percent-001.html]
expected: FAIL

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

2 changes: 2 additions & 0 deletions tests/wpt/mozilla/meta/css/white-space-pre-wrap.htm.ini
@@ -0,0 +1,2 @@
[white-space-pre-wrap.htm]
expected: FAIL

0 comments on commit a8339e0

Please sign in to comment.