Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash with no text in layout and very small widths #186

Closed
DJMcNab opened this issue Nov 28, 2024 · 2 comments · Fixed by #194
Closed

Crash with no text in layout and very small widths #186

DJMcNab opened this issue Nov 28, 2024 · 2 comments · Fixed by #194
Labels
bug Something isn't working

Comments

@DJMcNab
Copy link
Member

DJMcNab commented Nov 28, 2024

To reproduce, you can apply this patch then select all and press backspace or delete:

Patch to reproduce

diff --git a/examples/vello_editor/src/main.rs b/examples/vello_editor/src/main.rs
index 0422e76..8ab97cf 100644
--- a/examples/vello_editor/src/main.rs
+++ b/examples/vello_editor/src/main.rs
@@ -119,7 +119,7 @@ impl ApplicationHandler<accesskit_winit::Event> for SimpleVelloApp<'_> {
 
         self.editor.transact(|txn| {
             txn.set_scale(1.0);
-            txn.set_width(Some(size.width as f32 - 2f32 * text::INSET));
+            txn.set_width(Some(0.0)); // This can be a higher value and still reproduce
             txn.set_text(text::LOREM);
         });
 
@@ -238,9 +238,8 @@ impl ApplicationHandler<accesskit_winit::Event> for SimpleVelloApp<'_> {
                     .resize_surface(&mut render_state.surface, size.width, size.height);
                 self.editor.transact(|txn| {
                     txn.set_scale(1.0);
-                    txn.set_width(Some(size.width as f32 - 2f32 * text::INSET));
                     txn.set_default_style(Arc::new([
-                        StyleProperty::FontSize(32.0),
+                        StyleProperty::FontSize(16.0),
                         StyleProperty::LineHeight(1.2),
                         GenericFamily::SystemUi.into(),
                     ]));
diff --git a/examples/vello_editor/src/text.rs b/examples/vello_editor/src/text.rs
index 2211c7e..178b033 100644
--- a/examples/vello_editor/src/text.rs
+++ b/examples/vello_editor/src/text.rs
@@ -77,7 +77,7 @@ impl Editor {
     pub fn handle_event(&mut self, event: WindowEvent) {
         match event {
             WindowEvent::Resized(size) => {
-                self.transact(|txn| txn.set_width(Some(size.width as f32 - 2f32 * INSET)));
+                // self.transact(|txn| txn.set_width(Some(1.0)));
             }
             WindowEvent::ModifiersChanged(modifiers) => {
                 self.modifiers = Some(modifiers);

On my machine, any width less than an unknown value in [3.52, 3.521) causes a crash. The backtrace is:

Backtrace

thread 'main' panicked at parley/parley/src/layout/line/greedy.rs:740:26:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::panic
   3: core::option::unwrap_failed
   4: parley::layout::line::greedy::try_commit_line
   5: parley::layout::line::greedy::BreakLines<B>::break_remaining
   6: parley::layout::editor::PlainEditor<T>::update_layout
   7: parley::layout::editor::PlainEditor<T>::replace_selection
   8: parley::layout::editor::PlainEditor<T>::transact
   9: vello_editor::text::Editor::handle_event
  10: <vello_editor::SimpleVelloApp as winit::application::ApplicationHandler<accesskit_winit::Event>>::window_event
  11: core::ops::function::impls::<impl core::ops::function::FnMut<A> for &mut F>::call_mut
  12: winit::platform_impl::linux::wayland::event_loop::EventLoop<T>::pump_events
  13: winit::platform_impl::linux::wayland::event_loop::EventLoop<T>::run_on_demand
  14: vello_editor::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

And therefore the offending unwrap is:

let first_cluster = run
.get(cluster_range.start - run_data.cluster_range.start)
.unwrap();

DJMcNab added a commit to DJMcNab/xilem that referenced this issue Nov 28, 2024
DJMcNab added a commit to DJMcNab/xilem that referenced this issue Nov 28, 2024
github-merge-queue bot pushed a commit to linebender/xilem that referenced this issue Nov 28, 2024
The upstream PR has now been merged:
linebender/parley#170

I've had to add a hack to workaround
linebender/parley#186. This occurs because of
the first pass of layout with zero available area.
@nicoburns
Copy link
Contributor

The following patch fixes the crash for me, but causes the cursor position in an empty layout to display as if there was a line-break (it's on the "second line"):

diff --git a/parley/src/layout/line/greedy.rs b/parley/src/layout/line/greedy.rs
index 967df61..1c8e89e 100644
--- a/parley/src/layout/line/greedy.rs
+++ b/parley/src/layout/line/greedy.rs
@@ -759,7 +759,7 @@ fn try_commit_line<B: Brush>(

                 // Push run to line
                 let run = Run::new(layout, 0, 0, run_data, None);
-                let text_range = if run_data.cluster_range.is_empty() {
+                let text_range = if cluster_range.is_empty() {
                     0..0
                 } else {
                     let first_cluster = run

@nicoburns
Copy link
Contributor

nicoburns commented Dec 3, 2024

The LayoutData (post-shaping) seems to get itself into a pretty weird state after shaping the "dummy space" for empty text. There is an item, run, and cluster. But text_len is 0, and there are no glyphs:

[parley/src/layout/line/greedy.rs:760:17] &layout.data.text_len = 0
[parley/src/layout/line/greedy.rs:761:17] &layout.data.items = [
    LayoutItem {
        kind: TextRun,
        index: 0,
        bidi_level: 0,
    },
]
[parley/src/layout/line/greedy.rs:762:17] &layout.data.runs = [
    RunData {
        text_range: 0..1,
        glyph_start: 0,
    },
]
[parley/src/layout/line/greedy.rs:763:17] &layout.data.glyphs = []
[parley/src/layout/line/greedy.rs:764:17] &layout.data.clusters = [
    ClusterData {
        info:   sw,
        flags: 0,
        style_index: 0,
        glyph_len: 255,
        text_len: 1,
        glyph_offset: 3,
        text_offset: 0,
        advance: 6.5625,
    },
]

@nicoburns nicoburns added the bug Something isn't working label Dec 3, 2024
github-merge-queue bot pushed a commit that referenced this issue Dec 3, 2024
- Fixes #186

I don't really understand why this works (why it crashes with a smaller
width but not a larger one in the first place) But seeing as the
`max_advance` doesn't really makes any difference for "empty" layouts
that only contain a single space, it seems safe to ignore it here.

Signed-off-by: Nico Burns <[email protected]>
DJMcNab added a commit to DJMcNab/parley that referenced this issue Dec 4, 2024
DJMcNab added a commit to DJMcNab/parley that referenced this issue Dec 4, 2024
Remove workaround for linebender#186

Use `unchecked` name

Fix doc comment

Restore debug code

Remove comment from Masonry

Co-Authored-By: Tom Churchman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants