[PATCH weston] input: don't send to clients key events eaten by bindings

[Daniel Stone]

Hi,

On 21 November 2014 21:11, Bill Spitzak <spitzak at gmail.com
<javascript:_e(%7B%7D,'cvml','spitzak at gmail.com');>> wrote:

> On 11/21/2014 10:59 AM, Daniel Stone wrote:
>
>> Which you respond to by ceasing _all_ actions related to your current
>> keyboard state, such as cancelling key repeat. So, also not useless.
>>
>
> You are right, it is the "stop repeating keys" event. I didn't think of
> that. Depending on exactly what keystrokes the server is consuming, it will
> have to send this, and right away, rather than deferring it to immediately
> before the enter event, as I was thinking of.
>
> I personally feel it is a mistake to have the client do repeat rather than
> the compositor, because complexities of timing will not be identical
> between clients, and because it makes it really difficult to do remote
> display on a system that does the repeat itself. However this event would
> still be necessary to cancel "key held down for this long" timeouts.


I don't see how it makes it difficult to do remote display, and also
wl_keyboard does send the repeat rate and delay through. No reason to wake
the compositor up when the client could equally do it on its own.

Either way, it's a done deal now: for better or worse, that's how
wl_keyboard works. It's what we promise clients, so we can't be changing it
from under them.


>  The way you detect repeat in _core_ X11, is by a stream of paired
>> KeyPress and KeyRelease events. Look it up. Clients rely on this, and
>> conformance relies on it too. I don't know if you've heard of an
>> extension called the X Keyboard Extension (obscure name, I know), but
>> since its inclusion in X11R6.1 in 1996, the XkbSetDetectableAutorepeat
>> function has allowed you to request key repeat be sent as a stream of
>> press-press-press-press-release events, rather than
>> press-release-press-release-press-release. For bonus points, it's even
>> part of the core libX11.so.6 API/ABI. And it works perfectly, and every
>> real toolkit I've seen has made good use of it.
>>
>
> The "old" keyboard api (I think this is what you meant by core) in X sent
> repeated press events. This was certainly true for IRIX and I believe it
> was true for the first versions on Linux, because the errors would be
> obvious if it was different. Forms, XForms, FLTK, Tk and whatever Amazon
> paint used assumed it worked this way. Prisms and Alias Wavefront did too
> but they fixed them pretty quick.
>

Sorry, but this isn't true. If it was, XkbSetDetectableAutorepeat would
literally have no reason at all to exist. Its sole reason is to change the
repeat behaviour of the server from press-release-press-release to
press-press-release. See section 4.1.2 of the XKB spec.


> Also it looks like this function is allowed to fail and return false. This
> makes it pretty useless as the look-ahead solution has to be implemented
> anyway as a fallback. Due to the need to minimize code paths to reduce
> chances of bugs I certainly would not have used this call at all and used
> the fallback always. But I also suspect this is documentation over-kill and
> it actually always works: it should not have been defined as returning a
> failure code. This sort of mistake is what leads to misuse on non-use of
> documented API.
>

Yes, if the XKB extension isn't supported, then you have to cope with the
lack of that. Again, press-release-press-release predates XKB, to the core
protocol.


> There are also indications that various bugs make it return true but not
> work anyway: https://bugs.freedesktop.org/show_bug.cgi?id=22515.
> Encountering such a bug would also certainly have stopped us using the call.


I fixed that bug within literally two weeks of being filed! That's like
saying that you can't rely on Wayland to ever work because one version of
Weston had a segfault once (ho ho).

Anyway I'm sorry about being mad, but the experience certainly did piss me
> off at that time, enough that I am still pretty upset about it 20 years
> later. I had (and still have) great hopes for Wayland, but I am disturbed
> to see some of the same mistakes from then being repeated.
>

I agree with you that enter/leave could be improved here, with a maybe-NULL
surface pointer which would be set if focus was transferring to another
surface from the same client. Like I said, that's actually been somewhere
on my TODO for a while, pending some time to get back to Wayland work.

I don't think this thread is the place to discuss that though; it's more
than run its course. So, rather than muddy the waters any more, how about
we take it to a separate thread if we want to carry this on - but ideally
one with some patches behind it too.

Let's call this EOT for now and move on with our lives. It's Friday night,
after all.

Cheers,
Daniel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20141121/78c45daa/attachment.html>