Skip to content

GTK/Cocoa: give synthetic events a fresh timestamp instead of getLast… #3559

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
fedejeanne wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

GTK/Cocoa: give synthetic events a fresh timestamp instead of getLast… #3559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/widgets/Display.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4636,7 +4636,9 @@ void sendEvent (int eventType, Event event) {
if (event == null) event = new Event ();
event.display = this;
event.type = eventType;
if (event.time == 0) event.time = getLastEventTime ();
// See comment in Widget.sendEvent(int, Event, boolean) for why
// getLastEventTime() is not used as a fallback here.
if (event.time == 0) event.time = (int) (System.nanoTime() / 1_000_000L);

@fedejeanne fedejeanne Sep 3, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This millisecond-resolution clock can return the same value for consecutive synthetic display events, so it does not provide the fresh/distinct fallback described by the referenced Widget.sendEvent rationale.

Agreed, but the chances are very slim that 2 back-to-back events manage to get created and sent in the same millisecond and if they do then the deduplication logic should still notice that they are not the same event (event1 == event2 yields false), they were merely sent at the same time.

Route this through the same per-Display advancing synthetic timestamp allocator rather than reading and truncating the clock independently.

I don't understand that: do you mean the same logic that is being altered by this same PR and contain the exact same fix (event.time = (int) (System.nanoTime() / 1_000_000L);)?

sendEvent (eventTable, event);
}

Expand Down
12 changes: 11 additions & 1 deletion bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/widgets/Widget.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1674,7 +1674,17 @@ void sendEvent (int eventType, Event event, boolean send) {
event.display = display;
event.widget = this;
if (event.time == 0) {
event.time = display.getLastEventTime ();
/*
* display.getLastEventTime() only reflects the time of the last *native*
* NSEvent that was processed; it is not updated by synthetic events
* dispatched directly (e.g. notifyListeners()) without going through the
* native event queue. Relying on it here would make consecutive
* synthetic events (as commonly created by tests) carry the same, stale
* time, which can incorrectly look like a repeated/duplicate event to
* code (e.g. KeyBindingDispatcher) that de-duplicates by event time.
* Use a distinct, always-advancing timestamp instead.
*/
event.time = (int) (System.nanoTime() / 1_000_000L);
Comment thread
fedejeanne marked this conversation as resolved.
}
if (send) {
sendEvent (event);
Expand Down
4 changes: 3 additions & 1 deletion bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5875,7 +5875,9 @@ void sendEvent (int eventType, Event event) {
if (event == null) event = new Event ();
event.display = this;
event.type = eventType;
if (event.time == 0) event.time = getLastEventTime ();
// See comment in Widget.sendEvent(int, Event, boolean) for why
// getLastEventTime() is not used as a fallback here.
if (event.time == 0) event.time = (int) (System.nanoTime() / 1_000_000L);
Comment thread
fedejeanne marked this conversation as resolved.
if (!filterEvent (event)) {
if (eventTable != null) sendEvent (eventTable, event);
}
Expand Down
13 changes: 12 additions & 1 deletion bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Widget.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1679,7 +1679,18 @@ void sendEvent (int eventType, Event event, boolean send) {
event.display = display;
event.widget = this;
if (event.time == 0) {
event.time = display.getLastEventTime ();
/*
* display.getLastEventTime() only reflects the time of the last *native*
* GDK event that was processed; it is not updated by synthetic events
* dispatched directly (e.g. notifyListeners()) without going through the
* native event queue. Relying on it here would make consecutive
* synthetic events (as commonly created by tests) carry the same, stale
* time, which can incorrectly look like a repeated/duplicate event to
* code (e.g. KeyBindingDispatcher) that de-duplicates by event time.
* Use a distinct, always-advancing timestamp instead, consistent with
* how other synthetic events (see sendJDKInternalEvent) are timestamped.
*/
event.time = (int) (System.nanoTime() / 1_000_000L);
Comment thread
fedejeanne marked this conversation as resolved.

@akurtakov akurtakov Sep 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So 2 synthetic events fired one after the other will still share the same millisecond (quite likely for a test execution) and de-duplication will not happen , what am I missing?
Also mixing Java and Gtk (X11 and Wayland have different too) clock is something that could create a problem especially when adding remote sessions in the loop too.

@fedejeanne fedejeanne Sep 3, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So 2 synthetic events fired one after the other will still share the same millisecond (quite likely for a test execution) and de-duplication will not happen , what am I missing?

Same as above: the deduplication logic still compares event1 == event2 and that check returns false.

Also mixing Java and Gtk (X11 and Wayland have different too) clock is something that could create a problem especially when adding remote sessions in the loop too.

Can you please provide an example? I don't understand it.

The way I see it, the current code in all 4 (actually, all 6) implementations of sendEvent(...) methods in Widget and Display decides to assign the same timestamp as the last real (OS) event to all synthetic events, effectively saying "_I decided that this event you created and triggered by hand without explicitly assigning a timestamp to it (because event.time == 0) _ will have the same timestamp as the last real event the OS triggered". That sounds odd because the synthetic event may not even be related to the last real (OS) event, they could be about 2 totally different things e.g. moving the mouse vs pressing a key. I don't get why one would decide to have them both "happen at the same time" when they might not be related at all.

@akurtakov akurtakov Sep 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://xorg.freedesktop.org/archive/current/doc/xproto/x11protocol.html#glossary: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)." So these timestamps are with different base and using Java timestamp will put some value in the far, far future.

}
if (send) {
sendEvent (event);
Expand Down
Loading