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

wayland-client blocking dispatch with timeout #634

Open
fredizzimo opened this issue Jun 30, 2023 · 2 comments
Open

wayland-client blocking dispatch with timeout #634

fredizzimo opened this issue Jun 30, 2023 · 2 comments

Comments

@fredizzimo
Copy link
Contributor

fredizzimo commented Jun 30, 2023

It would be nice to have a blocking_dispatch with a timeout.

I'm trying to implement frame callbacks for Neovide, were we have the main logic and rendering loop in one thread, and the event processing in another thread. I want to add frame callbacks to the render thread, but with a timeout, so that it still can do some processing when a callback is not received for some time, maybe because the window is hidden or something.

I manged to get it to almost work by following the Integrating the event queue with other sources of events example, and polling the socket with a timeout.

However, there's one problem, I see regular short freezes, maybe once every other minute or so and tracked down the cause to the same as blocking_read is avoiding

// we need to check the queue again, just in case another thread did a read between
// dispatch_pending and prepare_read
if self.handle.inner.lock().unwrap().queue.is_empty() {
crate::conn::blocking_read(guard)?;
} else {
drop(guard);
}
self.dispatch_pending(data)

And I confirmed it by adding a short sleep between dispatch_pending and prepare_read, a 2 ms delay make the short pauses very frequent.

The problem is that I can't use the same workaround myself, since inner is private. So either that needs to be exposed, or better, provide a timeout for blocking_dispatch.

Edit: I think I could create yet another thread, and call 'blocking_dispatch` from that and handles the frame callbacks, and then uses a condition variable for example to signal when the frame callback happens, so that the rendering thread can wait for that with a timeout instead. But that seems like an unnecessary complicated thing to do.

My current work in progress code is here https://github.com/fredizzimo/neovide/blob/12aa5c74f330aed918a84eed561b262cbd1980c8/src/renderer/vsync_wayland.rs

@fredizzimo
Copy link
Contributor Author

One workaround that seems to work is to move the dispatch_pending call after calling prepare_read only actually poll the socket and read if it returns 0.

So something like this

pub fn wait_for_vsync(&mut self) {
    while !self.vsync_signaled.load(Ordering::Relaxed) {
        self.event_queue.flush().unwrap();
        let read_guard = self.event_queue.prepare_read().unwrap();
        if self
            .event_queue
            .dispatch_pending(&mut self.dispatcher)
            .unwrap()
            == 0
        {
            let mut fds = [PollFd::new(
                read_guard.connection_fd().as_raw_fd(),
                PollFlags::POLLIN | nix::poll::PollFlags::POLLERR,
            )];

            let n = loop {
                match nix::poll::poll(&mut fds, 1000) {
                    Ok(n) => break n,
                    Err(nix::errno::Errno::EINTR) => continue,
                    Err(_) => break 0,
                }
            };
            if n > 0 {
                read_guard.read().unwrap();
            }
        }
    }
    self.vsync_signaled.store(false, Ordering::Relaxed);
    let _callback = self.wl_surface.frame(&self.event_queue_handle, ());
}

There could be another dispatch_pending check before that to avoid locking if not necessary. But I don't think that matters in my use case, so I left it out.

@elinorbgr
Copy link
Member

Yeah, reading and processing events from multiple threads at the same time is kind of a pain point. I think your workaround is a good way to handle it, but I agree adding a blocking dispatch with timeout API to wayland-client would probably be the best, and I see no reason not to.

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

No branches or pull requests

2 participants