code review 2114042: exp/draw: unify a draw.Context's keyboard, mouse, etc. ...

exp/draw: unify a draw.Context's keyboard, mouse, etc. channels into a
single event channel.

A quit event is now represented by closing that channel.

[nigeltao, 2010-09-05 04:25:13]

Hello r, rsc (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[nigeltao_gnome, 2010-09-05 04:30:39]

The NaCl code under exp will be fixed in a separate change. GOOS=nacl
doesn't even compile at the moment.

[r, 2010-09-05 09:59:58]

We talked about this a little bit did we reach a conclusion?  I'm not
convinced yet.

-rob

[nigeltao_gnome, 2010-09-05 12:09:39]

On 5 September 2010 19:59, Rob 'Commander' Pike <r@golang.org> wrote:
> We talked about this a little bit did we reach a conclusion?  I'm not
> convinced yet.

I'm not sure if we reached a conclusion but my impression was that you
weren't opposed to the change.

The motivation comes from a previous discussion on golang-nuts:
http://groups.google.com/group/golang-nuts/browse_thread/thread/f5f8752056960...

[r, 2010-09-06 01:57:35]

LGTM with the understanding we might do something totally different later.

[nigeltao, 2010-09-06 09:22:44]

*** Submitted as http://code.google.com/p/go/source/detail?r=3dd6ee3ef9e9 ***

exp/draw: unify a draw.Context's keyboard, mouse, etc. channels into a
single event channel.

A quit event is now represented by closing that channel.

R=r, rsc, nigeltao
CC=golang-dev
http://codereview.appspot.com/2114042

[rog, 2010-09-07 08:38:01]

what happened to the mouse event's timestamp?

On 5 September 2010 05:25,  <nigeltao@golang.org> wrote:
> Reviewers: r, rsc,
>
> Message:
> Hello r, rsc (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change.
>
>
> Description:
> exp/draw: unify a draw.Context's keyboard, mouse, etc. channels into a
> single event channel.
>
> A quit event is now represented by closing that channel.
>
> Please review this at http://codereview.appspot.com/2114042/
>
> Affected files:
>  M src/pkg/exp/draw/event.go
>  M src/pkg/exp/draw/x11/conn.go
>
>
>

[nigeltao_gnome, 2010-09-07 09:15:23]

On 7 September 2010 18:37, roger peppe <rogpeppe@gmail.com> wrote:
> what happened to the mouse event's timestamp?

I removed it. The X11 backend never filled it in, so I presumed that
nobody used it.

This is still experimental; a future change could bring timestamps
back. If so, I think an unresolved issue is whether all events should
have timestamps, not just mouse events.

But for now, the simplest thing is to just not have them at all.

[rog, 2010-09-07 09:29:46]

i have a version that does fill them in (and code
that relies on that). of course, that is using a fork
of the code, but it would be nice to be able to merge
eventually.

they are useful, as without them a delay
in processing events from the channel
(e.g. when drawing complex objects in response
to mouse events) can mean that a user's
actions are interpreted incorrectly.

given that events are now unified, i think
it's reasonable that they should all contain a timestamp.

On 7 September 2010 10:15, Nigel Tao <nigel.tao.gnome@gmail.com> wrote:
> On 7 September 2010 18:37, roger peppe <rogpeppe@gmail.com> wrote:
>> what happened to the mouse event's timestamp?
>
> I removed it. The X11 backend never filled it in, so I presumed that
> nobody used it.
>
> This is still experimental; a future change could bring timestamps
> back. If so, I think an unresolved issue is whether all events should
> have timestamps, not just mouse events.
>
> But for now, the simplest thing is to just not have them at all.
>

[rsc, 2010-09-07 11:51:42]

please keep mouse time stamps.
they are the only reliable way to detect
a double-click.  using the time that
the event is received later on fails badly
under load.

russ

[nigeltao_gnome, 2010-09-07 12:53:22]

On 7 September 2010 21:51, Russ Cox <rsc@golang.org> wrote:
> please keep mouse time stamps.
> they are the only reliable way to detect
> a double-click.  using the time that
> the event is received later on fails badly
> under load.

OK. X11 gives a uint32 timestamp for some (but not all) events, but
http://www.x.org/docs/XProtocol/proto.pdf says that it wraps around,
so I'll just use time.Nanoseconds().

"Timestamp: A timestamp is a time value, expressed in milliseconds. It
typically is the time since the last server reset. Timestamp values
wrap around (after about 49.7 days). The server, given its current
time is represented by timestamp T, always interprets timestamps from
clients by treating half of the timestamp space as being earlier in
time than T and half of the timestamp space as being later in time
than T. One timestamp value (named CurrentTime) is never generated by
the server. This value is reserved for use in requests to represent
the current server time."

[rog, 2010-09-07 13:08:16]

On 7 September 2010 13:53, Nigel Tao <nigel.tao.gnome@gmail.com> wrote:
> On 7 September 2010 21:51, Russ Cox <rsc@golang.org> wrote:
>> please keep mouse time stamps.
>> they are the only reliable way to detect
>> a double-click.  using the time that
>> the event is received later on fails badly
>> under load.
>
> OK. X11 gives a uint32 timestamp for some (but not all) events, but
> http://www.x.org/docs/XProtocol/proto.pdf says that it wraps around,
> so I'll just use time.Nanoseconds().

when i did the implementation for this, i used time.Nanoseconds()
for the timestamp of the first mouse event (saving the uint32
timestamp of that event); for any subsequent event e, i used
(e.Timestamp - originalEvent.Timestamp) * 1e6 + originalNanoseconds.

this meant that at least time deltas were millisecond accurate regardless
of network transmission time, although the absolute times will be
subject to a constant offset according to the delay on the first
timestamp. i'm not sure it's possible to do better than that without
estimating network delay.