Differentiate shaped clusters and graphemes, introduce "atoms" - #715
Differentiate shaped clusters and graphemes, introduce "atoms"#715tomcur wants to merge 35 commits into
Conversation
There was a problem hiding this comment.
I changed this test in Fix ligature test after no longer treating graphemes as if they were ligatures, because with monotone character harfrust shaping, the graphemes didn't form "ligatures" anymore (and note they also weren't really ligatures, but that's how the old ClusterData encoded them).
There was a problem hiding this comment.
In this snapshot and the next, the gray-text graphemes under the rule that consist of multiple glyphs now render correctly.
8e2388a to
bcd591b
Compare
| /// Add the cluster(s) currently being evaluated to the current line. | ||
| /// Add the atom currently being evaluated to the current line. | ||
| /// | ||
| /// `metrics` provides the raw font ascent and descent of the cluster(s) (i.e. the distances | ||
| /// they extend above and below the baseline, *not* including leading) as well as the intrinsic | ||
| /// line height of the cluster(s) (i.e. including the full leading), which may be smaller than | ||
| /// `font_metrics` provides the raw font ascent and descent of the atom (i.e. the distances | ||
| /// it extends above and below the baseline, *not* including leading) as well as the intrinsic | ||
| /// line height of the atom (i.e. including the full leading), which may be smaller than | ||
| /// `ascent + descent` when the leading is negative. | ||
| pub fn append_cluster_to_line( | ||
| pub fn append_atom_to_line( | ||
| &mut self, | ||
| atom: &Atom<'_>, | ||
| next_x: f32, | ||
| font_metrics: &FontMetrics, | ||
| line_height: f32, | ||
| quantize: bool, | ||
| ) { |
There was a problem hiding this comment.
This probably needs to become private.
72bd129 to
a1faf3f
Compare
a1faf3f to
f726fe2
Compare
f726fe2 to
9f10cba
Compare
| /// Get a [`ShapedSlice`] of the run at `run_index`. | ||
| pub fn run_slice(&self, run_index: u32) -> ShapedSlice<'_> { | ||
| let run = &self.runs[run_index as usize]; | ||
| ShapedSlice { | ||
| characters: &self.characters, | ||
|
|
||
| shaped_clusters: &self.shaped_clusters, | ||
| glyphs: &self.glyphs, | ||
| bidi_level: run.bidi_level, | ||
|
|
||
| clusters: ( | ||
| run.shaped_clusters_range.start, | ||
| run.shaped_clusters_range.end, | ||
| ), | ||
| } | ||
| } |
There was a problem hiding this comment.
Thinking about this again today, I'm once more a bit into the weeds.
Currently, we enforce item boundaries to be a grapheme boundary, and we start new items on bidi and script changes (as well as with a user-provided split_after predicate). I believe that matches what Parley used to do in the swash days, but this doesn't fully match browsers.
In particular, Firefox and Chromium don't necessarily break graphemes when the script changes. It's admittedly a bit of a degenerate case, but I've been working today on also keeping graphemes alive across script changes. For bidi, if the direction doesn't match, you're not even visually adjacent, which is somewhat problematic. For scripts, I think we should match browser behavior. The issue is that a ShapedRun then no longer is guaranteed to be a grapheme boundary, and so you cannot unconditionally turn it into a ShapedSlice, as that type does guarantee those boundaries.
If what I'm trying works out, it should be a follow-up to this PR. My thoughts are to have the user-provided split_after callback still split the items, and thereby split graphemes (e.g., a font size change restarts grapheme segmentation, that matches browsers). We for now probably also do that for bidi, but script changes only start a new segment, where segments are shaped individually (so they can have their own font) but they don't break graphemes. Those segments are not user-facing. Within a segment, every time the font changes due to fallback, a new run starts, as previously was the case.
This ShapedText::run_slice method changes to be something just used for rendering so you can iterate glyphs in visual left-to-right order. ShapedSlice should remain an atom boundary on both sides, which a shaped run no longer guarantees. I mostly need to think about what the ergonomics should be for narrowing ShapedSlices. In any case, parley really assumes its run boundaries to be grapheme boundaries, so fixing it completely within parley might be somewhat of an effort and perhaps out of scope even for the next PR. The good news is that at least line boundaries are guaranteed to be atom boundaries, by construction. (Edit: this is not true in general, see: #715 (comment)).
There was a problem hiding this comment.
Can you share an example of mixed-script graphemes?
Those need to be split for shaping anyway.
There was a problem hiding this comment.
I'm not currently at my computer, but a Latin base character plus u+093f might do it.
aaिa
This afternoon I was surprised by a grapheme being rendered with two different fonts in a browser, which is somewhat annoying for proper selection and breaking.
There was a problem hiding this comment.
Yeah but that's nonsensical text and not expected to render anything other than two separate shaping runs.
There was a problem hiding this comment.
Yeah, definitely agree on the nonsensical part. The issue is matching browser behavior in regards to selection and line breaking, which uses the grapheme segmentation and crosses the shaped runs in this case.
We could decide to not care, of course.
There was a problem hiding this comment.
I think not matching is fine in these cases.
Browsers behave that way, and Parley can too, because they implement selection / delete etc solely on the Unicode input.
DJMcNab
left a comment
DJMcNab
left a comment
There was a problem hiding this comment.
I fear this is going to take a couple of passes to fully understand. I've made some comments which if nothing else should help you work out what's most confusing me!
| let clusters: &mut dyn Iterator<Item = &mut ClusterData> = | ||
| let (characters, shaped_clusters, _) = layout | ||
| .shaped_text | ||
| .characters_shaped_clusters_and_glyphs_mut(); |
There was a problem hiding this comment.
Oh my, that's a mouthful of a method name.
There was a problem hiding this comment.
It'll be deleted soon! It's a #[doc(hidden)] escape-hatch because parley currently mutates advances (for justification and such) in place. That'll get quite broken with reshaping, so should go.
| if cluster.is_grapheme_start() | ||
| && characters[cluster.chars_range().start as usize] | ||
| .info | ||
| .whitespace() | ||
| .is_space_or_nbsp() |
There was a problem hiding this comment.
I think this might need a comment.
There was a problem hiding this comment.
I've added a comment and a test for good measure, but it turns out browser behavior actually diverges here. And Chromium is just wrong, I think. Firefox could be considered to be wrong.
With the following text, the first space is not a grapheme boundary according to Unicode, i.e., the space between aaൎ bb.
aaൎ bb cc dd
But text-align: justify; text-justify: inter-word; that in browsers, and Firefox doesn't actually form the grapheme at all and stretches all spaces. Chromium does form the grapheme, but it stretches that space, and only that one. It probably counts correctly that it needs to stretch one space in the line, and then proceeds to erroneously stretch the first one it encounters.
| Firefox | Chromium | parley in this PR as of writing |
|---|---|---|
 |
 |
 |
https://developer.mozilla.org/en-US/play?id=d4126yYrbth%2F6uCumDnNwXehDoJYFVIlUTenAd%2B%2FpQqHg4w7ynFxZrye3TWhTHSX8%2B7URJRhR6b87gOX - may need to change the width to get the line break to fall between cc and dd.
There was a problem hiding this comment.
Wow! TIL that a scalar value + space can form a single grapheme!
There was a problem hiding this comment.
Yesterday I learned of another edge case. E.g., a \u{0301}b is three graphemes: the combining acute accent attaches to the space. However, the line segmenter's line break opportunity still falls at the end of that space (before the accent), and browsers are happy to take it. Unless line-break: anywhere is set: in that case, browsers split between graphemes (and only between them).
<div style="width: 1.5ch; margin: 1em; background: #eee;">a ́b</div>
<div style="width: 1.5ch; margin: 1em; background: #eee; line-break: anywhere;">a ́b</div>I think that's fine for what we have here, but that does mean that with reshaping, we actually cannot assume a line is a grapheme boundary on both sides. Because users will probably be interested in per-line advances, and because reshaped lines will likely be pushed to some arena (i.e., not mutating ShapedText), our Grapheme type should grow something like continues_{before,after}_slice methods. Those would indicate whether the grapheme continues past our current ShapedSlice view (consulting the global &[Characters]).
User's selection code should then consult this and extend the selection to cover the grapheme.
Note that a partial grapheme broken across lines takes its advance only from the shaped clusters on that line. My understanding is that the logic in this PR is therefore still pretty much correct, and by a happy accident also pretty much what you'd like to have as a per-line API. The thing that changes is that ShapedSlice no longer has the invariant that it it's an atom boundary on both sides.
There was a problem hiding this comment.
(I'd suggest mulling on these edge cases a bit more and updating parley_engine's documentation when landing the functionality in parley_engine that actually makes all these edge-cases possible. The PR's documentation is accurate as far as parley_engine's current functionality goes.)
| /// The first (logical) character of this cluster. | ||
| fn first_character(&self) -> &'a Character { | ||
| &self | ||
| .run | ||
| .full_slice() | ||
| .characters_in(self.grapheme.char_range())[0] | ||
| } | ||
|
|
There was a problem hiding this comment.
In what contexts is this useful/needed?
There was a problem hiding this comment.
parley encodes styles at the parley::layout::Cluster level. In the swash days, that used to effectively be a grapheme, and its style was the grapheme's first character's style. On main, Cluster is unfortunately 1-to-1 with characters, but it's still effectively a grapheme, through graphemes being encoded as ligatures: only the grapheme's first character's Cluster has glyphs. The "continuations" have none.
With this PR, Cluster once again becomes a proper grapheme. However, parley_engine exposes styles at the ShapedCluster level, which isn't a grapheme; it's just a mapping of some logically contiguous characters to some visually contiguous glyphs.
This method is used by some other methods to, for now, keep the old logic of e.g. the style index coming from the grapheme's first character. I think this is at least mostly correct, even though our ShapedClusters no longer necessarily align with graphemes, because shaping-relevant style changes like font size split the items, and thereby do split graphemes. That means this is only about things like glyph color.
| self.grapheme.is_atom_start() && !self.grapheme.is_atom_end() | ||
| } | ||
|
|
||
| /// Returns `true` if the cluster is a ligature continuation. |
There was a problem hiding this comment.
This doc isn't very useful. I comment this here because of the link to this method in the docs for advance. This doesn't block this PR.
| /// For our purposes, the glyphs of a shaped cluster belong to its first (logical) grapheme | ||
| /// cluster: for a ligature, the ligature start yields all of the ligature's glyphs and the | ||
| /// continuations yield none. |
There was a problem hiding this comment.
This comment is asking the reader to have an awful lot of context. That's not necessarily an issue, but just is my observation.
There was a problem hiding this comment.
I think I meant to make this a normal // comment. But perhaps writing it in a better way would actually make for a useful docstring.
| // The character after this boundary is outside this slice. Note that `Self::characters` | ||
| // is the full slice of characters that were shaped, so the character after | ||
| // `char_range.end` is not necessarily the next character in the source text. We use the |
There was a problem hiding this comment.
I believe you, but this comment doesn't make it at all clear why the "not necessarily the next character in the source text" holds. I obviously can see it if this slice is the end of the text, but that doesn't feel like what this is trying to express.
There was a problem hiding this comment.
Daniel and I also talked out of band, but for posterity: currently, users don't necessarily need to shape all items produced by the itemizer. That probably should be changed for simplicity, as it doesn't seem very useful to be able to shape only certain items.
The user's font selection currently is also allowed to fail to return a font, in which case the run doesn't get shaped (though, if font selection ever succeeded before, we currently fall back to using the previous font). Instead, we should probably make font selection be required to be infallible. The user should just have a some last-resort font they can pass to us.
There was a problem hiding this comment.
That probably should be changed for simplicity, as it doesn't seem very useful to be able to shape only certain items.
Agree! I think it would make working with Parley Engine simpler and safer too - caveats like the below (from atom_at_char) smell like unnecessary footguns
/// Note: this is the index into this [`ShapedSlice`]'s character slice, which is not
/// necessarily the same as the underlying source text's characters.
taj-p
left a comment
taj-p
left a comment
There was a problem hiding this comment.
Some review comments while I was learning about what these changes mean for Parley. Don't consider any of them blocking.
| if cluster.is_grapheme_start() | ||
| && characters[cluster.chars_range().start as usize] | ||
| .info | ||
| .whitespace() | ||
| .is_space_or_nbsp() |
There was a problem hiding this comment.
Wow! TIL that a scalar value + space can form a single grapheme!
| /// [UAX #29 § 3][uax-grapheme]. Grapheme edges are caret/selection/hit-testing edges. | ||
| /// | ||
| /// [uax-grapheme]: https://www.unicode.org/reports/tr29/#Grapheme_Cluster_Boundaries | ||
| pub struct Cluster<'a, B: Brush> { |
There was a problem hiding this comment.
Do we plan on renaming this to grapheme in the future?
There was a problem hiding this comment.
A few days ago I would've said yes! Now, I'm not quite sure.
The type is intersected with parley's Run, which is the intersection of a single ShapedRun with a line. Graphemes can cross shaped runs (e.g. a grapheme with two fonts due to font fallback), and as per #715 (comment) can cross lines.
I've noted that in Cluster's docstring for now. parley needs some careful thought.
There was a problem hiding this comment.
Ohhhh I see. Yes, that does make sense.
As we do for Parley Engine, should graphemes be materialised on demand as a separate concept?
| // The character after this boundary is outside this slice. Note that `Self::characters` | ||
| // is the full slice of characters that were shaped, so the character after | ||
| // `char_range.end` is not necessarily the next character in the source text. We use the |
There was a problem hiding this comment.
That probably should be changed for simplicity, as it doesn't seem very useful to be able to shape only certain items.
Agree! I think it would make working with Parley Engine simpler and safer too - caveats like the below (from atom_at_char) smell like unnecessary footguns
/// Note: this is the index into this [`ShapedSlice`]'s character slice, which is not
/// necessarily the same as the underlying source text's characters.
| /// Note this is not a character index into the source text: the shaped character array only | ||
| /// contains the characters of runs that were actually shaped. Mapping back to the source text | ||
| /// goes through [`Character::text_byte_start`]. | ||
| pub(crate) chars_range: (u32, u32), |
There was a problem hiding this comment.
Do we necessarily need to store both the start and end? Are ShapedClusters meaningful on their own without the context of their ShapedText::characters? If not, then maybe we just store the start and rely on the next cluster's start being this cluster's end?
I'm actually unsure what shape and vision you have for Parley Engine and whether this change might conflict with that. So I'll defer to you (as with all my comments) here.
There was a problem hiding this comment.
I suppose it's not strictly necessary to store the full range here. The main question is whether we want to provide an API like ShapedCluster::chars_range(&self) -> Range<u32>.
If ShapedCluster doesn't have the range in hand, users have to go through the thing containing ShapedCluster. At the moment, that's only ShapedText; with reshaping we'll also have fragments of ShapedClusters covering just part of the text.
Are ShapedClusters meaningful on their own without the context of their ShapedText::characters?
They're only meaningful when combined either with characters or glyphs.
I've added a TODO note to track this. FWIW, in the current state, storing only the start would shrink the struct from 20 to 16 bytes.
There was a problem hiding this comment.
FWIW, in the current state, storing only the start would shrink the struct from 20 to 16 bytes.
Yup! I think this is the motivation. But, happy to proceed, land re-shaping after breaking, and then re-evaluate later.
| #[derive(Copy, Clone, Debug, PartialEq)] | ||
| pub struct ClusterInfo { | ||
| boundary: Boundary, | ||
| source_char: char, |
There was a problem hiding this comment.
I think you're right. source_char might not be worth the memory and instead can be derived via text_byte_start when required on demand
There was a problem hiding this comment.
Now that I think about it, is there a representation of Character that is even smaller than what is currently encoded?
For example, could we bit pack something like the whitespace, boundary, grapheme start, is emoji, utf8 len into a 16 bit flag to reduce its memory footprint?
There was a problem hiding this comment.
Now that I think about it, is there a representation of Character that is even smaller than what is currently encoded?
For example, could we bit pack something like the whitespace, boundary, grapheme start, is emoji, utf8 len into a 16 bit flag to reduce its memory footprint?
Something like this is what I was also thinking!
| /// Cluster flags (see impl methods for details). | ||
| pub flags: u16, | ||
| /// Style index for this character. | ||
| pub style_index: u16, |
There was a problem hiding this comment.
Do we need style_index on both Character and ShapedCluster? IIUC, a cluster - as is today represented - cannot span multiple style boundaries? Are we planning to change that in the future? I also don't think anyone reads this yet.
But, I guess we may want to support multiple styles across a ShapedCluster, in which case, shouldn't Character (genuinely unsure?) be the better home so that we can support this type of rendering:
reference: https://faultlore.com/blah/text-hates-you/
There was a problem hiding this comment.
IIUC, a cluster - as is today represented - cannot span multiple style boundaries?
If I'm reading this correctly: yes, that's mostly true.
A shaped cluster is a mapping of logically contiguous characters to visually contiguous glyphs, and within the shaped cluster you don't know how each maps to the other. During itemization you can enforce splits (browsers split on things like font size change, language change, etc., but not color change). Only splitting gives you separately shaped clusters.
I believe Firefox styles the same way it places carets: if source characters in the shaped cluster have different styles, it uses the partial advance to color partial rectangles (but maybe it's doing something fancier!). I believe Chromium just uses the shaped cluster's first character's style.
Once we ensure ShapedText covers the entire source text, we could also choose to remove style_index from Character (the user likely has it in hand). Storing it on ShapedCluster can still be nice just for cache locality, but it's not strictly needed because we have the cluster's character range anyway.
| pub flags: u16, | ||
| /// Style index for this character. | ||
| pub style_index: u16, | ||
| pub grapheme_start: bool, |
There was a problem hiding this comment.
We seem to have a lot of cases where we iterate Character and only read this byte. I'm wondering whether a more compact and faster representation would be a bitvec
There was a problem hiding this comment.
That sounds like a good idea, it may well be true.
As an aside, I remember I wanted to change (and rename) ClusterInfo, at least folding this grapheme start flag into it... but a nicely packed allocation could make sense.
Perhaps we should do this once ShapedText is guaranteed to cover the entire source &str; we need to revisit Character at that point anyway.
4cabdca to
7681683
Compare
| self.grapheme.is_atom_start() && !self.grapheme.is_atom_end() | ||
| } | ||
|
|
||
| /// Returns `true` if the cluster is a ligature continuation. |
There was a problem hiding this comment.
The current API doesn't allow a consumer to project Layout into grapheme boundaries against their source text. I'm wondering whether it's worth exposing the below to enable that.
/// Returns `true` if this cluster begins a grapheme in the source text.
pub fn is_grapheme_start(&self) -> bool {
self.first_character().is_grapheme_start()
}I'm assuming this consumption pattern would only be safe after we make font selection infallible?
| // TODO: we force a grapheme break at the start of the run, as itemization could have split | ||
| // a grapheme into two. Ideally, grapheme segmentation should be performed after | ||
| // itemization, at which case this will always be true. | ||
| self.characters[characters_start].grapheme_start = true; |
There was a problem hiding this comment.
Ah, I see. It's not possible for a consumer to project Layout into a sequence of graphemes (from the new Cluster semantics) because we may mutate this source of truth 🤔 .
Could a small(ish) diff like the below overcome this requirement?
diff to address this TODO
diff --git a/parley/src/layout/alignment.rs b/parley/src/layout/alignment.rs
index 7bb559e..922cb1d 100644
--- a/parley/src/layout/alignment.rs
+++ b/parley/src/layout/alignment.rs
@@ -167,30 +167,35 @@ fn align_impl<B: Brush, const UNDO_JUSTIFICATION: bool>(
line_items
.filter(|item| item.is_text_run())
.for_each(|line_item| {
+ let run_first_cluster = layout.shaped_text.runs()[line_item.index]
+ .shaped_clusters_range
+ .start as usize;
let (characters, shaped_clusters, _) = layout
.shaped_text
.characters_shaped_clusters_and_glyphs_mut();
- let clusters = &mut shaped_clusters[line_item.shaped_cluster_range.start
- as usize
- ..line_item.shaped_cluster_range.end as usize];
- let clusters: &mut dyn Iterator<Item = &mut ShapedCluster> =
+ let cluster_range = line_item.shaped_cluster_range.start as usize
+ ..line_item.shaped_cluster_range.end as usize;
+ let clusters = &mut shaped_clusters[cluster_range.clone()];
+ let clusters: &mut dyn Iterator<Item = (usize, &mut ShapedCluster)> =
if line_item.bidi_level.is_rtl() {
- &mut clusters.iter_mut().rev()
+ &mut cluster_range.zip(clusters.iter_mut()).rev()
} else {
- &mut clusters.iter_mut()
+ &mut cluster_range.zip(clusters.iter_mut())
};
- clusters.for_each(|cluster| {
+ clusters.for_each(|(cluster_idx, cluster)| {
if applied == line.num_spaces {
return;
}
// We check whether the cluster starts with a space or non-breaking
- // space, because we stretch that whitespace, *and* is a grapheme start,
- // i.e., an atom boundary.
+ // space, because we stretch that whitespace, *and* is an atom boundary.
+ // A cluster is an atom boundary if it starts a grapheme, or if it is
+ // its run's first cluster: itemization can split a grapheme, and atoms
+ // don't cross runs.
//
// Not all spaces are grapheme boundaries (see
// <https://www.unicode.org/reports/tr29/#GB9b>). Note the
// `line.num_spaces` above was also counted over atoms.
- if cluster.is_grapheme_start()
+ if (cluster.is_grapheme_start() || cluster_idx == run_first_cluster)
&& characters[cluster.chars_range().start as usize]
.info
.whitespace()
diff --git a/parley/src/layout/cluster.rs b/parley/src/layout/cluster.rs
index 3fff1c9..d43d009 100644
--- a/parley/src/layout/cluster.rs
+++ b/parley/src/layout/cluster.rs
@@ -218,6 +218,11 @@ impl<'a, B: Brush> Cluster<'a, B> {
!self.grapheme.is_atom_start()
}
+ /// Returns `true` if this cluster begins a grapheme in the source text.
+ pub fn is_grapheme_start(&self) -> bool {
+ self.first_character().is_grapheme_start()
+ }
+
/// Returns `true` if the cluster is a word boundary.
pub fn is_word_boundary(&self) -> bool {
self.info().is_boundary()
diff --git a/parley/src/layout/data.rs b/parley/src/layout/data.rs
index a46c4d6..ee8b499 100644
--- a/parley/src/layout/data.rs
+++ b/parley/src/layout/data.rs
@@ -145,13 +145,21 @@ impl LineItemData {
/// The number of graphemes in `slice`.
///
+/// Includes partial graphemes - that is, if this slice starts within a grapheme, that grapheme is
+/// included.
+///
/// This is `O(n)` in the slice's characters.
pub(crate) fn count_graphemes(slice: ShapedSlice<'_>) -> usize {
- slice
- .characters_in(slice.char_range())
+ let characters = slice.characters_in(slice.char_range());
+ characters
.iter()
- .filter(|character| character.grapheme_start)
+ .filter(|character| character.is_grapheme_start())
.count()
+ + usize::from(
+ characters
+ .first()
+ .is_some_and(|character| !character.is_grapheme_start()),
+ )
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
diff --git a/parley_engine/src/shape/data.rs b/parley_engine/src/shape/data.rs
index 1275bca..be32ed9 100644
--- a/parley_engine/src/shape/data.rs
+++ b/parley_engine/src/shape/data.rs
@@ -27,6 +27,12 @@ impl Character {
self.text_byte_start as usize
..self.text_byte_start as usize + self.info.source_char().len_utf8()
}
+
+ /// Whether this character begins a grapheme in the source text.
+ #[inline(always)]
+ pub fn is_grapheme_start(&self) -> bool {
+ self.grapheme_start
+ }
}
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
@@ -154,7 +160,7 @@ impl ShapedCluster {
self.flags.has_inline_glyph()
}
- /// Whether this shaped cluster's logical start also starts a grapheme.
+ /// Whether this shaped cluster's logical start also starts a grapheme in the source text.
#[inline(always)]
pub fn is_grapheme_start(self) -> bool {
self.flags.is_grapheme_start()
@@ -170,6 +176,9 @@ impl ShapedCluster {
}
/// The number of graphemes this cluster overlaps.
+ ///
+ /// Includes partial overlap - that is, if the shaped cluster starts in the middle of a grapheme,
+ /// that grapheme is included.
pub(crate) fn graphemes_overlapped(&self, characters: &[Character]) -> u32 {
let start = self.chars_range().start as usize + 1;
let end = self.chars_range().end as usize;
diff --git a/parley_engine/src/shape/shaped_text.rs b/parley_engine/src/shape/shaped_text.rs
index d82b483..2c06e51 100644
--- a/parley_engine/src/shape/shaped_text.rs
+++ b/parley_engine/src/shape/shaped_text.rs
@@ -331,10 +331,6 @@ impl ShapedText {
grapheme_start: info.is_grapheme_start(),
});
}
- // TODO: we force a grapheme break at the start of the run, as itemization could have split
- // a grapheme into two. Ideally, grapheme segmentation should be performed after
- // itemization, at which case this will always be true.
- self.characters[characters_start].grapheme_start = true;
let glyphs_start = self.glyphs.len();
if item.bidi_level.is_ltr() {
@@ -629,6 +625,14 @@ mod tests {
shaped
}
+ fn grapheme_flags(text: &str) -> Vec<bool> {
+ shape_with_font(text, ROBOTO)
+ .characters()
+ .iter()
+ .map(|character| character.is_grapheme_start())
+ .collect()
+ }
+
#[test]
fn single_cluster_run_advance_matches_its_cluster() {
let shaped = shape_with_font("A", ROBOTO);
@@ -639,17 +643,52 @@ mod tests {
assert_eq!(run.advance, cluster.advance);
}
+ #[test]
+ fn character_grapheme_flags_survive_shaping() {
+ assert_eq!(grapheme_flags("abc"), [true, true, true]);
+ assert_eq!(grapheme_flags("e\u{301}"), [true, false]);
+ assert_eq!(
+ grapheme_flags("👩\u{200D}🚀🇦🇺"),
+ [true, false, false, true, false]
+ );
+ assert_eq!(grapheme_flags("\r\n"), [true, false]);
+ }
+
+ #[test]
+ fn rtl_characters_keep_logical_metadata() {
+ let shaped = shape_with_font("ا\u{64E}ب", NOTO_KUFI_ARABIC);
+ let characters: Vec<_> = shaped
+ .characters()
+ .iter()
+ .map(|character| {
+ (
+ character.text_byte_range(),
+ character.info.source_char(),
+ character.is_grapheme_start(),
+ )
+ })
+ .collect();
+
+ assert_eq!(
+ characters,
+ [
+ (0..2, 'ا', true),
+ (2..4, '\u{64E}', false),
+ (4..6, 'ب', true),
+ ]
+ );
+ }
+
/// [U+0600 U+0623 U+064F] is a single grapheme in text-wide segmentation, but U+0600 has
/// bidi class AN while the following characters resolve to a different bidi level, so
- /// itemization splits the grapheme into the items [U+0600] [U+0623 U+064F]. The item
- /// boundary forces a grapheme start on U+0623.
+ /// itemization splits the grapheme into the items [U+0600] [U+0623 U+064F].
#[test]
fn one_grapheme_split_across_items() {
let shaped = shape_with_font("\u{0600}\u{0623}\u{064F}", NOTO_KUFI_ARABIC);
let grapheme_starts: Vec<bool> =
shaped.characters.iter().map(|c| c.grapheme_start).collect();
- assert_eq!(grapheme_starts, [true, true, false]);
+ assert_eq!(grapheme_starts, [true, false, false]);
let clusters: Vec<(u32, u32, bool)> = shaped
.shaped_clusters
@@ -662,7 +701,10 @@ mod tests {
)
})
.collect();
- assert_eq!(clusters, [(0, 1, true), (1, 3, true)]);
+ assert_eq!(clusters, [(0, 1, true), (1, 3, false)]);
+
+ assert_eq!(shaped.run_slice(0).graphemes_start().count(), 1);
+ assert_eq!(shaped.run_slice(1).graphemes_start().count(), 1);
}
/// "ffi" is three graphemes but shapes into one cluster.
@@ -708,9 +750,9 @@ mod tests {
}
}
- /// An itemization boundary forces a cluster break in shaping, and must force a grapheme start.
+ /// An itemization boundary forces a cluster break in shaping.
#[test]
- fn item_boundaries_force_grapheme_start() {
+ fn item_boundaries_preserve_grapheme_start() {
// Two regional indicators forming a single flag grapheme...
let text = "\u{1F1E6}\u{1F1E7}";
let analysis = analyze(text);
@@ -738,9 +780,9 @@ mod tests {
let grapheme_starts: Vec<bool> =
shaped.characters.iter().map(|c| c.grapheme_start).collect();
- assert_eq!(grapheme_starts, [true, true]);
+ assert_eq!(grapheme_starts, [true, false]);
- // But note that, had we had three regional indicator symbols, like `[R0 R1 R2]` itemized as
- // `[R0] [R1 R2]`, the last pair should form a single grapheme. We don't do that currently.
+ assert_eq!(shaped.run_slice(0).graphemes_start().count(), 1);
+ assert_eq!(shaped.run_slice(1).graphemes_start().count(), 1);
}
}
4 participants
This introduces shaped clusters, graphemes, and "atoms" in
parley_engine.Shaped clusters are clusters of glyphs in the HarfBuzz sense: a span of characters and the glyphs they shaped into. Quoting docs on
ShapedCluster:ShapedCluster's glyphs are what you can render in a specific style.Graphemes are extended grapheme clusters as per UAX #29 § 3. These are usually what you want to use for caret placement, selection, etc.
Shaped cluster boundaries are not necessarily grapheme boundaries and vice-versa, especially because this now uses the recommended "monotone characters" level of
harfrustshaping, which keeps clusters smaller and is useful for styling (though I believe with "monotone graphemes",harfrustalso doesn't follow Unicode's grapheme segmentation exactly). Both shaped clusters and graphemes can intersect the other's boundaries, and start or end in the middle of the other. They are both sequential / monotone in characters, though.Because you're often interested in minimal spans of text where both edges are a shaped cluster and grapheme boundary,
parley_enginenow encodes these withAtoms. Breaking an atom always requires reshaping (which we don't support yet; soon!).Atoms and graphemes are computed on demand, with cursors so you can walk both ways in text. Not all code will always need both atoms and graphemes, so calculating them on demand probably makes sense. A user can
collectthem if desired. Reshaping may also benefit from calculating these on demand.An atom is roughly what
ClusterDatatogether withis_ligature_{start,continuation}used to encode in theswashdays (though, asClusterDataback then was more or less a grapheme, the old model wouldn't quite work under "monotone characters" clustering). On main,ClusterDataencodes just a single character, and graphemes are seen as ligatures; this is a somewhat buggy state. This PR removes the oldClusterData.I've migrated
parleyto be on top of these new semantics, but kept its API mostly unchanged.parley::layout::Clusteris now once again a grapheme. I used an LLM for the initial migration ofparleyontoparley_engine's new API (actually, a few migrations... to figure out whatparley_engineshould look like). I've iterated on that code a bit since, so it should be in relatively good shape.We could probably improve
parley's code by doing a few more breaking changes toparley. We should at the very least consider renamingparley::layout::Clustertoparley::layout::Grapheme.This is a ~1.5% performance regression, but I think we can recoup some of this. A small win may be to shrink the new
Charactersomewhat (I don't think we really need to storesource_char: char, say). We perhaps could also do somewhat better with grapheme counting inparley.