On Thu, Mar 03, 2016 at 03:26:49PM -0800, Aaron Armstrong Skomra wrote:
> On Thu, Jan 14, 2016 at 8:34 PM, Peter Hutterer <peter.hutte...@who-t.net>
> wrote:
>
> > On Wed, Jan 13, 2016 at 10:48:31AM -0800, Aaron Skomra wrote:
> > > Libwacom_new_from_path() matches the vid/pid from a found device to
> > > an entry in the database. Devices with entries in libwacom/data have
> > > a name which identifies the physical device (e.g. “Wacom Intuos Pro
> > > M”), but the generic/fallback devices have a (sysfs) name with a
> > > suffix indicating the part of the device in question (e.g “Wacom
> > > Intuos Pro M _Pen_”). When gnome-control-center creates panels for
> > > the devices, the names of the related fallback/generic devices don’t
> > > match each other, and gcc attempts to create a new panel for each of
> > > the Pen, the Pad, etc. because gcc thinks they are separate devices.
> > > This results in the Pad “separate” device not being connected to its
> > > associated Pen device and the “Map Buttons...” button not being
> > > displayed. This patch removes the suffix off of the fallback device
> > > name and adds button position necessary for gnome to add the buttons.
> > >
> > > Signed-off-by: Aaron Skomra <aaron.sko...@wacom.com>
> > > ---
> > > data/generic.tablet | 6 ++++++
> > > libwacom/libwacom.c | 13 +++++++++++--
> > > 2 files changed, 17 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/data/generic.tablet b/data/generic.tablet
> > > index 8b7f867..ca91b41 100644
> > > --- a/data/generic.tablet
> > > +++ b/data/generic.tablet
> > > @@ -1,3 +1,6 @@
> > > +# Wacom
> > > +# Generic/Fallback Tablet
> > > +
> > > [Device]
> > > Name=Generic
> > > DeviceMatch=generic
> > > @@ -8,3 +11,6 @@ Stylus=true
> > > Ring=true
> > > NumStrips=2
> > > Buttons=4
> > > +
> > > +[Buttons]
> > > +Left=A;B;C;D
> > > diff --git a/libwacom/libwacom.c b/libwacom/libwacom.c
> > > index d20b66c..7d324fe 100644
> > > --- a/libwacom/libwacom.c
> > > +++ b/libwacom/libwacom.c
> > > @@ -486,7 +486,7 @@ libwacom_new_from_path(const WacomDeviceDatabase
> > *db, const char *path, WacomFal
> > > const WacomDevice *device;
> > > WacomDevice *ret = NULL;
> > > WacomIntegrationFlags integration_flags;
> > > - char *name, *match_name;
> > > + char *name, *match_name, *name_copy;
> > >
> > > if (!db) {
> > > libwacom_error_set(error, WERROR_INVALID_DB, "db is NULL");
> > > @@ -521,7 +521,16 @@ libwacom_new_from_path(const WacomDeviceDatabase
> > *db, const char *path, WacomFal
> > >
> > > if (name != NULL) {
> > > g_free (ret->name);
> > > - ret->name = g_strdup(name);
> > > + name_copy = g_strdup(name);
> > > +
> > > + /* Remove the tool name suffix from the device
> > name. */
> > > + char * new_end = g_strrstr(name_copy, " ");
> > > + new_end[0] = '\0';
> >
> > two style nits: space before (, though I admit htis is very much all over
> > the place in the code. and we don't use ad-hoc declarations, please move
> > the
> > char *new_end to the top.
> >
> > > +
> > > + ret->name = g_strdup(name_copy);
> > > +
> > > + new_end[0] = ' ';
> > > + g_free (name_copy);
> > > }
> > > }
> >
> > I'm not sure we should have this code in libwacom, it would effectively
> > become part of the API, I'd rather have this in the caller where adjusting
> > to future changes is easier.
>
>
> Sorry for the delay in responding, I have an excuse, really!
>
> Just wanted to follow up on this to make sure we understand the API concerns
> here. It seems like gnome could use something other than name for its
> purposes.
>
> Gnome needs to know if items are part of a the same physical device so
> it can group them together on a CcWacomPage[1], and it does this with
> devices that are in the libwacom database by simply by matching on their
> name. (The pen, the pad, and the touch devices all have the same name
> coming
> from libacom.) The matching fails, though, for the default device's name
> because
> its name doesn't come from libwacom. You want to leave the name for
> default (not
> in libwacom) devices as is because the name from X could change and we
> don't
> want to be responsible for knowing how the name from X will look (or now
> libinput I
> guess). I'm thinking here about your recent blog post [2].
side-note: libwacom doesn't deal with X, so it has little effect here.
libwacom merely uses its own database and where it does use an actual device
to initialize it gets udev to provide it with the vid/pid.
exception is libwacom_new_from_name() but g-c-c doesn't use that for the
normal paths anyway.
> ...Maybe in this case gnome should be matching on VID/PID instead of name?
> In that case VID/PID would need to be added to GsdWacomDevicePrivate. I
> could work on that, but I already have some patches on these same files
> which
> haven't been applied [3]. Maybe that code is going to be rewritten anyway
> for
> Wayland support so there is no point (at this time) in trying to make any
> changes
> to it?
the thing here is: IMO libwacom is the wrong place to fix this issue. AFAICT
it works fine for the devices libwacom supports and only breaks where we
tell libwacom to create a generic device. This requires telling libwacom
explicitly that we want a device that won't be our device, but is close
enough (fsvo close). we shouldn't put magic into libwacom to make the
generic device guess how to look correct for the caller, we should put the
code into the caller to remember that this is a generic device and then
handle it acordingly.
but in libwacom there's no good solution, whatever we come up still won't
work 100% correct on all faked tablets anyway and every caller may handle
the generic device differently.
Cheers,
Peter
> [1] update_current_page() in cc-wacom-panel.c
> https://git.gnome.org/browse/gnome-control-center/tree/panels/wacom/cc-wacom-panel.c#n272
> [2]
> http://who-t.blogspot.com/2016/03/libinput-and-graphics-tablet-support.html
> [3] https://bugzilla.gnome.org/show_bug.cgi?id=750745
>
>
>
> > If we do decide to ship this in libwacom, we
> > should restrict the suffix removal to known tool types only, rather than
> > stripping out the last word. In theory, we can have non-wacom tablets in
> > libwacom's database that may not have a Pen/Finger/... ending.
> >
> > Bastien, any comments?
> >
> > Cheers,
> > Peter
> >
------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://makebettercode.com/inteldaal-eval
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
