layout: Resolve word_spacing ahead of time.

It's not possible anymore, in the presence of min() / max(), to split a
<length-percentage> value into a <length> and a <percentage> component.

Tweak word_spacing to do what Gecko does (resolving it in advance).
This commit is contained in:
Emilio Cobos Álvarez 2020-02-10 16:50:57 +01:00
parent e227715aee
commit f03026b869
No known key found for this signature in database
GPG key ID: E1152D0994E4BF8A
8 changed files with 45 additions and 24 deletions

2
Cargo.lock generated
View file

@ -1751,7 +1751,6 @@ dependencies = [
"log", "log",
"malloc_size_of", "malloc_size_of",
"net_traits", "net_traits",
"ordered-float",
"range", "range",
"serde", "serde",
"servo-fontconfig", "servo-fontconfig",
@ -5659,7 +5658,6 @@ dependencies = [
"num-integer", "num-integer",
"num-traits", "num-traits",
"num_cpus", "num_cpus",
"ordered-float",
"owning_ref", "owning_ref",
"parking_lot", "parking_lot",
"precomputed-hash", "precomputed-hash",

View file

@ -27,7 +27,6 @@ libc = "0.2"
log = "0.4" log = "0.4"
malloc_size_of = { path = "../malloc_size_of" } malloc_size_of = { path = "../malloc_size_of" }
net_traits = {path = "../net_traits"} net_traits = {path = "../net_traits"}
ordered-float = "1.0"
range = {path = "../range"} range = {path = "../range"}
serde = "1.0" serde = "1.0"
servo_arc = {path = "../servo_arc"} servo_arc = {path = "../servo_arc"}

View file

@ -13,7 +13,6 @@ use crate::text::shaping::ShaperMethods;
use crate::text::Shaper; use crate::text::Shaper;
use app_units::Au; use app_units::Au;
use euclid::default::{Point2D, Rect, Size2D}; use euclid::default::{Point2D, Rect, Size2D};
use ordered_float::NotNan;
use servo_atoms::Atom; use servo_atoms::Atom;
use smallvec::SmallVec; use smallvec::SmallVec;
use std::borrow::ToOwned; use std::borrow::ToOwned;
@ -38,6 +37,7 @@ macro_rules! ot_tag {
pub const GPOS: u32 = ot_tag!('G', 'P', 'O', 'S'); pub const GPOS: u32 = ot_tag!('G', 'P', 'O', 'S');
pub const GSUB: u32 = ot_tag!('G', 'S', 'U', 'B'); pub const GSUB: u32 = ot_tag!('G', 'S', 'U', 'B');
pub const KERN: u32 = ot_tag!('k', 'e', 'r', 'n'); pub const KERN: u32 = ot_tag!('k', 'e', 'r', 'n');
pub const LAST_RESORT_GLYPH_ADVANCE: FractionalPixel = 10.0;
static TEXT_SHAPING_PERFORMANCE_COUNTER: AtomicUsize = AtomicUsize::new(0); static TEXT_SHAPING_PERFORMANCE_COUNTER: AtomicUsize = AtomicUsize::new(0);
@ -197,7 +197,7 @@ pub struct ShapingOptions {
/// NB: You will probably want to set the `IGNORE_LIGATURES_SHAPING_FLAG` if this is non-null. /// NB: You will probably want to set the `IGNORE_LIGATURES_SHAPING_FLAG` if this is non-null.
pub letter_spacing: Option<Au>, pub letter_spacing: Option<Au>,
/// Spacing to add between each word. Corresponds to the CSS 2.1 `word-spacing` property. /// Spacing to add between each word. Corresponds to the CSS 2.1 `word-spacing` property.
pub word_spacing: (Au, NotNan<f32>), pub word_spacing: Au,
/// The Unicode script property of the characters in this run. /// The Unicode script property of the characters in this run.
pub script: Script, pub script: Script,
/// Various flags. /// Various flags.
@ -278,8 +278,7 @@ impl Font {
let mut advance = Au::from_f64_px(self.glyph_h_advance(glyph_id)); let mut advance = Au::from_f64_px(self.glyph_h_advance(glyph_id));
if character == ' ' { if character == ' ' {
// https://drafts.csswg.org/css-text-3/#word-spacing-property // https://drafts.csswg.org/css-text-3/#word-spacing-property
let (length, percent) = options.word_spacing; advance += options.word_spacing;
advance = (advance + length) + Au((advance.0 as f32 * percent.into_inner()) as i32);
} }
if let Some(letter_spacing) = options.letter_spacing { if let Some(letter_spacing) = options.letter_spacing {
advance += letter_spacing; advance += letter_spacing;
@ -343,7 +342,7 @@ impl Font {
.or_insert_with(|| { .or_insert_with(|| {
match self.handle.glyph_h_advance(glyph) { match self.handle.glyph_h_advance(glyph) {
Some(adv) => adv, Some(adv) => adv,
None => 10f64 as FractionalPixel, // FIXME: Need fallback strategy None => LAST_RESORT_GLYPH_ADVANCE as FractionalPixel, // FIXME: Need fallback strategy
} }
}) })
} }
@ -516,8 +515,8 @@ impl RunMetrics {
RunMetrics { RunMetrics {
advance_width: advance, advance_width: advance,
bounding_box: bounds, bounding_box: bounds,
ascent: ascent, ascent,
descent: descent, descent,
} }
} }
} }

View file

@ -434,9 +434,7 @@ impl Shaper {
// We elect to only space the two required code points. // We elect to only space the two required code points.
if character == ' ' || character == '\u{a0}' { if character == ' ' || character == '\u{a0}' {
// https://drafts.csswg.org/css-text-3/#word-spacing-property // https://drafts.csswg.org/css-text-3/#word-spacing-property
let (length, percent) = options.word_spacing; advance += options.word_spacing;
advance =
(advance + length) + Au::new((advance.0 as f32 * percent.into_inner()) as i32);
} }
advance advance

View file

@ -10,7 +10,7 @@ use crate::fragment::{ScannedTextFragmentInfo, SpecificFragmentInfo, UnscannedTe
use crate::inline::{InlineFragmentNodeFlags, InlineFragments}; use crate::inline::{InlineFragmentNodeFlags, InlineFragments};
use crate::linked_list::split_off_head; use crate::linked_list::split_off_head;
use app_units::Au; use app_units::Au;
use gfx::font::{FontMetrics, FontRef, RunMetrics, ShapingFlags, ShapingOptions}; use gfx::font::{self, FontMetrics, FontRef, RunMetrics, ShapingFlags, ShapingOptions};
use gfx::text::glyph::ByteIndex; use gfx::text::glyph::ByteIndex;
use gfx::text::text_run::TextRun; use gfx::text::text_run::TextRun;
use gfx::text::util::{self, CompressionMode}; use gfx::text::util::{self, CompressionMode};
@ -195,7 +195,24 @@ impl TextRunScanner {
}; };
text_transform = inherited_text_style.text_transform; text_transform = inherited_text_style.text_transform;
letter_spacing = inherited_text_style.letter_spacing; letter_spacing = inherited_text_style.letter_spacing;
word_spacing = inherited_text_style.word_spacing.to_hash_key(); word_spacing = inherited_text_style
.word_spacing
.to_length()
.map(|l| l.into())
.unwrap_or_else(|| {
let space_width = font_group
.borrow_mut()
.find_by_codepoint(&mut font_context, ' ')
.and_then(|font| {
let font = font.borrow();
font.glyph_index(' ')
.map(|glyph_id| font.glyph_h_advance(glyph_id))
})
.unwrap_or(font::LAST_RESORT_GLYPH_ADVANCE);
inherited_text_style
.word_spacing
.to_used_value(Au::from_f64_px(space_width))
});
text_rendering = inherited_text_style.text_rendering; text_rendering = inherited_text_style.text_rendering;
word_break = inherited_text_style.word_break; word_break = inherited_text_style.word_break;
} }

View file

@ -601,13 +601,6 @@ impl TextRun {
flags.insert(ShapingFlags::KEEP_ALL_FLAG); flags.insert(ShapingFlags::KEEP_ALL_FLAG);
} }
let shaping_options = gfx::font::ShapingOptions {
letter_spacing,
word_spacing: inherited_text_style.word_spacing.to_hash_key(),
script: unicode_script::Script::Common,
flags,
};
crate::context::with_thread_local_font_context(layout_context, |font_context| { crate::context::with_thread_local_font_context(layout_context, |font_context| {
let font_group = font_context.font_group(font_style); let font_group = font_context.font_group(font_style);
let font = font_group let font = font_group
@ -616,6 +609,25 @@ impl TextRun {
.expect("could not find font"); .expect("could not find font");
let mut font = font.borrow_mut(); let mut font = font.borrow_mut();
let word_spacing = &inherited_text_style.word_spacing;
let word_spacing = word_spacing
.to_length()
.map(|l| l.into())
.unwrap_or_else(|| {
let space_width = font
.glyph_index(' ')
.map(|glyph_id| font.glyph_h_advance(glyph_id))
.unwrap_or(gfx::font::LAST_RESORT_GLYPH_ADVANCE);
word_spacing.to_used_value(Au::from_f64_px(space_width))
});
let shaping_options = gfx::font::ShapingOptions {
letter_spacing,
word_spacing,
script: unicode_script::Script::Common,
flags,
};
let (runs, break_at_start) = gfx::text::text_run::TextRun::break_and_shape( let (runs, break_at_start) = gfx::text::text_run::TextRun::break_and_shape(
&mut font, &mut font,
&self.text, &self.text,

View file

@ -55,7 +55,6 @@ num_cpus = {version = "1.1.0"}
num-integer = "0.1" num-integer = "0.1"
num-traits = "0.2" num-traits = "0.2"
num-derive = "0.2" num-derive = "0.2"
ordered-float = "1.0"
owning_ref = "0.4" owning_ref = "0.4"
parking_lot = "0.9" parking_lot = "0.9"
precomputed-hash = "0.1.1" precomputed-hash = "0.1.1"

View file

@ -72,7 +72,6 @@ extern crate num_cpus;
extern crate num_derive; extern crate num_derive;
extern crate num_integer; extern crate num_integer;
extern crate num_traits; extern crate num_traits;
extern crate ordered_float;
extern crate owning_ref; extern crate owning_ref;
extern crate parking_lot; extern crate parking_lot;
extern crate precomputed_hash; extern crate precomputed_hash;