After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 752752 - Patches needed for relative pointer and pointer confinement in mutter
Patches needed for relative pointer and pointer confinement in mutter
Status: RESOLVED FIXED
Product: clutter
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: clutter-maint
clutter-maint
Depends on:
Blocks:
 
 
Reported: 2015-07-23 02:44 UTC by Jonas Ådahl
Modified: 2016-02-16 11:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
evdev: Drop redundant stage set check (1.21 KB, patch)
2015-07-23 02:44 UTC, Jonas Ådahl
committed Details | Review
evdev: Expose the original relative motion deltas from events (9.64 KB, patch)
2015-07-23 02:44 UTC, Jonas Ådahl
accepted-commit_now Details | Review
ClutterActor: Add an inverse of the relative_transform_to_point function (4.35 KB, patch)
2015-07-23 02:44 UTC, Jonas Ådahl
rejected Details | Review
evdev: Pass a motion delta to pointer constrain callback (2.05 KB, patch)
2015-07-23 02:44 UTC, Jonas Ådahl
none Details | Review
evdev: Provide the relative pointer motion event deltas (8.28 KB, patch)
2015-12-03 07:46 UTC, Jonas Ådahl
committed Details | Review
evdev: Use microsecond granularity for internal timestamps (24.40 KB, patch)
2015-12-03 07:46 UTC, Jonas Ådahl
committed Details | Review
evdev: Expose microsecond timestamps via the clutter evdev API (6.04 KB, patch)
2015-12-03 07:46 UTC, Jonas Ådahl
committed Details | Review
evdev: Pass a motion delta to pointer constrain callback (2.78 KB, patch)
2015-12-03 07:46 UTC, Jonas Ådahl
committed Details | Review

Description Jonas Ådahl 2015-07-23 02:44:08 UTC
One patch introduces a new API that doesn't round input coordinates to int when transforming from stage point to actor point. It is simply the mathematical inverse of the function clutter_actor_apply_relative_transform_to_point().

One patch changes the constrain callback API used by mutter to contain a motion delta instead of a position. This is needed because mutter cannot by itself find this delta.
Comment 1 Jonas Ådahl 2015-07-23 02:44:12 UTC
Created attachment 307950 [details] [review]
evdev: Drop redundant stage set check

Every path creating a input device in the evdev backend sets a stage,
so the check is unnecessary.
Comment 2 Jonas Ådahl 2015-07-23 02:44:18 UTC
Created attachment 307951 [details] [review]
evdev: Expose the original relative motion deltas from events

Provide a backend specific API that enables the application to fetch
the original motion deltas, including both the accelerated (dx, dy) and
the unaccelerated (dx, dy) motions.
Comment 3 Jonas Ådahl 2015-07-23 02:44:23 UTC
Created attachment 307952 [details] [review]
ClutterActor: Add an inverse of the relative_transform_to_point function

clutter_actor_transform_stage_point() transforms a coordinate from the
stage coordinate space to the actor coordinate space, but does so with
some loss of accuracy because it casts the input coordinate to int.
The accuracy loss is often negigible, but sometimes, for example for
Wayland compositors, it can cause issues. Therefore provide API for
more accurate transformation from the stage coordinate space to actor
coordinate space by providing a helper that does the inverse of
clutter_actor_relative_transform_to_point().
Comment 4 Jonas Ådahl 2015-07-23 02:44:29 UTC
Created attachment 307953 [details] [review]
evdev: Pass a motion delta to pointer constrain callback

The constrain callback cannot rely on the pointer position of the
corresponding ClutterInputDevice to get the actual delta of the motion
event that is to be constrained since it is only updated when an event is
dispatched. So change the API to pass the previous pointer position when
constraining.
Comment 5 Emmanuele Bassi (:ebassi) 2015-07-23 11:36:21 UTC
Review of attachment 307952 [details] [review]:

I really don't like it.

The integer handling in transform_stage_point() is just an internal detail — or an historical accident.

I removed all the integer handling in transform_stage_point() in master, so this should not be needed at all. If the error is in the math, then we can drop most of it, and just use a proper inverse transformation.
Comment 6 Emmanuele Bassi (:ebassi) 2015-07-23 11:36:42 UTC
Review of attachment 307950 [details] [review]:

Okay.
Comment 7 Emmanuele Bassi (:ebassi) 2015-07-23 11:37:49 UTC
Review of attachment 307951 [details] [review]:

Okay.
Comment 8 Emmanuele Bassi (:ebassi) 2015-07-23 11:38:52 UTC
Review of attachment 307953 [details] [review]:

This will require a note in the README, under the 1.24 Release Notes.

Looks good.
Comment 9 Jonas Ådahl 2015-07-23 11:42:19 UTC
(In reply to Emmanuele Bassi (:ebassi) from comment #5)
> Review of attachment 307952 [details] [review] [review]:
> 
> I really don't like it.
> 
> The integer handling in transform_stage_point() is just an internal detail —
> or an historical accident.
> 
> I removed all the integer handling in transform_stage_point() in master, so
> this should not be needed at all. If the error is in the math, then we can
> drop most of it, and just use a proper inverse transformation.

Ah, I had missed you made that change. The branch was from a week or so older commit than that. I will try again with transform_stage_point().
Comment 10 Jonas Ådahl 2015-12-03 07:38:30 UTC
Comment on attachment 307950 [details] [review]
evdev: Drop redundant stage set check

Attachment 307950 [details] pushed as 9c26a98 - evdev: Drop redundant stage set check
Comment 11 Jonas Ådahl 2015-12-03 07:46:12 UTC
Created attachment 316694 [details] [review]
evdev: Provide the relative pointer motion event deltas

Compositors need more detailed information about motion events. Make it
possible to retrieve this information when running the evdev backend by
adding the information to the backend specific event struct.
Comment 12 Jonas Ådahl 2015-12-03 07:46:17 UTC
Created attachment 316695 [details] [review]
evdev: Use microsecond granularity for internal timestamps

It's what libinput uses internally, and it'll be exposed in a later
patch.
Comment 13 Jonas Ådahl 2015-12-03 07:46:22 UTC
Created attachment 316696 [details] [review]
evdev: Expose microsecond timestamps via the clutter evdev API

This is needed by Wayland compositors for certain types of events.
Comment 14 Jonas Ådahl 2015-12-03 07:46:26 UTC
Created attachment 316697 [details] [review]
evdev: Pass a motion delta to pointer constrain callback

The constrain callback cannot rely on the pointer position of the
corresponding ClutterInputDevice to get the actual delta of the motion
event that is to be constrained since it is only updated when an event is
dispatched. So change the API to pass the previous pointer position when
constraining.
Comment 15 Jonas Ådahl 2015-12-03 07:48:43 UTC
A couple of the previous patches were made partly obsolete by recent patches also introducing the ClutterEventEvdev struct, so I went and rewrote part of those. These are needed by the mutter relative pointer and pointer constraints patches, and should be pushed together because of the API/ABI break.
Comment 16 Emmanuele Bassi (:ebassi) 2015-12-18 11:01:46 UTC
Review of attachment 316694 [details] [review]:

Okay.
Comment 17 Emmanuele Bassi (:ebassi) 2015-12-18 11:02:59 UTC
Review of attachment 316695 [details] [review]:

Okay.
Comment 18 Emmanuele Bassi (:ebassi) 2015-12-18 11:08:05 UTC
Review of attachment 316696 [details] [review]:

Looks okay, but the scale and unit of the 'time' field inside the ClutterEvent data structure is platform-defined — i.e. nothing says that the time must be in milliseconds, just that the timestamps can be used with the platform-specific API. It should be possible to move all input-related events in the evdev backend to a microsecond unit, unless, obviously, at some point they get fed to the Wayland backend and it expects milliseconds.
Comment 19 Emmanuele Bassi (:ebassi) 2015-12-18 11:09:17 UTC
Review of attachment 316697 [details] [review]:

::: README.in
@@ +294,3 @@
+
+• The evdev backend specific ClutterPointerConstrainCallback type was changed
+  to include not only the target position but also the previous position.

Please add: "This is an API change, and will require a version check in any caller code."
Comment 20 Jonas Ådahl 2016-02-16 11:04:30 UTC
Attachment 316694 [details] pushed as a598917 - evdev: Provide the relative pointer motion event deltas
Attachment 316695 [details] pushed as 9214d50 - evdev: Use microsecond granularity for internal timestamps
Attachment 316696 [details] pushed as 52e38d1 - evdev: Expose microsecond timestamps via the clutter evdev API
Attachment 316697 [details] pushed as 8181ef1 - evdev: Pass a motion delta to pointer constrain callback