TouchListenerGone cleans up if a client disappears. Having this in
FreeGrab() triggers cyclic removal of grabs, emitting wrong events. In
particular, it would clean up a passive grab record while that grab is
active.
Move it to CloseDownClient() instead, cleaning up before we go.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Introduced in xorg-server-1.13.99.901-2-g9ad0fdb. Storing the grab pointer
in the listener turns out to be a bad idea. If the grab is not an active
grab or an implicit grab, the pointer stored is the one to the grab attached
on the window. This grab may be removed if the client calls UngrabButton or
similar while the touch is still active, leaving a dangling pointer.
To avoid this, copy the grab wherever we need to reference it later. This
is also what we do for pointer/keyboard grabs, where we copy the grab as
soon as it becomes active.
Reported-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Obsolete since 4bc2761ad5. This struct
existed so copying a passive grab could be simply done by
activeGrab = *grab
and thus have a copy of the GrabPtr we'd get from various sources but still
be able to check device->grab for NULL.
Since 4bc2761 activeGrab is a pointer itself and points to the same memory
as grabinfo->grab, leaving us with the potential of dangling pointers if
either calls FreeGrab() and doesn't reset the other one.
There is no reader of activeGrab anyway, so simply removing it is
sufficient.
Note: field is merely renamed to keep the ABI. Should be removed in the
future.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Change the single if condition in the loop body to a
if (!foo) continue;
and re-indent the rest.
No functional changes.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
A sync grab is the owner once it gets events. If it doesn't replay the
event it will get all events from this touch, equivalent to accepting it.
If the touch has ended before XAllowEvents() is called, we also now need to
send the TouchEnd event and clean-up since we won't see anything more from
this touch.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If a device is frozen in results to a grab, we need to enqueue the events.
This makes things complicated, and hard to follow since touch events are now
replayed in the history, pushed into EnqueueEvent, then replayed later
during PlayReleasedEvents in response to an XAllowEvents.
While the device is frozen, no touch events are processed, so if there is a
touch client with ownership mask _below_ the grab this will delay the
delivery and potentially screw gesture recognition. However, this is the
behaviour we have already anyway if the top-most client is a sync pgrab or
there is a sync grab active on the device when the TouchBegin was generated.
(also note, such a client would only reliably work in case of ReplayPointer
anyway)
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If the device is currently grabbed as the result of a passive grab
activating, do not prepend that grab to the listeners (unlike active grabs).
Otherwise, a client with a passive pointer grab will prevent touch grabs
from activating higher up in the window stack.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If drivers supply incorrect values don't just quietly return False, spew to
the log so we can detect what's going on. All these cases are driver bugs
and should be fixed immediately.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Dave Airlie <airlied@redhat.com>
InitPointerClassDeviceStruct/InitKeyboardDeviceStruct allocate a
proximity/focus class, respectively. If a driver calls
InitFocusClassDeviceStruct or InitProximityClassDeviceStruct beforehand,
the previously allocated class is overwritten, leaking the memory.
Neither takes a parameter other than the device, so we can simply skip
initialising it if we already have one.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Dave Airlie <airlied@redhat.com>
==21860== 24 bytes in 1 blocks are still reachable in loss record 85 of 397
==21860== at 0x4C2B3F8: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==21860== by 0x61ED93: AllocateOutputBuffer (io.c:1037)
==21860== by 0x61E15A: WriteToClient (io.c:764)
==21860== by 0x457B30: ProcQueryExtension (extension.c:275)
==21860== by 0x43596B: Dispatch (dispatch.c:432)
==21860== by 0x425DAB: main (main.c:295)
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Dave Airlie <airlied@redhat.com>
DeleteCallbackManager() introduced for better symmetry in the caller, they
do the same thing.
==20085== 24 bytes in 1 blocks are still reachable in loss record 11 of 103
==20085== at 0x4C2A4CD: malloc (vg_replace_malloc.c:236)
==20085== by 0x43A097: CreateCallbackList (dixutils.c:837)
==20085== by 0x43A1D3: AddCallback (dixutils.c:869)
==20085== by 0x4B1736: GEExtensionInit (geext.c:209)
==20085== by 0x41C8A8: InitExtensions (miinitext.c:389)
==20085== by 0x5AC918: main (main.c:208)
==2042== 8 bytes in 1 blocks are still reachable in loss record 2 of 97
==2042== at 0x4C2A4CD: malloc (vg_replace_malloc.c:236)
==2042== by 0x4C2A657: realloc (vg_replace_malloc.c:525)
==2042== by 0x4802F5: XNFrealloc (utils.c:1095)
==2042== by 0x43A17A: CreateCallbackList (dixutils.c:855)
==2042== by 0x43A1EF: AddCallback (dixutils.c:870)
==2042== by 0x4B1752: GEExtensionInit (geext.c:209)
==2042== by 0x41C8A8: InitExtensions (miinitext.c:389)
==2042== by 0x5AC9E4: main (main.c:208)
==2042==
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Heaps of these:
==2042== 15,360 bytes in 120 blocks are still reachable in loss record 94 of
97
==2042== at 0x4C2A4CD: malloc (vg_replace_malloc.c:236)
==2042== by 0x4C2A657: realloc (vg_replace_malloc.c:525)
==2042== by 0x45FB91: double_size (registry.c:65)
==2042== by 0x45FC97: RegisterRequestName (registry.c:85)
==2042== by 0x460095: RegisterExtensionNames (registry.c:179)
==2042== by 0x460729: dixResetRegistry (registry.c:334)
==2042== by 0x5AC992: main (main.c:201)
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
==2547== 1 bytes in 1 blocks are still reachable in loss record 1 of 111
==2547== at 0x4C2A4CD: malloc (vg_replace_malloc.c:236)
==2547== by 0x64D1551: strdup (strdup.c:43)
==2547== by 0x4802FB: Xstrdup (utils.c:1113)
==2547== by 0x585B6C: XkbSetRulesUsed (xkbInit.c:219)
==2547== by 0x58700F: InitKeyboardDeviceStruct (xkbInit.c:595)
==2547== by 0x419FA3: vfbKeybdProc (InitInput.c:74)
==2547== by 0x425A3D: ActivateDevice (devices.c:540)
==2547== by 0x425F65: InitAndStartDevices (devices.c:713)
==2547== by 0x5ACA57: main (main.c:259)
and a few more of the above.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
I didn't think we needed this before, but after doing some more
work with reverse optimus it seems like it should be called.
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
this allows the pixmap dirty helper to be used for reverse optimus,
where the GPU wants to copy from the shared pixmap to its VRAM copy.
[airlied: slave_dst is wrong name now but pointless ABI churn at this point]
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
We should have no problem allowing output/offload from the same slave,
I asserted here, but in order to implement reverse optimus this makes
perfect sense. (reverse optimus is intel outputting to nvidia).
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
==15562== 1,800 bytes in 1 blocks are definitely lost in loss record 298 of 330
==15562== at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
==15562== by 0x4312C7: InitTouchClassDeviceStruct (devices.c:1644)
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Slave devices don't need these and the matching code in CloseDevice() has a
IsMaster() condition on freeing these, causing a leak.
==16111== 384 bytes in 4 blocks are definitely lost in loss record 72 of 105
==16111== at 0x4C28BB4: calloc (vg_replace_malloc.c:467)
==16111== by 0x42AEE2: AllocDevicePair (devices.c:2707)
==16111== by 0x4BAA27: AllocXTestDevice (xtest.c:617)
==16111== by 0x4BA89A: InitXTestDevices (xtest.c:570)
==16111== by 0x425F5E: InitCoreDevices (devices.c:690)
==16111== by 0x5ACB2D: main (main.c:257)
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Chase Douglas <chase.douglas@canonical.com>
Fixes reading random memory read beyond the end of original event.
sizeof device_event: 424
sizeof internal_event: 2800
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Unlike pointer/keyboard events, the flags field for ET_Touch* is a set of
server-internal defines that we need to convert to XI protocol defines.
Currently only two of those defines actually translate to the protocol, so
make sure we don't send internal garbage down the wire.
No effect to current clients since they shouldn't look at undefined bits
anyway.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
e02f864fdf "Suppress cursor display until the first XDefineCursor() request"
disabled cursor display a priori unless -retro is given.
On a plain server, caling XFixesHideCursor() and XFixesShowCursor() would
show the default root cursor, despite no client actually defining a cursor.
Change the logic, disable CursorVisible by default and only enable it from
the window's CWCursor logic. If no window ever defines a cursor, said cursor
stays invisible.
X.Org Bug 58398 <http://bugs.freedesktop.org/show_bug.cgi?id=58398>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Tested-by: Bastien Nocera <hadess@hadess.net>
Reviewed-by: Daniel Martin <consume.noise@gmail.com>
==5712== 6 bytes in 1 blocks are still reachable in loss record 17 of 585
==5712== at 0x4A074CD: malloc (vg_replace_malloc.c:236)
==5712== by 0x3D1DE885B1: strndup (strndup.c:46)
==5712== by 0x41CB71: MakeAtom (atom.c:121)
==5712== by 0x55AE3E: XIGetKnownProperty (xiproperty.c:401)
==5712== by 0x4251C9: AddInputDevice (devices.c:312)
==5712== by 0x42AC0C: AllocDevicePair (devices.c:2657)
==5712== by 0x425E6E: InitCoreDevices (devices.c:677)
==5712== by 0x5ACA05: main (main.c:257)
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Chase Douglas <chase.douglas@canonical.com>
If we're about to abort, we're already in the signal handler and cannot call
down to the default device cleanup routines (which reset, free, alloc, and
do a bunch of other things).
Add a new DEVICE_ABORT mode to signal a driver's DeviceProc that it must
reset the hardware if needed but do nothing else. An actual HW reset is only
required for some drivers dealing with the HW directly.
This is largely backwards-compatible, hence the input ABI minor bump only.
Drivers we care about either return BadValue on a mode that's not
DEVICE_{INIT|ON|OFF|CLOSE} or print an error and return BadValue. Exception
here is vmmouse, which currently ignores it and would not reset anything.
This should be fixed if the reset is required.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
The transformation matrix we previously stored was a scaled matrix based on
the axis ranges of the device. For relative movements, the scaling is not
required (or desired).
Store two separate matrices, one as requested by the client, one as the
product of [scale . matrix . inv_scale]. Depending on the type of movement,
apply the respective matrix.
For relative movements, also drop the translation component since it doesn't
really make sense to use that bit.
Input ABI 19
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Set on DeviceEnterLeaveEvent() the xXIEnterEvent->focus field
similarly to how the CoreEnterLeaveEvent() function above does
for core events.
This fixes bug https://bugzilla.gnome.org/show_bug.cgi?id=677329
reported to GTK+, where focus handling on window managers with
sloppy focus or no window manager present was broken due to this
field being always set to FALSE.
Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
XTest devices are the first ones in the list, being initialised together
with the master devices. If we disable the devices in-order and a device has
a button down when being disabled, the XTest device is checked for a
required button release (xkbAccessX.c's ProcessPointerEvent). This fails if
the device is already NULL.
Instead of putting the check there, disable the devices in the reverse order
they are initialised. Disable physical slaves first, then xtest devices,
then the master devices.
Testcase: shut down server with a button still held down on a physical
device
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Absolute devices may send relative events depending on the mode (synaptics
by default, wacom per option). The relative events are added to the previous
position, converted into device coordinates and then scaled into desktop
coordinates for pointer movement.
Because the device range must be mapped into the desktop coordinate range,
this results in uneven scaling depending dimensions, e.g. on a setup with
width == 2 * height, a relative movement of 10/10 in device coordinates
results in a cursor movement of 20/10 (+ acceleration)
Other commonly user-visible results:
* the touchpad changing acceleration once an external monitor as added.
* drawing a circle on a wacom tablet in relative mode gives an ellipsis in
the same ratio as the desktop dimensions.
Solution: pre-scale the incoming relative x/y coordinates by width/height
ratio of the total desktop size. Then add them to the previous
coordinates and scale back with the previous mapping, which will undo the
pre-scaling and give us the right movement.
X.Org Bug 31636 <http://bugs.freedesktop.org/show_bug.cgi?id=31636>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
If we're already using our own custom macro, might as well use it properly.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
This is mostly sigsafe code, so use sigsave printf. And update some fields
to double that used to be int.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
For absolute events, if the client specifies a screen number offset the
coordinates by that. And add a new flag so we know when _not_ to add the
screen offset in GPE.
Without this offset and the flag, GPE would simply add the offset of the
current screen if POINTER_SCREEN is set.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Scaled is already in desktop coordinates, take the total width into account,
not just the current screen's width.
Fixes Xdmx pointer position calculation.
X.Org Bug 51904 <http://bugs.freedesktop.org/show_bug.cgi?id=51904>
Signed-off-by: Sybren van Elderen <sowmestno@msn.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
GetTouchEvents is usually called in a signal context.
Calling ErrorF for the error messages leads to X complaining about log:
(EE) BUG: triggered 'if (inSignalContext)'
(EE) BUG: log.c:484 in LogVMessageVerb()
(EE) Warning: attempting to log data in a signal unsafe manner while in signal context.
Please update to check inSignalContext and/or use LogMessageVerbSigSafe() or ErrorFSigSafe().
The offending log format message is:
%s: Attempted to start touch without x/y (driver bug)
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If a XI2.1+ client has a grab on a non-root window, it must still receive
raw events on the root window.
Test case: register for XI_ButtonPress on window and XI_RawMotion on root.
No raw events are received once the press activates an implicit grab on the
window.
X.Org Bug 53897 <http://bugs.freedesktop.org/show_bug.cgi?id=53897>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Should've been removed in bc1f90a615018c05994fae3e678dd2341256cd82a, but got
left here due to a botched rebase.
Fixes stray button events sent to clients after deactivating an async
pointer grab on a pointer-emulating-touch.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
events.c: In function 'DeactivatePointerGrab':
events.c:1524:51: warning: 'dev' may be used uninitialized in this function
[-Wuninitialized
dev is unset when we get here, the device to check is "mouse".
Introduced in ece8157a59.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Instead of guessing what resource type the listener is and what property to
retrieve, store the resource type in the listener directly.
Breaks XIT test cases:
TouchGrabTestMultipleTaps.PassiveGrabPointerEmulationMultipleTouchesFastSuccession
Fixes https://bugs.freedesktop.org/show_bug.cgi?id=56557
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Chase Douglas <chase.douglas@ubuntu.com>
