When XIM + ibus is in use XIM creates an invisible window for its own
purposes, we sometimes get a _GTK_LOAD_ICONTHEMES ClientMessage event on
this window. Since this window was not explicitly created by spicec, it
does not have a Window Context (with the event handling function for the
window in question) set. This would cause spicec to throw an unhandled
exception and exit.
This patch replaces the exception throwing with silently ignoring
ClientMessage events on Windows without a Context and logging a warning
for other event types.
XRRSet* calls wait for a XReply, so add a missing XLockDisplay,
this fixes a hang (due to a race so not always) when switching between
windowed and fullscreen mode.
Since libX11-1.3.4 the multi-threading handling code of libX11 has been
changed, see:
http://cgit.freedesktop.org/xorg/lib/libX11/commit/?id=933aee1d5c53b0cc7d608011a29188b594c8d70b
This causes several issues. One of them is the display inside the
client not getting updated when there are no XEvents being generated,
this is caused by the following part of the referenced commit message:
Caveats:
- If one thread is waiting for events and another thread tries to read a reply,
both will hang until an event arrives. Previously, if this happened it might
work sometimes, but otherwise would trigger either an assertion failure or
a permanent hang.
We were depending on the otherwise behavior and apparently were lucky.
This can be seen by starting F14 in runlevel 3 and then doing startx
and not touching the mouse / keyb afterwards. Once you move the mouse
(generate an event you see the UI contents being updated but not before.
Another thing both I and Alon (iirc) have seen are hangs where 2
threads are stuck in XSync waiting for a reply simultaneously. This might
be related to libxcb version, according to the libX11 commit a libxcb
newer then 1.6 was needed, and my system had 1.5 at the time I saw this
after updating to libxcb 1.7 I can no longer reproduce.
This patch tackles both problems (and is needed for the 1st one
indepently of the 2nd one possibly being fixed) by adding XLockDisplay
calls around all libX11 calls which wait for a reply or an event.
Every time an events comes past where the Window is None (which happens
about once every 5 minutes or so), this annoying "invalid window" message
gets printed. Remove it!
Well almost remove it, it was possible that another x11 app would acquire
selection ownership, and we would receive a release message from the
agent before having processed the xselection ownership change event.
Then we would set the selection owner to none, overriding the new owner.
As the comment in the patch indicates there still is a minute window left
where something similar can happen after this patch. Nothing we can do
about that (I blame the libX11 selection API).
Atleast under x11 there is a race condition when setting the clipboard
owner to guest from the RedClient code rather then doing it in Platform.
After the XSetSelectionOwner() in Platform::on_clipboard_grab(), which runs
from the main message loop thread, the x11 event thread can receive a
SelectionRequest event from another x11 app, before the RedClient code
has set the clipboard owner, which will trigger the owner != guest
check in the SelectionRequest event handling code.
By moving the setting of the owner in to Platform::on_clipboard_grab() it
gets protected by the clipboard lock, which closes this tiny race.
XSelectionRequest events must be answered in the order they were
received. But for TARGETS request we can answer directly, where as
other requests need to go through the agent. This causes us to handle
things out of order, and this can cause us to have more then one
requets outstanding with the agent, which is also not what we want.
So this patch introduces a queue for XSelectionRequest events, causing
us to handle them 1 at a time and always in order.
Since we do not always "pump" libX11's event loop by calling
XPending (we only call XPending when there is data to read from the
display fd), we must always explictly flush any outstanding requests.
This patch adds a whole bunch of missing XFlush calls.
Make sure we process the XFixesSetSelectionOwnerNotify event caused by
us setting the clipboard owner to none, directly after setting the owner
to none. Otherwise we may end up changing the clipboard owner to none, after
it has already been re-owned because the XFixesSetSelectionOwnerNotify event
to owner none is event is still pending when we set the new owner, and
then changes the owner back to none once processed messing up our clipboard
ownership state tracking.
I saw this happening when doing copy twice in succession inside the guest.
Request targets from new clipboard owner, rather then assuming UTF-8
(not entirely complete yet, the last pieces will be in another patch).
Atleast as important this code unifies the selection getting code
for incr and non incr getting of selection data so that it can be
used for both getting regular selection data and for getting targets
selection data.
This also fixes a big bug in the (I believe untested sofar) incr support
code which was interpreting the contents of PropertyNotify events as
XSelectionEvents rather then as XpropertyEvents which are completely
differen structs!
Instead of keeping a flag, we can and should simply check wether the
new owner reported in the event it us or not. Also check for the
new owner being none and send a clipboard_release when that is the
case (through set_clipboard_owner(owner_none)).
Given that all clipboard handling is async, it is possible to for
example receive a request for clipboard data from the agent
while the client no longer owns the clipboard (ie a
VD_AGENT_CLIPBOARD_RELEASE message is in transit to the agent).
Thus it is necessary to keep track of our notion of clipboard ownership
and check received clipboard messages (both from other apps on the client
machine and from the agent) to see if they match our notion and if not
drop, or in case were a counter message is expected nack the clipboard
message.
Rename the 4 platform clipboard functions which get called
upon receival of an agent clipboard message to on_clipboard_*
The old set_clipboard_* names were confusing as they suggest being
a class property setter (like set_event_listener) rather then
event handler, and set_clipboard_owner was causing a name conflict
with the next patch in this series.
A clipboard owner can indicate that it can supply the data the clipboard
owns in multiple formats, so make the data passed with a
VD_AGENT_CLIPBOARD_GRAB message an array of types rather then a single
type.
-includes most of Hans' review fixes (up to the SelectionRequest comment [4]) & X11 wips sent by Hans (10x!)
-use the VD_AGENT_CLIPBOARD_* types in the platform code
-add ifs for VD_AGENT_CAP_CLIPBOARD_BY_DEMAND in both sides
-support the GRAB/REQUEST/DATA/RELEASE verbs in both ways
-pasting clipboard data is now "only-by-demand" from both sides (client and agent), whose behavior is symmetric
-client and agent don't read or send the contents of the clipboard unnecessarily (e.g. copy, internal paste, repeating paste, focus change)
-set client as clipboard listener instead of application
-add atexit(cleanup) in win platform
linux:
-instead of clipboard atom selection instead of XA_PRIMARY
-enable USE_XRANDR_1_2 and support clipboard in MultyMonScreen
-send utf8 with no null termination, remove ++size
-add xfixes in configure.ac & Makefile.am
windows:
-bonus: support image cut & paste, currently only on windows
not done yet:
-clipboards formats are still uint32_t, not mime types stores as strings
-platform_win is still used, not the root window
-not replaced the ugly windows CF_DIB in agent/winclient
When handling an xrandr event the event_listener->on_monitors_change()
callback destroys and re-creates the monitor object(s) which results
in the DynamicScreen or MultyMonconstructor being called, which triggers
more xrandr events. This causes a never ending event handling loop making
spicec hang, and eventually making the X-server crash as a backlog
of events builds up and it oom's.
This patches this by explictly processing the xrandr event caused
by the constructor inside the constructor surrounded by the already
present guard code against recursive xrandr events.
* windows - untested
* linux - small strings both ways, large implemented differently:
* client to guest - support INCR
* guest to client - we supply a single possibly very large property
* requires server changes in next patch to work with spice-vmc
The OpenGL renderer isn't really useful right now, its not quite up
to date, its not really faster than software and it only supports a limited
subset of drivers. So, lets disable it for now.
Long term opengl rendering of the 2d part of spice is important if we want
to combine 2d and 3d rendering (say if spice adds opengl support in the
protocol). But until then this is isn't useful for normal use.
The current glx code is looking for a rgb32 visual and always failing
if there is none. This means not even software rendering starts up
on e.g. 16bit visuals. This commit makes it pick software fallbacks
on 16bit visuals.
Long term we need to fix the gl implementation to do 16bpp too.
XIM ClientMessage events to an XIM internal window was triggering
the check for a window message procs. We need to properly filter
events with XFilterEvent to avoid this happening.
