From a859b4fbd0ec6a7cbf5c7a6185a48c4f49839367 Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Fri, 6 Dec 2024 16:42:49 +0100
Subject: [PATCH 1/3] Handle inline boxes in cluster geometry and path
Inline boxes are currently silently ignored in some places, making text
selection wrong when they are used, as their advance is not used in
offset calculation and the run indices are counted wrong as the boxes
are skipped.
To see the issues currently present, insert the following in
`PlainEditor::update_layout` and run vello_editor.
```rust
builder.push_inline_box(crate::InlineBox {
id: 0,
index: 7,
width: 50.0,
height: 50.0,
});
```
This PR fixes the issues I've found, though the cursor position when the
cursor has a downstream affinity right on an inline box is still wrong,
I believe. For example, in "AB" with an inline box between "A" and "B",
when the cursor is between the two characters with downstream affinity,
still shows up on the right edge of the A.
---
parley/src/layout/cluster.rs | 36 +++++++++++++++++++++---------------
parley/src/layout/cursor.rs | 19 ++++++++++++++++---
2 files changed, 37 insertions(+), 18 deletions(-)
diff --git a/parley/src/layout/cluster.rs b/parley/src/layout/cluster.rs
index 9504dd6c..0dcde58d 100644
--- a/parley/src/layout/cluster.rs
+++ b/parley/src/layout/cluster.rs
@@ -21,7 +21,10 @@ impl<'a, B: Brush> Cluster<'a, B> {
let mut path = ClusterPath::default();
if let Some((line_index, line)) = layout.line_for_byte_index(byte_index) {
path.line_index = line_index as u32;
- for (run_index, run) in line.runs().enumerate() {
+ for run_index in 0..line.len() {
+ let Some(run) = line.run(run_index) else {
+ continue;
+ };
path.run_index = run_index as u32;
if !run.text_range().contains(&byte_index) {
continue;
@@ -44,13 +47,17 @@ impl<'a, B: Brush> Cluster<'a, B> {
path.line_index = line_index as u32;
let mut offset = line.metrics().offset;
let last_run_index = line.len().saturating_sub(1);
- for (run_index, run) in line.runs().enumerate() {
+ for run_index in 0..line.len() {
+ let advance = line.item(run_index).unwrap().advance;
+ let Some(run) = line.run(run_index) else {
+ offset += advance;
+ continue;
+ };
let is_last_run = run_index == last_run_index;
- let run_advance = run.advance();
path.run_index = run_index as u32;
path.logical_index = 0;
- if x > offset + run_advance && !is_last_run {
- offset += run_advance;
+ if x > offset + advance && !is_last_run {
+ offset += advance;
continue;
}
let last_cluster_index = run.cluster_range().len().saturating_sub(1);
@@ -354,16 +361,15 @@ impl<'a, B: Brush> Cluster<'a, B> {
pub fn visual_offset(&self) -> Option<f32> {
let line = self.path.line(self.run.layout)?;
let mut offset = line.metrics().offset;
- for run_index in 0..=self.path.run_index() {
- let run = line.run(run_index)?;
- if run_index != self.path.run_index() {
- offset += run.advance();
- } else {
- let visual_index = run.logical_to_visual(self.path.logical_index())?;
- for cluster in run.visual_clusters().take(visual_index) {
- offset += cluster.advance();
- }
- }
+ for run_index in 0..self.path.run_index() {
+ let item = line.item(run_index)?;
+ offset += item.advance;
+ }
+
+ let run = line.run(self.path.run_index())?;
+ let visual_index = run.logical_to_visual(self.path.logical_index())?;
+ for cluster in run.visual_clusters().take(visual_index) {
+ offset += cluster.advance();
}
Some(offset)
}
diff --git a/parley/src/layout/cursor.rs b/parley/src/layout/cursor.rs
index d25c1d6f..323fa07b 100644
--- a/parley/src/layout/cursor.rs
+++ b/parley/src/layout/cursor.rs
@@ -5,7 +5,9 @@
#[cfg(feature = "accesskit")]
use super::LayoutAccessibility;
-use super::{Affinity, BreakReason, Brush, Cluster, ClusterSide, Layout, Line};
+use super::{
+ Affinity, BreakReason, Brush, Cluster, ClusterSide, Layout, Line, PositionedLayoutItem,
+};
#[cfg(feature = "accesskit")]
use accesskit::TextPosition;
use alloc::vec::Vec;
@@ -826,10 +828,18 @@ impl Selection {
let mut start_x = metrics.offset as f64;
let mut cur_x = start_x;
let mut cluster_count = 0;
- for run in line.runs() {
- for cluster in run.visual_clusters() {
+ for item in line.items() {
+ let PositionedLayoutItem::GlyphRun(run) = item else {
+ continue;
+ };
+ let offset = run.offset();
+ let run = run.run();
+ for (ix, cluster) in run.visual_clusters().enumerate() {
let advance = cluster.advance() as f64;
if text_range.contains(&cluster.text_range().start) {
+ if ix == 0 {
+ cur_x = offset as f64;
+ }
cluster_count += 1;
cur_x += advance;
} else {
@@ -837,6 +847,9 @@ impl Selection {
let width = (cur_x - start_x).max(MIN_RECT_WIDTH);
f(Rect::new(start_x as _, line_min, start_x + width, line_max));
}
+ if ix == 0 {
+ cur_x = offset as f64;
+ }
cur_x += advance;
start_x = cur_x;
}
From c7497f4984d7c32f7be951f57df5265db15936b4 Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Fri, 6 Dec 2024 17:02:54 +0100
Subject: [PATCH 2/3] Simplify
---
parley/src/layout/cluster.rs | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/parley/src/layout/cluster.rs b/parley/src/layout/cluster.rs
index 0dcde58d..cad4fcdb 100644
--- a/parley/src/layout/cluster.rs
+++ b/parley/src/layout/cluster.rs
@@ -21,11 +21,8 @@ impl<'a, B: Brush> Cluster<'a, B> {
let mut path = ClusterPath::default();
if let Some((line_index, line)) = layout.line_for_byte_index(byte_index) {
path.line_index = line_index as u32;
- for run_index in 0..line.len() {
- let Some(run) = line.run(run_index) else {
- continue;
- };
- path.run_index = run_index as u32;
+ for run in line.runs() {
+ path.run_index = run.index;
if !run.text_range().contains(&byte_index) {
continue;
}
From c8c42f73826cb702474a6a0aed9d24903fb84aec Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Fri, 6 Dec 2024 17:10:58 +0100
Subject: [PATCH 3/3] Fix (but maybe there's a neater way to account for the
boxes...)
---
parley/src/layout/cursor.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/parley/src/layout/cursor.rs b/parley/src/layout/cursor.rs
index 323fa07b..d685e335 100644
--- a/parley/src/layout/cursor.rs
+++ b/parley/src/layout/cursor.rs
@@ -832,7 +832,7 @@ impl Selection {
let PositionedLayoutItem::GlyphRun(run) = item else {
continue;
};
- let offset = run.offset();
+ let offset = run.offset() + metrics.offset;
let run = run.run();
for (ix, cluster) in run.visual_clusters().enumerate() {
let advance = cluster.advance() as f64;