Re: weston: weston randr protocol for testing and configuration
On Thu, 27 Mar 2014 04:08:15 + "Wang, Quanxian" wrote: > Hi, Pq > > The information to identify the unique mode: width, height and refresh are > enough? Not enough in theory. But is enough in real world. I have checked > with xrandr. Read the following comment. > > Welcome any comment for that. > > Thanks > > >> + > >> + > >> + > >> + Set the mode of output. > >> + > >> + >> + summary="the output object"/> > >> + > >> + > >> + > > > >So this is the simple mode set request. > > > >Do you think width/height/refresh is really enough to identify a mode? I > >don't > >think so. I think in the early days of X11 RandR, NVidia hit the same > >problem, and > >started to expose fake refresh values, only to be able to distinguish modes > >that > >were identical in width/height/refresh but still different in timings. Or > >actually I > >think it was much more complicated than that, but this is the point in > >simple terms. > > > >So we need something else here to identify a mode. > > > >Check what kind of protocol GNOME uses, and how current RandR protocol works. > [Wang, Quanxian] Hi, Pq > Your understanding are right in theory. But in reality, it is barely > possible. Width and height could be easily same, not easy for refresh at the > same time. Currently in xrandr, they use mode name to identify one mode (for > example widthxheight_refresh). in xrandr process, I don't find they take mode > information to compare. Maybe I missed, but from xrandr parameters, there is > no such option. They just take width, height, refresh as mode name to > identify one mode. Sometimes, only width and height. If we want to fully > support one unique mode, all the mode information have to be compared. > (clock, hdisplay, hsync_start, hsync_end, htotal, vdisplay, vsync_start, > vsync_end, vtotal, flags). But it is not convenient. Sometime, user basically > don't know what hdisplay is at all. They just know widthxheight_refresh. My > idea is just take width, height, refresh as the unique id for mode. > I assume the X server has the database of modes. I cannot think of any reason why the xrandr client would be comparing modes at all. Why did you expect that? Isn't the name of a mode in RandR just an arbitrary string (perhaps mapped to an XID)? These were introduced in RandR 1.2 according to the xrandr man page. If you are looking at RandR 1.1, it is not sufficient for distinguishing custom modes. Say, how will you separate a normal mode from a reduced-blanking mode? They can have the same width, height, and refresh rate, but one works while the other is not reliable. I have had such monitors before. Of course, you have the choice to not support custom modes with custom timings. Is your intention to support only custom modes with "standard" timings, but not with custom timings? If so, which standard do you pick? VESA CVT? Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH fullscreen-shell v5 18/18] Add a screen sharing plugin
Hi Jason, A few comments I have from trying out your fullscreen-shell changes: On Tue, 18 Mar 2014, Jason Ekstrand wrote: +static struct ss_seat * +ss_seat_create(struct shared_output *so, uint32_t id) +{ + struct ss_seat *seat; + + seat = zalloc(sizeof *seat); + if (seat == NULL) + return NULL; + + weston_seat_init(&seat->base, so->output->compositor, "default"); It would be great if we could transfer the seat name from the parent seat. In the case of VNC or RDP this could identify the remote user. + seat->output = so; + seat->parent.seat = wl_registry_bind(so->parent.registry, id, + &wl_seat_interface, 1); + wl_list_insert(so->seat_list.prev, &seat->link); + + wl_seat_add_listener(seat->parent.seat, &ss_seat_listener, seat); + wl_seat_set_user_data(seat->parent.seat, seat); + + return seat; +} +static struct shared_output * +shared_output_create(struct weston_output *output, int parent_fd) +{ + struct shared_output *so; + struct wl_event_loop *loop; + struct ss_seat *seat; + int epoll_fd; + + so = zalloc(sizeof *so); + if (so == NULL) + goto err_close; + + wl_list_init(&so->seat_list); + + so->parent.display = wl_display_connect_to_fd(parent_fd); + if (!so->parent.display) + goto err_alloc; + + so->parent.registry = wl_display_get_registry(so->parent.display); + if (!so->parent.registry) + goto err_display; + wl_registry_add_listener(so->parent.registry, +®istry_listener, so); + wl_display_roundtrip(so->parent.display); + if (so->parent.shm == NULL) { + weston_log("Screen share failed: No wl_shm found\n"); + goto err_display; + } + if (so->parent.fshell == NULL) { + weston_log("Screen share failed: " + "Parent does not support wl_fullscreen_shell\n"); + goto err_display; + } + if (so->parent.compositor == NULL) { + weston_log("Screen share failed: No wl_compositor found\n"); + goto err_display; + } + + /* Get SHM formats */ + wl_display_roundtrip(so->parent.display); + if (!(so->parent.shm_formats & (1 << WL_SHM_FORMAT_XRGB))) { + weston_log("Screen share failed: " + "WL_SHM_FORMAT_XRGB not available\n"); + goto err_display; + } + + so->parent.surface = + wl_compositor_create_surface(so->parent.compositor); + if (!so->parent.surface) { + weston_log("Screen share failed: %m"); + goto err_display; + } + + so->parent.mode_feedback = + _wl_fullscreen_shell_present_surface_for_mode(so->parent.fshell, + so->parent.surface, + so->parent.output, + output->current_mode->refresh); + if (!so->parent.mode_feedback) { + weston_log("Screen share failed: %m"); + goto err_display; + } + _wl_fullscreen_shell_mode_feedback_add_listener(so->parent.mode_feedback, + &mode_feedback_listener, + so); + + loop = wl_display_get_event_loop(output->compositor->wl_display); + + epoll_fd = wl_display_get_fd(so->parent.display); + so->event_source = + wl_event_loop_add_fd(loop, epoll_fd, WL_EVENT_READABLE, +shared_output_handle_event, so); + if (!so->event_source) { + weston_log("Screen share failed: %m"); + goto err_display; + } + + /* Ok, everything's created. We should be good to go */ + wl_list_init(&so->shm.buffers); + wl_list_init(&so->shm.free_buffers); + + so->output = output; + so->output_destroyed.notify = output_destroyed; + wl_signal_add(&so->output->destroy_signal, &so->output_destroyed); + + so->frame_listener.notify = shared_output_repainted; + wl_signal_add(&output->frame_signal, &so->frame_listener); + output->disable_planes++; I think you should only disable planes if the fullscreen_shell doesn't have the cursor_plane capability. In that case, there also needs to be a way to update the pointer surface on the parent seat. Although I think the difficulty may be in detecting when the pointer image changes - at least I remember that being a problem in my previous attempt at doing this. I guess you also need a corresponding disable_planes-- when screen sharing stops? + weston_output_damage(output); + + return so; + +err_display: + wl_list_for_each(seat, &so->se
Re: weston: weston randr protocol for testing and configuration
I don't think the user really knows what refresh is either. I'm actually curious: is there a reason to ever expose different modes to the user that have the same width/height but different timings? What's the rationale for choosing one instead of the other? I know nothing about display panels, keep in mind. I'm also assuming a modern system here with a modern display panel, so no saying "the refresh means the CRT updates faster and it's brighter and less less flicker" On Thu, Mar 27, 2014 at 6:35 AM, Pekka Paalanen wrote: > On Thu, 27 Mar 2014 04:08:15 + > "Wang, Quanxian" wrote: > > > Hi, Pq > > > > The information to identify the unique mode: width, height and refresh > are enough? Not enough in theory. But is enough in real world. I have > checked with xrandr. Read the following comment. > > > > Welcome any comment for that. > > > > Thanks > > > > >> + > > >> + > > >> + > > >> + Set the mode of output. > > >> + > > >> + > >> + summary="the output object"/> > > >> + > > >> + > > >> + > > > > > >So this is the simple mode set request. > > > > > >Do you think width/height/refresh is really enough to identify a mode? > I don't > > >think so. I think in the early days of X11 RandR, NVidia hit the same > problem, and > > >started to expose fake refresh values, only to be able to distinguish > modes that > > >were identical in width/height/refresh but still different in timings. > Or actually I > > >think it was much more complicated than that, but this is the point in > simple terms. > > > > > >So we need something else here to identify a mode. > > > > > >Check what kind of protocol GNOME uses, and how current RandR protocol > works. > > [Wang, Quanxian] Hi, Pq > > Your understanding are right in theory. But in reality, it is barely > possible. Width and height could be easily same, not easy for refresh at > the same time. Currently in xrandr, they use mode name to identify one mode > (for example widthxheight_refresh). in xrandr process, I don't find they > take mode information to compare. Maybe I missed, but from xrandr > parameters, there is no such option. They just take width, height, refresh > as mode name to identify one mode. Sometimes, only width and height. If we > want to fully support one unique mode, all the mode information have to be > compared. (clock, hdisplay, hsync_start, hsync_end, htotal, vdisplay, > vsync_start, vsync_end, vtotal, flags). But it is not convenient. Sometime, > user basically don't know what hdisplay is at all. They just know > widthxheight_refresh. My idea is just take width, height, refresh as the > unique id for mode. > > > > I assume the X server has the database of modes. I cannot think of any > reason why the xrandr client would be comparing modes at all. Why did > you expect that? Isn't the name of a mode in RandR just an arbitrary > string (perhaps mapped to an XID)? > > These were introduced in RandR 1.2 according to the xrandr man page. If > you are looking at RandR 1.1, it is not sufficient for distinguishing > custom modes. > > Say, how will you separate a normal mode from a reduced-blanking mode? > They can have the same width, height, and refresh rate, but one works > while the other is not reliable. I have had such monitors before. > > Of course, you have the choice to not support custom modes with custom > timings. Is your intention to support only custom modes with "standard" > timings, but not with custom timings? If so, which standard do you > pick? VESA CVT? > > > Thanks, > pq > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > -- Jasper ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: weston: weston randr protocol for testing and configuration
On Thu, 27 Mar 2014 08:31:34 -0400 "Jasper St. Pierre" wrote: > I don't think the user really knows what refresh is either. > > I'm actually curious: is there a reason to ever expose different modes to > the user that have the same width/height but different timings? What's the > rationale for choosing one instead of the other? I know nothing about > display panels, keep in mind. This is not for users, the whole weston-randr protocol is for developer and administrator tools for testing different modes without having to restart Weston and edit a config file in between. I cannot see a reason to offer the same mode with different timings during normal operations, but it would be useful for the very kind of experimentation that this protocol extension is designed for. At least, as far as I have understood. How far this interface should go is still unclear to me. If Quanxian wants to stick with RandR 1.1 features, i.e. no dynamic custom modelines, that's ok, though to me it seems like losing a good part of the possible benefits. > I'm also assuming a modern system here with a modern display panel, so no > saying "the refresh means the CRT updates faster and it's brighter and less > less flicker" My 1600x1200 monitor, connected via DVI-D, was unstable, going black once in a while when ran with a normal timings mode at 60 Hz. Switching to reduced blanking mode made it stable and got rid of the random green snow. Yes, you can blame the hardware being on its edge. The refresh rate was exactly the same. The reduced blanking mode just made the pixel clock a bit lower, enough to make the picture stable. Thanks, pq > On Thu, Mar 27, 2014 at 6:35 AM, Pekka Paalanen wrote: > > > On Thu, 27 Mar 2014 04:08:15 + > > "Wang, Quanxian" wrote: > > > > > Hi, Pq > > > > > > The information to identify the unique mode: width, height and refresh > > are enough? Not enough in theory. But is enough in real world. I have > > checked with xrandr. Read the following comment. > > > > > > Welcome any comment for that. > > > > > > Thanks > > > > > > >> + > > > >> + > > > >> + > > > >> + Set the mode of output. > > > >> + > > > >> + > > >> + summary="the output object"/> > > > >> + > > > >> + > > > >> + > > > > > > > >So this is the simple mode set request. > > > > > > > >Do you think width/height/refresh is really enough to identify a mode? > > I don't > > > >think so. I think in the early days of X11 RandR, NVidia hit the same > > problem, and > > > >started to expose fake refresh values, only to be able to distinguish > > modes that > > > >were identical in width/height/refresh but still different in timings. > > Or actually I > > > >think it was much more complicated than that, but this is the point in > > simple terms. > > > > > > > >So we need something else here to identify a mode. > > > > > > > >Check what kind of protocol GNOME uses, and how current RandR protocol > > works. > > > [Wang, Quanxian] Hi, Pq > > > Your understanding are right in theory. But in reality, it is barely > > possible. Width and height could be easily same, not easy for refresh at > > the same time. Currently in xrandr, they use mode name to identify one mode > > (for example widthxheight_refresh). in xrandr process, I don't find they > > take mode information to compare. Maybe I missed, but from xrandr > > parameters, there is no such option. They just take width, height, refresh > > as mode name to identify one mode. Sometimes, only width and height. If we > > want to fully support one unique mode, all the mode information have to be > > compared. (clock, hdisplay, hsync_start, hsync_end, htotal, vdisplay, > > vsync_start, vsync_end, vtotal, flags). But it is not convenient. Sometime, > > user basically don't know what hdisplay is at all. They just know > > widthxheight_refresh. My idea is just take width, height, refresh as the > > unique id for mode. > > > > > > > I assume the X server has the database of modes. I cannot think of any > > reason why the xrandr client would be comparing modes at all. Why did > > you expect that? Isn't the name of a mode in RandR just an arbitrary > > string (perhaps mapped to an XID)? > > > > These were introduced in RandR 1.2 according to the xrandr man page. If > > you are looking at RandR 1.1, it is not sufficient for distinguishing > > custom modes. > > > > Say, how will you separate a normal mode from a reduced-blanking mode? > > They can have the same width, height, and refresh rate, but one works > > while the other is not reliable. I have had such monitors before. > > > > Of course, you have the choice to not support custom modes with custom > > timings. Is your intention to support only custom modes with "standard" > > timings, but not with custom timings? If so, which standard do you > > pick? VESA CVT? > > > > > > Thanks, > > pq > > ___ > > wayland-devel mailing list > > wayland-devel@lists.freedeskto
Re: weston: weston randr protocol for testing and configuration
I mean, I'm mostly talking about user configuration in the case of a desktop, when the user wants to change the mode of the display. Obviously, it would be best if we could detect hardware edge cases like that where it's going on the fritz and showing a green tint automatically, and simply not offer the user a broken mode in the first place, but I don't want to get a scolding from Adam Jackson, so I'll shut up. On Thu, Mar 27, 2014 at 9:05 AM, Pekka Paalanen wrote: > On Thu, 27 Mar 2014 08:31:34 -0400 > "Jasper St. Pierre" wrote: > > > I don't think the user really knows what refresh is either. > > > > I'm actually curious: is there a reason to ever expose different modes to > > the user that have the same width/height but different timings? What's > the > > rationale for choosing one instead of the other? I know nothing about > > display panels, keep in mind. > > This is not for users, the whole weston-randr protocol is for > developer and administrator tools for testing different modes without > having to restart Weston and edit a config file in between. > > I cannot see a reason to offer the same mode with different timings > during normal operations, but it would be useful for the very kind of > experimentation that this protocol extension is designed for. > > At least, as far as I have understood. How far this interface should go > is still unclear to me. If Quanxian wants to stick with RandR 1.1 > features, i.e. no dynamic custom modelines, that's ok, though to me it > seems like losing a good part of the possible benefits. > > > I'm also assuming a modern system here with a modern display panel, so no > > saying "the refresh means the CRT updates faster and it's brighter and > less > > less flicker" > > My 1600x1200 monitor, connected via DVI-D, was unstable, going black > once in a while when ran with a normal timings mode at 60 Hz. Switching > to reduced blanking mode made it stable and got rid of the random green > snow. Yes, you can blame the hardware being on its edge. > > The refresh rate was exactly the same. The reduced blanking mode just > made the pixel clock a bit lower, enough to make the picture stable. > > > Thanks, > pq > > > On Thu, Mar 27, 2014 at 6:35 AM, Pekka Paalanen > wrote: > > > > > On Thu, 27 Mar 2014 04:08:15 + > > > "Wang, Quanxian" wrote: > > > > > > > Hi, Pq > > > > > > > > The information to identify the unique mode: width, height and > refresh > > > are enough? Not enough in theory. But is enough in real world. I have > > > checked with xrandr. Read the following comment. > > > > > > > > Welcome any comment for that. > > > > > > > > Thanks > > > > > > > > >> + > > > > >> + > > > > >> + > > > > >> + Set the mode of output. > > > > >> + > > > > >> + > > > >> + summary="the output object"/> > > > > >> + > > > > >> + > > > > >> + > > > > > > > > > >So this is the simple mode set request. > > > > > > > > > >Do you think width/height/refresh is really enough to identify a > mode? > > > I don't > > > > >think so. I think in the early days of X11 RandR, NVidia hit the > same > > > problem, and > > > > >started to expose fake refresh values, only to be able to > distinguish > > > modes that > > > > >were identical in width/height/refresh but still different in > timings. > > > Or actually I > > > > >think it was much more complicated than that, but this is the point > in > > > simple terms. > > > > > > > > > >So we need something else here to identify a mode. > > > > > > > > > >Check what kind of protocol GNOME uses, and how current RandR > protocol > > > works. > > > > [Wang, Quanxian] Hi, Pq > > > > Your understanding are right in theory. But in reality, it is barely > > > possible. Width and height could be easily same, not easy for refresh > at > > > the same time. Currently in xrandr, they use mode name to identify one > mode > > > (for example widthxheight_refresh). in xrandr process, I don't find > they > > > take mode information to compare. Maybe I missed, but from xrandr > > > parameters, there is no such option. They just take width, height, > refresh > > > as mode name to identify one mode. Sometimes, only width and height. > If we > > > want to fully support one unique mode, all the mode information have > to be > > > compared. (clock, hdisplay, hsync_start, hsync_end, htotal, vdisplay, > > > vsync_start, vsync_end, vtotal, flags). But it is not convenient. > Sometime, > > > user basically don't know what hdisplay is at all. They just know > > > widthxheight_refresh. My idea is just take width, height, refresh as > the > > > unique id for mode. > > > > > > > > > > I assume the X server has the database of modes. I cannot think of any > > > reason why the xrandr client would be comparing modes at all. Why did > > > you expect that? Isn't the name of a mode in RandR just an arbitrary > > > string (perhaps mapped to an XID)? > > > > > > These were introduced in RandR 1.2 according to the xrandr man page. I
Re: [PATCH libinput] test: add litest helper functions for creating uinput devices
On Thu, Mar 27, 2014 at 08:48:02AM +1000, Peter Hutterer wrote: > Both functions accept a series of event types/codes tuples, terminated by -1. > For the even type INPUT_PROP_MAX (an invalid type otherwise) the code is used > as a property to enable. > > The _abs function als takes an array of absinfo, with absinfo.value > determining the axis to change. If none are given, abs axes are initialized > with default settings. > > Both functions abort on failure, so the caller does not need to check the > return value. > > Example code for creating a rel device: > > struct libevdev_uinput *uinput; > struct input_id id = { ... }; > uinput = litest_create_uinput_device("foo", &id, > EV_REL, REL_X, > EV_REL, REL_Y, > EV_KEY, BTN_LEFT, >INPUT_PROP_MAX, INPUT_PROP_BUTTONPAD, > -1); > libevdev_uinput_write_event(uinput, EV_REL, REL_X, -1); > libevdev_uinput_write_event(uinput, EV_SYN, SYN_REPORT, 0); > .. > libevdev_uinput_destroy(uinput); The problem with this approach is that its no longer possible to use the litest_touch_down, litest_touch_up etc any more. Especially for touch events thats an annoyance to reimplement in the test case. We could add helpers for that but then we'd have double set of helpers for every kind of event we want to test. The way I'd want to use something like this is to create a fake device, possibly with some special parameters, and then use it in the same way I'd use a "normal" test device. I'd rather have a way to somehow create a special fake device that can be operated in the same way using the same functions as a "normal" fake device. Jonas > > Signed-off-by: Peter Hutterer > --- > test/litest-bcm5974.c | 43 +++- > test/litest-generic-highres-touch.c | 34 +++--- > test/litest-synaptics-st.c | 37 +++ > test/litest-synaptics.c | 43 +++- > test/litest-trackpoint.c| 32 +++--- > test/litest-wacom-touch.c | 33 ++ > test/litest.c | 89 > test/litest.h | 9 ++ > test/path.c | 207 > > 9 files changed, 244 insertions(+), 283 deletions(-) > > diff --git a/test/litest-bcm5974.c b/test/litest-bcm5974.c > index 5a8ce8a..c76a892 100644 > --- a/test/litest-bcm5974.c > +++ b/test/litest-bcm5974.c > @@ -95,7 +95,6 @@ static struct litest_device_interface interface = { > void > litest_create_bcm5974(struct litest_device *d) > { > - struct libevdev *dev; > struct input_absinfo abs[] = { > { ABS_X, 1472, 5472, 75 }, > { ABS_Y, 1408, 4448, 129 }, > @@ -107,34 +106,24 @@ litest_create_bcm5974(struct litest_device *d) > { ABS_MT_TRACKING_ID, 0, 65535, 0 }, > { ABS_MT_PRESSURE, 0, 255, 0 } > }; > - struct input_absinfo *a; > - int rc; > + struct input_id id = { > + .bustype = 0x3, > + .vendor = 0x5ac, > + .product = 0x249, > + }; > > d->interface = &interface; > - > - dev = libevdev_new(); > - ck_assert(dev != NULL); > - > - libevdev_set_name(dev, "bcm5974"); > - libevdev_set_id_bustype(dev, 0x3); > - libevdev_set_id_vendor(dev, 0x5ac); > - libevdev_set_id_product(dev, 0x249); > - libevdev_enable_event_code(dev, EV_KEY, BTN_LEFT, NULL); > - libevdev_enable_event_code(dev, EV_KEY, BTN_TOOL_FINGER, NULL); > - libevdev_enable_event_code(dev, EV_KEY, BTN_TOOL_QUINTTAP, NULL); > - libevdev_enable_event_code(dev, EV_KEY, BTN_TOUCH, NULL); > - libevdev_enable_event_code(dev, EV_KEY, BTN_TOOL_DOUBLETAP, NULL); > - libevdev_enable_event_code(dev, EV_KEY, BTN_TOOL_TRIPLETAP, NULL); > - libevdev_enable_event_code(dev, EV_KEY, BTN_TOOL_QUADTAP, NULL); > - > - ARRAY_FOR_EACH(abs, a) > - libevdev_enable_event_code(dev, EV_ABS, a->value, a); > - > - rc = libevdev_uinput_create_from_device(dev, > - LIBEVDEV_UINPUT_OPEN_MANAGED, > - &d->uinput); > - ck_assert_int_eq(rc, 0); > - libevdev_free(dev); > + d->uinput = litest_create_uinput_abs_device("bcm5974", &id, > + ARRAY_LENGTH(abs), > + abs, > + EV_KEY, BTN_LEFT, > + EV_KEY, BTN_TOOL_FINGER, > + EV_KEY, BTN_TOOL_QUINTTAP, > + EV_KEY, BTN_TOUCH, > + EV_KEY, BTN_TOOL_DOUBLETAP, > + EV_KEY, BTN_TOOL_T
[ANNOUNCE] libxkbcommon-0.4.1
We've accumulated some changes and bug fixes, so here's a new release. libxkbcommon 0.4.1 == - Converted README to markdown and added a Quick Guide to the documentation, which breezes through the most common parts of xkbcommon. Link: http://xkbcommon.org/doc/current/md_doc_quick-guide.html - Added two new functions, xkb_state_key_get_utf{8,32}(). They combine the operations of xkb_state_key_get_syms() and xkb_keysym_to_utf{8,32}(), and provide a nicer interface for it (espcially for multiple-keysyms-per-level). - The xkb_state_key_get_utf{8,32}() functions now apply Control transformation: when the Control modifier is active, the string is converted to an appropriate control character. This matches the behavior of libX11's XLookupString(3), and required by the XKB specification: http://www.x.org/releases/current/doc/kbproto/xkbproto.html#Interpreting_the_Control_Modifier https://bugs.freedesktop.org/show_bug.cgi?id=75892 - The consumed modifiers for a key are now calculated similarly to libX11. The previous behavior caused a bug where Shift would not cancel an active Caps Lock. - Make xkbcommon-x11 work with the keymap reported by the XQuartz X server. https://bugs.freedesktop.org/show_bug.cgi?id=75798 - Reduce memory usage during keymap compilation some more. - New API: xkb_state_key_get_consumed_mods() xkb_state_key_get_utf8() xkb_state_key_get_utf32() - Deprecated API: XKB_MAP_COMPILE_PLACEHOLDER, XKB_MAP_NO_FLAGS use XKB_KEYMAP_NO_FLAGS instead. - Bug fixes. Tarballs: git tag: xkbcommon-0.4.1 http://xkbcommon.org/download/libxkbcommon-0.4.1.tar.xz MD5: b70f4ed97b6c9432dc956e4177f3336a libxkbcommon-0.4.1.tar.xz SHA1: 38814e7edea16d032463b853490dda9040e5b1 libxkbcommon-0.4.1.tar.xz Ran ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [ANNOUNCE] libxkbcommon-0.4.1
Hi On Thu, Mar 27, 2014 at 8:22 PM, Ran Benita wrote: > - Added two new functions, xkb_state_key_get_utf{8,32}(). They > combine the operations of xkb_state_key_get_syms() and > xkb_keysym_to_utf{8,32}(), and provide a nicer interface for it > (espcially for multiple-keysyms-per-level). Slightly off-topic, but looking at the code, you ignore any multi-sym values (nsyms != 1). Users of that API might rely on that behavior, so have we at some point defined what exactly multi-sym means? Because if we add more and more APIs and if the user-base grows, we might at some point be unable to make use of multi-symbol behavior. I'm still not sure whether nsyms > 1 just means multiple sequential keysyms, or whether they should be handled as _one_ atomic combined keysym? > - The xkb_state_key_get_utf{8,32}() functions now apply Control > transformation: when the Control modifier is active, the string > is converted to an appropriate control character. > This matches the behavior of libX11's XLookupString(3), and > required by the XKB specification: > > http://www.x.org/releases/current/doc/kbproto/xkbproto.html#Interpreting_the_Control_Modifier > > https://bugs.freedesktop.org/show_bug.cgi?id=75892 So xkb_state_key_get_utf8(state, code) != xkb_keysym_to_utf8(xkb_state_key_get_one_sym(state, code))? (at least for ctrl+) Could we at least add a note/warning to the documentation? I cannot find anything here: http://xkbcommon.org/doc/current/group__state.html#ga0774b424063b45c88ec0354c77f9a247 For instance, for libtsm, I accept combined modifier+keysym+ucs4string input from the GUI layer and transform that internally. If the upper layer uses xkb_state_key_get_utf32() instead of xkb_keysym_to_utf32(), the logic will force the control-seq even though it might be mapped differently by the vte-layer (for instance, there's stuff like shifting values by 64 if ALT is pressed, and more legacy crap we need to support for eternity). Sure, most libtsm code uses the keysym, but for glyph transformation we have to use the ucs4 value. Imagine the ctrl+c input was shifted by the term-layer, you still end up with the \003 glyph, because the ucs4 string was mapped by xkbcommon. I'm not saying the patch is wrong, in fact, the layout-search logic is actually what you wrote for kmscon and I appreciate that. I'm just saying that it's a really subtle API difference that can introduce weird bugs. Lets see how it works out, but if people start using xkb_state_key_get_utf32(), I might send a patch adding an xkb-state flag that disables this transformation. Or just force users to not use it (which would be unfortunate). Btw., same is true for the implicit caps-lock magic in xkb_state_key_get_one_sym(). I'm now quite confused whether xkb_state_key_get_syms() users have to do caps-lock handling explicitly? Or is that done by keymaps? Anyhow, thanks for the release! David ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [ANNOUNCE] libxkbcommon-0.4.1
On Thu, Mar 27, 2014 at 09:04:07PM +0100, David Herrmann wrote: > Hi Hi David > On Thu, Mar 27, 2014 at 8:22 PM, Ran Benita wrote: > > - Added two new functions, xkb_state_key_get_utf{8,32}(). They > > combine the operations of xkb_state_key_get_syms() and > > xkb_keysym_to_utf{8,32}(), and provide a nicer interface for it > > (espcially for multiple-keysyms-per-level). > > Slightly off-topic, but looking at the code, you ignore any multi-sym > values (nsyms != 1). Users of that API might rely on that behavior, so > have we at some point defined what exactly multi-sym means? Because if > we add more and more APIs and if the user-base grows, we might at some > point be unable to make use of multi-symbol behavior. I'm still not > sure whether nsyms > 1 just means multiple sequential keysyms, or > whether they should be handled as _one_ atomic combined keysym? First, the reason I added these functions is that https://bugs.freedesktop.org/show_bug.cgi?id=75892 made it necessary, I would otherwise like to avoid adding new API at this point. But given the situation, I tried to make the best of it. I only ignore multiple-keysyms for the utf32 function -- I only added it to match the existing xkb_keysym_to_utf32() function. I don't think it makes sense to encode multiple-keysyms into a UTF-32 string, nobody uses it. A better name would be xkb_keysym_to_unicode_codepoint() or whatever, but I wanted to keep the existing name. For the utf8 function, it is handled correctly, and is actually much easier to use for multiple-keysyms than the old function. It builds the string internally and finally ensures it's valid UTF-8. Another advantage of adding new functions is that existing users don't break due to the new behavior, they must opt-in. Regarding intended use-case for multiple-keysyms, I consider it mainly to be for sequences with combining characters - not everything has precomposed codepoints, so if you want one of these, you don't have a way to do it with single-keysym, but it still conceptually fits in a keymap. However for the original intent you have to ask Daniel. > > - The xkb_state_key_get_utf{8,32}() functions now apply Control > > transformation: when the Control modifier is active, the string > > is converted to an appropriate control character. > > This matches the behavior of libX11's XLookupString(3), and > > required by the XKB specification: > > > > http://www.x.org/releases/current/doc/kbproto/xkbproto.html#Interpreting_the_Control_Modifier > > > > https://bugs.freedesktop.org/show_bug.cgi?id=75892 > > So xkb_state_key_get_utf8(state, code) != > xkb_keysym_to_utf8(xkb_state_key_get_one_sym(state, code))? (at least > for ctrl+) Yes. > Could we at least add a note/warning to the documentation? I cannot > find anything here: > http://xkbcommon.org/doc/current/group__state.html#ga0774b424063b45c88ec0354c77f9a247 > > For instance, for libtsm, I accept combined modifier+keysym+ucs4string > input from the GUI layer and transform that internally. If the upper > layer uses xkb_state_key_get_utf32() instead of xkb_keysym_to_utf32(), > the logic will force the control-seq even though it might be mapped > differently by the vte-layer (for instance, there's stuff like > shifting values by 64 if ALT is pressed, and more legacy crap we need > to support for eternity). Sure, most libtsm code uses the keysym, but > for glyph transformation we have to use the ucs4 value. Imagine the > ctrl+c input was shifted by the term-layer, you still end up with the > \003 glyph, because the ucs4 string was mapped by xkbcommon. Sure, the old functions are still useful for getting the "raw" translation if you want it. And you're right about the docs - I'll add some hopefully-not-too-confusing details instead of just "prefer the new ones". Full details are in the bugs and commit msgs but of course I don't expect anyone to read that :) > I'm not saying the patch is wrong, in fact, the layout-search logic is > actually what you wrote for kmscon and I appreciate that. I'm just > saying that it's a really subtle API difference that can introduce > weird bugs. Lets see how it works out, but if people start using > xkb_state_key_get_utf32(), I might send a patch adding an xkb-state > flag that disables this transformation. Or just force users to not use > it (which would be unfortunate). A flag makes sense; I try to to add stuff when someone really needs it, not preemptively. Btw, xkb_state_new() doesn't take a flags arg - argh. But in general this stuff should be the default, we initially tried to avoid these nonsensical US-centric behavior (let's say intentionally :), but both the keymaps and existing users actually depend on them, like the capitalization and Control thing (and they are also there in the spec). > Btw., same is true for the implicit caps-lock magic in > xkb_state_key_get_one_sym(). I'm now quite confused whether > xkb_state_key_get_syms() users have to do caps-lock handling > expli
Re: [RFC] libinput configuration interface
On Thu, Mar 27, 2014 at 07:34:37PM +0600, Alexander E. Patrakov wrote: > 2014-03-27 19:22 GMT+06:00 Alexander E. Patrakov : > > (regarding slow scrolling) > > > I don't think so. I have tried to reproduce the bug by hand. It still > > exists, but ./tools/event-debug still picks the scrolling up > > correctly. I guess that this tool output already has the finger motion > > averaging logic applied, so this speaks against your hypothesis. An > > interesting observation that can ring some bells: Chromium picks up > > the scroll events if and only if event-debug prints values whose > > absolute value is greater than 10. Maybe the driver fails to combine > > several small scroll events into a big one, or rounds the individual > > scroll amounts instead of the accumulated value? > > I have also tested regular (X/Y) pointer motion, and noticed the same > pattern. event-debug properly reports sub-pixel motion (i.e. prints > values below 1.0 if I move the finger slowly), but such multiple > sub-pixel motions are never added up into proper pointer events that > actually move the X pointer by at least one pixel. yeah, I found that yesterday and it's a xf86-input-libinput issue. Conversion to integers would just drop small values. I've pushed a fix to use valuator masks with doubles now and that seems to fix the issue. Try 1ab6637b56272bb5cef0568a785e2e5948e6b022. Cheers, Peter ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH fullscreen-shell v5 18/18] Add a screen sharing plugin
Andrew, Thanks for reviewing. Comments follow. On Mar 27, 2014 7:02 AM, "Andrew Wedgbury" wrote: > > Hi Jason, > > A few comments I have from trying out your fullscreen-shell changes: > > > On Tue, 18 Mar 2014, Jason Ekstrand wrote: > >> +static struct ss_seat * >> +ss_seat_create(struct shared_output *so, uint32_t id) >> +{ >> + struct ss_seat *seat; >> + >> + seat = zalloc(sizeof *seat); >> + if (seat == NULL) >> + return NULL; >> + >> + weston_seat_init(&seat->base, so->output->compositor, "default"); > > > It would be great if we could transfer the seat name from the parent seat. In the case of VNC or RDP this could identify the remote user. That's not a bad idea. I'll look into it. > > >> + seat->output = so; >> + seat->parent.seat = wl_registry_bind(so->parent.registry, id, >> + &wl_seat_interface, 1); >> + wl_list_insert(so->seat_list.prev, &seat->link); >> + >> + wl_seat_add_listener(seat->parent.seat, &ss_seat_listener, seat); >> + wl_seat_set_user_data(seat->parent.seat, seat); >> + >> + return seat; >> +} > > >> +static struct shared_output * >> +shared_output_create(struct weston_output *output, int parent_fd) >> +{ >> + struct shared_output *so; >> + struct wl_event_loop *loop; >> + struct ss_seat *seat; >> + int epoll_fd; >> + >> + so = zalloc(sizeof *so); >> + if (so == NULL) >> + goto err_close; >> + >> + wl_list_init(&so->seat_list); >> + + so->parent.display = wl_display_connect_to_fd(parent_fd); >> + if (!so->parent.display) >> + goto err_alloc; >> + + so->parent.registry = wl_display_get_registry(so->parent.display); >> + if (!so->parent.registry) >> + goto err_display; >> + wl_registry_add_listener(so->parent.registry, >> +®istry_listener, so); >> + wl_display_roundtrip(so->parent.display); >> + if (so->parent.shm == NULL) { >> + weston_log("Screen share failed: No wl_shm found\n"); >> + goto err_display; >> + } >> + if (so->parent.fshell == NULL) { >> + weston_log("Screen share failed: " >> + "Parent does not support wl_fullscreen_shell\n"); >> + goto err_display; >> + } >> + if (so->parent.compositor == NULL) { >> + weston_log("Screen share failed: No wl_compositor found\n"); >> + goto err_display; >> + } >> + >> + /* Get SHM formats */ >> + wl_display_roundtrip(so->parent.display); >> + if (!(so->parent.shm_formats & (1 << WL_SHM_FORMAT_XRGB))) { >> + weston_log("Screen share failed: " >> + "WL_SHM_FORMAT_XRGB not available\n"); >> + goto err_display; >> + } >> + >> + so->parent.surface = >> + wl_compositor_create_surface(so->parent.compositor); >> + if (!so->parent.surface) { >> + weston_log("Screen share failed: %m"); >> + goto err_display; >> + } >> + >> + so->parent.mode_feedback = >> + _wl_fullscreen_shell_present_surface_for_mode(so->parent.fshell, >> + so->parent.surface, >> + so->parent.output, >> + output->current_mode->refresh); >> + if (!so->parent.mode_feedback) { >> + weston_log("Screen share failed: %m"); >> + goto err_display; >> + } >> + _wl_fullscreen_shell_mode_feedback_add_listener(so->parent.mode_feedback, >> + &mode_feedback_listener, >> + so); >> + >> + loop = wl_display_get_event_loop(output->compositor->wl_display); >> + >> + epoll_fd = wl_display_get_fd(so->parent.display); >> + so->event_source = >> + wl_event_loop_add_fd(loop, epoll_fd, WL_EVENT_READABLE, >> +shared_output_handle_event, so); >> + if (!so->event_source) { >> + weston_log("Screen share failed: %m"); >> + goto err_display; >> + } >> + + /* Ok, everything's created. We should be good to go */ >> + wl_list_init(&so->shm.buffers); >> + wl_list_init(&so->shm.free_buffers); >> + >> + so->output = output; >> + so->output_destroyed.notify = output_destroyed; >> + wl_signal_add(&so->output->destroy_signal, &so->output_destroyed); >> + >> + so->frame_listener.notify = shared_output_repainted; >> + wl_signal_add(&output->frame_signal, &so->frame_listener); >> + output->disable_planes++; > > > I think you should only disable planes if the fullscreen_shell doesn't have the cursor_plane capability. In that case, there also needs to be a way to update the pointer surface on the parent seat. Although I think the difficulty may be in detecting when the pointer image changes - at least I remember that being a problem in my pre
RE: weston: weston randr protocol for testing and configuration
Note: Take user as developer or admin for weston randr discussion (in the following discussion, I always think user as developer or admin). >-Original Message- >From: Pekka Paalanen [mailto:ppaala...@gmail.com] >Sent: Thursday, March 27, 2014 6:35 PM >To: Wang, Quanxian >Cc: wayland-devel@lists.freedesktop.org >Subject: Re: weston: weston randr protocol for testing and configuration > >On Thu, 27 Mar 2014 04:08:15 + >"Wang, Quanxian" wrote: > >> Hi, Pq >> >> The information to identify the unique mode: width, height and refresh are >enough? Not enough in theory. But is enough in real world. I have checked with >xrandr. Read the following comment. >> >> Welcome any comment for that. >> >> Thanks >> >> >> + >> >> + >> >> + >> >> + Set the mode of output. >> >> + >> >> + > >> + summary="the output object"/> >> >> + >> >> + >> >> + >> > >> >So this is the simple mode set request. >> > >> >Do you think width/height/refresh is really enough to identify a >> >mode? I don't think so. I think in the early days of X11 RandR, >> >NVidia hit the same problem, and started to expose fake refresh >> >values, only to be able to distinguish modes that were identical in >> >width/height/refresh but still different in timings. Or actually I think it >> >was >much more complicated than that, but this is the point in simple terms. >> > >> >So we need something else here to identify a mode. >> > >> >Check what kind of protocol GNOME uses, and how current RandR protocol >works. >> [Wang, Quanxian] Hi, Pq >> Your understanding are right in theory. But in reality, it is barely >> possible. Width >and height could be easily same, not easy for refresh at the same time. >Currently >in xrandr, they use mode name to identify one mode (for example >widthxheight_refresh). in xrandr process, I don't find they take mode >information >to compare. Maybe I missed, but from xrandr parameters, there is no such >option. >They just take width, height, refresh as mode name to identify one mode. >Sometimes, only width and height. If we want to fully support one unique mode, >all the mode information have to be compared. (clock, hdisplay, hsync_start, >hsync_end, htotal, vdisplay, vsync_start, vsync_end, vtotal, flags). But it is >not >convenient. Sometime, user basically don't know what hdisplay is at all. They >just >know widthxheight_refresh. My idea is just take width, height, refresh as the >unique id for mode. >> > >I assume the X server has the database of modes. I cannot think of any reason >why the xrandr client would be comparing modes at all. Why did you expect that? >Isn't the name of a mode in RandR just an arbitrary string (perhaps mapped to >an >XID)? [Wang, Quanxian] Ok, Xserver has xid, does user knows the XID from interface? They don't know except you provide that when they query(in command line). In development, If they want that, they need to talk with xserver before running randr request(complex process). The same as wayland, if we want to support that, we need update mode attributes in compositor. In current mode of compositor, mode has width, height, refresh and flags attribute and no others. Mode information and others are available in backend. Weston mode id(wid same as xid) will be allocated as unique id to identify a mode. Also we should provide this information when user query mode list. (This is also another request to be designed. Provide detailed mode timing information for user including wid). So without xid, the origin right process for set mode is like that weston_randr_set_mode(hdisplay, hsync_start, hsync_end, ), that means, before user set the mode, he needs to get all the parameters of mode information. But is it a real friendly user interface for common user? (10 or 11 parameters, except the structure is shared between client and compositor) Of course there is another way provide a unique wid to mode, so user could use it when he want to set the assigned mode. But it will change some global structure of compositor.(if we provide wid for mode, I don't think it is only for mode, it should be for every elements of compositor. For example, output, mode, surface, view ..., so I don't want to add this at this time. It touches more.) For friendly user interface to set mode, I plan to provide two ways for user. One is the detailed timing setting. Provide all the parameters which could identify the unique id(special user special request). Another is provide a simple interface, for example, only width, height or refresh and find the first one matched to set the mode(most of users will like that and most of time they are unique by these 3 parameters.) At the same time, provide a request for user to list mode detailed timing information. > >These were introduced in RandR 1.2 according to the xrandr man page. If you are >looking at RandR 1.1, it is not sufficient for distinguishing custom modes. [Wang, Quanxian] xid is j
RE: weston: weston randr protocol for testing and configuration
>-Original Message- >From: Pekka Paalanen [mailto:ppaala...@gmail.com] >Sent: Thursday, March 27, 2014 9:06 PM >To: Jasper St. Pierre >Cc: Wang, Quanxian; wayland-devel@lists.freedesktop.org >Subject: Re: weston: weston randr protocol for testing and configuration > >On Thu, 27 Mar 2014 08:31:34 -0400 >"Jasper St. Pierre" wrote: > >> I don't think the user really knows what refresh is either. >> >> I'm actually curious: is there a reason to ever expose different modes >> to the user that have the same width/height but different timings? >> What's the rationale for choosing one instead of the other? I know >> nothing about display panels, keep in mind. > >This is not for users, the whole weston-randr protocol is for developer and >administrator tools for testing different modes without having to restart >Weston >and edit a config file in between. > >I cannot see a reason to offer the same mode with different timings during >normal >operations, but it would be useful for the very kind of experimentation that >this >protocol extension is designed for. > >At least, as far as I have understood. How far this interface should go is >still unclear >to me. If Quanxian wants to stick with RandR 1.1 features, i.e. no dynamic >custom >modelines, that's ok, though to me it seems like losing a good part of the >possible >benefits. [Wang, Quanxian] RandR protocol is just a reference. We take the ideas from it based on wayland/weston current mechanism. Custom should be supported. In my v2, I have provided newmode request, it provides the custom mode. But it is need to be enhanced in the future. I am planning to provide two ways for common and special case. > >> I'm also assuming a modern system here with a modern display panel, so >> no saying "the refresh means the CRT updates faster and it's brighter >> and less less flicker" > >My 1600x1200 monitor, connected via DVI-D, was unstable, going black once in a >while when ran with a normal timings mode at 60 Hz. Switching to reduced >blanking mode made it stable and got rid of the random green snow. Yes, you can >blame the hardware being on its edge. > >The refresh rate was exactly the same. The reduced blanking mode just made the >pixel clock a bit lower, enough to make the picture stable. > > >Thanks, >pq > >> On Thu, Mar 27, 2014 at 6:35 AM, Pekka Paalanen >wrote: >> >> > On Thu, 27 Mar 2014 04:08:15 + >> > "Wang, Quanxian" wrote: >> > >> > > Hi, Pq >> > > >> > > The information to identify the unique mode: width, height and >> > > refresh >> > are enough? Not enough in theory. But is enough in real world. I >> > have checked with xrandr. Read the following comment. >> > > >> > > Welcome any comment for that. >> > > >> > > Thanks >> > > >> > > >> + >> > > >> + >> > > >> +Set the >> > > >> + mode of output. >> > > >> + >> > > >> + > > > >> + summary="the output object"/> >> > > >> + >> > > >> + >> > > >> + >> > > > >> > > >So this is the simple mode set request. >> > > > >> > > >Do you think width/height/refresh is really enough to identify a mode? >> > I don't >> > > >think so. I think in the early days of X11 RandR, NVidia hit the >> > > >same >> > problem, and >> > > >started to expose fake refresh values, only to be able to >> > > >distinguish >> > modes that >> > > >were identical in width/height/refresh but still different in timings. >> > Or actually I >> > > >think it was much more complicated than that, but this is the >> > > >point in >> > simple terms. >> > > > >> > > >So we need something else here to identify a mode. >> > > > >> > > >Check what kind of protocol GNOME uses, and how current RandR >> > > >protocol >> > works. >> > > [Wang, Quanxian] Hi, Pq >> > > Your understanding are right in theory. But in reality, it is >> > > barely >> > possible. Width and height could be easily same, not easy for >> > refresh at the same time. Currently in xrandr, they use mode name to >> > identify one mode (for example widthxheight_refresh). in xrandr >> > process, I don't find they take mode information to compare. Maybe I >> > missed, but from xrandr parameters, there is no such option. They >> > just take width, height, refresh as mode name to identify one mode. >> > Sometimes, only width and height. If we want to fully support one >> > unique mode, all the mode information have to be compared. (clock, >> > hdisplay, hsync_start, hsync_end, htotal, vdisplay, vsync_start, >> > vsync_end, vtotal, flags). But it is not convenient. Sometime, user >> > basically don't know what hdisplay is at all. They just know >> > widthxheight_refresh. My idea is just take width, height, refresh as the >> > unique >id for mode. >> > > >> > >> > I assume the X server has the database of modes. I cannot think of >> > any reason why the xrandr client would be comparing modes at all. >> > Why did you expect that? Isn't the name of a mode in RandR just an >> > arbitrary string (perhaps mapped to an XID)? >> > >>
[PATCH libinput 2/9] Fix up ARRAY_FOR_EACH macro
Remove compiler warning about signed/unsigned comparison. And while we're at it, rename i to _i in the macro to avoid name clashes. Signed-off-by: Peter Hutterer --- src/libinput-util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libinput-util.h b/src/libinput-util.h index 2fbce72..c920568 100644 --- a/src/libinput-util.h +++ b/src/libinput-util.h @@ -71,7 +71,7 @@ int list_empty(const struct list *list); #define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0]) #define ARRAY_FOR_EACH(_arr, _elem) \ - for (int i = 0; (_elem = &_arr[i]) && i < ARRAY_LENGTH(_arr); i++) + for (size_t _i = 0; (_elem = &_arr[_i]) && _i < ARRAY_LENGTH(_arr); _i++) #define min(a, b) (((a) < (b)) ? (a) : (b)) #define max(a, b) (((a) > (b)) ? (a) : (b)) -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 6/9] test: automatically run the tests against valgrind for leaks
Signed-off-by: Peter Hutterer --- The main problem: valgrind is slow. I'm ok with taking the automatic hook out and requiring a make valgrind separate call. Makefile.am| 3 +++ configure.ac | 9 +++-- test/Makefile.am | 13 + test/valgrind.suppressions | 10 ++ 4 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 test/valgrind.suppressions diff --git a/Makefile.am b/Makefile.am index 08bf7ce..05698ba 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,3 +1,6 @@ SUBDIRS = src doc test tools ACLOCAL_AMFLAGS = -I m4 ${ACLOCAL_FLAGS} + +valgrind: + (cd test; $(MAKE) valgrind) diff --git a/configure.ac b/configure.ac index afd04a4..5c4073d 100644 --- a/configure.ac +++ b/configure.ac @@ -72,10 +72,15 @@ if test "x$build_tests" = "xauto"; then build_tests="yes" fi fi -if test "x$build_tests" = "xyes" -a "x$HAVE_CHECK" = "xno"; then - AC_MSG_ERROR([Cannot build tests, check is missing]) +if test "x$build_tests" = "xyes"; then + if test "x$HAVE_CHECK" = "xno"; then + AC_MSG_ERROR([Cannot build tests, check is missing]) + fi + + AC_PATH_PROG(VALGRIND, [valgrind]) fi +AM_CONDITIONAL(HAVE_VALGRIND, [test "x$VALGRIND" != "x"]) AM_CONDITIONAL(BUILD_TESTS, [test "x$build_tests" = "xyes"]) AC_CONFIG_FILES([Makefile diff --git a/test/Makefile.am b/test/Makefile.am index 9b71511..adb58e9 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -68,4 +68,17 @@ test_build_linker_SOURCES = build-pedantic.c test_build_linker_CFLAGS = -I$(top_srcdir)/src test_build_linker_LDADD = $(top_builddir)/src/libinput.la +if HAVE_VALGRIND +VALGRIND_FLAGS=--leak-check=full \ + --quiet \ + --error-exitcode=3 \ + --suppressions=$(srcdir)/valgrind.suppressions + +valgrind: + $(MAKE) check-TESTS LOG_COMPILER="$(VALGRIND)" LOG_FLAGS="$(VALGRIND_FLAGS)" + +check: valgrind + endif +endif +EXTRA_DIST=valgrind.suppressions diff --git a/test/valgrind.suppressions b/test/valgrind.suppressions new file mode 100644 index 000..3401496 --- /dev/null +++ b/test/valgrind.suppressions @@ -0,0 +1,10 @@ +{ + srunner_run::timer_create-uninitialized-bytes + Memcheck:Param + timer_create(evp) + fun:timer_create@@GLIBC_2.3.3 + fun:srunner_run + fun:litest_run + fun:main +} + -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 5/9] test: disable parallel build in test directory
We depend on device creation on the host system, having the tests run in parallel runs a risk of random failure. Ideally we'd just disable for the actual run target, but I couldn't figure out how to do that. Signed-off-by: Peter Hutterer --- test/Makefile.am | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Makefile.am b/test/Makefile.am index b59d4a9..9b71511 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -1,4 +1,6 @@ if BUILD_TESTS +# Disable parallel build, running tests in parallel will screw things up +AM_MAKEFLAGS = -j1 AM_CPPFLAGS = -I$(top_srcdir)/src $(CHECK_CFLAGS) $(LIBEVDEV_CFLAGS) TEST_LIBS = liblitest.la $(CHECK_LIBS) $(LIBUDEV_LIBS) $(LIBEVDEV_LIBS) $(top_builddir)/src/libinput.la -lm -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 9/9] Add libinput_device_get_name() to advertise the kernel name
Signed-off-by: Peter Hutterer --- I didn't end up needing this but it seems a nice to have. However, it does lead to a discussion on how much of the device should we expose to the caller through libinput. Right now libinput hides virtually everthing but the sysname and even that may not be enough to even get a udev reference. It is enough for now though because we only handle one subsystem. If a caller needs settings or get other information on the device, they need to be able to query udev or the fd, but the latter then needs needs open_restricted-like hooks. src/libinput.c | 6 ++ src/libinput.h | 12 tools/event-debug.c | 5 +++-- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/libinput.c b/src/libinput.c index 1e31be3..2863f1d 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1043,6 +1043,12 @@ libinput_device_get_user_data(struct libinput_device *device) } LIBINPUT_EXPORT const char * +libinput_device_get_name(struct libinput_device *device) +{ + return ((struct evdev_device *) device)->devname; +} + +LIBINPUT_EXPORT const char * libinput_device_get_sysname(struct libinput_device *device) { return evdev_device_get_sysname((struct evdev_device *) device); diff --git a/src/libinput.h b/src/libinput.h index 6e43181..f291ce8 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1158,6 +1158,18 @@ libinput_device_get_user_data(struct libinput_device *device); /** * @ingroup device * + * Get the name of the device. + * + * @param device A previously obtained device + * @return The product name of the device + * + */ +const char* +libinput_device_get_name(struct libinput_device *device); + +/** + * @ingroup device + * * Get the system name of the device. * * @param device A previously obtained device diff --git a/tools/event-debug.c b/tools/event-debug.c index c0a08a7..3d0a383 100644 --- a/tools/event-debug.c +++ b/tools/event-debug.c @@ -231,9 +231,10 @@ print_device_notify(struct libinput_event *ev) struct libinput_device *dev = libinput_event_get_device(ev); struct libinput_seat *seat = libinput_device_get_seat(dev); - printf("%s %s\n", + printf("%s %s %s\n", libinput_seat_get_physical_name(seat), - libinput_seat_get_logical_name(seat)); + libinput_seat_get_logical_name(seat), + libinput_device_get_name(dev)); } static void -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 3/9] tools: add --verbose to event-debug
And redirect the log to stdout. libinput logs to stderr by default, but if we're running with --verbose we want all msgs on the same stream. Signed-off-by: Peter Hutterer --- tools/event-debug.c | 20 +++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tools/event-debug.c b/tools/event-debug.c index 12a2df8..c0a08a7 100644 --- a/tools/event-debug.c +++ b/tools/event-debug.c @@ -46,11 +46,13 @@ static struct udev *udev; uint32_t start_time; static const uint32_t screen_width = 100; static const uint32_t screen_height = 100; +static int verbose = 0; static void usage(void) { - printf("Usage: %s [--udev []|--device /dev/input/event0]\n" + printf("Usage: %s [--verbose] [--udev []|--device /dev/input/event0]\n" + "--verbose ... Print debugging output.\n" "--udev Use udev device discovery (default).\n" " Specifying a seat ID is optional.\n" "--device /path/to/device open the given device only\n", @@ -67,6 +69,7 @@ parse_args(int argc, char **argv) { "device", 1, 0, 'd' }, { "udev", 0, 0, 'u' }, { "help", 0, 0, 'h' }, + { "verbose", 0, 0, 'v'}, { 0, 0, 0, 0} }; @@ -91,6 +94,9 @@ parse_args(int argc, char **argv) if (optarg) seat = optarg; break; + case 'v': /* --verbose */ + verbose = 1; + break; default: usage(); return 1; @@ -430,6 +436,13 @@ mainloop(struct libinput *li) close(fds[1].fd); } +static void log_handler(enum libinput_log_priority priority, + void *user_data, + const char *format, va_list args) +{ + vprintf(format, args); +} + int main(int argc, char **argv) { @@ -439,6 +452,11 @@ main(int argc, char **argv) if (parse_args(argc, argv)) return 1; + if (verbose) { + libinput_log_set_handler(log_handler, NULL); + libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_DEBUG); + } + if (mode == MODE_UDEV) { if (open_udev(&li)) return 1; -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 1/9] Use log_error instead of fprintf in old touchpad code
Signed-off-by: Peter Hutterer --- src/evdev-touchpad.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evdev-touchpad.c b/src/evdev-touchpad.c index 65c5ea0..1a48441 100644 --- a/src/evdev-touchpad.c +++ b/src/evdev-touchpad.c @@ -454,7 +454,7 @@ fsm_timeout_handler(void *data) /* This will only happen if the application made the fd * non-blocking, but this function should only be called * upon the timeout, so lets continue anyway. */ - fprintf(stderr, "timerfd read error: %m\n"); + log_error("timerfd read error: %m\n"); if (touchpad->fsm.events_count == 0) { clock_gettime(CLOCK_MONOTONIC, &ts); -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 4/9] test: mark the synaptics clickpad as buttonpad
Signed-off-by: Peter Hutterer --- This requires the uinput device creation patch I sent out yesterday. test/litest-synaptics.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/litest-synaptics.c b/test/litest-synaptics.c index d6aecc4..6df643c 100644 --- a/test/litest-synaptics.c +++ b/test/litest-synaptics.c @@ -125,6 +125,8 @@ litest_create_synaptics_clickpad(struct litest_device *d) EV_KEY, BTN_TOOL_DOUBLETAP, EV_KEY, BTN_TOOL_TRIPLETAP, EV_KEY, BTN_TOOL_QUADTAP, + INPUT_PROP_MAX, INPUT_PROP_POINTER, + INPUT_PROP_MAX, INPUT_PROP_BUTTONPAD, -1, -1); } -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 7/9] Add functions to convert back to the base event
A few functions only work on the base event but once we've converted to the target event we can't go back. Casting works for now but that would expose internal ABI. Signed-off-by: Peter Hutterer --- I got annoyed that I had to pass two arguments around or lose some type-safety just to e.g. get the event type from multiple places. src/libinput.c | 24 src/libinput.h | 43 +++ 2 files changed, 67 insertions(+) diff --git a/src/libinput.c b/src/libinput.c index 182c401..1e31be3 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1090,3 +1090,27 @@ libinput_device_has_capability(struct libinput_device *device, return evdev_device_has_capability((struct evdev_device *) device, capability); } + +LIBINPUT_EXPORT struct libinput_event * +libinput_event_device_notify_get_base_event(struct libinput_event_device_notify *event) +{ + return &event->base; +} + +LIBINPUT_EXPORT struct libinput_event * +libinput_event_keyboard_get_base_event(struct libinput_event_keyboard *event) +{ + return &event->base; +} + +LIBINPUT_EXPORT struct libinput_event * +libinput_event_pointer_get_base_event(struct libinput_event_pointer *event) +{ + return &event->base; +} + +LIBINPUT_EXPORT struct libinput_event * +libinput_event_touch_get_base_event(struct libinput_event_touch *event) +{ + return &event->base; +} diff --git a/src/libinput.h b/src/libinput.h index dab24b7..6e43181 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -271,6 +271,8 @@ libinput_event_get_device(struct libinput_event *event); * Return the pointer event that is this input event. If the event type does * not match the pointer event types, this function returns NULL. * + * The inverse of this function is libinput_event_pointer_get_base_event(). + * * @return A pointer event, or NULL for other events */ struct libinput_event_pointer * @@ -282,6 +284,8 @@ libinput_event_get_pointer_event(struct libinput_event *event); * Return the keyboard event that is this input event. If the event type does * not match the keyboard event types, this function returns NULL. * + * The inverse of this function is libinput_event_keyboard_get_base_event(). + * * @return A keyboard event, or NULL for other events */ struct libinput_event_keyboard * @@ -293,6 +297,8 @@ libinput_event_get_keyboard_event(struct libinput_event *event); * Return the touch event that is this input event. If the event type does * not match the touch event types, this function returns NULL. * + * The inverse of this function is libinput_event_touch_get_base_event(). + * * @return A touch event, or NULL for other events */ struct libinput_event_touch * @@ -304,12 +310,23 @@ libinput_event_get_touch_event(struct libinput_event *event); * Return the device event that is this input event. If the event type does * not match the device event types, this function returns NULL. * + * The inverse of this function is + * libinput_event_device_notify_get_base_event(). + * * @return A device event, or NULL for other events */ struct libinput_event_device_notify * libinput_event_get_device_notify_event(struct libinput_event *event); /** + * @ingroup event + * + * @return The generic libinput_event of this event + */ +struct libinput_event * +libinput_event_device_notify_get_base_event(struct libinput_event_device_notify *event); + +/** * @defgroup event_keyboard Keyboard events * * Key events are generated when a key changes its logical state, usually by @@ -340,6 +357,15 @@ libinput_event_keyboard_get_key(struct libinput_event_keyboard *event); enum libinput_keyboard_key_state libinput_event_keyboard_get_key_state(struct libinput_event_keyboard *event); + +/** + * @ingroup event_keyboard + * + * @return The generic libinput_event of this event + */ +struct libinput_event * +libinput_event_keyboard_get_base_event(struct libinput_event_keyboard *event); + /** * @defgroup event_pointer Pointer events * @@ -538,6 +564,15 @@ li_fixed_t libinput_event_pointer_get_axis_value(struct libinput_event_pointer *event); /** + * @ingroup event_pointer + * + * @return The generic libinput_event of this event + */ +struct libinput_event * +libinput_event_pointer_get_base_event(struct libinput_event_pointer *event); + + +/** * @defgroup event_touch Touch events * * Events from absolute touch devices. @@ -658,6 +693,14 @@ libinput_event_touch_get_y_transformed(struct libinput_event_touch *event, uint32_t height); /** + * @ingroup event_touch + * + * @return The generic libinput_event of this event + */ +struct libinput_event * +libinput_event_touch_get_base_event(struct libinput_event_touch *event); + +/** * @defgroup base Initialization and manipulation of libinput contexts */ -- 1.8.5.3 ___ wayland-devel mailing list waylan
[PATCH libinput 8/9] test: add tests for event conversion and back
Looks a bit excessive given how simple the base is but hey, we don't want to ever break that bit. That'd be embarrassing. And while we're at it make sure that the 'wrong' event getters return NULL for each event. Signed-off-by: Peter Hutterer --- This requires the uinput device creation patch I sent out yesterday. test/Makefile.am | 7 +- test/misc.c | 377 +++ 2 files changed, 383 insertions(+), 1 deletion(-) create mode 100644 test/misc.c diff --git a/test/Makefile.am b/test/Makefile.am index adb58e9..44afc95 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -20,7 +20,7 @@ liblitest_la_SOURCES = \ litest-generic-highres-touch.c \ litest.c -run_tests = test-udev test-path test-pointer test-touch test-log test-touchpad +run_tests = test-udev test-path test-pointer test-touch test-log test-touchpad test-misc build_tests = test-build-linker test-build-pedantic-c99 test-build-std-gnuc90 noinst_PROGRAMS = $(build_tests) $(run_tests) @@ -56,6 +56,11 @@ test_touchpad_CFLAGS = $(AM_CPPFLAGS) test_touchpad_LDADD = $(TEST_LIBS) test_touchpad_LDFLAGS = -static +test_misc_SOURCES = misc.c +test_misc_CFLAGS = $(AM_CPPFLAGS) +test_misc_LDADD = $(TEST_LIBS) +test_misc_LDFLAGS = -static + # build-test only test_build_pedantic_c99_SOURCES = build-pedantic.c test_build_pedantic_c99_CFLAGS = $(AM_CPPFLAGS) -std=c99 -pedantic -Werror diff --git a/test/misc.c b/test/misc.c new file mode 100644 index 000..5bf9477 --- /dev/null +++ b/test/misc.c @@ -0,0 +1,377 @@ +/* + * Copyright © 2014 Red Hat, Inc. + * + * Permission to use, copy, modify, distribute, and sell this software and + * its documentation for any purpose is hereby granted without fee, provided + * that the above copyright notice appear in all copies and that both that + * copyright notice and this permission notice appear in supporting + * documentation, and that the name of the copyright holders not be used in + * advertising or publicity pertaining to distribution of the software + * without specific, written prior permission. The copyright holders make + * no representations about the suitability of this software for any + * purpose. It is provided "as is" without express or implied warranty. + * + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include + +#include +#include +#include +#include +#include + +#include "litest.h" + +static int open_restricted(const char *path, int flags, void *data) +{ + int fd = open(path, flags); + return fd < 0 ? -errno : fd; +} +static void close_restricted(int fd, void *data) +{ + close(fd); +} + +const struct libinput_interface simple_interface = { + .open_restricted = open_restricted, + .close_restricted = close_restricted, +}; + +static struct libevdev_uinput * +create_simple_test_device(const char *name, ...) +{ + va_list args; + struct libevdev_uinput *uinput; + struct libevdev *evdev; + unsigned int type, code; + int rc; + struct input_absinfo abs = { + .value = -1, + .minimum = 0, + .maximum = 100, + .fuzz = 0, + .flat = 0, + .resolution = 100, + }; + + evdev = libevdev_new(); + ck_assert(evdev != NULL); + libevdev_set_name(evdev, name); + + va_start(args, name); + + while ((type = va_arg(args, unsigned int)) != -1 && + (code = va_arg(args, unsigned int)) != -1) { + const struct input_absinfo *a = NULL; + if (type == EV_ABS) + a = &abs; + libevdev_enable_event_code(evdev, type, code, a); + } + + va_end(args); + + rc = libevdev_uinput_create_from_device(evdev, + LIBEVDEV_UINPUT_OPEN_MANAGED, + &uinput); + ck_assert_int_eq(rc, 0); + libevdev_free(evdev); + + return uinput; +} + +START_TEST(event_conversion_device_notify) +{ + struct libevdev_uinput *uinput; + struct libinput *li; + struct libinput_event *event; + int device_added = 0, device_removed = 0; + + uinput = create_simple_test_device("test device", + EV_REL, REL_X, + EV_REL, REL_Y, + EV_KEY, BTN_LEFT, + EV_KEY, BTN_MIDDLE, +
Re: SynPS/2 Synaptics TouchPad firmware bug
On Thu, Mar 27, 2014 at 10:19:50PM +0600, Alexander E. Patrakov wrote: > Hello. > > This is a report of a touchpad firmware bug that I want you to work > around in libinput. I found this bug while testing > xf86-input-libinput. I guess that the synaptics driver already has a > workaround, or the bug does not manifest itself with the following > options: > > Option "SoftButtonAreas" "4360 0 4000 0 2880 4359 3500 0" > Option "AreaBottomEdge" "3500" > Option "VertTwoFingerScroll" "true" > Option "HorizTwoFingerScroll" "true" > Option "MinSpeed" "1" > Option "MaxSpeed" "10" > Option "FingerLow" "60" > Option "FingerHigh" "70" > > (indeed, FingerLow would have prevented this bug for the attached trace). for the xorg driver this is correct, for libinput it's a bit different. it largely depends on whether we want the pressure to filter events, or to filter completely. I haven't done anything in that direction yet at all though. > To reproduce the bug, get the affected touchpad, move the cursor as > you would normally do using a finger on the right hand, then use the > finger on the left hand to click in the bottom area of the touchpad. > Usually the driver would register the click as expected, but sometimes > it would move the pointer to the lower left corner and click there. > > I have managed to capture one of these cases using evemu-record. The > compressed file is attached. Please ignore the first 240 seconds or > so, they contain some movements, scrolling and clicks. > > The interesting part is: > > E: 249.206319 # SYN_REPORT (0) -- > E: 249.218008 0003 0035 3764# EV_ABS / ABS_MT_POSITION_X3764 > E: 249.218008 0003 0036 2221# EV_ABS / ABS_MT_POSITION_Y2221 > E: 249.218008 0003 003a 0065# EV_ABS / ABS_MT_PRESSURE 65 > E: 249.218008 0003 3764# EV_ABS / ABS_X3764 > E: 249.218008 0003 0001 2216# EV_ABS / ABS_Y2216 > E: 249.218008 0003 0018 0065# EV_ABS / ABS_PRESSURE 65 > E: 249.218008 # SYN_REPORT (0) -- > E: 249.230881 0003 0035 3752# EV_ABS / ABS_MT_POSITION_X3752 > E: 249.230881 0003 003a 0046# EV_ABS / ABS_MT_PRESSURE 46 > E: 249.230881 0003 3758# EV_ABS / ABS_X3758 > E: 249.230881 0003 0018 0046# EV_ABS / ABS_PRESSURE 46 > E: 249.230881 # SYN_REPORT (0) -- > E: 249.242648 0003 0035 1640# EV_ABS / ABS_MT_POSITION_X1640 > E: 249.242648 0003 0036 4681# EV_ABS / ABS_MT_POSITION_Y4681 > E: 249.242648 0003 003a 0025# EV_ABS / ABS_MT_PRESSURE 25 > E: 249.242648 0003 1640# EV_ABS / ABS_X1640 > E: 249.242648 0003 0001 4681# EV_ABS / ABS_Y4681 > E: 249.242648 0003 0018 0025# EV_ABS / ABS_PRESSURE 25 > E: 249.242648 # SYN_REPORT (0) -- > E: 249.254568 0003 0035 1648# EV_ABS / ABS_MT_POSITION_X1648 > E: 249.254568 0003 003a 0027# EV_ABS / ABS_MT_PRESSURE 27 > E: 249.254568 0003 1644# EV_ABS / ABS_X1644 > E: 249.254568 0003 0018 0027# EV_ABS / ABS_PRESSURE 27 > > As you see, the touchpad reports a huge jump in the touch position > without getting a new tracking ID. I.e. produces garbage data. A > non-buggy touchpad would have recognized that this is in fact a > different finger. > > So, I would like libinput to recognize such huge jumps (e.g. any > movements by more than 15% of the touchpad width in less than 0.03 > seconds) as firmware bugs and treat them as if this indicated a > completely new touch. please file this in bugzilla, this really is something that we need to keep in one place because it'll likely have a rather complex solution that I'd like to be able to link to. having said that, I'm currently struggling with getting libinput up to feature parity with the xorg drivers, hacking around broken devices will have to wait a bit, sorry. Cheers, Peter ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] test: add litest helper functions for creating uinput devices
On Thu, Mar 27, 2014 at 05:10:34PM +0100, Jonas Ådahl wrote: > On Thu, Mar 27, 2014 at 08:48:02AM +1000, Peter Hutterer wrote: > > Both functions accept a series of event types/codes tuples, terminated by > > -1. > > For the even type INPUT_PROP_MAX (an invalid type otherwise) the code is > > used > > as a property to enable. > > > > The _abs function als takes an array of absinfo, with absinfo.value > > determining the axis to change. If none are given, abs axes are initialized > > with default settings. > > > > Both functions abort on failure, so the caller does not need to check the > > return value. > > > > Example code for creating a rel device: > > > > struct libevdev_uinput *uinput; > > struct input_id id = { ... }; > > uinput = litest_create_uinput_device("foo", &id, > > EV_REL, REL_X, > > EV_REL, REL_Y, > > EV_KEY, BTN_LEFT, > > INPUT_PROP_MAX, INPUT_PROP_BUTTONPAD, > > -1); > > libevdev_uinput_write_event(uinput, EV_REL, REL_X, -1); > > libevdev_uinput_write_event(uinput, EV_SYN, SYN_REPORT, 0); > > .. > > libevdev_uinput_destroy(uinput); > > The problem with this approach is that its no longer possible to use the > litest_touch_down, litest_touch_up etc any more. Especially for touch > events thats an annoyance to reimplement in the test case. We could add > helpers for that but then we'd have double set of helpers for every kind > of event we want to test. how many devices are we talking about here. You're specifically creating a test device here that has one functionality that matters: a lot of slots. how many more devices do we expect similar to this? IMO at least for this case it's worth writing the extra couple of lines to have a custom device. Once we get past the number of devices we can look again to see if we have some sort of common denominator. also, litest_touch_down/up are already problematic. e.g. they don't release BTN_TOUCH or BTN_TOOL_FINGER because nothing refcounts the touches. they only solve the minimal test cases so far but the test suite needs a bit more work to be useful on a more general basis. > The way I'd want to use something like this is to create a fake device, > possibly with some special parameters, and then use it in the same way > I'd use a "normal" test device. I'd rather have a way to somehow create > a special fake device that can be operated in the same way using the > same functions as a "normal" fake device. the problem I see is with the special parameters. by the time you allow for all of them you end up re-implementing the current uinput API. e.g. your last device needed lots of slots, but what if the next device needs special pressure? which is exactly what I expect to be the next problem for the touchpad tests. I just start at the device description files and with the patch applied they are now little more than static descriptions of device capabilities with the odd for loop to write arrays out. Maybe a solution to this is to make it static but better exposed that you can literally take a device, adjust a few nobs in the cabability bits and then create it. Would that help? Cheers, Peter ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 0/9] Improvements for clickpad touchpads
I still need to write some tests for the buttons, but I'd like people to have a look at it already. Branch is also available from https://github.com/whot/libinput/tree/wip/clickpad-improvements Cheers, Peter ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 1/9] touchpad: set ntouches for single-touch pads depending on key bits
A single-touch touchpad that provides BTN_TOOL_TRIPLETAP has 3 touches, etc. There aren't a lot of these out there, but some touchpads don't have slots but do provide two- or three-finger detection. Signed-off-by: Peter Hutterer --- src/evdev-mt-touchpad.c | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index bbbd8f3..8021db2 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -715,9 +715,29 @@ tp_init_slots(struct tp_dispatch *tp, tp->slot = absinfo->value; tp->has_mt = true; } else { - tp->ntouches = 5; /* FIXME: based on DOUBLETAP, etc. */ + struct map { + unsigned int code; + int ntouches; + } max_touches[] = { + { BTN_TOOL_QUINTTAP, 5 }, + { BTN_TOOL_QUADTAP, 4 }, + { BTN_TOOL_TRIPLETAP, 3 }, + { BTN_TOOL_DOUBLETAP, 2 }, + }; + struct map *m; + tp->slot = 0; tp->has_mt = false; + tp->ntouches = 1; + + ARRAY_FOR_EACH(max_touches, m) { + if (libevdev_has_event_code(device->evdev, + EV_KEY, + m->code)) { + tp->ntouches = m->ntouches; + break; + } + } } tp->touches = calloc(tp->ntouches, sizeof(struct tp_touch)); -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 6/9] doc: add state machine SVG to EXTRA_DIST
Signed-off-by: Peter Hutterer --- doc/Makefile.am | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/Makefile.am b/doc/Makefile.am index 31b673b..75fa98a 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -1,3 +1,5 @@ +EXTRA_DIST = touchpad-tap-state-machine.svg + if HAVE_DOXYGEN noinst_DATA = html/index.html @@ -12,7 +14,7 @@ clean-local: $(AM_V_at)rm -rf html doc_src= $(shell find html -type f -printf "html/%P\n" 2>/dev/null) -EXTRA_DIST = $(builddir)/html/index.html $(doc_src) +EXTRA_DIST += $(builddir)/html/index.html $(doc_src) endif -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 8/9] touchpad: Only enable clickfingers on Apple touchpads
Apple touchpads don't have visible markings for the software button areas that almost all other vendors use. OS X provides clickfinger behaviour instead, where a click with two fingers on the touchpad generate a right button click. Use that same behaviour in libinput. For all other touchpads, use the software button areas introduced in a follow-up commit. Signed-off-by: Peter Hutterer --- This should probably be configurable at some point in the future... src/evdev-mt-touchpad-buttons.c | 7 ++- src/evdev-mt-touchpad.h | 1 + test/touchpad.c | 12 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index c3c97b0..08783a3 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -60,6 +60,11 @@ tp_init_buttons(struct tp_dispatch *tp, tp->buttons.motion_dist = diagonal * DEFAULT_BUTTON_MOTION_THRESHOLD; + if (libevdev_get_id_vendor(device->evdev) == 0x5ac) /* Apple */ + tp->buttons.use_clickfinger = true; + else + tp->buttons.use_clickfinger = false; + return 0; } @@ -142,7 +147,7 @@ tp_post_button_events(struct tp_dispatch *tp, uint32_t time) if (tp->buttons.has_buttons) rc = tp_post_physical_buttons(tp, time); - else + else if (tp->buttons.use_clickfinger) rc = tp_post_clickfinger_buttons(tp, time); return rc; diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 1dee663..f3e5b31 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -135,6 +135,7 @@ struct tp_dispatch { struct { bool has_buttons; /* true for physical LMR buttons */ + bool use_clickfinger; /* number of fingers decides button number */ uint32_t state; uint32_t old_state; uint32_t motion_dist; /* for pinned touches */ diff --git a/test/touchpad.c b/test/touchpad.c index f4d7839..bbae6cd 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -217,7 +217,7 @@ END_TEST START_TEST(touchpad_1fg_clickfinger) { - struct litest_device *dev = litest_current_device(); + struct litest_device *dev = litest_create_device(LITEST_BCM5974); struct libinput *li = dev->libinput; struct libinput_event *event; struct libinput_event_pointer *ptrev; @@ -237,12 +237,14 @@ START_TEST(touchpad_1fg_clickfinger) LIBINPUT_POINTER_BUTTON_STATE_PRESSED); assert_button_event(li, BTN_LEFT, LIBINPUT_POINTER_BUTTON_STATE_RELEASED); + + litest_delete_device(dev); } END_TEST START_TEST(touchpad_2fg_clickfinger) { - struct litest_device *dev = litest_current_device(); + struct litest_device *dev = litest_create_device(LITEST_BCM5974); struct libinput *li = dev->libinput; struct libinput_event *event; struct libinput_event_pointer *ptrev; @@ -264,6 +266,8 @@ START_TEST(touchpad_2fg_clickfinger) LIBINPUT_POINTER_BUTTON_STATE_PRESSED); assert_button_event(li, BTN_RIGHT, LIBINPUT_POINTER_BUTTON_STATE_RELEASED); + + litest_delete_device(dev); } END_TEST @@ -362,8 +366,8 @@ int main(int argc, char **argv) { litest_add("touchpad:tap", touchpad_1fg_tap_n_drag, LITEST_TOUCHPAD, LITEST_ANY); litest_add("touchpad:tap", touchpad_2fg_tap, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH); - litest_add("touchpad:clickfinger", touchpad_1fg_clickfinger, LITEST_TOUCHPAD, LITEST_ANY); - litest_add("touchpad:clickfinger", touchpad_2fg_clickfinger, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH); + litest_add_no_device("touchpad:clickfinger", touchpad_1fg_clickfinger); + litest_add_no_device("touchpad:clickfinger", touchpad_2fg_clickfinger); litest_add("touchpad:click", touchpad_btn_left, LITEST_TOUCHPAD, LITEST_CLICKPAD); litest_add("touchpad:click", clickpad_btn_left, LITEST_CLICKPAD, LITEST_ANY); -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 7/9] touchpad: save the active clickfinger button
To avoid having a button left press and a button right release if the number of fingers changes. Signed-off-by: Peter Hutterer --- src/evdev-mt-touchpad-buttons.c | 22 +- src/evdev-mt-touchpad.h | 1 + 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 8946fc7..c3c97b0 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -75,23 +75,27 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint32_t time) if (current == old) return 0; - switch (tp->nfingers_down) { + if (current) { + switch (tp->nfingers_down) { case 1: button = BTN_LEFT; break; case 2: button = BTN_RIGHT; break; case 3: button = BTN_MIDDLE; break; default: return 0; - } - - if (current) + } + tp->buttons.active = button; state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; - else + } else { + button = tp->buttons.active; + tp->buttons.active = 0; state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + } - pointer_notify_button(&tp->device->base, - time, - button, - state); + if (button) + pointer_notify_button(&tp->device->base, + time, + button, + state); return 1; } diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 0879776..1dee663 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -138,6 +138,7 @@ struct tp_dispatch { uint32_t state; uint32_t old_state; uint32_t motion_dist; /* for pinned touches */ + unsigned int active;/* currently active button, for release event */ } buttons; /* physical buttons */ struct { -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 4/9] touchpad: don't allow tapping while any button is down
Immediately set the state to DEAD, waiting for the tap release to go back to idle. Signed-off-by: Peter Hutterer --- src/evdev-mt-touchpad-tap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index bcc5700..863e004 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -508,7 +508,7 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint32_t time) struct tp_touch *t; int filter_motion = 0; - if (tp->queued & TOUCHPAD_EVENT_BUTTON_PRESS) + if (tp->buttons.state != 0) tp_tap_handle_event(tp, TAP_EVENT_BUTTON, time); tp_for_each_touch(tp, t) { -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 2/9] touchpad: after a click, lock the finger to its current position
On clickpads, releasing the button usually causes some motion events. To avoid erroneous movements, lock the finger into position on the up event and don't allow for motion events until we move past a given threshold (currently 2% of the touchpad diagonal). Signed-off-by: Peter Hutterer --- src/evdev-mt-touchpad.c | 70 + src/evdev-mt-touchpad.h | 18 - 2 files changed, 64 insertions(+), 24 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 8021db2..5318b8f 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -32,6 +32,7 @@ #define DEFAULT_MIN_ACCEL_FACTOR 0.16 #define DEFAULT_MAX_ACCEL_FACTOR 1.0 #define DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR 700.0 +#define DEFAULT_BUTTON_MOTION_THRESHOLD 0.02 /* in percent of size */ static inline int tp_hysteresis(int in, int center, int margin) @@ -346,16 +347,36 @@ tp_process_key(struct tp_dispatch *tp, } static void -tp_unpin_finger(struct tp_dispatch *tp) +tp_unpin_finger(struct tp_dispatch *tp, struct tp_touch *t) +{ + unsigned int xdist, ydist; + + if (t->pinned.state != PIN_STATE_UP) + return; + + xdist = abs(t->x - t->pinned.center_x); + ydist = abs(t->y - t->pinned.center_y); + + if (xdist * xdist + ydist * ydist < + tp->buttons.motion_dist * tp->buttons.motion_dist) + return; + + t->pinned.state = PIN_STATE_NONE; + + if (t->state != TOUCH_END && tp->nfingers_down == 1) + t->is_pointer = true; +} + +static void +tp_lift_pinned_finger(struct tp_dispatch *tp) { struct tp_touch *t; + tp_for_each_touch(tp, t) { - if (t->is_pinned) { - t->is_pinned = false; - - if (t->state != TOUCH_END && - tp->nfingers_down == 1) - t->is_pointer = true; + if (t->pinned.state == PIN_STATE_DOWN) { + t->pinned.state = PIN_STATE_UP; + t->pinned.center_x = t->x; + t->pinned.center_y = t->y; break; } } @@ -368,15 +389,14 @@ tp_pin_finger(struct tp_dispatch *tp) *pinned = NULL; tp_for_each_touch(tp, t) { - if (t->is_pinned) { + if (t->pinned.state != PIN_STATE_NONE) { pinned = t; break; } } - assert(!pinned); - - pinned = tp_current_touch(tp); + if (!pinned) + pinned = tp_current_touch(tp); if (tp->nfingers_down != 1) { tp_for_each_touch(tp, t) { @@ -388,7 +408,7 @@ tp_pin_finger(struct tp_dispatch *tp) } } - pinned->is_pinned = true; + pinned->pinned.state = PIN_STATE_DOWN; pinned->is_pointer = false; } @@ -409,16 +429,21 @@ tp_process_state(struct tp_dispatch *tp, uint32_t time) tp_motion_hysteresis(tp, t); tp_motion_history_push(t); + + tp_unpin_finger(tp, t); } - /* We have a physical button down event on a clickpad. For drag and - drop, this means we try to identify which finger pressed the - physical button and "pin" it, i.e. remove pointer-moving - capabilities from it. -*/ - if ((tp->queued & TOUCHPAD_EVENT_BUTTON_PRESS) && - !tp->buttons.has_buttons) - tp_pin_finger(tp); + if (!tp->buttons.has_buttons) { + /* We have a physical button down event on a clickpad. For drag and + drop, this means we try to identify which finger pressed the + physical button and "pin" it, i.e. remove pointer-moving + capabilities from it. +*/ + if (tp->queued & TOUCHPAD_EVENT_BUTTON_PRESS) + tp_pin_finger(tp); + else if ((tp->queued & TOUCHPAD_EVENT_BUTTON_RELEASE)) + tp_lift_pinned_finger(tp); + } } static void @@ -441,9 +466,6 @@ tp_post_process_state(struct tp_dispatch *tp, uint32_t time) tp->buttons.old_state = tp->buttons.state; - if (tp->queued & TOUCHPAD_EVENT_BUTTON_RELEASE) - tp_unpin_finger(tp); - tp->queued = TOUCHPAD_EVENT_NONE; } @@ -796,6 +818,8 @@ tp_init(struct tp_dispatch *tp, tp->hysteresis.margin_y = diagonal / DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR; + tp->buttons.motion_dist = diagonal * DEFAULT_BUTTON_MOTION_THRESHOLD; + if (libevdev_has_event_code(device->evdev, EV_KEY, BTN_MIDDLE) || libevdev_has_event_code(device->evdev, EV_KEY, BTN_RIGHT)) tp->buttons.has_buttons = true; diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 2bdb329..51e7702 1006
[PATCH libinput 5/9] touchpad: move button-related code into a separate file
This is about to become more complicated with the support for software button areas. Move it to a separate file to have it logically grouped together. No functional changes. Signed-off-by: Peter Hutterer --- src/Makefile.am | 1 + src/evdev-mt-touchpad-buttons.c | 146 src/evdev-mt-touchpad.c | 102 ++-- src/evdev-mt-touchpad.h | 11 +++ 4 files changed, 162 insertions(+), 98 deletions(-) create mode 100644 src/evdev-mt-touchpad-buttons.c diff --git a/src/Makefile.am b/src/Makefile.am index 579ed25..ffa6a29 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -14,6 +14,7 @@ libinput_la_SOURCES = \ evdev-mt-touchpad.c \ evdev-mt-touchpad.h \ evdev-mt-touchpad-tap.c \ + evdev-mt-touchpad-buttons.c \ evdev-touchpad.c\ filter.c\ filter.h\ diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c new file mode 100644 index 000..8946fc7 --- /dev/null +++ b/src/evdev-mt-touchpad-buttons.c @@ -0,0 +1,146 @@ +/* + * Copyright © 2014 Red Hat, Inc. + * + * Permission to use, copy, modify, distribute, and sell this software and + * its documentation for any purpose is hereby granted without fee, provided + * that the above copyright notice appear in all copies and that both that + * copyright notice and this permission notice appear in supporting + * documentation, and that the name of the copyright holders not be used in + * advertising or publicity pertaining to distribution of the software + * without specific, written prior permission. The copyright holders make + * no representations about the suitability of this software for any + * purpose. It is provided "as is" without express or implied warranty. + * + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include + +#include "evdev-mt-touchpad.h" + +#define DEFAULT_BUTTON_MOTION_THRESHOLD 0.02 /* in percent of size */ + +int +tp_process_button(struct tp_dispatch *tp, + const struct input_event *e, + uint32_t time) +{ + uint32_t mask = 1 << (e->code - BTN_LEFT); + if (e->value) { + tp->buttons.state |= mask; + tp->queued |= TOUCHPAD_EVENT_BUTTON_PRESS; + } else { + tp->buttons.state &= ~mask; + tp->queued |= TOUCHPAD_EVENT_BUTTON_RELEASE; + } + + return 0; +} + +int +tp_init_buttons(struct tp_dispatch *tp, + struct evdev_device *device) +{ + int width, height; + double diagonal; + + if (libevdev_has_event_code(device->evdev, EV_KEY, BTN_MIDDLE) || + libevdev_has_event_code(device->evdev, EV_KEY, BTN_RIGHT)) + tp->buttons.has_buttons = true; + + width = abs(device->abs.max_x - device->abs.min_x); + height = abs(device->abs.max_y - device->abs.min_y); + diagonal = sqrt(width*width + height*height); + + tp->buttons.motion_dist = diagonal * DEFAULT_BUTTON_MOTION_THRESHOLD; + + return 0; +} + +static int +tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint32_t time) +{ + uint32_t current, old, button; + enum libinput_pointer_button_state state; + + current = tp->buttons.state; + old = tp->buttons.old_state; + + if (current == old) + return 0; + + switch (tp->nfingers_down) { + case 1: button = BTN_LEFT; break; + case 2: button = BTN_RIGHT; break; + case 3: button = BTN_MIDDLE; break; + default: + return 0; + } + + if (current) + state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; + else + state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + + pointer_notify_button(&tp->device->base, + time, + button, + state); + return 1; +} + +static int +tp_post_physical_buttons(struct tp_dispatch *tp, uint32_t time) +{ + uint32_t current, old, button; + + current = tp->buttons.state; + old = tp->buttons.old_state; + button = BTN_LEFT; + + while (current || old) { + enum libinput_pointer_button_state state; + + if ((current & 0x1) ^ (old & 0x1)) { + if (!!(current & 0x1)) + st
[PATCH libinput 3/9] touchpad: reset the tap timer_fd to -1 on destroy
No real effect, just for safety Signed-off-by: Peter Hutterer --- src/evdev-mt-touchpad-tap.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index 5fa712f..bcc5700 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -615,6 +615,8 @@ tp_destroy_tap(struct tp_dispatch *tp) libinput_remove_source(tp->device->base.seat->libinput, tp->tap.source); tp->tap.source = NULL; } - if (tp->tap.timer_fd > -1) + if (tp->tap.timer_fd > -1) { close(tp->tap.timer_fd); + tp->tap.timer_fd = -1; + } } -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel