Remove comments that only restate the text code

# Conflicts:
#	core/src/render/primitive.rs

core/src/primitive/text.rs

@@ -1,12 +1,8 @@
-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},
@@ -14,18 +10,18 @@ use swash::{
 };
 
 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(),
         }
     }
@@ -81,13 +77,7 @@ 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>,
+    shaped: Option<(TextAttrs, Option<f32>)>,
 }
 
 impl TextBuffer {
@@ -95,7 +85,7 @@ impl TextBuffer {
         Self {
             text: text.into(),
             layout: Layout::new(),
-            layout_key: None,
+            shaped: None,
         }
     }
 
@@ -119,13 +109,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 +124,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,7 +140,7 @@ 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));
     }
 }
 
@@ -174,6 +160,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,21 +169,37 @@ 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 entry = self.glyph_entry(
-                        GlyphRaster {
-                            key,
-                            font: font_ref,
-                            font_size,
-                            coords,
-                            subpixel,
-                            glyph_id: glyph.id,
-                        },
-                        textures,
-                    );
+                    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,
@@ -209,60 +213,15 @@ 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()
-}
-
-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
+    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);
+    }
+    h
 }
 
 #[derive(Clone)]

core/src/render/atlas.rs

@@ -1,5 +1,5 @@
 use crate::{
-    GlyphPrimitive, PatchRect, TextureHandle, Textures,
+    PatchRect, TextureHandle, Textures,
     util::{HashMap, Vec2},
 };
 use image::RgbaImage;
@@ -27,28 +27,18 @@ 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 {
-    pub(crate) fn flags(&self) -> u32 {
-        if self.is_colored {
-            GlyphPrimitive::IS_COLORED
-        } else {
-            0
-        }
-    }
-}
-
 struct Page {
     handle: TextureHandle,
     x: u32,
@@ -105,13 +95,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,
         };
@@ -120,12 +110,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);
         }
 
@@ -152,65 +148,45 @@ 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 {
+    (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];
-        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]);
+    let w = image.placement.width;
+    let h = image.placement.height;
+    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::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]]);
+        }
+        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 => {
+            // 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]));
                 }
             }
         }

core/src/render/primitive.rs

@@ -6,7 +6,6 @@ use crate::{
         ArrBuf,
         data::{MaskIdx, PrimitiveInstance},
     },
-    util::Vec2,
 };
 use bytemuck::Pod;
 use wgpu::*;
@@ -235,8 +234,8 @@ pub struct TexturePrimitive {
 #[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>,
@@ -244,7 +243,7 @@ pub struct GlyphPrimitive {
 }
 
 impl GlyphPrimitive {
-    pub const IS_COLORED: u32 = 1;
+    pub const IS_COLOR: 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 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

@@ -101,6 +101,13 @@ 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;
@@ -115,7 +122,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

@@ -111,7 +111,7 @@ impl<'a> TextEditCtx<'a> {
         self.text.view.buf.layout()
     }
 
-    fn clamp_selection_to_layout(&mut self) {
+    fn refresh(&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 = std::mem::take(self.text.view.buf.edit());
-        self.text.selection = None;
+        let text = self.text.view.buf.text().to_string();
+        self.set("");
         text
     }
 
@@ -219,21 +219,23 @@ impl<'a> TextEditCtx<'a> {
         if end == 0 {
             return;
         }
-        let layout = self.layout();
-        let start = if word {
-            sel.focus().previous_logical_word(layout).index()
-        } else {
-            let Some(cluster) = sel.focus().logical_clusters(layout)[0] else {
-                return;
-            };
-            let range = cluster.text_range();
-            if cluster.is_hard_line_break() || cluster.is_emoji() {
-                range.start
+        let start = {
+            let layout = self.layout();
+            if word {
+                sel.focus().previous_logical_word(layout).index()
             } else {
-                self.text.view.buf.text()[..range.end]
-                    .char_indices()
-                    .next_back()
-                    .map_or(range.start, |(start, _)| start)
+                let Some(cluster) = sel.focus().logical_clusters(layout)[0] else {
+                    return;
+                };
+                let range = cluster.text_range();
+                if cluster.is_hard_line_break() || cluster.is_emoji() {
+                    range.start
+                } else {
+                    self.text.view.buf.text()[..range.end]
+                        .char_indices()
+                        .next_back()
+                        .map_or(range.start, |(start, _)| start)
+                }
             }
         };
         self.delete_range(start, end);
@@ -250,20 +252,27 @@ impl<'a> TextEditCtx<'a> {
         if start >= self.text.view.buf.text().len() {
             return;
         }
-        let layout = self.layout();
-        let end = if word {
-            sel.focus().next_logical_word(layout).index()
-        } else {
-            let clusters = sel.focus().logical_clusters(layout);
-            let Some(cluster) = clusters[1].as_ref() else {
-                return;
-            };
-            cluster.text_range().end
+        let end = {
+            let layout = self.layout();
+            if word {
+                sel.focus().next_logical_word(layout).index()
+            } else {
+                let clusters = sel.focus().logical_clusters(layout);
+                let Some(cluster) = clusters[1].as_ref() else {
+                    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,30 +294,31 @@ impl<'a> TextEditCtx<'a> {
         let prev_sel = self.text.selection;
         let prev_hit = self.text.double_hit;
 
-        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)
-        } 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)
-            } else if recent && prev_sel.map(|s| s.focus().index()) == Some(index) {
-                (
-                    Selection::word_from_point(layout, pos.x, pos.y),
-                    Some(index),
-                )
+        let outcome = {
+            let layout = self.layout();
+            if drag {
+                prev_sel.map(|sel| (Some(sel.extend_to_point(layout, pos.x, pos.y)), prev_hit))
             } else {
-                (hit, None)
+                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)
+                } else if recent && prev_sel.map(|s| s.focus().index()) == Some(index) {
+                    (
+                        Some(Selection::word_from_point(layout, pos.x, pos.y)),
+                        Some(index),
+                    )
+                } else {
+                    (Some(hit), None)
+                })
             }
         };
 
-        self.text.selection = Some(selection);
-        self.text.double_hit = double_hit;
+        if let Some((selection, double_hit)) = outcome {
+            self.text.selection = selection;
+            self.text.double_hit = double_hit;
+        }
     }
 
     pub fn deselect(&mut self) {
@@ -324,7 +334,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,7 +23,7 @@ pub struct TextView {
 }
 
 impl TextView {
-    fn is_empty(&self) -> bool {
+    fn is_blank(&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_empty()
+        if self.is_blank()
             && 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_empty()
+        if self.is_blank()
             && 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_empty()
+        if self.is_blank()
             && let Some(hint) = &self.hint
         {
             painter.widget(hint);