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

Missing X11 FocusIn event #3892

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rodrigorc
Copy link

@rodrigorc rodrigorc commented Aug 28, 2024

This PR fixes #2841.

Also it adds and extra commit that synthesizes a FocusIn event if ever the window receives a keyboard event while unfocused, that should not happen, but it does happen sometimes.

Feel free to comment on those two commits separately, if you prefer that.

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also need a changelog entry.

src/platform_impl/linux/x11/window.rs Show resolved Hide resolved
Comment on lines 879 to 894
let focused = self.with_window(window, |window| {
let mut shared_state_lock = window.shared_state_lock();
if !shared_state_lock.has_focus {
shared_state_lock.has_focus = true;
true
} else {
false
}
});
if focused == Some(true) {
let event = Event::WindowEvent { window_id, event: WindowEvent::Focused(true) };
callback(&self.target, event);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should only send focus event when we actually will be sending anything related to the keyboard input.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if this particular keyboard event does not end in notifying the user of anything, my argument was that if I'm getting this X11 event here it is because this window has the focus. And if shared_state_lock.has_focus is false, then I should already have received it, so I'm sending it now to compensate for the missing one.

src/platform_impl/linux/x11/event_processor.rs Outdated Show resolved Hide resolved
@madsmtm madsmtm added B - bug Dang, that shouldn't have happened DS - x11 labels Sep 4, 2024
It should be done before mapping the window, or we could
lose the firsst XInput2 events, such as the first FocusIn.
Fixes rust-windowing#2841.
@rodrigorc
Copy link
Author

Hi! This seems to have stalled a bit.

I think adding two unrelated "fixes" to the focus was a mistake. I'm rewriting the PR to just include the fix for #2841, that should be easier to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - bug Dang, that shouldn't have happened DS - x11
Development

Successfully merging this pull request may close these issues.

Borderless fullscreen mode in X11 "focus toggle" behavior; focus is lost when switching workspaces
3 participants