On Tue, Nov 25, 2014 at 09:30:59PM +0000, Daniel Stone wrote: > Hi, > > On 25 November 2014 at 02:38, Keith Packard <[email protected]> wrote: > > > Daniel Stone <[email protected]> writes: > > > Because that's still a bunch of work, > > > > I think it's just: > > > > - wl_array_for_each(k, &xwl_seat->keys) > > - QueueKeyboardEvents(xwl_seat->keyboard, KeyPress, *k + 8, &mask); > > + wl_array_for_each(k, &xwl_seat->keys) { > > + if (xwl_seat->keyboard->key->xkbInfo->desc->map->modmap[*k+8] != > > 0) > > + QueueKeyboardEvents(xwl_seat->keyboard, KeyPress, *k + 8, > > &mask); > > + } > > } > > > > Is it? You have to make sure to the focus is NULL at this point, otherwise > you send surprise events to windows that don't want it. And if the focus is > NULL, it has pretty much the same effect as my patch; it's just that it's > stripped of context when you're trying to debug things at lower levels. > > Think of the case where you enter with Alt down. If the user presses F, you > want the file menu to trigger, but when the user releases Alt without > pressing anything, you don't want to activate the menubar. > > I can also see the case for passing all keys along on enter: after all, > that's exactly what KeymapNotify does. If you think this is fine, would you > also accept a patch to do this on every focus change inside normal X11? And > if not, why not ... ? > > > > > This patch isn't the end of the road; in the end, it should be > > > entirely possible to achieve perfect nesting wrt KeymapNotify events > > > with XWayland, Xephyr, XQuartz, and any combination thereof. This is a > > > relatively start in that direction. > > > > Yeah, I'm fine with figuring out how to make it perfect; if the above > > patch will suffice to get modifiers handled correctly for now, I'd like > > to go with that for this release and pend the perfect solution until > > after 1.17; just too many changes outside of hw/xwayland to make me > > happy at this point in the release cycle. > > > > I think they're all pretty harmless as they're confined to new codepaths > which were not previously triggerable, and are still not triggerable > outside of Xwayland.
that's the theory :) if you forgot to add one check somewhere, you may now get weird events. I can't seem to find a 1.17 release schedule - Keith, where're we at? > > In particular, I can imagine passing a new boolean in, or setting some > > device state, that muted event generation and avoided having to check > > for ET_KeyFocusIn in a bunch of places now and in the future. That would > > also encourage us to actually deal with this case in Xephyr and Xnest. > > > > How is that better? You've shifted some local state (what's the event > type?) into global state (is this thing set?). The penalties for missing a > check are exactly the same for both, except the intention is much more > obvious with an explicit event. The event type also has the benefit of > being able to be posted through the queue, rather than OsBlockSignals -> > ProcessInputEvents -> mangle magic global flag -> QueueKeyboardEvents -> > ProcessKeyboardEvents -> OsUnblockSignals. I'd much rather take the hit of > tiny extra complexity in the core (really, it's not much at all) rather > than having all the users get it wrong, or simply not bother because it's > too hard. Much like they do today. You could add that state to the internal event, add a fake_event field to the DeviceEvent and check that where necessary, otherwise let it pass through as normal event. That way the penalty for forgetting a check only matters where you actually care about it. That should also reduce the risk of getting odd events. Cheers, Peter > If you're implacably opposed, then I'll ack the minimal change (once we can > be certain that the focus is indeed NULL whilst posting those events) for > 1.17, but I'll also request a revert immediately after master reopens, and > resubmit this patchset. Your patch does just happen to work, but it's an > extra whack of magic bonghits right where we least need them. > > Cheers, > Daniel _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
