Some initial impressions from looking over the branch:

1) Do we forsee ever adding properties to GdkDeviceTool ? In that case, it might be better to make it an object now, instead of a boxed type

2) gdk_device_update_tool just stores a pointer to the passed-in tool without using gdk_device_ref/unref - is that safe ?

3) Maybe more concerning, _gdk_wayland_seat_remove_tool appears to be cxalling g_object_unref on a GdkDeviceTool

4) The docs for ::tool-changed say it is emitted on proximity-in/out - what happens if I simply switch between two different pens - I guess it is assumed that there will always be proximity events around such a change ? Is that guaranteed ?

5) Related to the previous point, I see a call to gdk_device_update_tool in tablet_tool_handle_proximity_in, but not in tablet_tool_handle_proximity_out. Oversight ?

6) Is there always going to be a 1:1 relationship between devices and tools ? Could one imagine a fancy 'multi-pen' tablet that handles more than one tool simultaneously ?

7) Grepping for GDK_SEAT_CAPABILITY_TABLET_STYLUS in gdk/*/*.c comes up empty - I would have expected seats to expose that capabability (and associated master devices) now ? Also, should that name be shortened to GDK_SEAT_CAPABILITY_TABLET, maybe ? The stylus is just a tool, after all (right?)

(In reply to Matthias Clasen from comment #1)
> Some initial impressions from looking over the branch:
> 
> 1) Do we forsee ever adding properties to GdkDeviceTool ? In that case, it
> might be better to make it an object now, instead of a boxed type

Right, IIRC it started like that in the early branches from from lyude and I didn't put much thought in it. I wouldn't expect much extra/non-static info ahead, but it'll be probably safer to make it a GObject anyway.

> 
> 2) gdk_device_update_tool just stores a pointer to the passed-in tool
> without using gdk_device_ref/unref - is that safe ?

Oops yeah, ref/unref should probably happen there.

> 
> 3) Maybe more concerning, _gdk_wayland_seat_remove_tool appears to be
> cxalling g_object_unref on a GdkDeviceTool

Good catch, that path is kind of untested...

> 
> 4) The docs for ::tool-changed say it is emitted on proximity-in/out - what
> happens if I simply switch between two different pens - I guess it is
> assumed that there will always be proximity events around such a change ? Is
> that guaranteed ?

This is kinda related to #6. I don't have multiple pens to try with, but I do expect proximity_out from an earlier tool before a newer one takes over.

> 
> 5) Related to the previous point, I see a call to gdk_device_update_tool in
> tablet_tool_handle_proximity_in, but not in
> tablet_tool_handle_proximity_out. Oversight ?

It indeed is.

> 
> 6) Is there always going to be a 1:1 relationship between devices and tools
> ? Could one imagine a fancy 'multi-pen' tablet that handles more than one
> tool simultaneously ?

AFAIK, there were already such models, but those are mostly forgotten nowadays. It is true that the gdk layer couldn't cope as well with this case if it hw capability became trendy again, even though the wayland protocol itself can.

> 
> 7) Grepping for GDK_SEAT_CAPABILITY_TABLET_STYLUS in gdk/*/*.c comes up
> empty - I would have expected seats to expose that capabability (and
> associated master devices) now ? Also, should that name be shortened to
> GDK_SEAT_CAPABILITY_TABLET, maybe ? The stylus is just a tool, after all
> (right?)

Right, adding api to get devices with that capability should be in place with GdkDeviceManager API being deprecated. Agreed about the shortened name too.

Will be working on those.

(In reply to Carlos Garnacho from comment #2)
> (In reply to Matthias Clasen from comment #1)
> > Some initial impressions from looking over the branch:
> > 
> > 1) Do we forsee ever adding properties to GdkDeviceTool ? In that case, it
> > might be better to make it an object now, instead of a boxed type
> 
> Right, IIRC it started like that in the early branches from from lyude and I
> didn't put much thought in it. I wouldn't expect much extra/non-static info
> ahead, but it'll be probably safer to make it a GObject anyway.

this is more a comment from the other side of the protocol but any device-specific configuration on the tablet will go into the tools, so the tools may scale up to become more complicated. not quite sure how pressure curves etc will be handled but either way I agree that making this a GObject is prudent.

> > 6) Is there always going to be a 1:1 relationship between devices and tools
> > ? Could one imagine a fancy 'multi-pen' tablet that handles more than one
> > tool simultaneously ?
> 
> AFAIK, there were already such models, but those are mostly forgotten
> nowadays. It is true that the gdk layer couldn't cope as well with this case
> if it hw capability became trendy again, even though the wayland protocol
> itself can.

the Intuos2 (iirc) had that ability but it's too much of a niche and too expensive to put the sensors in the hardware. so no recent tablet can do it, at best you get touch + pen because they're two different devices. both libinput and the protocol can handle a potential multi-pen input.

also, just for clarification: the default case is 1 tabelet:2 tools for Intuos Pro (1 tablet, 1 tool with stylus + eraser). Some ex-Bamboo (now Intuos) have a stylus-only pen, no eraser, and that is also the case for some non-Wacom tablets. The surface pen is a special case because the eraser is triggered by a button press, but still at the bottom of the device. The datastream looks identical to turning the device around though.
the 1 tablet:many tools is a special case but common for professionals, 2:many is relatively common at some studios.

(In reply to Matthias Clasen from comment #1)
> Some initial impressions from looking over the branch:
> 
> 1) Do we forsee ever adding properties to GdkDeviceTool ? In that case, it
> might be better to make it an object now, instead of a boxed type

Fixed in the branch

> 2) gdk_device_update_tool just stores a pointer to the passed-in tool
> without using gdk_device_ref/unref - is that safe ?

Fixed
 
> 3) Maybe more concerning, _gdk_wayland_seat_remove_tool appears to be
> cxalling g_object_unref on a GdkDeviceTool

Fixed
 
> 4) The docs for ::tool-changed say it is emitted on proximity-in/out - what
> happens if I simply switch between two different pens - I guess it is
> assumed that there will always be proximity events around such a change ? Is
> that guaranteed ?

This was answered 

> 5) Related to the previous point, I see a call to gdk_device_update_tool in
> tablet_tool_handle_proximity_in, but not in
> tablet_tool_handle_proximity_out. Oversight ?

Fixed

> 6) Is there always going to be a 1:1 relationship between devices and tools
> ? Could one imagine a fancy 'multi-pen' tablet that handles more than one
> tool simultaneously ?

Answered

> 7) Grepping for GDK_SEAT_CAPABILITY_TABLET_STYLUS in gdk/*/*.c comes up
> empty - I would have expected seats to expose that capabability (and
> associated master devices) now ? Also, should that name be shortened to
> GDK_SEAT_CAPABILITY_TABLET, maybe ? The stylus is just a tool, after all
> (right?)

Decided to not do this for now.


Branch looks good to me now, fwiw.

After the last double checks, the branch has been merged to master.