winit
winit copied to clipboard
rust-windowing
Add integration tests
Note: This patch is based on my work in #1932 and cannot currently be merged. Please ignore all but the ~~last~~ two last commit. That is, ignore everything outside the it directory.
This patch adds integration tests than run as a github action. Currently only the X11 backend is being tested. It should not be hard to add support for the wayland backend.
You can see them in action here: https://github.com/mahkoh/winit/runs/4417323203
The tricky thing with porting from xlib to xcb is that xlib does more than just wrap the wire protocol, it also includes various utilities, with the most notable here maybe being the XFilterEvent mechanism that supports IME integration.
It doesn't seem ideal that this series ends up removing IME support.
Since xlib is based on xcb and an xcb connection can be got from xlib I would hope that xcb usage could be introduced incrementally where it's particularly beneficial, e.g. to help avoid synchronous round trips.
It looks like there's loads of really good work buried in this series - though it's a bit surprising how long of a series of changes this is stacked on top of.
The first commit in this series is apparently two years old :/
I was looking at some small changes in the x11 backend and now I'm worried about how much of a huge backlog of x11 changes there are for winit.
It doesn't seem ideal that this series ends up removing IME support.
Since xlib is based on xcb and an xcb connection can be got from xlib I would hope that xcb usage could be introduced incrementally where it's particularly beneficial, e.g. to help avoid synchronous round trips.
IME is broken on X11 with xlib anyway. And the only way to solve it is to move to xcb.
For more see https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/127 . I'm not sure there's a way to get it MR any further, so moving away from xlib is the only solution for this particular problem.
If we want to solve IME problem with xcb there's https://crates.io/crates/xcb-imdkit and upstream C library is maintained by fctix developers, so it's kind of battle tested.
It looks like there's loads of really good work buried in this series - though it's a bit surprising how long of a series of changes this is stacked on top of.
The first commit in this series is apparently two years old :/
Like I said in the OP, for this PR only the last two commits are relevant. The remaining commits are part of the linked PR.
Like I said in the OP, for this PR only the last two commits are relevant. The remaining commits are part of the linked PR.
Yeah, although I'd seen that this issue was only focused on the final patches, I still initially interpreted "This patch is based on my work in #1932" to mean that this series was a super set of #1932, so when I looked over the series of commits here I was under the impression it was the same series under #1932.
Taking another look at the commits for #1932 I do see now that some things like XInput2 enabling are now being done by @maroider so I probably got the wrong impression about what work is outstanding from looking at the commits here.
Is there another issue where you've maybe discussed more about upstreaming the XCB port - I couldn't find anything, so this was the only place I'd seen that work. I just found this old issue which didn't have any link to your work: https://github.com/rust-windowing/winit/issues/5
It doesn't seem ideal that this series ends up removing IME support.
Since xlib is based on xcb and an xcb connection can be got from xlib I would hope that xcb usage could be introduced incrementally where it's particularly beneficial, e.g. to help avoid synchronous round trips.
IME is broken on X11 with xlib anyway. And the only way to solve it is to move to xcb.
For more see https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/127 . I'm not sure there's a way to get it MR any further, so moving away from xlib is the only solution for this particular problem.
If we want to solve IME problem with xcb there's https://crates.io/crates/xcb-imdkit and upstream C library is maintained by fctix developers, so it's kind of battle tested.
Yeah, this makes sense. Tbh, when I was first skimming the patch series it just stood out that the series was removing IME support and I hadn't initially seen much discussion of that so I was concerned this change might potentially be for the wrong reasons (just to remove xlib for the sake of it even if it regresses features).
I'd also recently come across PR https://github.com/rust-windowing/winit/pull/2182 which gave me an initial impression their might be users actively using XIM based IMEs with winit considering going to the effort to fix issues with it. Actually though it looks like it might be more the opposite - they are trying to workaround the way in which key repeats are synthesized with im=ibus, and arguably that problem would also be solved by removing the XIM support from the current backend.
@mahkoh Do you have plans to rebase this work on the new XCB-based x11rb backend? I think that integration tests in this way would be very valuable. Otherwise I will close this PR for triage purposes.
