From 49edd69a3ac3a69edb27720bc95ce5941e35fab6 Mon Sep 17 00:00:00 2001
From: Simon Hausmann <simon.hausmann@sixtyfps.io>
Date: Tue, 22 Sep 2020 18:43:00 +0200
Subject: [PATCH] Fix multi-byte handling hit testing

Return a byte offset within the string for the hit test in TextInput.
---
 sixtyfps_runtime/corelib/font/canvasfont.rs |  8 ++++----
 sixtyfps_runtime/corelib/font/fontkit.rs    |  9 +++++----
 sixtyfps_runtime/corelib/items.rs           | 14 +++++++-------
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/sixtyfps_runtime/corelib/font/canvasfont.rs b/sixtyfps_runtime/corelib/font/canvasfont.rs
index 87d0c8593..79de1fb4e 100644
--- a/sixtyfps_runtime/corelib/font/canvasfont.rs
+++ b/sixtyfps_runtime/corelib/font/canvasfont.rs
@@ -27,17 +27,17 @@ impl Font {
         text_metrics.width() as _
     }
 
-    pub fn text_index_for_x_position(&self, text: &str, x: f32) -> usize {
+    pub fn text_offset_for_x_position(&self, text: &str, x: f32) -> usize {
         // This is pretty cruel ...
         let mut last_width = 0.;
-        for index in 1..text.len() {
-            let new_width = self.text_width(&text[0..index]);
+        for offset in text.char_indices().map(|(offset, _)| offset) {
+            let new_width = self.text_width(&text[0..offset]);
 
             if new_width > last_width {
                 let advance = new_width - last_width;
 
                 if last_width + advance / 2. >= x {
-                    return index;
+                    return offset;
                 }
 
                 last_width = new_width;
diff --git a/sixtyfps_runtime/corelib/font/fontkit.rs b/sixtyfps_runtime/corelib/font/fontkit.rs
index d26c6d77a..4429c9432 100644
--- a/sixtyfps_runtime/corelib/font/fontkit.rs
+++ b/sixtyfps_runtime/corelib/font/fontkit.rs
@@ -48,18 +48,19 @@ impl Font {
             .fold(0., |width, glyph| width + glyph.advance)
     }
 
-    pub fn text_index_for_x_position(&self, text: &str, x: f32) -> usize {
-        let mut index = 0;
+    pub fn text_offset_for_x_position<'a>(&self, text: &'a str, x: f32) -> usize {
+        let mut char_offset_it = text.char_indices().map(|(offset, _)| offset);
 
         let mut current_x = 0.;
+        let mut current_offset = 0;
         // This assumes a 1:1 mapping between glyphs and characters right now -- this is wrong.
         for (_, glyph_id) in self.string_to_glyphs(text) {
             let metrics = self.glyph_metrics(glyph_id);
 
             if current_x + metrics.advance / 2. >= x {
-                return index;
+                return current_offset;
             }
-            index += 1;
+            current_offset = char_offset_it.next().unwrap();
             current_x += metrics.advance;
         }
 
diff --git a/sixtyfps_runtime/corelib/items.rs b/sixtyfps_runtime/corelib/items.rs
index 7ce0a5a90..5e373bbe1 100644
--- a/sixtyfps_runtime/corelib/items.rs
+++ b/sixtyfps_runtime/corelib/items.rs
@@ -835,8 +835,8 @@ pub struct TextInput {
     pub y: Property<f32>,
     pub width: Property<f32>,
     pub height: Property<f32>,
-    pub cursor_position: Property<i32>,
-    pub anchor_position: Property<i32>,
+    pub cursor_position: Property<i32>, // byte offset,
+    pub anchor_position: Property<i32>, // byte offset
     pub text_cursor_width: Property<f32>,
     pub cursor_visible: Property<bool>,
     pub accepted: Signal<()>,
@@ -952,16 +952,16 @@ impl Item for TextInput {
             InputEventResult::GrabMouse
         };
 
-        let clicked_index = TextInput::with_font(self, window, |font| {
+        let clicked_offset = TextInput::with_font(self, window, |font| {
             let text = Self::FIELD_OFFSETS.text.apply_pin(self).get();
-            font.text_index_for_x_position(&text, event.pos.x)
+            font.text_offset_for_x_position(&text, event.pos.x)
         }) as i32;
 
         if matches!(event.what, MouseEventType::MousePressed) {
             self.as_ref().pressed.set(true);
             self.as_ref().show_cursor(window);
-            self.as_ref().anchor_position.set(clicked_index);
-            self.as_ref().cursor_position.set(clicked_index);
+            self.as_ref().anchor_position.set(clicked_offset);
+            self.as_ref().cursor_position.set(clicked_offset);
         }
 
         match event.what {
@@ -969,7 +969,7 @@ impl Item for TextInput {
                 self.as_ref().pressed.set(false);
             }
             MouseEventType::MouseMoved if self.as_ref().pressed.get() => {
-                self.as_ref().cursor_position.set(clicked_index);
+                self.as_ref().cursor_position.set(clicked_offset);
             }
             _ => {}
         }