leyoda/cosmic-text
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix #134 and include a test for it.

Browse source 
Try to ensure that using "the width computed during an unconstrained
layout" as the width constraint during a relayout produces the same
layout. This is useful for certain UI layout algorithms.

See https://github.com/pop-os/cosmic-text/issues/134

* Instead of computing the LayoutLine width from the positioned and
  aligned glyphs, we pass through width computed during line wrapping
  (unless justified alignment is used, in this case we use the old
  approach because the use case for measuring the width isn't really
  applicable to justified text since that will just expand to the
  provided width). For the produced width to later give the same
  wrapping results when passed in as the `line_width` it needs to use
  the same exact float arithmatic that was used to compute the width
  that is compared against `line_width` when making line wrapping
  choices. Passing this width through as the LayoutLine width is the
  most covenient option without making more major changse to the API.
  Nevertheless, I am imagining that if we get a dedicated measurement
  method (i.e. that doesn't do the final positioning and alignment of
  glyphs and which caches `Vec<VisualLine>`), then this width can just
  be exposed there instead of preservering it in LayoutLine.
* Incidentally, this fixes
  https://github.com/pop-os/cosmic-text/issues/169.
* Switch substraction from `fit_x` to checking whether potential
  addition to the current line width would exceed the `line_width`. This
  avoids the float error being dependent on the provided `line_width`
  value.
* When eliminating trailing space from the line width, we avoid
  backtracking with subtraction (which would not give the same exact
  value due to float error) and instead save the previous width and use
  that.
* If the previous word did not exceed the line_width, we now include a
  single blank word even if it would cross the width limit since its
  width won't be counted. This is necessary to get the same wrapping
  behavior when re-using the measured width (which doesn't count a
  single trailing blank word). Note, this whitespace logic may be
  reworked anyway if <https://github.com/pop-os/cosmic-text/issues/155>
  is addressed.
* Change tests to use `opt-level=1` to keep test runtime down.
* Add `fonts` folder for fonts used in tests.
* Fix an issue where a non-breaking whitespace was assumed to be the
  start of a section of spaces which included characters that weren't
  even whitespace.
* Add some TODOs about incongruencies between `is_whitespace`,
  justification, and line breaks.
This commit is contained in:
Imbris 2023-08-24 22:37:32 -04:00
parent 665d3d86b9
commit 91674d5a49
5 changed files with 260 additions and 48 deletions
Download patch file Download diff file Expand all files Collapse all files

3
Cargo.toml
View file

@ -58,3 +58,6 @@ members = [
[dev-dependencies]
criterion = { version = "0.5.1", default-features = false, features = ["cargo_bench_support"] }
[profile.test]
opt-level = 1

93
fonts/FiraMono-LICENSE Normal file
View file

@ -0,0 +1,93 @@
Digitized data copyright (c) 2012-2015, The Mozilla Foundation and Telefonica S.A.
This Font Software is licensed under the SIL Open Font License, Version 1.1.
This license is copied below, and is also available with a FAQ at:
http://scripts.sil.org/OFL
-----------------------------------------------------------
SIL OPEN FONT LICENSE Version 1.1 - 26 February 2007
-----------------------------------------------------------
PREAMBLE
The goals of the Open Font License (OFL) are to stimulate worldwide
development of collaborative font projects, to support the font creation
efforts of academic and linguistic communities, and to provide a free and
open framework in which fonts may be shared and improved in partnership
with others.
The OFL allows the licensed fonts to be used, studied, modified and
redistributed freely as long as they are not sold by themselves. The
fonts, including any derivative works, can be bundled, embedded,
redistributed and/or sold with any software provided that any reserved
names are not used by derivative works. The fonts and derivatives,
however, cannot be released under any other type of license. The
requirement for fonts to remain under this license does not apply
to any document created using the fonts or their derivatives.
DEFINITIONS
"Font Software" refers to the set of files released by the Copyright
Holder(s) under this license and clearly marked as such. This may
include source files, build scripts and documentation.
"Reserved Font Name" refers to any names specified as such after the
copyright statement(s).
"Original Version" refers to the collection of Font Software components as
distributed by the Copyright Holder(s).
"Modified Version" refers to any derivative made by adding to, deleting,
or substituting -- in part or in whole -- any of the components of the
Original Version, by changing formats or by porting the Font Software to a
new environment.
"Author" refers to any designer, engineer, programmer, technical
writer or other person who contributed to the Font Software.
PERMISSION & CONDITIONS
Permission is hereby granted, free of charge, to any person obtaining
a copy of the Font Software, to use, study, copy, merge, embed, modify,
redistribute, and sell modified and unmodified copies of the Font
Software, subject to the following conditions:
1) Neither the Font Software nor any of its individual components,
in Original or Modified Versions, may be sold by itself.
2) Original or Modified Versions of the Font Software may be bundled,
redistributed and/or sold with any software, provided that each copy
contains the above copyright notice and this license. These can be
included either as stand-alone text files, human-readable headers or
in the appropriate machine-readable metadata fields within text or
binary files as long as those fields can be easily viewed by the user.
3) No Modified Version of the Font Software may use the Reserved Font
Name(s) unless explicit written permission is granted by the corresponding
Copyright Holder. This restriction only applies to the primary font name as
presented to the users.
4) The name(s) of the Copyright Holder(s) or the Author(s) of the Font
Software shall not be used to promote, endorse or advertise any
Modified Version, except to acknowledge the contribution(s) of the
Copyright Holder(s) and the Author(s) or with their explicit written
permission.
5) The Font Software, modified or unmodified, in part or in whole,
must be distributed entirely under this license, and must not be
distributed under any other license. The requirement for fonts to
remain under this license does not apply to any document created
using the Font Software.
TERMINATION
This license becomes null and void if any of the above conditions are
not met.
DISCLAIMER
THE FONT SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO ANY WARRANTIES OF
MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT
OF COPYRIGHT, PATENT, TRADEMARK, OR OTHER RIGHT. IN NO EVENT SHALL THE
COPYRIGHT HOLDER BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
INCLUDING ANY GENERAL, SPECIAL, INDIRECT, INCIDENTAL, OR CONSEQUENTIAL
DAMAGES, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM, OUT OF THE USE OR INABILITY TO USE THE FONT SOFTWARE OR FROM
OTHER DEALINGS IN THE FONT SOFTWARE.

BIN
fonts/FiraMono-Medium.ttf Normal file
View file

Binary file not shown.

129
src/shape.rs
View file

@ -569,11 +569,14 @@ impl ShapeSpan {
let mut start_word = 0;
for (end_lb, _) in unicode_linebreak::linebreaks(span) {
let mut start_lb = end_lb;
for (i, c) in span[start_word..end_lb].char_indices() {
if start_word + i == end_lb {
break;
} else if c.is_whitespace() {
for (i, c) in span[start_word..end_lb].char_indices().rev() {
// TODO: Not all whitespace characters are linebreakable, e.g. 00A0 (No-break
// space)
// https://www.unicode.org/reports/tr14/#GL
// https://www.unicode.org/Public/UCD/latest/ucd/PropList.txt
if c.is_whitespace() {
start_lb = start_word + i;
} else {
break;
}
}
@ -946,9 +949,9 @@ impl ShapeLine {
);
}
} else {
let mut fit_x = line_width;
for (span_index, span) in self.spans.iter().enumerate() {
let mut word_range_width = 0.;
let mut width_before_last_blank = 0.;
let mut number_of_blanks: u32 = 0;
// Create the word ranges that fits in a visual line
@ -957,19 +960,30 @@ impl ShapeLine {
let mut fitting_start = (span.words.len(), 0);
for (i, word) in span.words.iter().enumerate().rev() {
let word_width = font_size * word.x_advance;
if fit_x - word_width >= 0. {
// Addition in the same order used to compute the final width, so that
// relayouts with that width as the `line_width` will produce the same
// wrapping results.
if current_visual_line.w + (word_range_width + word_width)
<= line_width
// Include on blank word over the width limit since it won't be counted
// in the final width
|| (word.blank
&& (current_visual_line.w + word_range_width) <= line_width)
{
// fits
fit_x -= word_width;
word_range_width += word_width;
if word.blank {
number_of_blanks += 1;
width_before_last_blank = word_range_width;
}
word_range_width += word_width;
continue;
} else if wrap == Wrap::Glyph {
for (glyph_i, glyph) in word.glyphs.iter().enumerate().rev() {
let glyph_width = font_size * glyph.x_advance;
if fit_x - glyph_width >= 0. {
fit_x -= glyph_width;
if current_visual_line.w + (word_range_width + glyph_width)
<= line_width
{
word_range_width += glyph_width;
continue;
} else {
@ -985,30 +999,33 @@ impl ShapeLine {
current_visual_line = VisualLine::default();
number_of_blanks = 0;
fit_x = line_width - glyph_width;
word_range_width = glyph_width;
fitting_start = (i, glyph_i + 1);
}
}
} else {
// Wrap::Word
let mut trailing_space_width = None;
if let Some(previous_word) = span.words.get(i + 1) {
// Current word causing a wrap is not whitespace, so we ignore the
// previous word if it's a whitespace
if previous_word.blank {
trailing_space_width =
Some(previous_word.x_advance * font_size);
number_of_blanks = number_of_blanks.saturating_sub(1);
}
}
if let Some(width) = trailing_space_width {
// TODO: What if the previous span ended with whitespace and the next
// span wraps a new line? Is that possible?
//
// TODO: This comment it outdated, the current word can be a
// whitespace.
//
// Current word causing a wrap is not whitespace, so we ignore the
// previous word if it's a whitespace
let trailing_blank = span
.words
.get(i + 1)
.map_or(false, |previous_word| previous_word.blank);
if trailing_blank {
number_of_blanks = number_of_blanks.saturating_sub(1);
add_to_visual_line(
&mut current_visual_line,
span_index,
(i + 2, 0),
fitting_start,
word_range_width - width,
width_before_last_blank,
number_of_blanks,
);
} else {
@ -1026,11 +1043,9 @@ impl ShapeLine {
number_of_blanks = 0;
if word.blank {
fit_x = line_width;
word_range_width = 0.;
fitting_start = (i, 0);
} else {
fit_x = line_width - word_width;
word_range_width = word_width;
fitting_start = (i + 1, 0);
}
@ -1049,19 +1064,26 @@ impl ShapeLine {
let mut fitting_start = (0, 0);
for (i, word) in span.words.iter().enumerate() {
let word_width = font_size * word.x_advance;
if fit_x - word_width >= 0. {
if current_visual_line.w + (word_range_width + word_width)
<= line_width
// Include on blank word over the width limit since it won't be counted
// in the final width.
|| (word.blank
&& (current_visual_line.w + word_range_width) <= line_width)
{
// fits
fit_x -= word_width;
word_range_width += word_width;
if word.blank {
number_of_blanks += 1;
width_before_last_blank = word_range_width;
}
word_range_width += word_width;
continue;
} else if wrap == Wrap::Glyph {
for (glyph_i, glyph) in word.glyphs.iter().enumerate() {
let glyph_width = font_size * glyph.x_advance;
if fit_x - glyph_width >= 0. {
fit_x -= glyph_width;
if current_visual_line.w + (word_range_width + glyph_width)
<= line_width
{
word_range_width += glyph_width;
continue;
} else {
@ -1077,32 +1099,24 @@ impl ShapeLine {
current_visual_line = VisualLine::default();
number_of_blanks = 0;
fit_x = line_width - glyph_width;
word_range_width = glyph_width;
fitting_start = (i, glyph_i);
}
}
} else {
// Wrap::Word
let mut trailing_space_width = None;
if i > 0 {
if let Some(previous_word) = span.words.get(i - 1) {
// Current word causing a wrap is not whitespace, so we ignore the
// previous word if it's a whitespace
if previous_word.blank {
trailing_space_width =
Some(previous_word.x_advance * font_size);
number_of_blanks = number_of_blanks.saturating_sub(1);
}
}
}
if let Some(width) = trailing_space_width {
// Current word causing a wrap is not whitespace, so we ignore the
// previous word if it's a whitespace
let trailing_blank = i > 0 && span.words[i - 1].blank;
if trailing_blank {
number_of_blanks = number_of_blanks.saturating_sub(1);
add_to_visual_line(
&mut current_visual_line,
span_index,
fitting_start,
(i - 1, 0),
word_range_width - width,
width_before_last_blank,
number_of_blanks,
);
} else {
@ -1120,11 +1134,9 @@ impl ShapeLine {
number_of_blanks = 0;
if word.blank {
fit_x = line_width;
word_range_width = 0.;
fitting_start = (i + 1, 0);
} else {
fit_x = line_width - word_width;
word_range_width = word_width;
fitting_start = (i, 0);
}
@ -1166,6 +1178,19 @@ impl ShapeLine {
(Align::Center, _) => (line_width - visual_line.w) / 2.0,
(Align::End, _) => line_width - visual_line.w,
(Align::Justified, _) => {
// TODO: Only certain `is_whitespace` chars are typically expanded.
//
// https://www.unicode.org/reports/tr14/#Introduction
// > When expanding or compressing interword space according to common
// > typographical practice, only the spaces marked by U+0020 SPACE and U+00A0
// > NO-BREAK SPACE are subject to compression, and only spaces marked by U+0020
// > SPACE, U+00A0 NO-BREAK SPACE, and occasionally spaces marked by U+2009 THIN
// > SPACE are subject to expansion. All other space characters normally have
// > fixed width.
//
// (also some spaces aren't followed by potential linebreaks but they could
// still be expanded)
// Don't justify the last line in a paragraph.
if visual_line.spaces > 0 && index != number_of_visual_lines - 1 {
(line_width - visual_line.w) / visual_line.spaces as f32
@ -1335,7 +1360,15 @@ impl ShapeLine {
let mut glyphs_swap = Vec::new();
mem::swap(&mut glyphs, &mut glyphs_swap);
layout_lines.push(LayoutLine {
w: if self.rtl { start_x - x } else { x },
w: if align != Align::Justified {
visual_line.w
} else {
if self.rtl {
start_x - x
} else {
x
}
},
max_ascent: max_ascent * font_size,
max_descent: max_descent * font_size,
glyphs: glyphs_swap,

83
tests/wrap_stability.rs Normal file
View file

@ -0,0 +1,83 @@
use cosmic_text::{
fontdb, Align, Attrs, AttrsList, BidiParagraphs, Family, FontSystem, LayoutLine, ShapeLine,
Shaping, Weight, Wrap,
};
// Test for https://github.com/pop-os/cosmic-text/issues/134
//
// Being able to get the same wrapping when feeding the measured width back into ShapeLine::layout
// as the new width limit is very useful for certain UI layout use cases.
#[test]
fn stable_wrap() {
let font_size = 18.0;
let attrs = AttrsList::new(
Attrs::new()
.family(Family::Name("FiraMono"))
.weight(Weight::MEDIUM),
);
let mut font_system =
FontSystem::new_with_locale_and_db("en-US".into(), fontdb::Database::new());
let font = std::fs::read("fonts/FiraMono-Medium.ttf").unwrap();
font_system.db_mut().load_font_data(font);
let mut check_wrap = |text: &_, wrap, start_width| {
let line = ShapeLine::new(&mut font_system, text, &attrs, Shaping::Advanced);
let layout_unbounded = line.layout(font_size, start_width, wrap, Some(Align::Left));
let max_width = layout_unbounded.iter().map(|l| l.w).fold(0.0, f32::max);
let new_limit = f32::min(start_width, max_width);
let layout_bounded = line.layout(font_size, new_limit, wrap, Some(Align::Left));
let bounded_max_width = layout_bounded.iter().map(|l| l.w).fold(0.0, f32::max);
// For debugging:
// dbg_layout_lines(text, &layout_unbounded);
// dbg_layout_lines(text, &layout_bounded);
assert_eq!(
(max_width, layout_unbounded.len()),
(bounded_max_width, layout_bounded.len()),
"Wrap \"{wrap:?}\" with text: \"{text}\"",
);
for (u, b) in layout_unbounded[1..].iter().zip(layout_bounded[1..].iter()) {
assert_eq!(u.w, b.w, "Wrap {wrap:?} with text: \"{text}\"",);
}
};
let hello_sample = std::fs::read_to_string("sample/hello.txt").unwrap();
let cases = [
"(6) SomewhatBoringDisplayTransform",
"",
" ",
" ",
" ",
" ",
]
.into_iter()
// This has several cases where the line would wrap when the computed width was used as the
// width limit.
.chain(BidiParagraphs::new(&hello_sample));
for text in cases {
for wrap in [Wrap::Word, Wrap::Glyph] {
for start_width in [f32::MAX, 80.0, 198.2132, 20.0, 4.0, 300.0] {
check_wrap(text, wrap, start_width);
let with_spaces = format!("{text} ");
check_wrap(&with_spaces, wrap, start_width);
let with_spaces_2 = format!("{text} ");
check_wrap(&with_spaces_2, wrap, start_width);
}
}
}
}
#[allow(dead_code)]
fn dbg_layout_lines(text: &str, lines: &[LayoutLine]) {
for line in lines {
let mut s = String::new();
for glyph in line.glyphs.iter() {
s.push_str(&text[glyph.start..glyph.end]);
}
println!("\"{s}\"");
}
}