From 24ef4e2fd9f73dac03480eb1aa22417dd5074612 Mon Sep 17 00:00:00 2001 From: Imbris Date: Sun, 13 Aug 2023 21:42:33 -0400 Subject: [PATCH 1/3] Small code organization changes in ShapeLine::layout: move some variables declarations down closer to where they are used, move variables that are reset every loop down to be declared in the loop, replace Vec::new + mem::swap with mem::take --- src/shape.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/shape.rs b/src/shape.rs index 3f4642c..b392944 100644 --- a/src/shape.rs +++ b/src/shape.rs @@ -916,12 +916,6 @@ impl ShapeLine { vl.spaces += number_of_blanks; } - let start_x = if self.rtl { line_width } else { 0.0 }; - let mut x; - let mut y; - let mut max_ascent: f32 = 0.; - let mut max_descent: f32 = 0.; - // This would keep the maximum number of spans that would fit on a visual line // If one span is too large, this variable will hold the range of words inside that span // that fits on a line. @@ -1159,6 +1153,10 @@ impl ShapeLine { } // Create the LayoutLines using the ranges inside visual lines + let start_x = if self.rtl { line_width } else { 0.0 }; + let mut max_ascent: f32 = 0.; + let mut max_descent: f32 = 0.; + let number_of_visual_lines = visual_lines.len(); for (index, visual_line) in visual_lines.iter().enumerate() { if visual_line.ranges.is_empty() { @@ -1166,8 +1164,8 @@ impl ShapeLine { } let new_order = self.reorder(&visual_line.ranges); let mut glyphs = Vec::with_capacity(1); - x = start_x; - y = 0.; + let mut x = start_x; + let mut y = 0.; max_ascent = 0.; max_descent = 0.; let alignment_correction = match (align, self.rtl) { @@ -1357,8 +1355,7 @@ impl ShapeLine { } } } - let mut glyphs_swap = Vec::new(); - mem::swap(&mut glyphs, &mut glyphs_swap); + layout_lines.push(LayoutLine { w: if align != Align::Justified { visual_line.w @@ -1371,7 +1368,7 @@ impl ShapeLine { }, max_ascent: max_ascent * font_size, max_descent: max_descent * font_size, - glyphs: glyphs_swap, + glyphs: mem::take(&mut glyphs), }); push_line = false; } From ae96bf26d1a5d9a309139256ed9e5eb6606f7232 Mon Sep 17 00:00:00 2001 From: Imbris Date: Mon, 14 Aug 2023 00:18:15 -0400 Subject: [PATCH 2/3] Deduplicate / reorganize / clarify code for creating LayoutLines * max_ascent and max_descent declarations moved into loop since they are reset each iteration and the one spot where they are used outside the loop for pushing an empty line is if all items are empty (so they would always be 0.0 there). * For `Align::Justified`, instead of repurposing `alignment_correction` variable for expanding blank spaces, there is a new `justification_expansion` variable. This helps clarify the code. * Common code for processing ranges factored out section where ranges are iterated in opposite orders for RTL vs LTR. * We don't need to use `take_mut` on `glyphs` since the variable is not used afterwards (i.e. we can just move out of `glyphs`). * Fix bug where `scratch.scripts` was being used for logging info instead of `scripts`. --- src/shape.rs | 256 +++++++++++++++------------------------------------ 1 file changed, 75 insertions(+), 181 deletions(-) diff --git a/src/shape.rs b/src/shape.rs index b392944..e2c0871 100644 --- a/src/shape.rs +++ b/src/shape.rs @@ -205,11 +205,7 @@ fn shape_run( } } - log::trace!( - " Run {:?}: '{}'", - &scratch.scripts, - &line[start_run..end_run], - ); + log::trace!(" Run {:?}: '{}'", &scripts, &line[start_run..end_run],); let attrs = attrs_list.get_span(start_run); @@ -1154,8 +1150,6 @@ impl ShapeLine { // Create the LayoutLines using the ranges inside visual lines let start_x = if self.rtl { line_width } else { 0.0 }; - let mut max_ascent: f32 = 0.; - let mut max_descent: f32 = 0.; let number_of_visual_lines = visual_lines.len(); for (index, visual_line) in visual_lines.iter().enumerate() { @@ -1166,8 +1160,8 @@ impl ShapeLine { let mut glyphs = Vec::with_capacity(1); let mut x = start_x; let mut y = 0.; - max_ascent = 0.; - max_descent = 0.; + let mut max_ascent: f32 = 0.; + let mut max_descent: f32 = 0.; let alignment_correction = match (align, self.rtl) { (Align::Left, true) => line_width - visual_line.w, (Align::Left, false) => 0., @@ -1175,184 +1169,84 @@ impl ShapeLine { (Align::Right, false) => line_width - visual_line.w, (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) + (Align::Justified, _) => 0., + }; - // 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 - } else { - 0. + if self.rtl { + x -= alignment_correction; + } else { + x += alignment_correction; + } + + // TODO: Only certain `is_whitespace` chars are typically expanded but this is what is + // currently used to compute `visual_line.spaces`. + // + // 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) + + // Amount of extra width added to each blank space within a line. + let justification_expansion = if matches!(align, Align::Justified) + && visual_line.spaces > 0 + // Don't justify the last line in a paragraph. + && index != number_of_visual_lines - 1 + { + (line_width - visual_line.w) / visual_line.spaces as f32 + } else { + 0. + }; + + let mut process_range = |range: Range| { + for &(span_index, (starting_word, starting_glyph), (ending_word, ending_glyph)) in + visual_line.ranges[range.clone()].iter() + { + let span = &self.spans[span_index]; + // If ending_glyph is not 0 we need to include glyphs from the ending_word + for i in starting_word..ending_word + usize::from(ending_glyph != 0) { + let word = &span.words[i]; + let included_glyphs = match (i == starting_word, i == ending_word) { + (false, false) => &word.glyphs[..], + (true, false) => &word.glyphs[starting_glyph..], + (false, true) => &word.glyphs[..ending_glyph], + (true, true) => &word.glyphs[starting_glyph..ending_glyph], + }; + for glyph in included_glyphs { + let x_advance = font_size * glyph.x_advance + + if word.blank { + justification_expansion + } else { + 0.0 + }; + let y_advance = font_size * glyph.y_advance; + glyphs.push(glyph.layout(font_size, x, y, x_advance, span.level)); + if self.rtl { + x -= x_advance; + } else { + x += x_advance; + } + y += y_advance; + max_ascent = max_ascent.max(glyph.ascent); + max_descent = max_descent.max(glyph.descent); + } } } }; - if self.rtl { - if align != Align::Justified { - x -= alignment_correction; - } - for range in new_order.iter().rev() { - for ( - span_index, - (starting_word, starting_glyph), - (ending_word, ending_glyph), - ) in visual_line.ranges[range.clone()].iter() - { - let span = &self.spans[*span_index]; - if starting_word == ending_word { - let word_blank = span.words[*starting_word].blank; - for glyph in span.words[*starting_word].glyphs - [*starting_glyph..*ending_glyph] - .iter() - { - let x_advance = font_size * glyph.x_advance; - let y_advance = font_size * glyph.y_advance; - x -= x_advance; - if word_blank && align == Align::Justified { - x -= alignment_correction; - glyphs.push(glyph.layout( - font_size, - x, - y, - x_advance + alignment_correction, - span.level, - )); - } else { - glyphs - .push(glyph.layout(font_size, x, y, x_advance, span.level)); - } - y += y_advance; - max_ascent = max_ascent.max(glyph.ascent); - max_descent = max_descent.max(glyph.descent); - } - } else { - for i in *starting_word..*ending_word + 1 { - if let Some(word) = span.words.get(i) { - let (g1, g2) = if i == *starting_word { - (*starting_glyph, word.glyphs.len()) - } else if i == *ending_word { - (0, *ending_glyph) - } else { - (0, word.glyphs.len()) - }; - let word_blank = word.blank; - for glyph in &word.glyphs[g1..g2] { - let x_advance = font_size * glyph.x_advance; - let y_advance = font_size * glyph.y_advance; - x -= x_advance; - if word_blank && align == Align::Justified { - x -= alignment_correction; - glyphs.push(glyph.layout( - font_size, - x, - y, - x_advance + alignment_correction, - span.level, - )); - } else { - glyphs - .push(glyph.layout( - font_size, x, y, x_advance, span.level, - )); - } - y += y_advance; - max_ascent = max_ascent.max(glyph.ascent); - max_descent = max_descent.max(glyph.descent); - } - } - } - } - } + if self.rtl { + for range in new_order.into_iter().rev() { + process_range(range); } } else { /* LTR */ - if align != Align::Justified { - x += alignment_correction; - } for range in new_order { - for ( - span_index, - (starting_word, starting_glyph), - (ending_word, ending_glyph), - ) in visual_line.ranges[range.clone()].iter() - { - let span = &self.spans[*span_index]; - if starting_word == ending_word { - let word_blank = span.words[*starting_word].blank; - for glyph in span.words[*starting_word].glyphs - [*starting_glyph..*ending_glyph] - .iter() - { - let x_advance = font_size * glyph.x_advance; - let y_advance = font_size * glyph.y_advance; - if word_blank && align == Align::Justified { - glyphs.push(glyph.layout( - font_size, - x, - y, - x_advance + alignment_correction, - span.level, - )); - x += alignment_correction; - } else { - glyphs - .push(glyph.layout(font_size, x, y, x_advance, span.level)); - } - x += x_advance; - y += y_advance; - max_ascent = max_ascent.max(glyph.ascent); - max_descent = max_descent.max(glyph.descent); - } - } else { - for i in *starting_word..*ending_word + 1 { - if let Some(word) = span.words.get(i) { - let (g1, g2) = if i == *starting_word { - (*starting_glyph, word.glyphs.len()) - } else if i == *ending_word { - (0, *ending_glyph) - } else { - (0, word.glyphs.len()) - }; - - let word_blank = word.blank; - for glyph in &word.glyphs[g1..g2] { - let x_advance = font_size * glyph.x_advance; - let y_advance = font_size * glyph.y_advance; - if word_blank && align == Align::Justified { - glyphs.push(glyph.layout( - font_size, - x, - y, - x_advance + alignment_correction, - span.level, - )); - x += alignment_correction; - } else { - glyphs - .push(glyph.layout( - font_size, x, y, x_advance, span.level, - )); - } - x += x_advance; - y += y_advance; - max_ascent = max_ascent.max(glyph.ascent); - max_descent = max_descent.max(glyph.descent); - } - } - } - } - } + process_range(range); } } @@ -1368,7 +1262,7 @@ impl ShapeLine { }, max_ascent: max_ascent * font_size, max_descent: max_descent * font_size, - glyphs: mem::take(&mut glyphs), + glyphs, }); push_line = false; } @@ -1376,8 +1270,8 @@ impl ShapeLine { if push_line { layout_lines.push(LayoutLine { w: 0.0, - max_ascent: max_ascent * font_size, - max_descent: max_descent * font_size, + max_ascent: 0.0, + max_descent: 0.0, glyphs: Default::default(), }); } From 5f28feef1fd03d3f19c801059aa2e64fb0facaf7 Mon Sep 17 00:00:00 2001 From: Imbris Date: Mon, 14 Aug 2023 00:54:50 -0400 Subject: [PATCH 3/3] Move variables down that are only used when creating LayoutLines and replaced used of push_line bool with checking is layout_lines is empty --- src/shape.rs | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/shape.rs b/src/shape.rs index e2c0871..9144b18 100644 --- a/src/shape.rs +++ b/src/shape.rs @@ -875,17 +875,6 @@ impl ShapeLine { align: Option, layout_lines: &mut Vec, ) { - let align = align.unwrap_or({ - if self.rtl { - Align::Right - } else { - Align::Left - } - }); - - // This is used to create a visual line for empty lines (e.g. lines with only a ) - let mut push_line = true; - // For each visual line a list of (span index, and range of words in that span) // Note that a BiDi visual line could have multiple spans or parts of them // let mut vl_range_of_spans = Vec::with_capacity(1); @@ -1149,6 +1138,14 @@ impl ShapeLine { } // Create the LayoutLines using the ranges inside visual lines + let align = align.unwrap_or({ + if self.rtl { + Align::Right + } else { + Align::Left + } + }); + let start_x = if self.rtl { line_width } else { 0.0 }; let number_of_visual_lines = visual_lines.len(); @@ -1264,10 +1261,10 @@ impl ShapeLine { max_descent: max_descent * font_size, glyphs, }); - push_line = false; } - if push_line { + // This is used to create a visual line for empty lines (e.g. lines with only a ) + if layout_lines.is_empty() { layout_lines.push(LayoutLine { w: 0.0, max_ascent: 0.0,