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

Having too many TextEdits causes an invalid_key error in timers.rs #3019

Closed
KrakenNikki opened this issue Jun 29, 2023 · 7 comments · Fixed by #3034
Closed

Having too many TextEdits causes an invalid_key error in timers.rs #3019

KrakenNikki opened this issue Jun 29, 2023 · 7 comments · Fixed by #3034
Assignees

Comments

@KrakenNikki
Copy link

KrakenNikki commented Jun 29, 2023

Setup Info

Currently using Rust and the latest version of Slint through the main git repository on the winit-femtovg renderer, on Arch Linux with the zen kernel, and using SwayWM on Wayland.

The Problem

The issue involves timers and the TextInput builtin element, for some reason, the more TextEdits there are on a window, the faster each consecutive TextInput cursor will blink, It will also keep creating new timers every time the elements are removed and recreated, such as through a VecModel that is displayed through a for x in model := TextEdit { text:x.text }. This will eventually cause the 'Slab' of timers to get corrupted (Still unsure how), and will crash the application due with calls to timers that have ID's outside of the 'Slab'.

The crash is also performance dependent, for my project, the VSCode preview runs at about 15 - 20 FPS, and will crash in 5-10 seconds. A release build will get 60FPS and last about 4 - 5 minutes before it crashes. Unsure if this has anything to do with it or is just a side effect.

Now I do have a few STD re-implementation elements using TextInput, but none of them use any functions other than binding to text and using the edited and cursor_position_changed callbacks.

Possible Cause?

I believe it is a form of memory corruption since I did some probing, if you do sanity checks between activate_timer and register_active_timer, Even though they are called consecutively without modification to self.timers, it will report that activate timer has a valid timer ID while register_active_timer does not, which leads to the invalid_key error.

I usually don't report bugs, but this has been getting in the way of my project and my quick fix was to essentially disable most timers. This significantly improved stability, but now things like blinking cursors and such do not work properly.

The Project

This project is going to be my take on what 'Manuskript' is, and the portion for editing character information involves lots and lots of TextEdits, LineEdits, and others. I can provide code to part of my STD Widgets on request, but beware that it's still very much in its 'sketch' phase, and is of rather poor quality (and documentation).

Thank you for your time.

@ogoffart
Copy link
Member

I could not reproduce the problem with a simple testcase. This is the testcase i used:

import {  ListView, TextEdit} from "std-widgets.slint";
export component MainWindow inherits Window {
    preferred-width: 200px;
    preferred-height: 400px;
    ListView {
        for t in 142: TextEdit { text: t;  height: 60px; }
    }
}

@KrakenNikki
Copy link
Author

Weird, it's not crashing mine either, and what's even more odd is that I can use the same logic in a test program that I use in the real thing, and it doesn't crash. The big difference being the widgets I use, and that the files are over 1500 lines long each, and there's a lot more tabbed stuff.

I recorded a video of it in action here

@ogoffart
Copy link
Member

That's a nice UI you've made. Looking forward to see that released.

As for the blinking cursor, maybe this has something to do with focus changes...
It is possible that some animation in the background causes something to re-create components.
I can't really say more.

They way i'd debug that is to "bisect" the UI by removing as many part of the UI as possible while still making the bug visible.

If i had access to your application code, i could have a look. Feel free to send me an email if you're comfortable sharing your source code.

@KrakenNikki
Copy link
Author

KrakenNikki commented Jun 30, 2023

Alright, I cut down the code as much as I could, and the crash only happens when a TextEdit is inside a NikkiScrollArea.
If using the default scroll area, everything behaves as expected

Steps to reproduce

  1. Click between two TextInputs timed half a second to a second apart from eachother about 5 - 7 times
  2. Switch tabs at the top of the screen on the strawberry bar

It should switch tabs, freeze, then crash the interpreter.

Also thank you for the compliment, I've been trying real hard on it!

Here's the files for it, just click preview on NikkiWindow, the stuff here is not sensitive so it's fine to share
CutDownNikkiBug.zip

@ogoffart
Copy link
Member

ogoffart commented Jul 3, 2023

I can Reproduce the bug

@ogoffart
Copy link
Member

ogoffart commented Jul 3, 2023

Small testcase:

component LineEdit {
    Rectangle {
        border-width: 1px;
        border-color: lightgray;
    }
    TextInput {}
}

export component NikkiWindow inherits Window
{
    preferred-height: 200px; preferred-width: 200px;
    if true : Rectangle {
        Flickable {
            HorizontalLayout {
                LineEdit { }
                LineEdit { }
            }
        }
    }
}

online editor

switching focus between the line edit around 4 times reproduce the blinking issue, then un-focussing the window panics:

thread 'main' panicked at 'invalid key', /home/olivier/.cargo/registry/src/index.crates.io-6f17d22bba15001f/i-slint-core-1.1.0/timers.rs:244:37
stack backtrace:
[...]
   3: std::thread::local::LocalKey<T>::with
   4: i_slint_core::platform::update_timers_and_animations
   5: i_slint_backend_winit::event_loop::CURRENT_WINDOW_TARGET::<impl i_slint_backend_winit::event_loop::CURRENT_WINDOW_TARGET>::set
   6: winit::platform_impl::platform::x11::EventLoop<T>::run_return::single_iteration

(the panic only happens with the winit backend, but the cursor blinking problem happens also with Qt)

@ogoffart
Copy link
Member

ogoffart commented Jul 3, 2023

The Flickable cause a "delayed click". So there is a 100ms timer which delay the event handling in case we want to flick the flickable. And then the delayed click event reset the cursor timer.
There is a bug when restarting another timer from a timer handler. This is being fixed in #3034

(When the window becomes inactive, the cursor blinking stops with winit, but not with qt, which axplain the difference in behavior with the two backend)

Thanks for reporting the issue and providing the testcase!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants