Subject: Re: bug#57012: Activating versus raising frames

From: Po Lu <[email-masked]>
Date: Sun, 07 Aug 2022 10:45:15 +0800

Daniel Colascione <[email-masked]> writes:

> The GDK code specifically mentions that programs that handle events
> themselves (like Emacs) need to explicitly update the event time (as
> my patch does)

The GDK documentation is unclear.  You only have to update the event
time if the event is not passed to GDK, by setting *finish to
X_EVENT_DROP, which really only happens with key press events.

> What is the bug?

Client messages sent to x-dnd.el did not automatically update the user
time, causing various selection-related functions to use an outdated
timestamp.

> Sorry, but I strongly disagree. The concept of signaling to the
> underlying window system that the user has interacted in some manner
> with a frame is generic and not X-specific. In fact --- doesn't the
> pgtk backend need an implementation of this hook too? It, like the
> conventional GTK backend, is blind to interactions with the frame
> performed using emacsclient.

No, the PGTK backend doesn't have a concept of "server time".  The GDK
Wayland backend implements them via event serials, which cannot be
generated.  It is also unnecessary to specify the server time when
trying to activate a toplevel window.

The only window system I know of that requires that to be specified is
X, so let's keep the code specific to X.

> I think server_time_monotonic_p is an unnecessary optimization.

It's used during frame synchronization, which naturally requires highly
accurate views of the server time.  Further more, we try to _reduce_ the
amount of calls to XSync, which leads to slow performance over display
connections with high latency.

> The style is fine, thanks.

No, it's not:

> +static bool
> +x_get_server_time (struct frame* f, Time* time)

should be

> +x_get_server_time (struct frame* f, Time *time)

and

> +  if (FRAME_LIVE_P (f)) {
> +    Time server_time;
> +    if (!x_get_server_time (f, &server_time))
> +      error ("Timed out waiting for server timestamp");
> +    x_display_set_last_user_time_1 (
> +      FRAME_DISPLAY_INFO (f), server_time, false, f);
> +  }

Should have the opening braces on a new line.

Thanks.