Hi, On 28 November 2014 at 06:09, Peter Hutterer <[email protected]> wrote:
> On Tue, Nov 25, 2014 at 09:30:59PM +0000, Daniel Stone wrote: > > On 25 November 2014 at 02:38, Keith Packard <[email protected]> wrote: > > > 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? Sure, but the same is true of the suggested global state. Forget to check it and you've got a bug. > > > 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. > So to be clear, it would be ET_KeyPress, but DeviceEvent grows an extra field that tells you that really it should be suppressed? I don't think that makes too much difference from above (again, if you miss a check, you're sending something out incorrectly; it also makes it less easy to tell at a glance what's going on), but if that's what you'd prefer, then fair enough. Cheers, Daniel > 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
