Re: XI2 branch ready for review (xi2-for-master)

[Carlos Garnacho <carlosg gnome org>]

On vie, 2010-05-21 at 14:00 -0400, Matthias Clasen wrote:
> On Fri, May 21, 2010 at 10:29 AM, Carlos Garnacho <carlosg gnome org> wrote:
> 
> >
> > So, reviewing and comments are most welcome :)
> 
> I've started to look over the branch, and so far things look great to
> me. Here are a few trivial things that I've noticed. I'll push fixes
> for those later:

Ouch, I now notice that you fixed some of these, when pulling git merged
both your and my fixes, I didn't notice and pushed. The merge makes
sense though.

> 
> * There are some "index" arguments in gdkdevice.h. They should be
> renamed to index_ to avoid trouble with lesser system headers...
> 
> * The GDK_MULTIDEVICE_SAFE should be documented in the section on
> 'compiling GTK+ applications' (look for GDK_MULTIHEAD_SAFE there).
> 
> * gtk_device_grab_add/remove are missing from the docs.
> 
> * The docs for gdk_enable_multidevice should state that it needs to be
> called before initializing GDK
> 
> * It would be good if the GdkDeviceType docs had a link to the
> conceptual explanation of the types in the GdkDevice intro

It could also be worth to ponder here about making these names not so
X-like, perhaps "virtual", "physical" and "detached".

> 
> * gdk_display_list_devices docs should state what kind of devices are
> listed - master, slave, both. Or should that function be deprecated in
> the first place ?

I've deprecated this function, it should together with
gdk_devices_list(). These aren't sufficient when the device hierarchy
changes.

For backwards compatibility, these 2 functions only return the "core"
pointer (i.e. the first master device found in XI2, although
XIGetClientPointer should be used here), and floating slave devices (not
sending events to any master device). This is most similar to XInput1
semantics.

> 
> * Is there any reason why the device_grab api is in gdk.h, not gdkdevice.h ?

Not really, I just put them together with their older sisters, probably
gdkdevice.h makes more sense.

> 
> The one bigger concern I have is the grab-notify handling in GTK+ with
> the device-specific grabs.
> It wasn't immediately obvious that that does the right thing if there
> are grabs for different devices in the stack. But maybe I just haven't
> stared long enough at the code yet.

The semantics there have changed quite a bit. The was_grabbed parameter
semantics aren't sufficient for multidevice case, but adding an
additional signal would be too cumbersome/ugly, so I added
gtk_widget_device_is_shadowed (GtkWidget*, GdkDevice*).

GtkWidgets should know which devices are they currently interacting
with, so on ::grab-notify, they should check that function for such
devices and see whether they're no longer going to receive events for
these. The was_grabbed parameter remains as a hint, still valid for the
presence of a single pointer.

I think I should be writing further docs about this, device ownership
and other new concepts (a "migrating"-like section), I'll be working on
this next.

Cheers,
  Carlos