[Xcb] Re: very first attempt of the keybinding
Vincent.Torri at iecn.u-nancy.fr
Thu May 5 23:03:11 PDT 2005
thank you for your answer. I'm doing the changes.
One question, though:
in atoms.c, there's following code in InternAtomFast:
if((cookie.u.atom = InternAtomPredefined(name_len, name)).xid !=
cookie.tag = TAG_VALUE;
I don't understand why you check cookie.u.atom, as you've just declared
cookie. Its values should be random, shouldn't they ?
On Thu, 5 May 2005, Jamey Sharp wrote:
> On Thu, 2005-05-05 at 10:07 +0200, Vincent Torri wrote:
> > Hello,
> Hi Vincent!
> > i've modified the code. At least it compiles fine. I've not ested it yet.
> > What do you think of it ? is the design good enough ?
> Sorry I never commented on your first draft. Looking at this one I've
> remembered that I read over that one too, but forgot to write anything
> down... Oops.
> I think it looks good generally. But there's at least one optimization
> that this design won't allow. The MappingNotify event may tell you that
> only a subset of the keys have changed, and it'd be nice to only force a
> round-trip to look up the new mapping when one of those keys are
> actually pressed. (I don't think this is an important optimization,
> since I think keyboard mappings change very rarely, but it'd be nice to
> not disallow it.)
> To allow future optimizations, I'd be inclined to use a single opaque
> XCBKeySymbols structure to track all per-display state. The simple
> version of that struct would hold a pointer to the XCBConnection that
> it's associated with, plus a tagged union of a GetKeyboardMapping cookie
> and reply. On initialization of this structure the cookie would be
> filled in with a query for the whole mapping; whenever a lookup is
> needed the cookie would be converted to a reply; and on MappingNotify
> events the reply would be discarded and the cookie reinitialized with a
> new query.
> There's an example of the tagged union model I'm proposing in xcb-atom:
> see InternAtomFast and InternAtomFastReply. Similar code also appears in
> xcb-wm, in manage.c. The reasons for using tagged unions are a little
> different in those cases but the code would be roughly the same.
> There are a couple little things I want to comment on in the code, too.
> > /* FIXME: is there an interest to use the iter system
> > compared to that loop */
> > for (j = 0; j < rep->keysyms_per_keycode; j++)
> No, iterators don't buy you anything here. They're only provided on
> arrays for consistency, so programmers (and XCB's code generator) don't
> have to remember whether a particular XCB type has an iterator or not.
> In XCBKeySymbolsReply:
> > ...
> > kc_to_ks->keysyms = XCBGetKeyboardMappingKeysyms (km_rep);
> > }
> > free (km_rep);
> In XCBRefreshKeyboardMapping:
> > free (rep->keysyms);
> You can't make either of those calls to free: all parts of an XCB reply
> are in one malloc'd chunk, so you can't free just one part.
> XCBGetKeyboardMappingKeysyms just returns a pointer into the reply. But
> if you just store the reply itself, as I'm suggesting with the tagged
> union, you won't have this problem.
> Hope that helps. I'm glad you're patient: you keep writing good code
> even when it takes me a while to give you feedback. :-)
> xcb mailing list
> xcb at lists.freedesktop.org
More information about the xcb