Created attachment 246950 [details]
test case, based on tests/motion-compression.c

Created attachment 246951 [details]
result of motion-snake.c test (old and new gtk version)

Idea: disable motion compression for applications/widgets which enable extended input events?

Jon, set_extension_events() is gone in GTK3. Pressure simply works :-)

I'm geting more and more convinced that this new "motion compression" has exactly the same goal as the GDK_POINTER_MOTION_HINT. From the docu:

"GDK_POINTER_MOTION_HINT_MASK is a special mask which is used to reduce the number of GDK_MOTION_NOTIFY events received. Normally a GDK_MOTION_NOTIFY event is received each time the mouse moves. However, if the application spends a lot of time processing the event [...]"

This is unfortunately absolutely broken for paint apps. In GIMP, we do
motion compression ourselves for tools which do not need exact events,
but for painting, we can't have the toolkit compress motion events.

About gdk_device_get_history(), GIMP used that, but as mentioned by Martin
this doesn't help if the backend doesn't implement it.

Concerning gdk_device_get_history(), here is a description of the problems: http://www.gtkforums.com/viewtopic.php?p=195468&sid=f5ce1ae2eb4bdd45cc5b28e73e7dd962#p195468

One possible way forward is to remove confusion about where the history starts and stops, and make sure it works reliably. It would have to report at least the compressed events, independent of platform-specific code.

The get_history() approach has one advantage, it avoids the per-event overhead of propagating through all abstraction layers. (However, for a desktop PC and 100 motion events per second, the overhead is neglegible.)

GDK_POINTER_MOTION_HINT is not even worth discussing - people have spent 20 years trying to use it (and the underlying X pointer motion hints), misunderstanding how it worksk, getting pointer positions out of order, etc.

Compare:

 GDK_POINTER_MOTION_HINT - has to be enabled and handled in a specific (very confusing) by every single widget and application that handles motion events. Naively written applications lag. Involves round trips to the X server and is inefficient.

 GTK+ 3.8 motion compression - makes almost all motion handling code just work. Painting is the only exception. In no other case do you want more than one motion event per frame.

Adding new API to disable motion compression for a specific GdkWindow would be conceivably possible, but since we already have motion history API, and some apps are already calling it, making it work properly is the clear path to take. (I meant to check on how well motion history was working before releasing 3.8, but I forgot to do that.)

We could potentially write an implementation of gdk_device_get_history() that was purely based on discarded events and the local implementation of GDK_POINTER_MOTION_HINT_MASK and didn't touch the server at all. In theory X servers can provide more granularity for event history than is delivered in events, but other than PointerMotionHintMask (which GTK+ never uses any more and always emulates) I don't think that ever happens with Xorg.

My observations on this issue:

* If not ameliorated in GDK itself, this bug will basically kill paint programs which rely on it.

* Motion history is an obscure corner of GDK, and is difficult to get working right for novice developers. The API is finicky. One must (apparently) shave off the first and last events to avoid duplicates (or is it just the possibility of duplicates?), and code must be written for axis mapping.

* In addition, support for device history appears to be lacking on all devices and platforms I've tried it with, including with fancy Wacom input drivers for X which push events at 200Hz. I have never been able to extract motion history using gdk_device_get_history(), and I believe I am using it quite correctly, on displays where XDisplayMotionBufferSize() returns > 0.

* Annoyingly, gdk_device_get_history() is no longer introspected. While MyPaint can just forward-port the PyGTK _wrap for it, other Python apps may not be able to do so as easily.

It would seem much cleaner to add new API for disabling motion compression for a specific GdkWindow rather than relying on obscure, old APIs with unclear semantics and patchy existing support.

My personal preference would be a simple GDK_*_HINT_MASK-style flag for disabling motion compression. Flag it with gdk_window_set_events() or gdk_window_add_events(), just like GDK_POINTER_MOTION_HINT_MASK but inverting its sense: make it a declaration that a program can cope with a higher rate of event delivery, rather than an admission that it can't or that it doesn't need to.

Discussion on #gtk+ was hinting at a possible solution of adding an api that returns all the events that have been compressed into a delivered event, something like gdk_event_get_compressed_events() or gdk_event_get_history()

I actually like

const GdkEvent ** gdk_event_decompress (GdkEvent *event, guint *n_events);

with code like this for the default case:

if (!event->allocated || event->compressed_events == NULL)
  {
    *n_events = 1;
    return &event;
  }

That way decompression never fails and is essentially idempotent. It also works for every event and with that for backends that don't support compression.

That's a much nicer API than the current device history.

But for painting I think coalescing will still pose a problem. Painting is CPU intensive and if we're only going to receive a package of events every 1/60s it's going to mean we're at least part of a frame behind trying to render things. If there's vsync that will easily push us back to a full frame of lag.

The perfect API from my perspective would be to have a setting to get events as soon as the operating system posts them, and then if events get backed up in the main loop coalesce them into something that your decompress function could explode.

Will that also return a touch or tablet device's history, or will we have
to do both? I also agree with Daniel that events should probably be delivered
in "real time" if the device is not in compression mode.

I'd like to point out that the compression is enabled only for MOTION events; TOUCH_UPDATE events are not compressed.

(In reply to comment #14)
> I'd like to point out that the compression is enabled only for MOTION events;
> TOUCH_UPDATE events are not compressed.

Is there a reason for this?

Tablet events are not touch events, my gut says this should be handled
uniformly tho.

Created attachment 258492 [details] [review]
Add an event_compression setting to GdkWindow

Because compression is already done per-window the GdkWindow seems like the most logical place to toggle this. Applications can set the value to false for drawing canvas type widgets without impacting anything else.

(In reply to comment #15)
> (In reply to comment #14)
> > I'd like to point out that the compression is enabled only for MOTION events;
> > TOUCH_UPDATE events are not compressed.
> 
> Is there a reason for this?

TOUCH_UPDATE is equivalent to a move under drag, rather than a plain no-button motion event.  The only three cases I can think of for handling touch update events are gesture recognition (often defeated by motion compression), drawing (ditto), and drag and drop.

I don't really have much intelligent comment on the patch, not knowing GTK+ internals, but it seems like the right thing to do, to me.

That patch almost looks too simple to be true, but correct :)

Review of attachment 258492 [details] [review]:

The documentation needs some more work - I would expect it to state when this might be considered useful (for drawing applications). Our current frame clock documentation is not very detailed about motion compression - that could be improved at the same as adding a note there that motion compression can be turned off using this new api.

Comment on attachment 258492 [details] [review]
Add an event_compression setting to GdkWindow

Yeah, that's the patch we agreed on in IRC. It's not ideal for forwards compat (where we want to move into a windowless world), but it's certainly a nice approach for backwards compatibility. And since we're kind of fixing a regression here, this is probably even a better approach.

Some details about the patch:

> + * Determines whether or not extra unprocessed motion events in
> + * the event queue can be discarded. If %TRUE only the most recent
> + * event will be delivered.
>
That blob should say that the default is %TRUE and events will be compressed.
And as Matthias says, giving an example for when this is useful to turn off compression sounds like a good idea.

> +{
> +  window->event_compression = event_compression;
> +}
>
Needs g_return_if_fail(GDK_IS_WINDOW (window));
Same for get_event_compression().

I like this patch, an absolute KISS solution.

Related to this, can we mark both GDK_POINTER_MOTION_HINT and gdk_device_get_history() as depreciated? From this discussion my impression is that they can not or should not be used currently. And because they are difficult to understand, maybe application developers should not waste time trying?

(In reply to comment #22)
> I like this patch, an absolute KISS solution.
> 
> Related to this, can we mark both GDK_POINTER_MOTION_HINT and
> gdk_device_get_history() as depreciated? From this discussion my impression is
> that they can not or should not be used currently. And because they are
> difficult to understand, maybe application developers should not waste time
> trying?

Yes, we should do that.

Attachment 258492 [details] pushed as 80dd1a8 - Add an event_compression setting to GdkWindow

It's good to see that something has been landed. Just for reference (and in case this needs to be revisited in the future), here's why I would have instead gone with actually implementing a compression history

 * Window-based event delivery and event masks are something that we want to move away from. We don't always want GTK+ programs to be piles of input-only windows.

 * With the setting, once you want accurate history, you have to restructure your program to call queue_redraw (queue_resize) and then do the heavy lifting in the draw handler - you can't simply *do* your work in the event handler. This makes teaching people how to to properly do event handlers harder - because there are *two* ways to do it. Rather than one way, and if you want the accurate history, you make a function call to get the history.

 * Having a per-window setting means that if there are multiple consumers of events from a single window, they have to be in agreement. E.g., it's problematical to call this function on the window of an existing widget and then try to connect to ::motion-event because the existing widget may start lagging and misbehaving.

Right, I agree with those things, so lets keep this bug open for adding device history

Event history you mean, not device history, right?

I'd still like something like in comment 11 to land.

But as I said in comment 21, the current approach is best for backwards compat but not perfect for forwards compat. And we can add the new approach and just deprecate this one whenever we want to.

Created attachment 259591 [details] [review]
proposed update of documentation

The patch adds pointers to gdk_window_set_event_compression() in the relevant docstrings. It advises against using MOTION_HINT, but doesn't formally deprecate any function. Feel free to edit. After grepping the source I decided that I don't understand if MOTION_HINT is still useful for something or not (same for the X11 event history).

Also, I've added code to MyPaint git, so it will use gdk_window_set_event_compression() if available, instead of the current gdk_window_add_filter() hack using XIDeviceEvent directly. I can confirm that the new API works for us.

(In reply to comment #28)
> I can confirm that the new API works for us.

Thanks, great to hear that.

Could we have the switch to disable motion compression in 3.8.x? It would solve the freeze introduced probably by motion compression, https://bugzilla.gnome.org/show_bug.cgi?id=719883.

we don't do api additions in stable branches, so this will be a 3.10-only fix, I'm afraid.

I understand, but it may be also solved without adding a new api. Please tell me whether I made a mistake in the following reasoning:

1. Problems are detected when motion event compression starts working.
2. To counteract the problems, a switch is introduced, to make it possible to disable this optimization.

I think those problems should also be addressed in stable branches. How about a text parameter like if (getenv("GDK_DISABLE_MOTION_COMPRESSION"))? That would be portable and would make gtk 3.8 usable. For some applications it's not usable at the moment.

I hope someone will discuss the problem that I mentioned, bug 719883. Possibly Owen. Maybe there are other solutions to that problem. If I am right and this bug is indeed a bug, it should be fixed, in stable branch too. If you want me to provide a proof for that bug, I'll give it a try.

We're moving to gitlab! As part of this move, we are moving bugs to NEEDINFO if they haven't seen activity in more than a year. If this issue is still important to you and still relevant with GTK+ 3.22 or master, please reopen it and we will migrate it to gitlab.

On IRC, from Carlos:
"
the motion compression off switch was eventually added. and the ideas about uncompressing received events into more detailed history is implemented in master. I think that can be closed
"