Re: [PATCH v2 2/2] weston-info: destroy wl_keyboard

2018-04-26 Thread Peter Hutterer
On Thu, Apr 26, 2018 at 05:01:24PM +0200, w...@ongy.net wrote:
> From: Markus Ongyerth 
> 
> Fixes a memory leak by calling wl_keyboard_destroy on a any keyboard
> that was used to listen for events.
> 
> Signed-off-by: Markus Ongyerth 

Reviewed-by: Peter Hutterer 

Cheers,
   Peter
> ---
>  clients/weston-info.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/clients/weston-info.c b/clients/weston-info.c
> index d1bdcbde..3dfea7d8 100644
> --- a/clients/weston-info.c
> +++ b/clients/weston-info.c
> @@ -118,6 +118,7 @@ struct seat_info {
>   struct wl_seat *seat;
>   struct weston_info *info;
>  
> + struct wl_keyboard *keyboard;
>   uint32_t capabilities;
>   char *name;
>  
> @@ -498,10 +499,8 @@ seat_handle_capabilities(void *data, struct wl_seat 
> *wl_seat,
>   return;
>  
>   if (caps & WL_SEAT_CAPABILITY_KEYBOARD) {
> - struct wl_keyboard *keyboard;
> -
> - keyboard = wl_seat_get_keyboard(seat->seat);
> - wl_keyboard_add_listener(keyboard, _listener,
> + seat->keyboard = wl_seat_get_keyboard(seat->seat);
> + wl_keyboard_add_listener(seat->keyboard, _listener,
>seat);
>  
>   seat->info->roundtrip_needed = true;
> @@ -531,6 +530,9 @@ destroy_seat_info(void *data)
>   if (seat->name != NULL)
>   free(seat->name);
>  
> + if (seat->keyboard)
> + wl_keyboard_destroy(seat->keyboard);
> +
>   wl_list_remove(>global_link);
>  }
>  
> -- 
> 2.17.0
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH libinput] touchpad: fix the trackpoint event counter for the T460s

2018-04-26 Thread Peter Hutterer
Introduced in 416fa44d80b0f2c53b652ddfa35dd4a156a65c65 but there was a logic
error: we claimed to require 3 events from a trackpoint before stopping the
touchpad but the timer was only set when we actually stopped the touchpad. So
if a trackpoint sends a single event every second, we'd disable the touchpad
after 3 seconds for the duration of the timeout, then again 3 seconds later,
etc.

Fix this by always setting the timeout and resetting the event counter if no
activity happened.

Signed-off-by: Peter Hutterer 
---
 src/evdev-mt-touchpad.c | 11 +++
 test/test-trackpoint.c  | 30 ++
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index 7b6825c0..2608cf0d 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -1945,8 +1945,10 @@ tp_trackpoint_timeout(uint64_t now, void *data)
 {
struct tp_dispatch *tp = data;
 
-   tp_tap_resume(tp, now);
-   tp->palm.trackpoint_active = false;
+   if (tp->palm.trackpoint_active) {
+   tp_tap_resume(tp, now);
+   tp->palm.trackpoint_active = false;
+   }
tp->palm.trackpoint_event_count = 0;
 }
 
@@ -1963,6 +1965,9 @@ tp_trackpoint_event(uint64_t time, struct libinput_event 
*event, void *data)
tp->palm.trackpoint_last_event_time = time;
tp->palm.trackpoint_event_count++;
 
+   libinput_timer_set(>palm.trackpoint_timer,
+  time + DEFAULT_TRACKPOINT_ACTIVITY_TIMEOUT);
+
/* Require at least three events before enabling palm detection */
if (tp->palm.trackpoint_event_count < 3)
return;
@@ -1972,8 +1977,6 @@ tp_trackpoint_event(uint64_t time, struct libinput_event 
*event, void *data)
tp->palm.trackpoint_active = true;
}
 
-   libinput_timer_set(>palm.trackpoint_timer,
-  time + DEFAULT_TRACKPOINT_ACTIVITY_TIMEOUT);
 }
 
 static void
diff --git a/test/test-trackpoint.c b/test/test-trackpoint.c
index a924f5e2..1f823331 100644
--- a/test/test-trackpoint.c
+++ b/test/test-trackpoint.c
@@ -379,6 +379,35 @@ START_TEST(trackpoint_palmdetect_require_min_events)
 }
 END_TEST
 
+START_TEST(trackpoint_palmdetect_require_min_events_timeout)
+{
+   struct litest_device *trackpoint = litest_current_device();
+   struct litest_device *touchpad;
+   struct libinput *li = trackpoint->libinput;
+
+   touchpad = litest_add_device(li, LITEST_SYNAPTICS_I2C);
+   litest_drain_events(li);
+
+   for (int i = 0; i < 10; i++) {
+   /* A single event does not trigger palm detection */
+   litest_event(trackpoint, EV_REL, REL_X, 1);
+   litest_event(trackpoint, EV_REL, REL_Y, 1);
+   litest_event(trackpoint, EV_SYN, SYN_REPORT, 0);
+   libinput_dispatch(li);
+   litest_drain_events(li);
+
+   litest_touch_down(touchpad, 0, 30, 30);
+   litest_touch_move_to(touchpad, 0, 30, 30, 80, 80, 10, 1);
+   litest_touch_up(touchpad, 0);
+   litest_assert_only_typed_events(li, 
LIBINPUT_EVENT_POINTER_MOTION);
+
+   litest_timeout_trackpoint();
+   }
+
+   litest_delete_device(touchpad);
+}
+END_TEST
+
 TEST_COLLECTION(trackpoint)
 {
litest_add("trackpoint:middlebutton", trackpoint_middlebutton, 
LITEST_POINTINGSTICK, LITEST_ANY);
@@ -392,4 +421,5 @@ TEST_COLLECTION(trackpoint)
litest_add("trackpoint:palmdetect", trackpoint_palmdetect, 
LITEST_POINTINGSTICK, LITEST_ANY);
litest_add("trackpoint:palmdetect", trackpoint_palmdetect_resume_touch, 
LITEST_POINTINGSTICK, LITEST_ANY);
litest_add("trackpoint:palmdetect", 
trackpoint_palmdetect_require_min_events, LITEST_POINTINGSTICK, LITEST_ANY);
+   litest_add("trackpoint:palmdetect", 
trackpoint_palmdetect_require_min_events_timeout, LITEST_POINTINGSTICK, 
LITEST_ANY);
 }
-- 
2.14.3

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v2 0/2] Add tablet_v2 support to weston-info

2018-04-26 Thread wl
From: Markus Ongyerth 

This is a v2 based on the suggestions from Simon on the v1.

Changes:
 * Changed C90/C++ style comment to C style
 * Changed the *_v2_listener struct instances to be const

Not changed:
 * struct tablet_path -> struct wl_array.
I think wl_array is lacking in usability for general purpose usage.
The struct is fine, but the surrounding code really doesn't scratch my itch.
If others think it's a better solution as well, I can change it, but I'm not a
fan

Cheers,
ongy

Markus Ongyerth (2):
  weston-info: Add support for tablet-unstable-v2
  weston-info: destroy wl_keyboard

 Makefile.am   |  14 +-
 clients/weston-info.c | 855 +-
 2 files changed, 860 insertions(+), 9 deletions(-)

-- 
2.17.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v2 2/2] weston-info: destroy wl_keyboard

2018-04-26 Thread wl
From: Markus Ongyerth 

Fixes a memory leak by calling wl_keyboard_destroy on a any keyboard
that was used to listen for events.

Signed-off-by: Markus Ongyerth 
---
 clients/weston-info.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/clients/weston-info.c b/clients/weston-info.c
index d1bdcbde..3dfea7d8 100644
--- a/clients/weston-info.c
+++ b/clients/weston-info.c
@@ -118,6 +118,7 @@ struct seat_info {
struct wl_seat *seat;
struct weston_info *info;
 
+   struct wl_keyboard *keyboard;
uint32_t capabilities;
char *name;
 
@@ -498,10 +499,8 @@ seat_handle_capabilities(void *data, struct wl_seat 
*wl_seat,
return;
 
if (caps & WL_SEAT_CAPABILITY_KEYBOARD) {
-   struct wl_keyboard *keyboard;
-
-   keyboard = wl_seat_get_keyboard(seat->seat);
-   wl_keyboard_add_listener(keyboard, _listener,
+   seat->keyboard = wl_seat_get_keyboard(seat->seat);
+   wl_keyboard_add_listener(seat->keyboard, _listener,
 seat);
 
seat->info->roundtrip_needed = true;
@@ -531,6 +530,9 @@ destroy_seat_info(void *data)
if (seat->name != NULL)
free(seat->name);
 
+   if (seat->keyboard)
+   wl_keyboard_destroy(seat->keyboard);
+
wl_list_remove(>global_link);
 }
 
-- 
2.17.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v2 1/2] weston-info: Add support for tablet-unstable-v2

2018-04-26 Thread wl
From: Markus Ongyerth 

This now prints each tablet seat with at least one tablet/pad/tool
attached.
For each tablet seat, each tablet, pad and tool is printed with as much
detail about the device as the protocol provides.
Seat info is stored to be referenced, because the protocol requires to
request a tablet_seat for each wl_seat and it's not guaranteed that the
tablet_v2_manager is available when seats are advertised.

Signed-off-by: Markus Ongyerth 
---
 Makefile.am   |  14 +-
 clients/weston-info.c | 845 ++
 2 files changed, 854 insertions(+), 5 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 69ca6cba..ac0f5471 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -824,11 +824,13 @@ weston_simple_im_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
 weston_info_SOURCES =  \
clients/weston-info.c   \
shared/helpers.h
-nodist_weston_info_SOURCES =   \
-   protocol/presentation-time-protocol.c   \
-   protocol/presentation-time-client-protocol.h\
-   protocol/linux-dmabuf-unstable-v1-protocol.c\
-   protocol/linux-dmabuf-unstable-v1-client-protocol.h
+nodist_weston_info_SOURCES =   \
+   protocol/presentation-time-protocol.c   \
+   protocol/presentation-time-client-protocol.h\
+   protocol/linux-dmabuf-unstable-v1-protocol.c\
+   protocol/linux-dmabuf-unstable-v1-client-protocol.h \
+   protocol/tablet-unstable-v2-protocol.c  \
+   protocol/tablet-unstable-v2-client-protocol.h
 weston_info_LDADD = $(WESTON_INFO_LIBS) libshared.la
 weston_info_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
 
@@ -888,6 +890,8 @@ BUILT_SOURCES +=\
protocol/ivi-application-client-protocol.h  \
protocol/linux-dmabuf-unstable-v1-protocol.c\
protocol/linux-dmabuf-unstable-v1-client-protocol.h \
+   protocol/tablet-unstable-v2-protocol.c  \
+   protocol/tablet-unstable-v2-client-protocol.h   \
protocol/input-timestamps-unstable-v1-protocol.c\
protocol/input-timestamps-unstable-v1-client-protocol.h
 
diff --git a/clients/weston-info.c b/clients/weston-info.c
index 386bd412..d1bdcbde 100644
--- a/clients/weston-info.c
+++ b/clients/weston-info.c
@@ -41,6 +41,7 @@
 #include "shared/zalloc.h"
 #include "presentation-time-client-protocol.h"
 #include "linux-dmabuf-unstable-v1-client-protocol.h"
+#include "tablet-unstable-v2-client-protocol.h"
 
 typedef void (*print_info_t)(void *info);
 typedef void (*destroy_info_t)(void *info);
@@ -113,6 +114,7 @@ struct linux_dmabuf_info {
 
 struct seat_info {
struct global_info global;
+   struct wl_list global_link;
struct wl_seat *seat;
struct weston_info *info;
 
@@ -123,6 +125,75 @@ struct seat_info {
int32_t repeat_delay;
 };
 
+struct tablet_v2_path {
+   struct wl_list link;
+   char *path;
+};
+
+struct tablet_tool_info {
+   struct wl_list link;
+   struct zwp_tablet_tool_v2 *tool;
+
+   uint64_t hardware_serial;
+   uint64_t hardware_id_wacom;
+   enum zwp_tablet_tool_v2_type type;
+   
+   int tilt;
+   int pressure;
+   int distance;
+   int rotation;
+   int slider;
+   int wheel;
+};
+
+struct tablet_pad_group_info {
+   struct wl_list link;
+   struct zwp_tablet_pad_group_v2 *group;
+
+   uint32_t modes;
+   size_t button_count;
+   int *buttons;
+   size_t strips;
+   size_t rings;
+};
+
+struct tablet_pad_info {
+   struct wl_list link;
+   struct zwp_tablet_pad_v2 *pad;
+
+   uint32_t buttons;
+   struct wl_list paths;
+   struct wl_list groups;
+};
+
+struct tablet_info {
+   struct wl_list link;
+   struct zwp_tablet_v2 *tablet;
+
+   char *name;
+   uint32_t vid, pid;
+   struct wl_list paths;
+};
+
+struct tablet_seat_info {
+   struct wl_list link;
+
+   struct zwp_tablet_seat_v2 *seat;
+   struct seat_info *seat_info;
+
+   struct wl_list tablets;
+   struct wl_list tools;
+   struct wl_list pads;
+};
+
+struct tablet_v2_info {
+   struct global_info global;
+   struct zwp_tablet_manager_v2 *manager;
+   struct weston_info *info;
+
+   struct wl_list seats;
+};
+
 struct presentation_info {
struct global_info global;
struct wp_presentation *presentation;
@@ -136,6 +207,10 @@ struct weston_info {
 
struct wl_list infos;
bool roundtrip_needed;
+
+   /* required for tablet-unstable-v2 */
+   struct wl_list seats;
+   struct tablet_v2_info *tablet_info;
 };
 
 static void
@@ -455,6 +530,767 @@ destroy_seat_info(void *data)
 
if (seat->name != NULL)
free(seat->name);
+
+ 

Re: [PATCH weston v9 9/9] Update copyrights for Collabora and General Electric Company

2018-04-26 Thread Pekka Paalanen
On Thu, 26 Apr 2018 14:48:14 +0100
Daniel Stone  wrote:

> Hi Pekka,
> 
> On 19 April 2018 at 13:09, Pekka Paalanen  wrote:
> > Looking at the diff statistics of the changes authored by me and landed
> > since 4.0.0 release points out these files as having major changes.
> > Update the copyright holders accordingly.
> >
> > I have kept the redundant "Copyright ©" form only to keep things
> > consistent, even when either the word or the mark would be enough.  
> 
> Might be worth noting that the GE copyright comes from the clone-mode
> work, since that's not necessarily obvious. That being said (and no
> need to send another version):
> Reviewed-by; Daniel Stone 

Hi,

I added a bit to the commit message and pushed this one patch:
   f39249ad..925788fc  master -> master

The touchscreen calibration series I'm revising needs to update
copyrights as well (or maybe doesn't after this one), so this helps
reducing the conflicts between the clone mode series and that one.


Thanks,
pq


pgpXUhwaMsLKr.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v9 9/9] Update copyrights for Collabora and General Electric Company

2018-04-26 Thread Daniel Stone
Hi Pekka,

On 19 April 2018 at 13:09, Pekka Paalanen  wrote:
> Looking at the diff statistics of the changes authored by me and landed
> since 4.0.0 release points out these files as having major changes.
> Update the copyright holders accordingly.
>
> I have kept the redundant "Copyright ©" form only to keep things
> consistent, even when either the word or the mark would be enough.

Might be worth noting that the GE copyright comes from the clone-mode
work, since that's not necessarily obvious. That being said (and no
need to send another version):
Reviewed-by; Daniel Stone 

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Finding a window's icon

2018-04-26 Thread Eike Hein


On 04/26/2018 07:49 AM, Nicholas Bishop wrote:
> My question is, does that work in practice? Do most applications
> actually set the application ID as suggested?

KDE/Qt and Gnome/GTK apps generally do. We've had to poke a few
apps in the broader ecosystem to fix things, but overall things
are a bit better and not any less reliable than the equivalent
.desktop<->WM_CLASS mapping on X.


Cheers,
Eike
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [Mesa-dev] [PATCH i-g-t] [RFC] CONTRIBUTING: commit rights docs

2018-04-26 Thread Emil Velikov
On 24 April 2018 at 20:14, Daniel Vetter  wrote:
> On Tue, Apr 24, 2018 at 7:30 PM, Emil Velikov  
> wrote:
>> On 13 April 2018 at 11:00, Daniel Vetter  wrote:
>>> This tries to align with the X.org communities's long-standing
>>> tradition of trying to be an inclusive community and handing out
>>> commit rights fairly freely.
>>>
>>> We also tend to not revoke commit rights for people no longer
>>> regularly active in a given project, as long as they're still part of
>>> the larger community.
>>>
>>> Finally make sure that commit rights, like anything happening on fd.o
>>> infrastructre, is subject to the fd.o's Code of Conduct.
>>>
>>> v2: Point at MAINTAINERS for contact info (Daniel S.)
>>>
>>> v3:
>>> - Make it clear that commit rights are voluntary and that committers
>>>   need to acknowledge positively when they're nominated by someone
>>>   else (Keith).
>>> - Encourage committers to drop their commit rights when they're no
>>>   longer active, and make it clear they'll get readded (Keith).
>>> - Add a line that maintainers and committers should actively nominate
>>>   new committers (me).
>>>
>>> v4: Typo (Petri).
>>>
>>> v5: Typo (Sean).
>>>
>>> v6: Wording clarifications and spelling (Jani).
>>>
>>> v7: Require an explicit commitment to the documented merge criteria
>>> and rules, instead of just the implied one through the Code of Conduct
>>> threat (Jani).
>>>
>>> Acked-by: Alex Deucher 
>>> Acked-by: Arkadiusz Hiler 
>>> Acked-by: Daniel Stone 
>>> Acked-by: Eric Anholt 
>>> Acked-by: Gustavo Padovan 
>>> Acked-by: Petri Latvala 
>>> Cc: Alex Deucher 
>>> Cc: Arkadiusz Hiler 
>>> Cc: Ben Widawsky 
>>> Cc: Daniel Stone 
>>> Cc: Dave Airlie 
>>> Cc: Eric Anholt 
>>> Cc: Gustavo Padovan 
>>> Cc: Jani Nikula 
>>> Cc: Joonas Lahtinen 
>>> Cc: Keith Packard 
>>> Cc: Kenneth Graunke 
>>> Cc: Kristian H. Kristensen 
>>> Cc: Maarten Lankhorst 
>>> Cc: Petri Latvala 
>>> Cc: Rodrigo Vivi 
>>> Cc: Sean Paul 
>>> Reviewed-by: Keith Packard 
>>> Signed-off-by: Daniel Vetter 
>>> ---
>>> If you wonder about the wide distribution list for an igt patch: I'd
>>> like to start a discussions about x.org community norms around commit
>>> rights at large, at least for all the shared repos. I plan to propose
>>> the same text for drm-misc and libdrm too, and hopefully others like
>>> mesa/xserver/wayland would follow.
>>>
>> I think the idea is pretty good, simply highlighting some bits.
>>
>> What you've outlined in this patch has been in practise for many years:
>>  a) undocumented, applicable to most xorg projects [1]
>>  b) documented, mesa
>
> Hm, I chatted with a few mesa devs about this, and I wasn't aware
> there's explicit documentation for mesa. Where is it? I'd very much
> want to align as much as we can.
>
See the "Developer git Access" section in [1]. FWIW I prefer the
wording used in this patch and the CoC reference is a big plus.

HTH
Emil

[1] https://www.mesa3d.org/repository.html
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [Mesa-dev] [PATCH i-g-t] [RFC] CONTRIBUTING: commit rights docs

2018-04-26 Thread Daniel Vetter
On Wed, Apr 25, 2018 at 01:27:20PM +0100, Emil Velikov wrote:
> On 24 April 2018 at 20:14, Daniel Vetter  wrote:
> > On Tue, Apr 24, 2018 at 7:30 PM, Emil Velikov  
> > wrote:
> >> On 13 April 2018 at 11:00, Daniel Vetter  wrote:
> >>> This tries to align with the X.org communities's long-standing
> >>> tradition of trying to be an inclusive community and handing out
> >>> commit rights fairly freely.
> >>>
> >>> We also tend to not revoke commit rights for people no longer
> >>> regularly active in a given project, as long as they're still part of
> >>> the larger community.
> >>>
> >>> Finally make sure that commit rights, like anything happening on fd.o
> >>> infrastructre, is subject to the fd.o's Code of Conduct.
> >>>
> >>> v2: Point at MAINTAINERS for contact info (Daniel S.)
> >>>
> >>> v3:
> >>> - Make it clear that commit rights are voluntary and that committers
> >>>   need to acknowledge positively when they're nominated by someone
> >>>   else (Keith).
> >>> - Encourage committers to drop their commit rights when they're no
> >>>   longer active, and make it clear they'll get readded (Keith).
> >>> - Add a line that maintainers and committers should actively nominate
> >>>   new committers (me).
> >>>
> >>> v4: Typo (Petri).
> >>>
> >>> v5: Typo (Sean).
> >>>
> >>> v6: Wording clarifications and spelling (Jani).
> >>>
> >>> v7: Require an explicit commitment to the documented merge criteria
> >>> and rules, instead of just the implied one through the Code of Conduct
> >>> threat (Jani).
> >>>
> >>> Acked-by: Alex Deucher 
> >>> Acked-by: Arkadiusz Hiler 
> >>> Acked-by: Daniel Stone 
> >>> Acked-by: Eric Anholt 
> >>> Acked-by: Gustavo Padovan 
> >>> Acked-by: Petri Latvala 
> >>> Cc: Alex Deucher 
> >>> Cc: Arkadiusz Hiler 
> >>> Cc: Ben Widawsky 
> >>> Cc: Daniel Stone 
> >>> Cc: Dave Airlie 
> >>> Cc: Eric Anholt 
> >>> Cc: Gustavo Padovan 
> >>> Cc: Jani Nikula 
> >>> Cc: Joonas Lahtinen 
> >>> Cc: Keith Packard 
> >>> Cc: Kenneth Graunke 
> >>> Cc: Kristian H. Kristensen 
> >>> Cc: Maarten Lankhorst 
> >>> Cc: Petri Latvala 
> >>> Cc: Rodrigo Vivi 
> >>> Cc: Sean Paul 
> >>> Reviewed-by: Keith Packard 
> >>> Signed-off-by: Daniel Vetter 
> >>> ---
> >>> If you wonder about the wide distribution list for an igt patch: I'd
> >>> like to start a discussions about x.org community norms around commit
> >>> rights at large, at least for all the shared repos. I plan to propose
> >>> the same text for drm-misc and libdrm too, and hopefully others like
> >>> mesa/xserver/wayland would follow.
> >>>
> >> I think the idea is pretty good, simply highlighting some bits.
> >>
> >> What you've outlined in this patch has been in practise for many years:
> >>  a) undocumented, applicable to most xorg projects [1]
> >>  b) documented, mesa
> >
> > Hm, I chatted with a few mesa devs about this, and I wasn't aware
> > there's explicit documentation for mesa. Where is it? I'd very much
> > want to align as much as we can.
> >
> See the "Developer git Access" section in [1]. FWIW I prefer the
> wording used in this patch and the CoC reference is a big plus.
> 
> HTH
> Emil
> 
> [1] https://www.mesa3d.org/repository.html

Ah missed this indeed. One thing to note wrt mesa is that this text here
relies heavily on _documented_ merge criteria. When I discussed it with
mesa we realized that the documented merge criteria do not really match
the actual criteria:

https://www.mesa3d.org/submittingpatches.html

E.g. for many drivers review is mandatory I think, same for core code. And
Intel folks require that you go through their CI too.

So the bigger part in adopting this for mesa would be in updating the
merge criteria doc to reflect reality.

Anyway, I'm happy that even the few terse lines match what I'm proposing
here (minus lots of details), I think we're good to go.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCHv6 wayland-protocols] Add name event to xdg-output

2018-04-26 Thread Jonas Ådahl
On Thu, Apr 26, 2018 at 01:20:28PM +0300, Pekka Paalanen wrote:
> On Thu, 26 Apr 2018 11:46:54 +0200
> Drew DeVault  wrote:
> 
> > On 2018-04-26 10:49 AM, Pekka Paalanen wrote:
> > > when someone merges this patch, please do add a commit message
> > > explaining why these events are added. See
> > > https://cgit.freedesktop.org/wayland/wayland/tree/doc/Contributing#n21
> > > for guidance on what to write.
> > > 
> > > Even if it seems obvious to everyone right now, it's not obvious after
> > > 5 years. I don't think the one line summary explains it.
> > > 
> > > For example, we had a long discussion about having just one event
> > > instead of two, what that one event would mean, and yet we ended up
> > > with two separate events (which I think is for the better). I would
> > > expect the commit message to explain why we have two events instead of
> > > one, since having one was the original and intuitive proposal.
> > > 
> > > Acked-by: Pekka Paalanen 
> > > 
> > > I would give R-b if the commit message was there. The protocol spec
> > > text looks good to me.  
> > 
> > I respectfully disagree. The commit message should pertain only to the
> > final approach, and historical information and timely commentary belongs
> > on the mailing list. 5 years from now, should it prove confusing,
> > looking up the mailing list posts will not be considerably more
> > difficult than looking up the commit message.
> 
> A commit must always document the "why exactly this change is being
> made", and here it is completely missing, even for the final approach.

There should be a commit message explaining the change, yes. Sorry for
missing that when reviewing.


Jonas

> 
> 
> Thanks,
> pq



> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCHv6 wayland-protocols] Add name event to xdg-output

2018-04-26 Thread Pekka Paalanen
On Thu, 26 Apr 2018 11:46:54 +0200
Drew DeVault  wrote:

> On 2018-04-26 10:49 AM, Pekka Paalanen wrote:
> > when someone merges this patch, please do add a commit message
> > explaining why these events are added. See
> > https://cgit.freedesktop.org/wayland/wayland/tree/doc/Contributing#n21
> > for guidance on what to write.
> > 
> > Even if it seems obvious to everyone right now, it's not obvious after
> > 5 years. I don't think the one line summary explains it.
> > 
> > For example, we had a long discussion about having just one event
> > instead of two, what that one event would mean, and yet we ended up
> > with two separate events (which I think is for the better). I would
> > expect the commit message to explain why we have two events instead of
> > one, since having one was the original and intuitive proposal.
> > 
> > Acked-by: Pekka Paalanen 
> > 
> > I would give R-b if the commit message was there. The protocol spec
> > text looks good to me.  
> 
> I respectfully disagree. The commit message should pertain only to the
> final approach, and historical information and timely commentary belongs
> on the mailing list. 5 years from now, should it prove confusing,
> looking up the mailing list posts will not be considerably more
> difficult than looking up the commit message.

A commit must always document the "why exactly this change is being
made", and here it is completely missing, even for the final approach.


Thanks,
pq


pgpq6jfsrYP1W.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCHv6 wayland-protocols] Add name event to xdg-output

2018-04-26 Thread Drew DeVault
On 2018-04-26 10:49 AM, Pekka Paalanen wrote:
> when someone merges this patch, please do add a commit message
> explaining why these events are added. See
> https://cgit.freedesktop.org/wayland/wayland/tree/doc/Contributing#n21
> for guidance on what to write.
> 
> Even if it seems obvious to everyone right now, it's not obvious after
> 5 years. I don't think the one line summary explains it.
> 
> For example, we had a long discussion about having just one event
> instead of two, what that one event would mean, and yet we ended up
> with two separate events (which I think is for the better). I would
> expect the commit message to explain why we have two events instead of
> one, since having one was the original and intuitive proposal.
> 
> Acked-by: Pekka Paalanen 
> 
> I would give R-b if the commit message was there. The protocol spec
> text looks good to me.

I respectfully disagree. The commit message should pertain only to the
final approach, and historical information and timely commentary belongs
on the mailing list. 5 years from now, should it prove confusing,
looking up the mailing list posts will not be considerably more
difficult than looking up the commit message.

--
Drew DeVault
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH 1/1] libweston: add weston_view_set_output()

2018-04-26 Thread Fabien Lahoudere
From: Semi Malinen 

Instead of desktop shell assigning view outputs directly,
use a new method, weston_view_set_output(). The method can
set up an output destroy listener to make sure that views
do not have stale output pointers.

Without this patch it is possible to end up in a scenario
where, e.g. configure_static_view() accesses memory that
has already been freed. The scenario can be provoked by
repeatedly plugging and unplugging a display. The faulty
memory accesses are reported by valgrind.

Signed-off-by: Semi Malinen 
Signed-off-by: Fabien Lahoudere 
---
 desktop-shell/shell.c  | 11 +++
 libweston/compositor.c | 43 +--
 libweston/compositor.h |  4 
 3 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index b846e30..1e153cb 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -580,7 +580,7 @@ create_focus_surface(struct weston_compositor *ec,
free(fsurf);
return NULL;
}
-   fsurf->view->output = output;
+   weston_view_set_output(fsurf->view, output);
fsurf->view->is_mapped = true;
 
weston_surface_set_size(surface, output->width, output->height);
@@ -2464,7 +2464,7 @@ map(struct desktop_shell *shell, struct shell_surface 
*shsurf,
shsurf->view->is_mapped = true;
if (shsurf->state.maximized) {
surface->output = shsurf->output;
-   shsurf->view->output = shsurf->output;
+   weston_view_set_output(shsurf->view, shsurf->output);
}
 
if (!shell->locked) {
@@ -2881,6 +2881,9 @@ configure_static_view(struct weston_view *ev, struct 
weston_layer *layer, int x,
 {
struct weston_view *v, *next;
 
+   if (!ev->output)
+   return;
+
wl_list_for_each_safe(v, next, >view_list.link, layer_link.link) 
{
if (v->output == ev->output && v != ev) {
weston_view_unmap(v);
@@ -2970,7 +2973,7 @@ desktop_shell_set_background(struct wl_client *client,
surface->committed_private = shell;
weston_surface_set_label_func(surface, background_get_label);
surface->output = weston_head_from_resource(output_resource)->output;
-   view->output = surface->output;
+   weston_view_set_output(view, surface->output);
 
sh_output = find_shell_output_from_weston_output(shell, 
surface->output);
if (sh_output->background_surface) {
@@ -3067,7 +3070,7 @@ desktop_shell_set_panel(struct wl_client *client,
surface->committed_private = shell;
weston_surface_set_label_func(surface, panel_get_label);
surface->output = weston_head_from_resource(output_resource)->output;
-   view->output = surface->output;
+   weston_view_set_output(view, surface->output);
 
sh_output = find_shell_output_from_weston_output(shell, 
surface->output);
if (sh_output->panel_surface) {
diff --git a/libweston/compositor.c b/libweston/compositor.c
index dbf61ef..9be66fc 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -1021,6 +1021,45 @@ weston_surface_update_output_mask(struct weston_surface 
*es, uint32_t mask)
}
 }
 
+static void
+notify_view_output_destroy(struct wl_listener *listener, void *data)
+{
+   struct weston_view *view =
+   container_of(listener,
+struct weston_view, output_destroy_listener);
+
+   view->output = NULL;
+   view->output_destroy_listener.notify = NULL;
+}
+
+/** Set the primary output of the view
+ *
+ * \param view The view whose primary output to set
+ * \param output The new primary output for the view
+ *
+ * Set \a output to be the primary output of the \a view.
+ *
+ * Notice that the assignment may be temporary; the primary output could be
+ * automatically changed. Hence, one cannot rely on the value persisting.
+ *
+ * Passing NULL as /a output will set the primary output to NULL.
+ */
+WL_EXPORT void
+weston_view_set_output(struct weston_view *view, struct weston_output *output)
+{
+   if (view->output_destroy_listener.notify) {
+   wl_list_remove(>output_destroy_listener.link);
+   view->output_destroy_listener.notify = NULL;
+   }
+   view->output = output;
+   if (output) {
+   view->output_destroy_listener.notify =
+   notify_view_output_destroy;
+   wl_signal_add(>destroy_signal,
+ >output_destroy_listener);
+   }
+}
+
 /** Recalculate which output(s) the surface has views displayed on
  *
  * \param es  The surface to remap to outputs
@@ -1112,7 +1151,7 @@ weston_view_assign_output(struct weston_view *ev)
}
pixman_region32_fini();
 
-   ev->output = new_output;
+   weston_view_set_output(ev, new_output);
ev->output_mask = 

Re: [PATCHv6 wayland-protocols] Add name event to xdg-output

2018-04-26 Thread Pekka Paalanen
On Tue, 24 Apr 2018 12:07:26 +0200
Drew DeVault  wrote:

> Signed-off-by: Drew DeVault 
> Reviewed-by: Simon Ser 
> Reviewed-by: Jonas Ådahl 
> ---
> Only change is the additional 

Hi,

when someone merges this patch, please do add a commit message
explaining why these events are added. See
https://cgit.freedesktop.org/wayland/wayland/tree/doc/Contributing#n21
for guidance on what to write.

Even if it seems obvious to everyone right now, it's not obvious after
5 years. I don't think the one line summary explains it.

For example, we had a long discussion about having just one event
instead of two, what that one event would mean, and yet we ended up
with two separate events (which I think is for the better). I would
expect the commit message to explain why we have two events instead of
one, since having one was the original and intuitive proposal.

Acked-by: Pekka Paalanen 

I would give R-b if the commit message was there. The protocol spec
text looks good to me.


Thanks,
pq

> 
>  .../xdg-output/xdg-output-unstable-v1.xml | 47 ++-
>  1 file changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/unstable/xdg-output/xdg-output-unstable-v1.xml 
> b/unstable/xdg-output/xdg-output-unstable-v1.xml
> index 0c0c481..6eed8b0 100644
> --- a/unstable/xdg-output/xdg-output-unstable-v1.xml
> +++ b/unstable/xdg-output/xdg-output-unstable-v1.xml
> @@ -54,7 +54,7 @@
>  reset.
>
>  
> -  
> +  
>  
>A global factory interface for xdg_output objects.
>  
> @@ -77,7 +77,7 @@
>  
>
>  
> -  
> +  
>  
>An xdg_output describes part of the compositor geometry.
>  
> @@ -157,5 +157,48 @@
>
>  
>  
> +
> +
> +  
> +Many compositors will assign names to their outputs, show them to the 
> user,
> +allow them to be configured by name, etc. The client may wish to know 
> this
> +name as well to offer the user similar behaviors.
> +
> +The naming convention is compositor defined, but limited to alphanumeric
> +characters and dashes (-). Each name is unique among all wl_output
> +globals, but if a wl_output global is destroyed the same name may be 
> reused
> +later. The names will also remain consistent across sessions with the 
> same
> +hardware and software configuration. 
>   
> +
> +Examples of names include 'HDMI-A-1', 'WL-1', 'X11-1', etc. However, do 
> not
> +assume that the name is a reflection of an underlying DRM connector, X11
> +connection, etc.
> +
> +The name event is sent after creating an xdg_output (see
> +xdg_output_manager.get_xdg_output). This event is only sent once per
> +xdg_output, and the name does not change over the lifetime of the 
> wl_output
> +global.
> +  
> +  
> +
> +
> +
> +  
> +Many compositors can produce human-readable descriptions of their 
> outputs.
> +The client may wish to know this description as well, to communicate the
> +user for various purposes.
> +
> +The description is a UTF-8 string with no convention defined for its
> +contents. Examples might include 'Foocorp 11" Display' or 'Virtual X11
> +output via :1'.
> +
> +The description event is sent after creating an xdg_output (see
> +xdg_output_manager.get_xdg_output). This event is only sent once per
> +xdg_output, and the description does not change over the lifetime of the
> +wl_output global. The description is optional, and may not be sent at 
> all.
> +  
> +  
> +
> +
>
>  



pgpbStU8rQG6A.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v5 0/3] simple-dmabuf-drm: Support etnaviv and freedreno cleanups

2018-04-26 Thread Pekka Paalanen
On Wed, 25 Apr 2018 15:19:24 +0200
Guido Günther  wrote:

> On Tue, Mar 20, 2018 at 09:41:57AM +0100, Guido Günther wrote:
> > Former patches 1-2 where already applied thanks Derek.
> > 
> > Changes from v4
> > - configure: use true if etnaviv drm was not found as with
> >   other drm backends (avoids an unused variable assignment)
> > - configure: use (hopefully) correct english
> > - simplify several if (cond) return 1; return 0; statements
> > - unmap buffer
> > 
> > Former patches 1-3 where already applied thanks Peka.
> > 
> > Changes from v3:
> > - Use ALIGN macro by default
> > - Don't multiply by bpp twice
> > - unmap buffer
> > 
> > Changes from v2:
> > - Use stride to calculate buffer size (etnaviv)
> > - Use stride to calculate buffer size (freedreno)
> > - Use less generic name for ALIGN macro
> > 
> > Changes from v1:
> > - Split up changes
> > - autoconf:
> >   - don't define unused have_ variables. Use true instead to
> > prevent abort of PKG_CHECK_MODULES
> >   - use && instead of -a in test
> >   - properly check variables
> > - use vfunc for drm_device_destroy
> > 
> > 
> > Guido Günther (3):
> >   simple-dmabuf-drm: 0 is a valid fd (freedreno)
> >   simple-dmabuf-drm: simplify fd_map_bo
> >   simple-dmabuf-drm: support etnaviv drm as well
> > 
> >  Makefile.am |  1 +
> >  clients/simple-dmabuf-drm.c | 79 
> > +++--
> >  configure.ac|  5 ++-
> >  3 files changed, 75 insertions(+), 10 deletions(-)  
> 
> Ping?

Still in my todo, no time so far to look into it, sorry. I wouldn't
mind at all if someone else reviewed and landed these before I get to
them.


Thanks,
pq


pgp515hiyn3Oh.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel