dvdhrm / kmscon

Linux KMS/DRM based virtual Console Emulator
http://www.freedesktop.org/wiki/Software/kmscon
Other
433 stars 81 forks source link

Update xkb keyboard backend to new libxkbcommon API #15

Closed bluetech closed 12 years ago

bluetech commented 12 years ago

Hi David,

Here's a few patches to update to the current libxkbcommon master. The current code hasn't been working with it for a while.

I think you will be pleased with the changelog :) Most of what we had in kbd_xkb.c is now in the library proper, and it also stopped leaking memory. It should be pretty lightweight now, after the initial keymap compilation, which takes a few milliseconds during startup.

Some comments on the commits:

A couple trivial fixes: 6f5e383 kbd: remove "dev" from kbd_dev_keysym_to_string d4e27ce monitor: fix small memory leak

[ I also run into a memleak which I preferred leaving to you. The eloop isn't free'd, caused by this cycle: ev_eloop_new -> ev_eloop_new_counter -> ev_eloop_add_counter -> ev_eloop_add_fd -> ev_eloop_ref ev_eloop_unref -> ev_eloop_rm_counter -> ev_eloop_rm_fd -> ev_eloop_unref Of course this ^^ one never happens because the ref doesn't go to 0. ]

It seems I caught the codebase during some transition. I didn't want to update both the kbd and the uterm variants, so I removed the old files and fixed the fallout. It was pretty straightforward, so I think it's fine. If not, you can skip it, but it would not build because of the libxkbcommon API change. 4bcca3d input: remove old input and kbd subsystems

And these do the updating. There's still some things left to do, some on the xkbcommon side, and later on kmscon. And xkbcommon is definitely not bug free. But it will get some more stuff soon, some mentioned in the comments, and also keysym-to-unicode (which we still do ourselves), a common config file, logging into an fd instead of stderr, and a few more. I should also mention that libxkbcommon does not handle OOM gracefully, basically it does abort(). I don't think anyone cares about that, so it probably won't be fixed soon. But you should know. e25740b xkb: add a TODO note to kbd_dev_reset stub b9ecae0 xkb: produce key events b47dcae xkb: initialize the xkb objects 8977572 xkb: remove old xkb code and add stubs

Finally, I think that once xkbcommon is stable, we could remove the dumb backend, remove the xproto dependency, and utilize the library to a larger extent. But there's still some time for that.

Thanks, Ran

dvdhrm commented 12 years ago

I've been following the discussions on xorg-devel/wayland-devel and am quite happy about the changes. Thanks btw ;) And yes, the changelog looks pretty nice. Most of the code is redundant now and can be removed. I am also looking forward to the other changes like the global xkbrc file.

I've fixed the eloop refcnt issue, too (as you might have seen). Thanks for spotting it!

The removal of the old input API will not work, though. At least for now. The new input API does not add input devices if used without the uterm-monitor. However, I wasn't able to add the monitor, yet, as I am still discussing multi-seat related issues with the systemd developers. However, I will do the transition to uterm_input and uterm_monitor next week and after that I will merge your patches. Sorry for the delay but I hope that's fine.

I already suspected that xkbcommon does not care for OOM. There are also other libraries which have issues with it (like glib). That's the reason why I decided to keep the dumb backend. Not because I care for OOM, but because I want kmscon to be able to work as rescue console. That is, I decided (driven by your libudev removal idea and Alan Cox' comments) to allow building a very limited kmscon binary which has almost no dependencies and works under memory-pressure and/or failing system devices. For the same reason I am working on an fbdev backend and on some kernel drivers that allow printing kernel-log to framebuffers with CONFIG_VT=n.

The uterm efforts are part of the idea to split kmscon into several libraries so other software can use the VT that is provided by kmscon as library.

Well, that's it for now. Thanks for the xkb updates. As I said, I will merge them next week but after first review they look very nice. Thanks! David

dvdhrm commented 12 years ago

I've merged your changes. After debugging a "uterm_input_ref" typo for about 2 hours as it was writing wildly into memory, I now got everything working. Thanks again for the patches!