[PATCH weston 1/2] compositor: add weston_surface_set_description()

[Pekka Paalanen]

On Thu, 27 Nov 2014 10:17:21 +0200
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

> Just a quick comment below, I haven't looked at it carefully yet.
> 
> --
> Giulio
> 
> 2014-11-27 9:07 GMT+02:00 Pekka Paalanen <ppaalanen at gmail.com>:
> > From: Pekka Paalanen <pq at iki.fi>
> >
> > When printing out logs from Weston's actions, mainly for debugging, it
> > can be very difficult to identify the different surfaces.  Inspecting
> > the configure function pointer is not useful, as the configure functions
> > may live in modules.
> >
> > Add vfunc get_description to weston_surface, which will produce a short,
> > human-readable description of the surface, which allows identifying it
> > better, rather than just looking at the surface size, for instance.
> >
> > Set the description from most parts of Weston, to identify cursors and
> > drag icons, and panels, backgrounds, screensavers and lock surfaces, and
> > the desktop shell's application surfaces.
> >
> > Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > ---
> >  desktop-shell/input-panel.c |   9 ++++
> >  desktop-shell/shell.c       | 101 ++++++++++++++++++++++++++++++++++++++++++++
> >  src/compositor.c            |  17 ++++++++
> >  src/compositor.h            |   7 +++
> >  src/data-device.c           |  19 +++++++++
> >  src/input.c                 |  10 +++++
> >  6 files changed, 163 insertions(+)

...

> > diff --git a/src/compositor.c b/src/compositor.c
> > index 17e930c..ea5cc14 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -2736,6 +2736,13 @@ weston_subsurface_parent_commit(struct weston_subsurface *sub,
> >                 weston_subsurface_synchronized_commit(sub);
> >  }
> >
> > +static int
> > +subsurface_get_description(struct weston_surface *surface,
> > +                          char *buf, size_t len)
> > +{
> > +       return snprintf(buf, len, "sub-surface");
> > +}
> > +
> >  static void
> >  subsurface_configure(struct weston_surface *surface, int32_t dx, int32_t dy)
> >  {
> > @@ -2819,6 +2826,14 @@ weston_surface_set_role(struct weston_surface *surface,
> >         return -1;
> >  }
> >
> > +WL_EXPORT void
> > +weston_surface_set_description(struct weston_surface *surface,
> 
> I find the naming a bit confusing, I'd expect this to set the
> description of the surface, that is a char*. The get_description vfunc
> on the other hand doesn't return the function passed here, but the
> char*. I'd call it something like
> weston_surface_set_description_func(), or maybe
> weston_surface_set_descriptor().

Yes, weston_surface_set_get_description_func() it would be indeed, but
it was getting a bit long, especially when the argument is usually
a fairly long word too.

Descriptor smells a bit like an fd thing, though.

weston_surface_set_... eh... umm

What if I did s/description/label/g?

So we'd have e.g.:
weston_surface_set_label_func(surface, shell_surface_get_label)
and weston_surface::get_label field.

How's that?


Thanks,
pq


> 
> > +                              int (*desc)(struct weston_surface *,
> > +                                          char *, size_t))
> > +{
> > +       surface->get_description = desc;
> > +}
> > +