Fix RTL justification; clear TODOs

Author: valadaptive

Cargo.lock

@@ -3949,7 +3949,7 @@ dependencies = [
 [[package]]
 name = "swash"
 version = "0.2.1"
-source = "git+https://github.com/valadaptive/swash?branch=tight-bounds#059903eb7175c6a0a838fc1e4b7d445150cfe7d9"
+source = "git+https://github.com/valadaptive/swash?branch=tight-bounds#2a6429fb81b692d7345a78986354f939f424d1f3"
 dependencies = [
  "skrifa",
  "yazi",

crates/egui/src/text_selection/text_cursor_state.rs

@@ -105,7 +105,6 @@ pub fn cursor_rect(galley: &Galley, cursor: &ByteCursor, row_height: f32) -> Rec
     let mut cursor_pos = galley.pos_from_cursor(*cursor);
 
     // Handle completely empty galleys
-    // TODO(valadaptive): had to adjust this because Parley's row height is less than egui's. Is this necessary anymore?
     if cursor_pos.height() < 1.0 {
         cursor_pos.max.y = cursor_pos.max.y.at_least(cursor_pos.min.y + row_height);
     }

crates/epaint/src/text/glyph_atlas.rs

@@ -238,13 +238,14 @@ impl GlyphAtlas {
         let image = &mut self.scratch;
         let color = glyph_run.style().brush;
 
-        // TODO(valadaptive): there's also a faux embolden property, but it's always true with the defaut font because
-        // it's "light" and we technically asked for "normal"
+        // There's also a faux embolden property, but it's always true with the default font because it's "light" and we
+        // technically asked for "normal", so we can't use it
         let skew = run.synthesis().skew();
 
         let style_key = StyleKey::<'b>::new(
             font_id,
             size,
+            // Parley stores skew as an i8 internally, so it's fine to convert it back into one
             skew.unwrap_or_default() as i8,
             hinting_enabled,
             normalized_coords,

crates/epaint/src/text/parley_layout.rs

@@ -154,9 +154,9 @@ pub(super) fn layout(fonts: &mut FontsLayoutView<'_>, job: LayoutJob) -> Galley
         Some(alignment_width),
         match (justify, job.halign) {
             (true, _) => parley::Alignment::Justified,
-            (false, emath::Align::Min) => parley::Alignment::Start,
+            (false, emath::Align::Min) => parley::Alignment::Left,
             (false, emath::Align::Center) => parley::Alignment::Middle,
-            (false, emath::Align::Max) => parley::Alignment::End,
+            (false, emath::Align::Max) => parley::Alignment::Right,
         },
         AlignmentOptions::default(),
     );

crates/epaint/src/texture_atlas.rs

@@ -290,8 +290,6 @@ impl TextureAtlas {
         self.overflowed = false;
         self.discs.clear();
         self.initialize();
-
-        // TODO(valadaptive): reset to initial size?
     }
 }
 

parley-todo.md

@@ -13,6 +13,7 @@
 - [ ] Support the tab character (https://github.com/linebender/parley/issues/302)
 - [ ] AccessKit improvements (https://github.com/linebender/parley/issues/310)
 - [ ] Not Parley, but Swash: tighter glyph bounds (https://github.com/dfrg/zeno/pull/15)
+- [ ] SystemUi doesn't properly fallback in some cases (e.g. Arabic text, macOS shortcut symbols) on my machine; SansSerif does (https://github.com/linebender/parley/issues/323)
 
 ## Here:
 - [ ] Text layout
@@ -30,9 +31,15 @@
   - [ ] `LayoutJob::break_on_newline`
   - [ ] RTL considerations
     - [ ] Label wrapping only occurs in LTR layouts, but make sure it doesn't do anything weird with RTL labels
-    - [ ] Text is right-justified if a label is split in the middle of RTL text
+    - [ ] Do label wrapping in RTL too?
+    - [x] Text is right-justified if a label is split in the middle of RTL text, or if a line contains only an RTL label
       - This may be desirable behavior, but is weird
+      - Whoops; we asked Parley for RTL-aware behavior and got it
     - [ ] overflow_character should appear at the "start" of the line for RTL text
+    - [ ] RTL support for `egui::Align`
+  - [ ] Once Parley has vertical alignment, remove the hack for leading_space/first_row_min_height
+  - [ ] Align multiple consecutive labels to the same baseline, even with different fonts
+  - [ ] Line height discrepancy between old and new layout (Parley line height is just the font size)
 - [ ] Text rendering
   - [x] Investigate whether swash is being too conservative with its shape bounds and cutting off rendered glyphs
     - The reverse is true https://github.com/dfrg/zeno/pull/15
@@ -52,12 +59,13 @@
     - Forgot to clear the glyph atlas
 - [ ] Text selection and editing
   - [x] Unify the three different cursor types and move to a Parley-like API before moving to the actual Parley API (done)
-  - [ ] Rewrite selection code to use parley's API
+  - [x] Rewrite selection code to use parley's API
     - [x] Basic API mapping (done)
     - [x] Selection painting (done but kinda janky)
-    - [ ] Probably rework label_text_selection (does it take bidirectional text into account?)
+    - [x] Probably rework label_text_selection (does it take bidirectional text into account?)
       - [x] Still some jank when the cursor is kinda below the first label and it selects the "rest of the line" (fixed)
-      - [ ] Make it support bidirectional text (fun!)
+      - [x] Make it support bidirectional text (fun!)
+        - I think this... just works already?
     - [x] finish the gnarly parts that i've been putting off
       - [x] indentation (done, but untested because parley's tab character support is broken)
       - [x] selecting a range without having a Galley rendered already (done)
@@ -68,6 +76,7 @@
   - [ ] Do another pass over TextBuffer's API
   - [ ] Test IME support
   - [ ] Smoothe out AccessKit API integration (and reduce temp allocations)
+  - [ ] Test AccessKit text bounding boxes (horiz_offset for alignment and vertical_offset for wrapped labels working)
 - [ ] Text styling
   - [x] Fix FontDefinitions and adding fonts
     - [x] Get fallback/ordering working properly
@@ -99,20 +108,16 @@
   - [ ] Smoothe out the janky parts of the new API
     - [ ] For mixed-DPI purposes, and because we don't need to store the FontStore as a mutex, ctx.fonts() now returns a "fonts view" that's technically read/write. But there are no operations that *semantically* modify the fonts from it
     - [ ] FontStore and Fonts are different and we should just expose them separately instead of passing through all the FontStore methods onto Fonts
-  - [ ] SystemUi doesn't properly fallback in some cases (e.g. Arabic text, macOS shortcut symbols) on my machine; SansSerif does
   - [ ] Work around https://github.com/jslegers/emoji-icon-font/issues/18 / https://github.com/emilk/egui/issues/1284
-- [ ] Cross-cutting concerns
-  - [ ] Global/scoped RTL? Do we get bidirectional support for free if we use Parley's APIs?
-    - [ ] RTL support for `egui::Align`
-    - [ ] RTL/bidi support for cross-label text selection
-    - [ ] RTL support for cross-label text wrapping
+- [x] Cross-cutting concerns
   - [x] Actually remove all the ab_glyph stuff
     - Sayonara, ab_glyph 🫡
 - [ ] Perf optimizations!
   - [ ] Stop using TreeBuilder so we don't have to allocate a bunch of strings
   - [ ] https://github.com/emilk/egui/issues/1098
   - [ ] Line-level layout memoization (https://github.com/emilk/egui/pull/5411)
 - [ ] The other 90%
+  - [ ] Comment the new code better
   - [ ] update All Of The Doctests...
   - [ ] Go over APIs and clean them up
   - [ ] New documentation for the new APIs