Merge remote-tracking branch 'upstream/main' into split/08-parley-text

Cargo.lock

@@ -1189,7 +1189,6 @@ dependencies = [
  "bytemuck",
  "fxhash",
  "image",
- "log",
  "parley",
  "swash",
  "wgpu",

Cargo.toml

@@ -35,7 +35,6 @@ image = "0.25.6"
 parley = "0.11.1"
 swash = "0.2.10"
 fxhash = "0.2.1"
-log = "0.4.29"
 arboard = "3.6.1"
 iris-core = { path = "core" }
 iris-macro = { path = "macro" }

TODO

@@ -1,6 +1,19 @@
 images
     settings (sampler)
-    consider typed TextureHandle<T> variants for distinct texture uses
+
+text
+    figure out ways to speed up / what costs the most
+    resizing (per frame) is really slow (assuming painter isn't griefing)
+    j is weird / fix x offset
+
+masks r just made to bare minimum work
+
+scaling
+    could be just a simple scaling factor that multiplies abs
+    and need to ensure text uses raw abs and not scaled abs
+    naming? (pt, px)
+    want to keep (drawn) regions using px? or should I add another field to UiScalar/Vec
+    field could be best solution so redrawing stuff isn't needed & you can specify both as user
 
 WidgetRef<W> or smth instead of Id
     enum that's either an Id or an actual concrete instance of W
@@ -11,6 +24,17 @@ WidgetRef<W> or smth instead of Id
     maybe introduce InnerWidget trait to allow for editors to expose & modify inner type
     maybe could also store a parent widget and keep using InnerWidget trait? unsure if possible
 
+really weird limitation:
+    I don't think you can currently remove an element from a parent and put it in a child of the same parent
+    because it removes the unused children after the entire parent redraw
+    but the child gets drawn during that, so it will think the child is still active !!!
+    or something like that idk, maybe I need a special enum for parent that includes a undecided state where it may or may not get redrawn by the parent
+    or just do ref counting and ensure all drawn things == 1 afterwards (seems like best way)
+    ok so I'm removing the limit for now
+
+don't forget I'm streaming
+
+tags
 vecs for each widget type?
 
 POTENTIAL BUG: closures that store IDs will not decrement the id!!! need to not increment id if moved into closure somehow??? wait no, need to decrement ID every time an event fn is added...... only if the id is used in it..??

core/Cargo.toml

@@ -10,4 +10,3 @@ image = { workspace = true }
 parley = { workspace = true }
 swash = { workspace = true }
 fxhash = { workspace = true }
-log = { workspace = true }

core/src/primitive/color.rs

@@ -10,6 +10,7 @@ pub struct Color<T> {
     pub a: T,
 }
 
+/// Defaults to visible, unstyled text for Parley's brush requirement.
 impl<T: ColorNum> Default for Color<T> {
     fn default() -> Self {
         Self::BLACK

core/src/primitive/text.rs

@@ -1,36 +1,34 @@
-use crate::{
-    Align, GlyphAtlas, GlyphEntry, GlyphKey, PlacedGlyph, RegionAlign, Textures, UiColor,
-    util::Vec2,
-};
+use crate::{Align, GlyphAtlas, 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},
     zeno::{Format, Vector},
 };
 
+/// Shared font, layout, rasterization, and glyph-atlas state.
 pub struct TextData {
-    pub font_ctx: FontContext,
-    pub layout_ctx: LayoutContext<UiColor>,
-    scale_ctx: ScaleContext,
+    pub font_cx: FontContext,
+    pub layout_cx: LayoutContext<UiColor>,
+    scale_cx: ScaleContext,
     pub atlas: GlyphAtlas,
 }
 
 impl Default for TextData {
     fn default() -> Self {
         Self {
-            font_ctx: FontContext::new(),
-            layout_ctx: LayoutContext::new(),
-            scale_ctx: ScaleContext::new(),
+            font_cx: FontContext::new(),
+            layout_cx: LayoutContext::new(),
+            scale_cx: ScaleContext::new(),
             atlas: GlyphAtlas::default(),
         }
     }
 }
 
+/// An owned font family that can be stored by a widget.
 #[derive(Clone, PartialEq)]
 pub enum Family {
     SansSerif,
@@ -58,6 +56,7 @@ pub struct TextAttrs {
     pub line_height: f32,
     pub family: Family,
     pub wrap: bool,
+    /// Alignment within the text's region.
     pub align: RegionAlign,
 }
 
@@ -81,13 +80,8 @@ impl Default for TextAttrs {
 pub struct TextBuffer {
     text: String,
     layout: Layout<UiColor>,
-    layout_key: Option<LayoutKey>,
-}
-
-#[derive(PartialEq)]
-struct LayoutKey {
-    attrs: TextAttrs,
-    max_width: Option<f32>,
+    /// The inputs used to build the cached layout.
+    shaped: Option<(TextAttrs, Option<f32>)>,
 }
 
 impl TextBuffer {
@@ -95,7 +89,7 @@ impl TextBuffer {
         Self {
             text: text.into(),
             layout: Layout::new(),
-            layout_key: None,
+            shaped: None,
         }
     }
 
@@ -119,13 +113,13 @@ impl TextBuffer {
         let text = text.into();
         if text != self.text {
             self.text = text;
-            self.layout_key = None;
+            self.shaped = None;
         }
     }
 
     /// Invalidates the layout and returns the underlying string for editing.
     pub fn edit(&mut self) -> &mut String {
-        self.layout_key = None;
+        self.shaped = None;
         &mut self.text
     }
 
@@ -134,16 +128,12 @@ impl TextBuffer {
     }
 
     pub fn shape(&mut self, data: &mut TextData, attrs: &TextAttrs, width: Option<f32>) {
-        let layout_key = LayoutKey {
-            attrs: attrs.clone(),
-            max_width: width,
-        };
-        if self.layout_key.as_ref() == Some(&layout_key) {
+        if self.shaped.as_ref() == Some(&(attrs.clone(), width)) {
             return;
         }
         let mut builder = data
-            .layout_ctx
-            .ranged_builder(&mut data.font_ctx, &self.text, 1.0, true);
+            .layout_cx
+            .ranged_builder(&mut data.font_cx, &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(
@@ -154,11 +144,12 @@ impl TextBuffer {
         self.layout.break_all_lines(width);
         self.layout
             .align(Alignment::Start, AlignmentOptions::default());
-        self.layout_key = Some(layout_key);
+        self.shaped = Some((attrs.clone(), width));
     }
 }
 
 impl TextData {
+    /// Rasterizes uncached glyphs and places them relative to the text origin.
     pub fn place(&mut self, buffer: &TextBuffer, textures: &mut Textures) -> Vec<PlacedGlyph> {
         let mut placed = Vec::new();
         for line in buffer.layout.lines() {
@@ -174,6 +165,8 @@ 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() {
@@ -181,23 +174,38 @@ impl TextData {
                     let key = GlyphKey {
                         font: font_id,
                         glyph: glyph.id,
-                        size: glyph_size_key(font_size),
+                        size: (font_size * 16.0).round() as u32,
                         subpixel,
                         coords: coords_hash,
                     };
-                    let Some(entry) = self.glyph_entry(
-                        GlyphRaster {
-                            key,
-                            font: font_ref,
-                            font_size,
-                            coords,
-                            subpixel,
-                            glyph_id: glyph.id,
-                        },
-                        textures,
-                    ) else {
-                        continue;
+                    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 Some(entry) = entry else { continue };
                     placed.push(PlacedGlyph {
                         entry,
                         offset: Vec2::new(
@@ -210,64 +218,21 @@ 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 hasher = DefaultHasher::new();
-    coords.hash(&mut hasher);
-    hasher.finish()
+    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);
     }
-
-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
+    h
 }
 
+/// Cached glyph placement for a laid-out string.
+#[derive(Clone)]
 pub struct RenderedText {
-    pub glyphs: Vec<PlacedGlyph>,
+    pub glyphs: std::sync::Arc<Vec<PlacedGlyph>>,
     pub size: Vec2,
     pub color: UiColor,
 }
@@ -283,7 +248,7 @@ impl TextData {
         buffer.shape(self, attrs, width);
         let glyphs = self.place(buffer, textures);
         RenderedText {
-            glyphs,
+            glyphs: std::sync::Arc::new(glyphs),
             size: buffer.size(),
             color: attrs.color,
         }

core/src/primitive/texture.rs

@@ -29,6 +29,7 @@ pub struct Textures {
 pub enum TextureUpdate<'a> {
     Push(&'a DynamicImage),
     Set(u32, &'a DynamicImage),
+    /// Overwrite one rectangle without uploading the entire texture.
     Patch(u32, PatchRect, &'a DynamicImage),
     Free(u32),
     PushFree,
@@ -91,6 +92,7 @@ impl Textures {
         }
     }
 
+    /// The stored image for a handle, to be written into before `patch`.
     pub fn image_mut(&mut self, handle: &TextureHandle) -> &mut DynamicImage {
         self.images[handle.inner.view_idx as usize]
             .as_mut()

core/src/render/atlas.rs

@@ -1,3 +1,5 @@
+//! Packs reusable rasterized glyphs into shared texture pages.
+
 use crate::{
     PatchRect, TextureHandle, Textures,
     util::{HashMap, Vec2},
@@ -5,12 +7,14 @@ use crate::{
 use image::RgbaImage;
 use swash::scale::image::{Content, Image};
 
+/// A 1024-pixel RGBA8 page occupies 4 MiB.
 const PAGE: u32 = 1024;
 
 /// Transparent margin kept around every glyph, so that sampling one cannot
 /// pick up its neighbour along a shared edge.
 const PAD: u32 = 1;
 
+/// Includes every input that can change the rasterized pixels.
 #[derive(Clone, Copy, PartialEq, Eq, Hash)]
 pub struct GlyphKey {
     pub font: u64,
@@ -27,28 +31,21 @@ pub struct GlyphKey {
 
 #[derive(Clone, Copy)]
 pub struct GlyphEntry {
-    pub uv_min: Vec2,
-    pub uv_max: Vec2,
+    pub uv_min: [f32; 2],
+    pub uv_max: [f32; 2],
     /// 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_colored: bool,
+    pub is_color: bool,
     pub view_idx: u32,
     pub sampler_idx: u32,
 }
 
-impl GlyphEntry {
-    const IS_COLORED: u32 = 1;
-
-    pub(crate) fn flags(&self) -> u32 {
-        if self.is_colored { Self::IS_COLORED } else { 0 }
-    }
-}
-
 struct Page {
     handle: TextureHandle,
+    /// Shelf packing is effective because glyphs at one size have similar heights.
     x: u32,
     y: u32,
     shelf_height: u32,
@@ -67,6 +64,7 @@ impl GlyphAtlas {
         self.entries.get(key).copied()
     }
 
+    /// Returns `None` when a glyph has no pixels or cannot fit on a page.
     pub fn insert(
         &mut self,
         key: GlyphKey,
@@ -76,20 +74,11 @@ impl GlyphAtlas {
         let w = image.placement.width;
         let h = image.placement.height;
         if w == 0 || h == 0 {
-            log::warn!(
-                "glyph {} in font {} rasterized at {w}x{h}; skipping it",
-                key.glyph,
-                key.font,
-            );
             self.entries.insert(key, None);
             return None;
         }
-        if w > PAGE - PAD * 2 || h > PAGE - PAD * 2 {
-            log::warn!(
-                "glyph {} in font {} rasterized at {w}x{h}, too large for the {PAGE}x{PAGE} atlas; skipping it",
-                key.glyph,
-                key.font,
-            );
+        if w + PAD * 2 > PAGE || h + PAD * 2 > PAGE {
+            // Never silently crop an oversized glyph.
             self.entries.insert(key, None);
             return None;
         }
@@ -113,13 +102,13 @@ impl GlyphAtlas {
         let page = &self.pages[page_idx];
         let scale = 1.0 / PAGE as f32;
         let entry = GlyphEntry {
-            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),
+            uv_min: [x as f32 * scale, y as f32 * scale],
+            uv_max: [(x + w) as f32 * scale, (y + h) as f32 * scale],
             left: image.placement.left,
             top: image.placement.top,
             width: w,
             height: h,
-            is_colored: matches!(image.content, Content::Color),
+            is_color: matches!(image.content, Content::Color),
             view_idx: page.handle.primitive().view_idx,
             sampler_idx: page.handle.primitive().sampler_idx,
         };
@@ -128,12 +117,18 @@ impl GlyphAtlas {
     }
 
     fn allocate(&mut self, w: u32, h: u32, textures: &mut Textures) -> (usize, u32, u32) {
-        if let Some((i, (x, y))) = self
-            .pages
-            .iter_mut()
-            .enumerate()
-            .find_map(|(i, page)| page.allocate(w, h).map(|position| (i, position)))
-        {
+        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);
             return (i, x, y);
         }
 
@@ -147,6 +142,7 @@ impl GlyphAtlas {
         (self.pages.len() - 1, PAD, PAD)
     }
 
+    /// Record that a glyph has no pixels, so it is not re-rasterised.
     pub fn insert_empty(&mut self, key: GlyphKey) {
         self.entries.insert(key, None);
     }
@@ -160,71 +156,53 @@ impl GlyphAtlas {
     }
 }
 
-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)
-    }
+fn fits(page: &Page, need_w: u32, need_h: u32) -> bool {
+    // On the current shelf, or on a new one above it.
+    (page.x + need_w <= PAGE && page.y + need_h <= PAGE)
+        || (need_w + PAD <= PAGE && page.y + page.shelf_height + need_h <= PAGE)
 }
 
 /// 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 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];
+    let w = image.placement.width;
+    let h = image.placement.height;
     match image.content {
-            Content::Color => {
-                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]);
+            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));
+                }
             }
         }
         Content::SubpixelMask => {
-                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]]);
+            // 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]));
                 }
             }
         }
     }
 }
 
+/// Where a glyph goes on screen, in pixels relative to the text's origin.
 #[derive(Clone, Copy)]
 pub struct PlacedGlyph {
     pub entry: GlyphEntry,

core/src/render/primitive.rs

@@ -6,7 +6,6 @@ use crate::{
         ArrBuf,
         data::{MaskIdx, PrimitiveInstance},
     },
-    util::Vec2,
 };
 use bytemuck::Pod;
 use wgpu::*;
@@ -94,6 +93,7 @@ macro_rules! primitives {
             }
         )*
     };
+    // Preserve the whitespace-separated token shape across recursion.
     (@count $t1:tt $($t:tt)+) => { 1 + primitives!(@count $($t)+) };
     (@count $t:tt) => { 1 };
 }
@@ -232,17 +232,22 @@ pub struct TexturePrimitive {
     pub sampler_idx: u32,
 }
 
+/// An atlas subrectangle, tinted for masks or unchanged for color glyphs.
 #[repr(C)]
 #[derive(Debug, Copy, Clone)]
 pub struct GlyphPrimitive {
-    pub uv_min: Vec2,
-    pub uv_max: Vec2,
+    pub uv_min: [f32; 2],
+    pub uv_max: [f32; 2],
     pub view_idx: u32,
     pub sampler_idx: u32,
     pub color: Color<u8>,
     pub flags: u32,
 }
 
+impl GlyphPrimitive {
+    pub const IS_COLOR: u32 = 1;
+}
+
 pub struct PrimitiveVec<T> {
     vec: Vec<T>,
     free: Vec<usize>,

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 bindings_changed = false;
+        let mut changed = false;
         for update in textures.updates() {
-            bindings_changed |= match update {
+            match update {
                 TextureUpdate::Push(image) => {
                     self.push(image);
-                    true
+                    changed = true;
                 }
                 TextureUpdate::Set(i, image) => {
                     self.set(i, image);
-                    true
+                    changed = 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;
-                    true
+                    changed = true;
                 }
                 TextureUpdate::Free(i) => {
                     self.free(i);
-                    true
+                    changed = true;
                 }
                 TextureUpdate::PushFree => {
                     self.push_free();
-                    true
+                    changed = true;
                 }
-            };
             }
-        bindings_changed
+        }
+        changed
     }
     fn set(&mut self, i: u32, image: &DynamicImage) {
         self.view_count += 1;

core/src/ui/painter.rs

@@ -100,7 +100,15 @@ impl<'a> Painter<'a> {
         ui.text.render(buffer, attrs, width, &mut ui.textures)
     }
 
+    /// Draws one atlas-sampling quad per glyph.
     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;
@@ -115,7 +123,7 @@ impl<'a> Painter<'a> {
                     view_idx: glyph.entry.view_idx,
                     sampler_idx: glyph.entry.sampler_idx,
                     color: text.color,
-                    flags: glyph.entry.flags(),
+                    flags: flags_for(glyph.entry.is_color),
                 },
                 region,
             );

src/widget/text/edit.rs

@@ -104,6 +104,7 @@ pub struct TextEditCtx<'a> {
 }
 
 impl<'a> TextEditCtx<'a> {
+    /// Returns a layout synchronized with the current text.
     fn layout(&mut self) -> &Layout<UiColor> {
         let attrs = self.text.view.attrs.clone();
         let width = self.text.view.wrap_width();
@@ -111,7 +112,8 @@ impl<'a> TextEditCtx<'a> {
         self.text.view.buf.layout()
     }
 
-    fn clamp_selection_to_layout(&mut self) {
+    /// Keep the selection valid after the text underneath it changed.
+    fn refresh(&mut self) {
         if let Some(sel) = self.text.selection {
             let layout = self.layout();
             self.text.selection = Some(sel.refresh(layout));
@@ -119,8 +121,8 @@ impl<'a> TextEditCtx<'a> {
     }
 
     pub fn take(&mut self) -> String {
-        let text = std::mem::take(self.text.view.buf.edit());
-        self.text.selection = None;
+        let text = self.text.view.buf.text().to_string();
+        self.set("");
         text
     }
 
@@ -178,6 +180,7 @@ impl<'a> TextEditCtx<'a> {
         self.set_caret(at + text.len());
     }
 
+    /// True when there was a span to remove.
     pub fn clear_span(&mut self) -> bool {
         let Some(sel) = self.text.selection else {
             return false;
@@ -219,8 +222,9 @@ impl<'a> TextEditCtx<'a> {
         if end == 0 {
             return;
         }
+        let start = {
             let layout = self.layout();
-        let start = if word {
+            if word {
                 sel.focus().previous_logical_word(layout).index()
             } else {
                 let Some(cluster) = sel.focus().logical_clusters(layout)[0] else {
@@ -235,6 +239,7 @@ impl<'a> TextEditCtx<'a> {
                         .next_back()
                         .map_or(range.start, |(start, _)| start)
                 }
+            }
         };
         self.delete_range(start, end);
     }
@@ -250,8 +255,9 @@ impl<'a> TextEditCtx<'a> {
         if start >= self.text.view.buf.text().len() {
             return;
         }
+        let end = {
             let layout = self.layout();
-        let end = if word {
+            if word {
                 sel.focus().next_logical_word(layout).index()
             } else {
                 let clusters = sel.focus().logical_clusters(layout);
@@ -259,11 +265,17 @@ 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);
@@ -285,31 +297,32 @@ impl<'a> TextEditCtx<'a> {
         let prev_sel = self.text.selection;
         let prev_hit = self.text.double_hit;
 
+        let outcome = {
             let layout = self.layout();
-        let (selection, double_hit) = if drag {
-            let Some(selection) = prev_sel else {
-                return;
-            };
-            (selection.extend_to_point(layout, pos.x, pos.y), prev_hit)
+            if drag {
+                prev_sel.map(|sel| (Some(sel.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.
-            if recent && prev_hit == Some(index) {
-                (Selection::line_from_point(layout, pos.x, pos.y), None)
+                Some(if recent && prev_hit == Some(index) {
+                    (Some(Selection::line_from_point(layout, pos.x, pos.y)), None)
                 } else if recent && prev_sel.map(|s| s.focus().index()) == Some(index) {
                     (
-                    Selection::word_from_point(layout, pos.x, pos.y),
+                        Some(Selection::word_from_point(layout, pos.x, pos.y)),
                         Some(index),
                     )
                 } else {
-                (hit, None)
+                    (Some(hit), None)
+                })
             }
         };
 
-        self.text.selection = Some(selection);
+        if let Some((selection, double_hit)) = outcome {
+            self.text.selection = selection;
             self.text.double_hit = double_hit;
         }
+    }
 
     pub fn deselect(&mut self) {
         self.text.selection = None;
@@ -324,7 +337,7 @@ impl<'a> TextEditCtx<'a> {
             if let Some((old, selection)) = self.text.history.pop() {
                 self.set(&old);
                 self.text.selection = selection;
-                self.clamp_selection_to_layout();
+                self.refresh();
             }
         } else if self.text.view.buf.text() != old.0 {
             self.text.history.push(old);

src/widget/text/mod.rs

@@ -23,10 +23,11 @@ pub struct TextView {
 }
 
 impl TextView {
-    fn is_empty(&self) -> bool {
+    fn is_blank(&self) -> bool {
         self.buf.is_empty()
     }
 
+    /// The width used by the cached layout.
     pub fn wrap_width(&self) -> Option<f32> {
         self.width
     }
@@ -52,25 +53,31 @@ impl TextView {
             .align(self.align)
     }
 
-    fn render(&mut self, ctx: &mut SizeCtx) -> &RenderedText {
+    fn render(&mut self, ctx: &mut SizeCtx) -> RenderedText {
         let width = if self.attrs.wrap {
             Some(ctx.px_size().x)
         } else {
             None
         };
-        if width != self.width || self.tex.is_none() || self.attrs.changed || self.buf.changed {
+        if width == self.width
+            && let Some(tex) = &self.tex
+            && !self.attrs.changed
+            && !self.buf.changed
+        {
+            return tex.clone();
+        }
         self.width = width;
-            self.tex = Some(ctx.draw_text(&mut self.buf, &self.attrs, width));
+        let tex = ctx.draw_text(&mut self.buf, &self.attrs, width);
+        self.tex = Some(tex.clone());
         self.attrs.changed = false;
         self.buf.changed = false;
-        }
-        self.tex.as_ref().unwrap()
+        tex
     }
     pub fn tex(&self) -> Option<&RenderedText> {
         self.tex.as_ref()
     }
     pub fn desired_width(&mut self, ctx: &mut SizeCtx) -> Len {
-        if self.is_empty()
+        if self.is_blank()
             && let Some(hint) = &self.hint
         {
             ctx.width(hint)
@@ -79,7 +86,7 @@ impl TextView {
         }
     }
     pub fn desired_height(&mut self, ctx: &mut SizeCtx) -> Len {
-        if self.is_empty()
+        if self.is_blank()
             && let Some(hint) = &self.hint
         {
             ctx.height(hint)
@@ -88,19 +95,16 @@ impl TextView {
         }
     }
     pub fn draw(&mut self, painter: &mut Painter) -> UiRegion {
-        let align = self.align;
-        if self.is_empty() && self.hint.is_some() {
-            let region = self.render(&mut painter.size_ctx()).size.align(align);
-            if let Some(hint) = &self.hint {
-                painter.widget(hint);
-            }
-            return region;
-        }
-
         let tex = self.render(&mut painter.size_ctx());
-        let region = tex.size.align(align);
+        let region = tex.size.align(self.align);
+        if self.is_blank()
+            && let Some(hint) = &self.hint
+        {
+            painter.widget(hint);
+        } else {
             let within = region.within(&painter.region());
-        painter.glyphs(tex, within);
+            painter.glyphs(&tex, within);
+        }
         region
     }