Simplify text editing state changes

core/src/primitive/text.rs

@@ -1,8 +1,12 @@
-use crate::{Align, GlyphAtlas, GlyphKey, PlacedGlyph, RegionAlign, Textures, UiColor, util::Vec2};
+use crate::{
+    Align, GlyphAtlas, GlyphEntry, GlyphKey, PlacedGlyph, RegionAlign, Textures, UiColor,
+    util::Vec2,
+};
 use parley::{
     Alignment, AlignmentOptions, FontContext, FontFamily, FontFamilyName, GenericFamily, Layout,
     LayoutContext, LineHeight, PositionedLayoutItem, StyleProperty,
 };
+use std::hash::{DefaultHasher, Hash, Hasher};
 use swash::{
     FontRef,
     scale::{Render, ScaleContext, Source, StrikeWith},
@@ -10,18 +14,18 @@ use swash::{
 };
 
 pub struct TextData {
-    pub font_cx: FontContext,
-    pub layout_cx: LayoutContext<UiColor>,
-    scale_cx: ScaleContext,
+    pub font_ctx: FontContext,
+    pub layout_ctx: LayoutContext<UiColor>,
+    scale_ctx: ScaleContext,
     pub atlas: GlyphAtlas,
 }
 
 impl Default for TextData {
     fn default() -> Self {
         Self {
-            font_cx: FontContext::new(),
-            layout_cx: LayoutContext::new(),
-            scale_cx: ScaleContext::new(),
+            font_ctx: FontContext::new(),
+            layout_ctx: LayoutContext::new(),
+            scale_ctx: ScaleContext::new(),
             atlas: GlyphAtlas::default(),
         }
     }
@@ -77,7 +81,13 @@ impl Default for TextAttrs {
 pub struct TextBuffer {
     text: String,
     layout: Layout<UiColor>,
-    shaped: Option<(TextAttrs, Option<f32>)>,
+    layout_key: Option<LayoutKey>,
+}
+
+#[derive(PartialEq)]
+struct LayoutKey {
+    attrs: TextAttrs,
+    max_width: Option<f32>,
 }
 
 impl TextBuffer {
@@ -85,7 +95,7 @@ impl TextBuffer {
         Self {
             text: text.into(),
             layout: Layout::new(),
-            shaped: None,
+            layout_key: None,
         }
     }
 
@@ -109,13 +119,13 @@ impl TextBuffer {
         let text = text.into();
         if text != self.text {
             self.text = text;
-            self.shaped = None;
+            self.layout_key = None;
         }
     }
 
     /// Invalidates the layout and returns the underlying string for editing.
     pub fn edit(&mut self) -> &mut String {
-        self.shaped = None;
+        self.layout_key = None;
         &mut self.text
     }
 
@@ -124,12 +134,16 @@ impl TextBuffer {
     }
 
     pub fn shape(&mut self, data: &mut TextData, attrs: &TextAttrs, width: Option<f32>) {
-        if self.shaped.as_ref() == Some(&(attrs.clone(), width)) {
+        let layout_key = LayoutKey {
+            attrs: attrs.clone(),
+            max_width: width,
+        };
+        if self.layout_key.as_ref() == Some(&layout_key) {
             return;
         }
         let mut builder = data
-            .layout_cx
-            .ranged_builder(&mut data.font_cx, &self.text, 1.0, true);
+            .layout_ctx
+            .ranged_builder(&mut data.font_ctx, &self.text, 1.0, true);
         builder.push_default(StyleProperty::FontFamily(attrs.family.family()));
         builder.push_default(StyleProperty::FontSize(attrs.font_size));
         builder.push_default(StyleProperty::LineHeight(LineHeight::Absolute(
@@ -140,7 +154,7 @@ impl TextBuffer {
         self.layout.break_all_lines(width);
         self.layout
             .align(Alignment::Start, AlignmentOptions::default());
-        self.shaped = Some((attrs.clone(), width));
+        self.layout_key = Some(layout_key);
     }
 }
 
@@ -160,8 +174,6 @@ impl TextData {
                     continue;
                 };
                 let coords_hash = hash_coords(coords);
-                // `font.data.id()` rather than the pointer, so the same font
-                // loaded twice is still one set of entries.
                 let font_id = font.data.id();
 
                 for glyph in run.positioned_glyphs() {
@@ -169,37 +181,21 @@ impl TextData {
                     let key = GlyphKey {
                         font: font_id,
                         glyph: glyph.id,
-                        size: (font_size * 16.0).round() as u32,
+                        size: glyph_size_key(font_size),
                         subpixel,
                         coords: coords_hash,
                     };
-                    let entry = match self.atlas.get(&key) {
-                        Some(entry) => entry,
-                        None => {
-                            let mut scaler = self
-                                .scale_cx
-                                .builder(font_ref)
-                                .size(font_size)
-                                .hint(true)
-                                .normalized_coords(coords)
-                                .build();
-                            let image = Render::new(&[
-                                Source::ColorOutline(0),
-                                Source::ColorBitmap(StrikeWith::BestFit),
-                                Source::Outline,
-                            ])
-                            .format(Format::Alpha)
-                            .offset(Vector::new(subpixel as f32 / 4.0, 0.0))
-                            .render(&mut scaler, glyph.id as u16);
-                            match image {
-                                Some(image) => self.atlas.insert(key, &image, textures),
-                                None => {
-                                    self.atlas.insert_empty(key);
-                                    None
-                                }
-                            }
-                        }
-                    };
+                    let entry = self.glyph_entry(
+                        GlyphRaster {
+                            key,
+                            font: font_ref,
+                            font_size,
+                            coords,
+                            subpixel,
+                            glyph_id: glyph.id,
+                        },
+                        textures,
+                    );
                     let Some(entry) = entry else { continue };
                     placed.push(PlacedGlyph {
                         entry,
@@ -213,15 +209,60 @@ impl TextData {
         }
         placed
     }
+
+    fn glyph_entry(
+        &mut self,
+        glyph: GlyphRaster<'_>,
+        textures: &mut Textures,
+    ) -> Option<GlyphEntry> {
+        if let Some(entry) = self.atlas.get(&glyph.key) {
+            return entry;
+        }
+
+        let mut scaler = self
+            .scale_ctx
+            .builder(glyph.font)
+            .size(glyph.font_size)
+            .hint(true)
+            .normalized_coords(glyph.coords)
+            .build();
+        let image = Render::new(&[
+            Source::ColorOutline(0),
+            Source::ColorBitmap(StrikeWith::BestFit),
+            Source::Outline,
+        ])
+        .format(Format::Alpha)
+        .offset(Vector::new(glyph.subpixel as f32 / 4.0, 0.0))
+        .render(&mut scaler, glyph.glyph_id as u16);
+
+        if let Some(image) = image {
+            self.atlas.insert(glyph.key, &image, textures)
+        } else {
+            self.atlas.insert_empty(glyph.key);
+            None
+        }
+    }
+}
+
+struct GlyphRaster<'a> {
+    key: GlyphKey,
+    font: FontRef<'a>,
+    font_size: f32,
+    coords: &'a [i16],
+    subpixel: u8,
+    glyph_id: u32,
 }
 
 fn hash_coords(coords: &[i16]) -> u64 {
-    let mut h: u64 = 0xcbf2_9ce4_8422_2325;
-    for c in coords {
-        h ^= *c as u16 as u64;
-        h = h.wrapping_mul(0x1000_0000_01b3);
+    let mut hasher = DefaultHasher::new();
+    coords.hash(&mut hasher);
+    hasher.finish()
 }
-    h
+
+const GLYPH_SIZE_STEPS_PER_PIXEL: f32 = 16.0;
+
+fn glyph_size_key(font_size: f32) -> u32 {
+    (font_size * GLYPH_SIZE_STEPS_PER_PIXEL).round() as u32
 }
 
 #[derive(Clone)]

core/src/render/atlas.rs

@@ -1,5 +1,5 @@
 use crate::{
-    PatchRect, TextureHandle, Textures,
+    GlyphPrimitive, PatchRect, TextureHandle, Textures,
     util::{HashMap, Vec2},
 };
 use image::RgbaImage;
@@ -27,18 +27,28 @@ pub struct GlyphKey {
 
 #[derive(Clone, Copy)]
 pub struct GlyphEntry {
-    pub uv_min: [f32; 2],
-    pub uv_max: [f32; 2],
+    pub uv_min: Vec2,
+    pub uv_max: Vec2,
     /// Offset from the glyph's pen position to the top-left of its pixels.
     pub left: i32,
     pub top: i32,
     pub width: u32,
     pub height: u32,
-    pub is_color: bool,
+    pub is_colored: bool,
     pub view_idx: u32,
     pub sampler_idx: u32,
 }
 
+impl GlyphEntry {
+    pub(crate) fn flags(&self) -> u32 {
+        if self.is_colored {
+            GlyphPrimitive::IS_COLORED
+        } else {
+            0
+        }
+    }
+}
+
 struct Page {
     handle: TextureHandle,
     x: u32,
@@ -95,13 +105,13 @@ impl GlyphAtlas {
         let page = &self.pages[page_idx];
         let scale = 1.0 / PAGE as f32;
         let entry = GlyphEntry {
-            uv_min: [x as f32 * scale, y as f32 * scale],
-            uv_max: [(x + w) as f32 * scale, (y + h) as f32 * scale],
+            uv_min: Vec2::new(x as f32 * scale, y as f32 * scale),
+            uv_max: Vec2::new((x + w) as f32 * scale, (y + h) as f32 * scale),
             left: image.placement.left,
             top: image.placement.top,
             width: w,
             height: h,
-            is_color: matches!(image.content, Content::Color),
+            is_colored: matches!(image.content, Content::Color),
             view_idx: page.handle.primitive().view_idx,
             sampler_idx: page.handle.primitive().sampler_idx,
         };
@@ -110,18 +120,12 @@ impl GlyphAtlas {
     }
 
     fn allocate(&mut self, w: u32, h: u32, textures: &mut Textures) -> (usize, u32, u32) {
-        let need_w = w + PAD;
-        let need_h = h + PAD;
-        if let Some(i) = self.pages.iter().position(|p| fits(p, need_w, need_h)) {
-            let page = &mut self.pages[i];
-            if page.x + need_w > PAGE {
-                page.y += page.shelf_height;
-                page.x = PAD;
-                page.shelf_height = 0;
-            }
-            let (x, y) = (page.x, page.y);
-            page.x += need_w;
-            page.shelf_height = page.shelf_height.max(need_h);
+        if let Some((i, (x, y))) = self
+            .pages
+            .iter_mut()
+            .enumerate()
+            .find_map(|(i, page)| page.allocate(w, h).map(|position| (i, position)))
+        {
             return (i, x, y);
         }
 
@@ -148,45 +152,65 @@ impl GlyphAtlas {
     }
 }
 
-fn fits(page: &Page, need_w: u32, need_h: u32) -> bool {
-    (page.x + need_w <= PAGE && page.y + need_h <= PAGE)
-        || (need_w + PAD <= PAGE && page.y + page.shelf_height + need_h <= PAGE)
+impl Page {
+    fn allocate(&mut self, w: u32, h: u32) -> Option<(u32, u32)> {
+        let need_w = w + PAD;
+        let need_h = h + PAD;
+        if self.x + need_w > PAGE {
+            if need_w + PAD > PAGE || self.y + self.shelf_height + need_h > PAGE {
+                return None;
+            }
+            self.y += self.shelf_height;
+            self.x = PAD;
+            self.shelf_height = 0;
+        } else if self.y + need_h > PAGE {
+            return None;
+        }
+
+        let position = (self.x, self.y);
+        self.x += need_w;
+        self.shelf_height = self.shelf_height.max(need_h);
+        Some(position)
+    }
 }
 
 /// Mask glyphs keep coverage in alpha so their raster can be tinted at draw time.
 fn write_glyph(page: &mut RgbaImage, image: &Image, x: u32, y: u32) {
-    let w = image.placement.width;
-    let h = image.placement.height;
+    let width = image.placement.width as usize;
+    let height = image.placement.height as usize;
+    let page_stride = page.width() as usize * 4;
+    let x = x as usize * 4;
+    let y = y as usize;
+    let page = page.as_mut();
+
+    for row in 0..height {
+        let start = (y + row) * page_stride + x;
+        let target = &mut page[start..start + width * 4];
         match image.content {
-        Content::Mask => {
-            for row in 0..h {
-                for col in 0..w {
-                    let a = image.data[(row * w + col) as usize];
-                    page.put_pixel(x + col, y + row, image::Rgba([255, 255, 255, a]));
-                }
-            }
-        }
             Content::Color => {
-            for row in 0..h {
-                for col in 0..w {
-                    let i = ((row * w + col) * 4) as usize;
-                    let px = [
-                        image.data[i],
-                        image.data[i + 1],
-                        image.data[i + 2],
-                        image.data[i + 3],
-                    ];
-                    page.put_pixel(x + col, y + row, image::Rgba(px));
+                let start = row * width * 4;
+                target.copy_from_slice(&image.data[start..start + width * 4]);
             }
+            Content::Mask => {
+                let start = row * width;
+                for (target, &alpha) in target
+                    .as_chunks_mut::<4>()
+                    .0
+                    .iter_mut()
+                    .zip(&image.data[start..start + width])
+                {
+                    target.copy_from_slice(&[255, 255, 255, alpha]);
                 }
             }
             Content::SubpixelMask => {
-            // Preserve readable output if the rasterizer returns a subpixel mask.
-            for row in 0..h {
-                for col in 0..w {
-                    let i = ((row * w + col) * 4) as usize;
-                    let a = image.data[i + 1];
-                    page.put_pixel(x + col, y + row, image::Rgba([255, 255, 255, a]));
+                let start = row * width * 4;
+                for (target, source) in target
+                    .as_chunks_mut::<4>()
+                    .0
+                    .iter_mut()
+                    .zip(image.data[start..start + width * 4].as_chunks::<4>().0)
+                {
+                    target.copy_from_slice(&[255, 255, 255, source[1]]);
                 }
             }
         }

core/src/render/primitive.rs

@@ -6,6 +6,7 @@ use crate::{
         ArrBuf,
         data::{MaskIdx, PrimitiveInstance},
     },
+    util::Vec2,
 };
 use bytemuck::Pod;
 use wgpu::*;
@@ -234,8 +235,8 @@ pub struct TexturePrimitive {
 #[repr(C)]
 #[derive(Debug, Copy, Clone)]
 pub struct GlyphPrimitive {
-    pub uv_min: [f32; 2],
-    pub uv_max: [f32; 2],
+    pub uv_min: Vec2,
+    pub uv_max: Vec2,
     pub view_idx: u32,
     pub sampler_idx: u32,
     pub color: Color<u8>,
@@ -243,7 +244,7 @@ pub struct GlyphPrimitive {
 }
 
 impl GlyphPrimitive {
-    pub const IS_COLOR: u32 = 1;
+    pub const IS_COLORED: u32 = 1;
 }
 
 pub struct PrimitiveVec<T> {

core/src/render/texture.rs

@@ -17,36 +17,36 @@ pub struct GpuTextures {
 
 impl GpuTextures {
     pub fn update(&mut self, textures: &mut Textures) -> bool {
-        let mut changed = false;
+        let mut bindings_changed = false;
         for update in textures.updates() {
-            match update {
+            bindings_changed |= match update {
                 TextureUpdate::Push(image) => {
                     self.push(image);
-                    changed = true;
+                    true
                 }
                 TextureUpdate::Set(i, image) => {
                     self.set(i, image);
-                    changed = true;
+                    true
                 }
                 TextureUpdate::Patch(i, rect, image) => {
-                    // Patching contents leaves the binding array unchanged.
                     self.patch(i, rect, image);
+                    false
                 }
                 TextureUpdate::SetFree => {
                     self.view_count += 1;
-                    changed = true;
+                    true
                 }
                 TextureUpdate::Free(i) => {
                     self.free(i);
-                    changed = true;
+                    true
                 }
                 TextureUpdate::PushFree => {
                     self.push_free();
-                    changed = true;
+                    true
                 }
+            };
         }
-        }
-        changed
+        bindings_changed
     }
     fn set(&mut self, i: u32, image: &DynamicImage) {
         self.view_count += 1;

core/src/ui/painter.rs

@@ -101,13 +101,6 @@ impl<'a> Painter<'a> {
     }
 
     pub fn glyphs(&mut self, text: &RenderedText, origin: UiRegion) {
-        let flags_for = |is_color| {
-            if is_color {
-                GlyphPrimitive::IS_COLOR
-            } else {
-                0
-            }
-        };
         for glyph in text.glyphs.iter() {
             let mut region = origin;
             region.x.end = region.x.start;
@@ -122,7 +115,7 @@ impl<'a> Painter<'a> {
                     view_idx: glyph.entry.view_idx,
                     sampler_idx: glyph.entry.sampler_idx,
                     color: text.color,
-                    flags: flags_for(glyph.entry.is_color),
+                    flags: glyph.entry.flags(),
                 },
                 region,
             );

src/widget/text/edit.rs

@@ -111,7 +111,7 @@ impl<'a> TextEditCtx<'a> {
         self.text.view.buf.layout()
     }
 
-    fn refresh(&mut self) {
+    fn clamp_selection_to_layout(&mut self) {
         if let Some(sel) = self.text.selection {
             let layout = self.layout();
             self.text.selection = Some(sel.refresh(layout));
@@ -119,8 +119,8 @@ impl<'a> TextEditCtx<'a> {
     }
 
     pub fn take(&mut self) -> String {
-        let text = self.text.view.buf.text().to_string();
-        self.set("");
+        let text = std::mem::take(self.text.view.buf.edit());
+        self.text.selection = None;
         text
     }
 
@@ -219,9 +219,8 @@ impl<'a> TextEditCtx<'a> {
         if end == 0 {
             return;
         }
-        let start = {
         let layout = self.layout();
-            if word {
+        let start = if word {
             sel.focus().previous_logical_word(layout).index()
         } else {
             let Some(cluster) = sel.focus().logical_clusters(layout)[0] else {
@@ -236,7 +235,6 @@ impl<'a> TextEditCtx<'a> {
                     .next_back()
                     .map_or(range.start, |(start, _)| start)
             }
-            }
         };
         self.delete_range(start, end);
     }
@@ -252,9 +250,8 @@ impl<'a> TextEditCtx<'a> {
         if start >= self.text.view.buf.text().len() {
             return;
         }
-        let end = {
         let layout = self.layout();
-            if word {
+        let end = if word {
             sel.focus().next_logical_word(layout).index()
         } else {
             let clusters = sel.focus().logical_clusters(layout);
@@ -262,17 +259,11 @@ impl<'a> TextEditCtx<'a> {
                 return;
             };
             cluster.text_range().end
-            }
         };
         self.delete_range(start, end);
     }
 
     fn delete_range(&mut self, start: usize, end: usize) {
-        let len = self.text.view.buf.text().len();
-        let (start, end) = (start.min(end).min(len), start.max(end).min(len));
-        if start == end {
-            return;
-        }
         self.text.view.buf.edit().replace_range(start..end, "");
         self.text.view.buf.changed = true;
         self.set_caret(start);
@@ -294,32 +285,31 @@ impl<'a> TextEditCtx<'a> {
         let prev_sel = self.text.selection;
         let prev_hit = self.text.double_hit;
 
-        let outcome = {
         let layout = self.layout();
-            if drag {
-                prev_sel.map(|sel| (Some(sel.extend_to_point(layout, pos.x, pos.y)), prev_hit))
+        let (selection, double_hit) = if drag {
+            let Some(selection) = prev_sel else {
+                return;
+            };
+            (selection.extend_to_point(layout, pos.x, pos.y), prev_hit)
         } else {
             let hit = Selection::from_point(layout, pos.x, pos.y);
             let index = hit.focus().index();
             // Successive clicks at one index select the word, then the line.
-                Some(if recent && prev_hit == Some(index) {
-                    (Some(Selection::line_from_point(layout, pos.x, pos.y)), None)
+            if recent && prev_hit == Some(index) {
+                (Selection::line_from_point(layout, pos.x, pos.y), None)
             } else if recent && prev_sel.map(|s| s.focus().index()) == Some(index) {
                 (
-                        Some(Selection::word_from_point(layout, pos.x, pos.y)),
+                    Selection::word_from_point(layout, pos.x, pos.y),
                     Some(index),
                 )
             } else {
-                    (Some(hit), None)
-                })
+                (hit, None)
             }
         };
 
-        if let Some((selection, double_hit)) = outcome {
-            self.text.selection = selection;
+        self.text.selection = Some(selection);
         self.text.double_hit = double_hit;
     }
-    }
 
     pub fn deselect(&mut self) {
         self.text.selection = None;
@@ -334,7 +324,7 @@ impl<'a> TextEditCtx<'a> {
             if let Some((old, selection)) = self.text.history.pop() {
                 self.set(&old);
                 self.text.selection = selection;
-                self.refresh();
+                self.clamp_selection_to_layout();
             }
         } else if self.text.view.buf.text() != old.0 {
             self.text.history.push(old);

src/widget/text/mod.rs

@@ -23,7 +23,7 @@ pub struct TextView {
 }
 
 impl TextView {
-    fn is_blank(&self) -> bool {
+    fn is_empty(&self) -> bool {
         self.buf.is_empty()
     }
 
@@ -76,7 +76,7 @@ impl TextView {
         self.tex.as_ref()
     }
     pub fn desired_width(&mut self, ctx: &mut SizeCtx) -> Len {
-        if self.is_blank()
+        if self.is_empty()
             && let Some(hint) = &self.hint
         {
             ctx.width(hint)
@@ -85,7 +85,7 @@ impl TextView {
         }
     }
     pub fn desired_height(&mut self, ctx: &mut SizeCtx) -> Len {
-        if self.is_blank()
+        if self.is_empty()
             && let Some(hint) = &self.hint
         {
             ctx.height(hint)
@@ -96,7 +96,7 @@ impl TextView {
     pub fn draw(&mut self, painter: &mut Painter) -> UiRegion {
         let tex = self.render(&mut painter.size_ctx());
         let region = tex.size.align(self.align);
-        if self.is_blank()
+        if self.is_empty()
             && let Some(hint) = &self.hint
         {
             painter.widget(hint);