Re: [PATCH v2 2/2] weston-info: destroy wl_keyboard
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
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
From: Markus OngyerthThis 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
From: Markus OngyerthFixes 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
From: Markus OngyerthThis 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
On Thu, 26 Apr 2018 14:48:14 +0100 Daniel Stonewrote: > 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
Hi Pekka, On 19 April 2018 at 13:09, Pekka Paalanenwrote: > 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
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
On 24 April 2018 at 20:14, Daniel Vetterwrote: > 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
On Wed, Apr 25, 2018 at 01:27:20PM +0100, Emil Velikov wrote: > On 24 April 2018 at 20:14, Daniel Vetterwrote: > > 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
On Thu, Apr 26, 2018 at 01:20:28PM +0300, Pekka Paalanen wrote: > On Thu, 26 Apr 2018 11:46:54 +0200 > Drew DeVaultwrote: > > > 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
On Thu, 26 Apr 2018 11:46:54 +0200 Drew DeVaultwrote: > 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
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()
From: Semi MalinenInstead 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
On Tue, 24 Apr 2018 12:07:26 +0200 Drew DeVaultwrote: > 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
On Wed, 25 Apr 2018 15:19:24 +0200 Guido Güntherwrote: > 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