RFC: Use libxkbcommon in Xorg

[Tiago Vignatti]

Hi Kristian.

I tried the library. First impression is good - it works :) I had some
comments bellow though.

On Fri, Oct 22, 2010 at 04:26:38AM +0200, ext Kristian H�gsberg wrote:
> 
> At this point libxkbcommon only depends on xproto (for keycodes and a few
> other defines) and kbproto. 

it seems that "only" here is a lot actually. I mean, I thought the real
motivation of the library was to get rid of X11 dependency, so one could use
for instance with Wayland. Otherwise, it seems that we would be just moving
code around. So, the plan next is to keep digging towards the X11 removal or
what?

BTW, did you tried already do glue the library with Wayland already?


> I added a flex generated lexer instead of the handcoded one in xkbcomp -
> sadly this didn't give much of a performance boost.  It did make it easy to
> make the compiler read from a string, which avoids messing with pipes/temp
> files.
> 
> The Xorg side of the work is over here:
> 
>   http://cgit.freedesktop.org/~krh/xserver/log/?h=xkbcommon

BTW, I needed this:
http://people.freedesktop.org/~vignatti/tmp/0001-xkb-fix-missing-xkbcommon-conversion.patch
 
I had also some different warnings on Xorg log, that using xkbcomp I didn't
have:
Internal error:   Could not resolve keysym SunProps
Internal error:   Could not resolve keysym SunFront
Internal error:   Could not resolve keysym SunOpen


> and the diffstat for the changes look pretty good:
> 
>   [krh at hinata xserver]$ git diff --shortstat origin/master HEAD
>    66 files changed, 1554 insertions(+), 5845 deletions(-)

again, this seems a false illusion - if we're still hook up with X prototypes
then it's just moving things around.

 
> Now, it's all looking pretty promising, but there are a few open
> issues I'd like to throw to the list.  First of all, there's the issue
> of how this intersects/dovetails/conflicts with xkb2.  Part of the
> original plan for libxkbcommon as part of xkb2 was to also increase
> the size of certain datatypes in xkb: bump keycodes and virtual mods
> to 32 bits.  We can't do that with the existing xkb protocol and will
> require xkb2 protocol with new requests to serialize the xkb
> descriptions and we need to rethink some of the ways we represent
> certain state (per keycode repeat info makes a pretty big bit vector
> for 32 bit keycodes).  I don't think that's realistic for a 1.10
> xserver release, and I've changed the libxkbcommon structs back to be
> compatible with the current xkb protocol and implementation.  We can
> extend libxkbcommon to support bigger keycodes and vmods in a later
> release.  It's not going to be as pretty as if we did it all in the
> first release, but I think there's too much potential here to block it
> on xkb2.

as the guys already commented on, I'd not expect xkb2 (whatever it will be) to
release the library.

 
> The remaining issues are smaller and easier to sort out;
> 
> 1) the libxkbcommon API still needs review cleaning up from an
> namespace point of view.
> 
> 2) I'm considering whether to include a copy of the Xorg keysym under
> a XKB_KEY_* namespace to break the dependency on xproto?

I'm in favour, exactly. That what I meant before! :) Then we should get rid of
kbproto.h also, right?

BTW, what are you using setxkbmap to change the settings or what? setxkbmap
seems to be very self-contained, not using much X stuff.


> 3) Right now libxkbcommon is essentially just an xkb keymap parser
> library, should we include the xkb state machine logic too (basically
> the action processing from xkbAction.c and the keysym lookup logic
> from libX11)? Any user of libxkbcommon (xcb, wayland, clutter with
> evdev input on kms etc) will have to implement this logic...
> 
> 4) One of the big chunks of code left in the xkb modules deals with
> writing out an xkb keymap based on a struct xkb_desc and a few changes
> only to then parse it back into a new struct xkb_desc
> (XkbWriteXKBKeymapForNames followed by xkb_compile_keymap_from_string
> in XkbDDXLoadKeymapByNames).  If we moved this to libxkbcommon, we
> could avoid the dump+parse step and just create the struct xkb_desc
> directly... not sure if it's worth it.
> 
> and I'm sure there's more.  Like, exposing an API that's 90% hairy
> structs is recipe for disaster, but it's kinda late to change that.
> Anyway, looking forward to some feedback.

Thanks!

             Tiago