[PATCH libinput] evdev: fix inverted mouse normalization
Regression introduced in 9f8edc5fd880e0a9c482b36e6b4120ccc056ee0b where it
changed from delta / (dpi/default) to delta * dpi/default, causing the inverse
effect of what the dpi setting is supposed to achieve.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/evdev.c b/src/evdev.c
index 00450bf..243cd22 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -230,8 +230,8 @@ normalize_delta(struct evdev_device *device,
const struct device_coords *delta,
struct normalized_coords *normalized)
{
- normalized-x = delta-x * (double)device-dpi / DEFAULT_MOUSE_DPI;
- normalized-y = delta-y * (double)device-dpi / DEFAULT_MOUSE_DPI;
+ normalized-x = delta-x * DEFAULT_MOUSE_DPI / (double)device-dpi;
+ normalized-y = delta-y * DEFAULT_MOUSE_DPI / (double)device-dpi;
}
static void
--
2.3.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 06/17] xdg-shell: Require a buffer and a wl_surface.commit for mapping a window
On Wed, Apr 08, 2015 at 11:32:20AM +0800, Jonas Ådahl wrote: On Tue, Apr 07, 2015 at 06:36:56PM -0700, Jasper St. Pierre wrote: On Tue, Apr 7, 2015 at 6:03 PM, Bryce Harrington br...@osg.samsung.com wrote: On Tue, Apr 07, 2015 at 05:01:21PM +0800, Jonas Ådahl wrote: Require the client to have attached (either previously committed, or newly) a buffer to the corresponding wl_surface, and that the window will not be potentially mapped until calling wl_surface.commit after having created the window. This is required to make valid double buffered xdg_surface state possible when creating a window. Currently there is no double buffered state in xdg_popup, but it should behave the same as xdg_surface, and for making it future proof in case we want to add double buffered state to xdg_popup. Signed-off-by: Jonas Ådahl jad...@gmail.com --- protocol/xdg-shell.xml | 8 1 file changed, 8 insertions(+) diff --git a/protocol/xdg-shell.xml b/protocol/xdg-shell.xml index 0b6c999..d316e06 100644 --- a/protocol/xdg-shell.xml +++ b/protocol/xdg-shell.xml @@ -149,6 +149,10 @@ It provides requests to treat surfaces like windows, allowing to set properties like maximized, fullscreen, minimized, and to move and resize them, and associate metadata like title and app id. + + For a xdg_surface to be mapped by the compositor, the wl_surface must + have a buffer attached to it, and wl_surface.commit must have been called + after having created the xdg_surface object. That reads a bit awkward to me; could it be improved with the following? In order for the compositor to map a created xdg_surface object, a buffer must first be attached to the wl_surface and wl_surface.commit called. It doesn't describe the full story. wl_surface.commit needs to be called *after* the xdg_surface is created. I believe the wl_surface.attach and get_xdg_surface can happen in either order, though it might be nice to establish some form of standard order for that. Not sure that is a good idea. It'd mean we have to disallow otherwise valid non-role-assigned surfaces which seems a bit odd. We also need to allow committing the surface without attaching a buffer so that the compositor can configure the xdg_surface before mapping it (e.g. when starting as maximized). Anyway, how about: The client must call wl_surface.commit on the corresponding wl_surface for the xdg_surface state to take effect. Prior to committing the new state, it can set up initial configuration, such as maximizing or setting a window geometry. Even without attaching a buffer the compositor must respond to initial committed configuration, for instance sending a configure event with expected window geometry if the client maximized its surface during initialization. For a surface to be mapped by the compositor the client must have committed both an xdg_surface state and a buffer. Yes, that's a lot clearer. Bryce Jonas /description request name=destroy type=destructor @@ -473,6 +477,10 @@ The x and y arguments specify where the top left of the popup should be placed, relative to the local surface coordinates of the parent surface. + + For a xdg_popup to be mapped by the compositor, the wl_surface must + have a buffer attached to it, and wl_surface.commit must have been + called after having created the xdg_popup object. As above /description request name=destroy type=destructor -- 2.1.4 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ 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
[PATCH libinput v3] evdev: Add support for POINTINGSTICK_CONST_ACCEL udev property
There is quite a wide spread in the delta events generated by trackpoints,
some generate deltas of 1-2 under normal use, while others generate deltas
from 1-20.
It is desirable to normalize trackpoint deltas just like we are normalizing
mouse deltas to 1000 dpi, so as to give different model laptops aprox.
the same trackpoint cursor speed ootb.
Recent versions of udev + hwdb set a POINTINGSTICK_CONST_ACCEL udev property
which can be used to adjust trackpoints which are too slow / too fast
ootb, this commit implements support for that property.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v3:
-Fix a couple of typos
-Document TRACKPOINT_CONST_ACCEL udev property in
doc/device-configuration-via-udev.dox
-Added a test for parse_trackpoint_accel_property
Changes in v2:
-Use POINTINGSTICK_CONST_ACCEL instead of TRACKPOINT_CONST_ACCEL to match
changes to the udev patchset
foo
---
doc/device-configuration-via-udev.dox | 4
src/evdev.c | 33 +
src/evdev.h | 7 +++
src/libinput-util.c | 31 +++
src/libinput-util.h | 1 +
test/misc.c | 27 +++
6 files changed, 103 insertions(+)
diff --git a/doc/device-configuration-via-udev.dox
b/doc/device-configuration-via-udev.dox
index fc1c0af..6579041 100644
--- a/doc/device-configuration-via-udev.dox
+++ b/doc/device-configuration-via-udev.dox
@@ -57,6 +57,10 @@ See @ref motion_normalization for details.
ddThe angle in degrees for each click on a mouse wheel. See
libinput_pointer_get_axis_source() for details.
/dd
+dtPOINTINGSTICK_CONST_ACCEL/dt
+ddA constant (linear) acceleration factor to apply to pointingstick deltas
+to normalize them. See evdev_get_trackpoint_dpi() for details.
+/dd
/dl
Below is an example udev rule to assign seat1 to a device from vendor
diff --git a/src/evdev.c b/src/evdev.c
index 4205c2d..1730dae 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -1319,6 +1319,31 @@ evdev_read_wheel_click_prop(struct evdev_device *device)
return angle;
}
+
+static inline int
+evdev_get_trackpoint_dpi(struct evdev_device *device)
+{
+ struct libinput *libinput = device-base.seat-libinput;
+ const char *trackpoint_accel;
+ double accel = DEFAULT_TRACKPOINT_ACCEL;
+
+ trackpoint_accel = udev_device_get_property_value(
+ device-udev_device,
POINTINGSTICK_CONST_ACCEL);
+ if (trackpoint_accel) {
+ accel = parse_trackpoint_accel_property(trackpoint_accel);
+ if (accel == 0.0) {
+ log_error(libinput, Trackpoint accel property for
+ '%s' is present but invalid,
+ using %.2f instead\n,
+ device-devname,
+ DEFAULT_TRACKPOINT_ACCEL);
+ accel = DEFAULT_TRACKPOINT_ACCEL;
+ }
+ }
+
+ return DEFAULT_MOUSE_DPI / accel;
+}
+
static inline int
evdev_read_dpi_prop(struct evdev_device *device)
{
@@ -1326,6 +1351,14 @@ evdev_read_dpi_prop(struct evdev_device *device)
const char *mouse_dpi;
int dpi = DEFAULT_MOUSE_DPI;
+ /*
+* Trackpoints do not have dpi, instead hwdb may contain a
+* POINTINGSTICK_CONST_ACCEL value to compensate for sensitivity
+* differences between models, we translate this to a fake dpi.
+*/
+ if (libevdev_has_property(device-evdev, INPUT_PROP_POINTING_STICK))
+ return evdev_get_trackpoint_dpi(device);
+
mouse_dpi = udev_device_get_property_value(device-udev_device,
MOUSE_DPI);
if (mouse_dpi) {
diff --git a/src/evdev.h b/src/evdev.h
index c7a9e75..b63ae86 100644
--- a/src/evdev.h
+++ b/src/evdev.h
@@ -36,6 +36,13 @@
/* The HW DPI rate we normalize to before calculating pointer acceleration */
#define DEFAULT_MOUSE_DPI 1000
+
+/*
+ * The constant (linear) acceleration factor we use to normalize trackpoint
+ * deltas before calculating pointer acceleration.
+ */
+#define DEFAULT_TRACKPOINT_ACCEL 1.0
+
/* The fake resolution value for abs devices without resolution */
#define EVDEV_FAKE_RESOLUTION 1
diff --git a/src/libinput-util.c b/src/libinput-util.c
index 49e297a..4857435 100644
--- a/src/libinput-util.c
+++ b/src/libinput-util.c
@@ -29,6 +29,7 @@
#include config.h
#include ctype.h
+#include locale.h
#include stdarg.h
#include stdbool.h
#include stdio.h
@@ -201,3 +202,33 @@ parse_mouse_wheel_click_angle_property(const char *prop)
return angle;
}
+
+/**
+ * Helper function to parse the TRACKPOINT_CONST_ACCEL property from udev.
+ * Property is of the form:
+ * TRACKPOINT_CONST_ACCEL=float
+ *
+ * @param prop The value of the udev
RE: [PATCH] weston-launch: Fixed TTY switching
Hi
On Wed, Apr 8, 2015 at 2:03 AM, Bryce Harrington br...@osg.samsung.com
wrote:
On Wed, Apr 01, 2015 at 08:10:44AM +0100, mateuszx.potr...@intel.com
wrote:
From: Mateusz Polrola mateuszx.potr...@intel.com
After weston-launch is executing weston it cannot close TTY file,
because it is still required to properly handle SIGUSR1 and SIGUSR2
signals that are used for switching TTY.
Additionally after opening TTY it has to be activated, so that user
don't have to manually switch to TTY used by weston first to be able
to switch to other TTY.
During shutdown TTY file has to be reopened, as device was already
deinitialize by child process, but it is still required to cleanup VT
handling and leave system in sane state.
Signed-off-by: Mateusz Polrola mateuszx.potr...@intel.com
---
src/weston-launch.c | 20 +---
1 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/src/weston-launch.c b/src/weston-launch.c index
10c66de..312b955 100644
--- a/src/weston-launch.c
+++ b/src/weston-launch.c
@@ -46,6 +46,7 @@
#include linux/vt.h
#include linux/major.h
#include linux/kd.h
+#include linux/limits.h
#include pwd.h
#include grp.h
@@ -105,6 +106,7 @@ struct weston_launch {
pid_t child;
int verbose;
char *new_user;
+ char tty_path[PATH_MAX];
};
union cmsg_data { unsigned char b[4]; int fd; }; @@ -419,6 +421,13
@@ quit(struct weston_launch *wl, int status)
pam_end(wl-ph, err);
}
+ /* tty will be deinitialized by child process, so it has to be
+ * opened again to correctly cleanup vt handling. */
+ if (wl-tty != STDIN_FILENO) {
+ close(wl-tty);
+ wl-tty = open(wl-tty_path, O_RDWR | O_NOCTTY);
+ }
+
Why this? I don't get why wl-tty is not good enough and you need to reopen
the fd.
Trying to use wl-tty after child process is terminated will return errors from
ioctl.
I found when fd is reopened ioctls are working correctly. I'm suspecting that
child process closes tty's fd (it's passed to child process via environment
variable) when terminates and somehow deinitializes tty , wl-tty fd itself is
still valid, but ioctls are failing for some reason.
if (ioctl(wl-tty, KDSKBMUTE, 0)
ioctl(wl-tty, KDSKBMODE, wl-kb_mode))
fprintf(stderr, failed to restore keyboard mode:
%m\n); @@ -521,8 +530,10 @@ setup_tty(struct weston_launch *wl, const
char *tty)
t = ttyname(STDIN_FILENO);
if (t strcmp(t, tty) == 0)
wl-tty = STDIN_FILENO;
- else
+ else {
wl-tty = open(tty, O_RDWR | O_NOCTTY);
+ strcpy(wl-tty_path, tty);
+ }
I'm sure this is not going to ever be a problem since tty filenames
and paths are on the short side, but since the tty string is an input
parameter to this routine, it would be better defensive programming to
use strncpy.
I will change to strncpy.
} else {
int tty0 = open(/dev/tty0, O_WRONLY | O_CLOEXEC);
char filename[16];
@@ -535,6 +546,7 @@ setup_tty(struct weston_launch *wl, const char
*tty)
snprintf(filename, sizeof filename, /dev/tty%d, wl-ttynr);
wl-tty = open(filename, O_RDWR | O_NOCTTY);
+ strcpy(wl-tty_path, filename);
Since filename is set to a fixed length, I'm less worried about the
strcpy here, but for code consistency you might use strncpy here as
well.
close(tty0);
}
@@ -555,6 +567,10 @@ setup_tty(struct weston_launch *wl, const char
*tty)
wl-ttynr = minor(buf.st_rdev);
}
+ /* Activate tty, otherwise tty switches won't work right away. */
+ ioctl(wl-tty, VT_ACTIVATE, wl-ttynr);
+ ioctl(wl-tty, VT_WAITACTIVE, wl-ttynr);
+
Check the ioctl returns and issue perror() on failure.
Googling VT_WAITACTIVE shows misc. reports about it causing hangs in
years past. No idea if that's at all likely here in Wayland. But if
it is, it might be better to use a timed wait, checking the active tty
each cycle, like was done in this fix:
http://permalink.gmane.org/gmane.linux.kernel.suspend.devel/7119
Why do this at all? There is no reason to wait for the VT to become active.
Just
issue the VT_ACTIVATE and continue, it's async and that's good.
Btw., I'm no big fan of activating a VT when starting a compositor.
Xorg requires this as it cannot initialize in background, but new compositors
should really support this. Imagine you're started by a login-manager. You
really
want the compositor to initialize in background and wait to be switched to by
the login manager.
Yes, weston-launch is special, as it is its own login-manager. So I'd be fine
with
the VT_ACTIVATE. But i cannot see why we'd need
Re: modules=xwayland.so ignored (Was: Xfig menus don't appear - bug?)
On Wed, 8 Apr 2015 08:47:41 +0100 Felix E. Klee felix.k...@inka.de wrote: On Wed, Apr 8, 2015 at 8:20 AM, Pekka Paalanen ppaala...@gmail.com wrote: if you check Weston's output, it says which config file it is reading. Is it reading the one you are editing? To create the attached log file I ran: $ weston-launch –-verbose /tmp/weston.log 21 Starting `xterm` from within the Weston terminal emulator fails: $ xterm xterm Xt error: Can’t open display: xterm DISPLAY is not set Hi, try removing that tab from your .ini. The parser is more picky that you'd expect. That is, instead of: [core] modules=xwayland.so try: [core] modules=xwayland.so Should probably fix that one day... Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 3/3] xwm: Test hash_table_lookup() returns
On Tue, 07 Apr 2015 13:48:59 -0500 Derek Foreman der...@osg.samsung.com wrote: I don't like the temporary variable, it seems like you can just put the call to hash_table_lookup in the if statement and it will be clearer. I dunno, the if statement pretty much always exceeds 80 cols and I have to break it into a couple of lines and it starts to look pretty crappy. Yeah, let's prefer helper variables. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/3] xwm: fix extra break
On Tue, 7 Apr 2015 12:12:13 -0500
Derek Foreman der...@osg.samsung.com wrote:
The first break in TYPE_WM_PROTOCOLS was almost certainly intended to be
nested within the if statement.
Even if it wasn't, it makes sense there.
Signed-off-by: Derek Foreman der...@osg.samsung.com
---
xwayland/window-manager.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 7018c92..83ebfae 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -459,10 +459,10 @@ weston_wm_window_read_properties(struct
weston_wm_window *window)
case TYPE_WM_PROTOCOLS:
atom = xcb_get_property_value(reply);
for (i = 0; i reply-value_len; i++)
- if (atom[i] == wm-atom.wm_delete_window)
+ if (atom[i] == wm-atom.wm_delete_window) {
window-delete_window = 1;
- break;
-
+ break;
+ }
break;
case TYPE_WM_NORMAL_HINTS:
memcpy(window-size_hints,
R-b me and pushed.
8cb2587..b4deec6 master - master
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 1/5] evdev: fix crash for missing ABS_X/Y
Hi, On 08-04-15 01:54, Peter Hutterer wrote: libevdev_set_abs_info() is a noop if the event code isn't enabled on the device. This leaves ABS_X/Y on NULL, causing a crash later when dereferencing the absinfo. https://bugs.freedesktop.org/show_bug.cgi?id=89783 Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Entire series looks good, I've added my Rev-by and pushed this. Regards, Hans --- src/evdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/evdev.c b/src/evdev.c index a972b9d..115dc99 100644 --- a/src/evdev.c +++ b/src/evdev.c @@ -1444,9 +1444,9 @@ evdev_fix_android_mt(struct evdev_device *device) !libevdev_has_event_code(evdev, EV_ABS, ABS_MT_POSITION_Y)) return; - libevdev_set_abs_info(evdev, ABS_X, + libevdev_enable_event_code(evdev, EV_ABS, ABS_X, libevdev_get_abs_info(evdev, ABS_MT_POSITION_X)); - libevdev_set_abs_info(evdev, ABS_Y, + libevdev_enable_event_code(evdev, EV_ABS, ABS_Y, libevdev_get_abs_info(evdev, ABS_MT_POSITION_Y)); } ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] touchpad: count the tapping fingers separately from the main touchpad code
Hi,
On 08-04-15 05:54, Peter Hutterer wrote:
tp-nfingers_down gives us the current state of the touchpad but in the case
of the tapping state we need the touchpoints separately. If all touchpoints
end in the same SYN_REPORT frame, tp-nfingers_down is 0 when we handle the
touch releases. This changes the tap state to IDLE on the first release and
then logs a bug when the remaining touches are released while the touchpad is
in IDLE.
Avoid this by counting the fingers separately for the tap state, this way we
can count up/down with the down/up events as we process them for the tapping
state machine.
This also adds tests for 4 and 5-finger tapping which is how the bug was
discovered in the first place.
https://bugs.freedesktop.org/show_bug.cgi?id=89800
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
Looks good, I've added my Rev-by and pushed this.
Regards,
Hans
---
src/evdev-mt-touchpad-tap.c | 5 +-
src/evdev-mt-touchpad.h | 1 +
test/touchpad.c | 233
3 files changed, 238 insertions(+), 1 deletion(-)
diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index 4ba4ad2..78d4a0f 100644
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -452,7 +452,7 @@ tp_tap_dead_handle_event(struct tp_dispatch *tp,
switch (event) {
case TAP_EVENT_RELEASE:
- if (tp-nfingers_down == 0)
+ if (tp-tap.tap_finger_count == 0)
tp-tap.state = TAP_STATE_IDLE;
break;
case TAP_EVENT_TOUCH:
@@ -567,10 +567,12 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time)
t-tap.state = TAP_TOUCH_STATE_DEAD;
if (t-state == TOUCH_BEGIN) {
+ tp-tap.tap_finger_count++;
t-tap.state = TAP_TOUCH_STATE_TOUCH;
t-tap.initial = t-point;
tp_tap_handle_event(tp, t, TAP_EVENT_TOUCH, time);
} else if (t-state == TOUCH_END) {
+ tp-tap.tap_finger_count--;
tp_tap_handle_event(tp, t, TAP_EVENT_RELEASE, time);
t-tap.state = TAP_TOUCH_STATE_IDLE;
} else if (tp-tap.state != TAP_STATE_IDLE
@@ -754,6 +756,7 @@ tp_release_all_taps(struct tp_dispatch *tp, uint64_t now)
}
tp-tap.state = tp-nfingers_down ? TAP_STATE_DEAD : TAP_STATE_IDLE;
+ tp-tap.tap_finger_count = 0;
}
void
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 19a262e..b88dadd 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -255,6 +255,7 @@ struct tp_dispatch {
struct libinput_timer timer;
enum tp_tap_state state;
uint32_t buttons_pressed;
+ unsigned int tap_finger_count;
} tap;
struct {
diff --git a/test/touchpad.c b/test/touchpad.c
index 6fa2301..b06e00d 100644
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -369,6 +369,38 @@ START_TEST(touchpad_2fg_tap_inverted)
}
END_TEST
+START_TEST(touchpad_2fg_tap_quickrelease)
+{
+ struct litest_device *dev = litest_current_device();
+ struct libinput *li = dev-libinput;
+
+ libinput_device_config_tap_set_enabled(dev-libinput_device,
+ LIBINPUT_CONFIG_TAP_ENABLED);
+
+ litest_drain_events(dev-libinput);
+
+ litest_touch_down(dev, 0, 50, 50);
+ litest_touch_down(dev, 1, 70, 70);
+ litest_event(dev, EV_ABS, ABS_MT_SLOT, 0);
+ litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
+ litest_event(dev, EV_ABS, ABS_MT_SLOT, 1);
+ litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
+ litest_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 0);
+ litest_event(dev, EV_KEY, BTN_TOUCH, 0);
+ litest_event(dev, EV_SYN, SYN_REPORT, 0);
+
+ libinput_dispatch(li);
+
+ litest_assert_button_event(li, BTN_RIGHT,
+ LIBINPUT_BUTTON_STATE_PRESSED);
+ litest_timeout_tap();
+ litest_assert_button_event(li, BTN_RIGHT,
+ LIBINPUT_BUTTON_STATE_RELEASED);
+
+ litest_assert_empty_queue(li);
+}
+END_TEST
+
START_TEST(touchpad_1fg_tap_click)
{
struct litest_device *dev = litest_current_device();
@@ -705,6 +737,46 @@ START_TEST(touchpad_3fg_tap)
}
END_TEST
+START_TEST(touchpad_3fg_tap_quickrelease)
+{
+ struct litest_device *dev = litest_current_device();
+ struct libinput *li = dev-libinput;
+
+ if (libevdev_get_abs_maximum(dev-evdev,
+ABS_MT_SLOT) = 2)
+ return;
+
+ libinput_device_config_tap_set_enabled(dev-libinput_device,
+ LIBINPUT_CONFIG_TAP_ENABLED);
+
+ litest_drain_events(li);
+
+ litest_touch_down(dev, 0, 50, 50);
+
Re: [PATCH libinput] evdev: fix inverted mouse normalization
Hi,
On 08-04-15 08:05, Peter Hutterer wrote:
Regression introduced in 9f8edc5fd880e0a9c482b36e6b4120ccc056ee0b where it
changed from delta / (dpi/default) to delta * dpi/default, causing the inverse
effect of what the dpi setting is supposed to achieve.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
Looks good, I've added my Rev-by and pushed this.
Regards,
Hans
---
src/evdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/evdev.c b/src/evdev.c
index 00450bf..243cd22 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -230,8 +230,8 @@ normalize_delta(struct evdev_device *device,
const struct device_coords *delta,
struct normalized_coords *normalized)
{
- normalized-x = delta-x * (double)device-dpi / DEFAULT_MOUSE_DPI;
- normalized-y = delta-y * (double)device-dpi / DEFAULT_MOUSE_DPI;
+ normalized-x = delta-x * DEFAULT_MOUSE_DPI / (double)device-dpi;
+ normalized-y = delta-y * DEFAULT_MOUSE_DPI / (double)device-dpi;
}
static void
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2 0/9] Enable test configuration
On Tue, 7 Apr 2015 15:32:51 -0700
Bryce Harrington br...@osg.samsung.com wrote:
On Tue, Apr 07, 2015 at 02:44:59PM +0300, Pekka Paalanen wrote:
On Thu, 2 Apr 2015 19:16:49 -0700
Bryce Harrington br...@osg.samsung.com wrote:
This enables tests to provide their own .ini files for configuring
weston, via the recently added --config option. Also included in this
set is some related cleanup in weston-tests-env.
[Update v2]
Implemented review comments from pekka:
* Dropped ~/.config as default location of weston.ini in help text.
* Look for generated .ini files in build dir, as well as static ini
files in the source tree (in tests/ subdir)
* Simplified the .la/.so logic.
* Added a test case for config loading
Also did some further fussy cleanup of weston-tests-env.
Now, previously I was concerned that these clean-ups (patches 2, 5, 6)
would actually make weston-tests-env harder to read. I still wasn't
sure, so I tried it and rebased my ivi-test-5 branch on top your
complete series:
http://cgit.collabora.com/git/user/pq/weston.git/log/?h=ivi-test-5b
After my rebase, weston-tests-env now looks like this:
--
#!/bin/bash
TEST_FILE=${1##*/}
TEST_NAME=${TEST_FILE%.*}
if [ -z $TEST_NAME ]; then
echo usage: $(basename $0) test name
exit 1;
fi
WESTON=$abs_builddir/weston
LOGDIR=$abs_builddir/logs
mkdir -p $LOGDIR || exit
OUTLOG=$LOGDIR/${TEST_NAME}-log.txt
SERVERLOG=$LOGDIR/${TEST_NAME}-serverlog.txt
rm -f $SERVERLOG || exit
client=
BACKEND=${BACKEND:-headless-backend.so}
MODDIR=$abs_builddir/.libs
TEST_MODULE=$MODDIR/${TEST_FILE/.la/.so}
SHELL_PLUGIN=$MODDIR/desktop-shell.so
XWAYLAND_PLUGIN=$MODDIR/xwayland.so
if [[ ${TEST_MODULE} != *.so ]]; then
export WESTON_TEST_CLIENT_PATH=$abs_builddir/$TEST_FILE
client=$($WESTON_TEST_CLIENT_PATH --params)
TEST_MODULE=$MODDIR/weston-test.so
fi
CONFIG_FILE=${TEST_NAME}.ini
if [ -e ${abs_builddir}/${CONFIG_FILE} ]; then
CONFIG=--config=${abs_builddir}/${CONFIG_FILE}
elif [ -e ${abs_top_srcdir}/tests/${CONFIG_FILE} ]; then
CONFIG=--config=${abs_top_srcdir}/tests/${CONFIG_FILE}
else
CONFIG=--no-config
fi
if [[ ${TEST_NAME} = ivi-* ]]; then
SHELL_PLUGIN=$MODDIR/ivi-shell.so
if [[ $CONFIG = --no-config ]]; then
CONFIG=--config=$abs_builddir/tests/weston-ivi.ini
fi
XWAYLAND_PLUGIN=
fi
if [[ ${TEST_FILE} = ivi-*.la ]]; then
client=$client --ivi-module=$TEST_MODULE
TEST_MODULE=$MODDIR/weston-test.so
fi
export WESTON_BUILD_DIR=$abs_builddir
$WESTON \
--shell=$SHELL_PLUGIN \
--socket=test-${TEST_NAME} \
--modules=$TEST_MODULE,$XWAYLAND_PLUGIN \
--backend=$MODDIR/$BACKEND \
--log=$SERVERLOG \
$CONFIG \
$client \
$OUTLOG
--
Is this what you meant?
Yes, that looks good.
Hi,
sorry, I find it very confusing to read. It took me half an hour
to figure out what I need to add to add my ivi-shell test cases. With
the old way
http://cgit.collabora.com/git/user/pq/weston.git/tree/tests/weston-tests-env?h=ivi-test-5
it was as simple as copying the block and changing what I needed.
When you read the above, is it clear that there are fundamentally four
different types of test setups? What are the runtime parameters to each
of them? I really can't see what they are without running that code
either mentally or for real, because there are so many combinations of
if-clauses that overwrite previously assigned variables. IOW, I find it
hard to read.
If I want to change the parameters to one test case, I cannot just
change one place. I have to again parse through every possible
combination of the if-clauses to see that I don't break an unrelated
case.
Sure, the above code is compact and duplicates nothing, but it also
intertwines things that would be best kept separate.
The old way in ivi-test-5 is not perfect either. It duplicates more
things than it should. May I suggest a compromise solution? Keep the
big case structure, but perhaps refactor the actual weston invocation
outside it. The case structure could assign variables, just once and
never overwriting, so it would be clear to read yet duplicate the
minimum. Things like --socket, --backend and --log shouldn't be
duplicated since they never vary by test type.
That's just the overall structure, there would still be many details to
clean up. Would that work?
If you want to work on that, I can hold up with the ivi testing series
another day or two, but I'd really like to land it this week.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] config: use simpler regexp syntax to get dot version
On Thu, 2 Apr 2015 19:20:00 -0700 Bill Spitzak spit...@gmail.com wrote: I wasted a lot of time before I figured out that I needed to add those square brackets to get this to work. Sigh... --- configure.ac |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 6bbec59..271eec3 100644 --- a/configure.ac +++ b/configure.ac @@ -141,7 +141,7 @@ if test x$enable_documentation = xyes; then AC_MSG_ERROR([Documentation build requested but graphviz's dot not found. Install graphviz or disable the documentation using --disable-documentation]) fi AC_MSG_CHECKING([for compatible dot version]) - dot_version=`$DOT -V 21|$GREP -oP '(?=version\W)@:@0-9.@:@*(?=\W(.*))'` + dot_version=`$DOT -V 21|$GREP -o ['[0-9]*\.[0-9]*\.[0-9]*']` AS_VERSION_COMPARE([$dot_version], [2.26.0], [AC_MSG_RESULT([no]) AC_MSG_ERROR([Graphviz dot $dot_version too old. Graphviz 2.26+ required for documentation build. Install required graphviz version or disable the documentation using --disable-documentation])], For the record, I think those @:@ and @:@ are square brackets. They are just an M4 escape thing to prevent M4 from interpreting those square brackets and possibly removing them. Oh yeah, quadrigraphs: http://stackoverflow.com/questions/2308721/how-do-i-escape-text-in-autoconf-m4 It's just another way to protect the square brackets. Your way should work fine here too, since these are not M4 macro arguments. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
modules=xwayland.so ignored (Was: Xfig menus don't appear - bug?)
On Wed, Apr 8, 2015 at 8:20 AM, Pekka Paalanen ppaala...@gmail.com wrote: if you check Weston's output, it says which config file it is reading. Is it reading the one you are editing? To create the attached log file I ran: $ weston-launch –-verbose /tmp/weston.log 21 Starting `xterm` from within the Weston terminal emulator fails: $ xterm xterm Xt error: Can’t open display: xterm DISPLAY is not set weston.ini Description: Binary data weston.log Description: Binary data ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 2/6] compositor-drm: Allow instant start of repaint loop.
On Wed, 08 Apr 2015 03:12:25 +0200
Mario Kleiner mario.kleiner...@gmail.com wrote:
On 04/07/2015 09:34 AM, Pekka Paalanen wrote:
On Sat, 04 Apr 2015 19:45:10 +0200
Mario Kleiner mario.kleiner...@gmail.com wrote:
On 04/02/2015 01:37 PM, Pekka Paalanen wrote:
On Thu, 2 Apr 2015 07:10:50 +0200
Mario Kleiner mario.kleiner...@gmail.com wrote:
drm_output_start_repaint_loop() incurred a delay of
one refresh cycle by using a no-op page-flip to get
an accurate vblank timestamp as reference. This causes
unwanted lag whenever Weston exited its repaint loop, e.g.,
whenever an application wants to repaint with less than
full video refresh rate but still minimum lag.
Try to use the drmWaitVblank ioctl to get a proper
timestamp instantaneously without lag. If that does
not work, fall back to the old method of idle page-flip.
This optimization should work on any drm/kms driver
which supports high precision vblank timestamping.
As of Linux 4.0 these would be intel, radeon and
nouveau on all supported gpu's.
Signed-off-by: Mario Kleiner mario.kleiner...@gmail.com
---
src/compositor-drm.c | 41 -
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index fe59811..4a7baa1 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -225,6 +225,9 @@ static const char default_seat[] = seat0;
static void
drm_output_set_cursor(struct drm_output *output);
+static void
+drm_output_update_msc(struct drm_output *output, unsigned int seq);
+
static int
drm_sprite_crtc_supported(struct drm_output *output, uint32_t
supported)
{
@@ -704,6 +707,12 @@ err_pageflip:
return -1;
}
+static int64_t
+timespec_to_nsec(const struct timespec *a)
+{
+return (int64_t)a-tv_sec * 1LL + a-tv_nsec;
Hi,
are you sure this cannot overflow? I think tv_sec could be a int64_t.
That no-one gets the idea of initializing the clock in the kernel to
some huge value just to fish out this kind of overflows?
I almost literally copied it from compositor.c, your repaint delay
handling. I think it can't overflow the way it is used here to only
remap from kernel timestamps. The kernel delivers its timestamps in
struct timeval, so tv_sec (== long int) from kernel could be a 64-bit
int on 64-Bit kernels, but the values put into it are always derived
from ktime_t which is nanoseconds stored in 64 bit signed integers. So
at least with the current kernel implementation it can't overflow. And
32-Bit kernels would hit such overflows before user space as their
struct timeval seems to have 32 bit int for tv_sec. So i think with the
current kernel ioctl interfaces it should be safe.
Hi,
that's nice to know about the kernel. Isn't tv_sec actually a time_t? I
once or twice tried to find out what was guaranteed about time_t and I
couldn't really find much, except that it may be even 64-bit or it
maybe be less.
Yes. Trying to follow the userspace headers creates headache due to so
many indirections. Looking at the kernel side of timestamps returned by
the DRM ends up with long on 32-Bit or 64-Bit kernels = int32_t on
32-Bit kernel, int64_t on 64-Bit kernel. For the x32 interface it is
defined as long long = int64_t with the comment should match the size
of the regular 64-Bit kernel.
So tv_sec could be 32-Bit or 64-Bit. But the DRM does all its timestamp
calculations internally as ktime_t which is always Nanoseconds inside
signed int64_t and only converts into struct timeval at the end. That's
why i think it can't overflow because the kernel payload we get out of
our struct timespec can't be more than int64_t nanoseconds. The kernel
would overflow things internally before they would reach us, although
with CLOCK_MONOTONIC as timebase that would require an uptime of almost
300 years, so no immediate danger here.
It's hard for me to draw the line between trusting only specs and
trusting the implementation I have at hand.
Btw. I always try to do a timespec_to_nsec() on a timespec that already
is a difference, never an absolute time. Below you are subtracting
nsecs instead of timespecs, so that got me wondering.
I could change it if you want, but then i'd need to duplicate more
helper functions in compositor-drm.c and as you said you'll have some
shared helpers prepared soon anyway i would just wait until those are
merged.
Yeah, you don't have to do that. :-)
I'm not sure when I'd get to adding those functions, but if/when I do,
it's up to me to convert the then current code, so no worries.
+}
+
static void
drm_output_start_repaint_loop(struct weston_output *output_base)
{
@@ -711,7 +720,13 @@ drm_output_start_repaint_loop(struct weston_output
*output_base)
struct drm_compositor *compositor =
Re: [PATCH] weston-launch: Fixed TTY switching
David,
would you happen to have a moment to look at this, please?
Thanks,
pq
On Wed, 1 Apr 2015 08:10:44 +0100
mateuszx.potr...@intel.com wrote:
From: Mateusz Polrola mateuszx.potr...@intel.com
After weston-launch is executing weston it cannot close TTY file,
because it is still required to properly handle SIGUSR1 and SIGUSR2
signals that are used for switching TTY.
Additionally after opening TTY it has to be activated, so that user
don't have to manually switch to TTY used by weston first to be able to switch
to other TTY.
During shutdown TTY file has to be reopened, as device was already
deinitialize by child
process, but it is still required to cleanup VT handling and leave
system in sane state.
Signed-off-by: Mateusz Polrola mateuszx.potr...@intel.com
---
src/weston-launch.c | 20 +---
1 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/src/weston-launch.c b/src/weston-launch.c
index 10c66de..312b955 100644
--- a/src/weston-launch.c
+++ b/src/weston-launch.c
@@ -46,6 +46,7 @@
#include linux/vt.h
#include linux/major.h
#include linux/kd.h
+#include linux/limits.h
#include pwd.h
#include grp.h
@@ -105,6 +106,7 @@ struct weston_launch {
pid_t child;
int verbose;
char *new_user;
+ char tty_path[PATH_MAX];
};
union cmsg_data { unsigned char b[4]; int fd; };
@@ -419,6 +421,13 @@ quit(struct weston_launch *wl, int status)
pam_end(wl-ph, err);
}
+ /* tty will be deinitialized by child process, so it has to be
+ * opened again to correctly cleanup vt handling. */
+ if (wl-tty != STDIN_FILENO) {
+ close(wl-tty);
+ wl-tty = open(wl-tty_path, O_RDWR | O_NOCTTY);
+ }
+
if (ioctl(wl-tty, KDSKBMUTE, 0)
ioctl(wl-tty, KDSKBMODE, wl-kb_mode))
fprintf(stderr, failed to restore keyboard mode: %m\n);
@@ -521,8 +530,10 @@ setup_tty(struct weston_launch *wl, const char *tty)
t = ttyname(STDIN_FILENO);
if (t strcmp(t, tty) == 0)
wl-tty = STDIN_FILENO;
- else
+ else {
wl-tty = open(tty, O_RDWR | O_NOCTTY);
+ strcpy(wl-tty_path, tty);
+ }
} else {
int tty0 = open(/dev/tty0, O_WRONLY | O_CLOEXEC);
char filename[16];
@@ -535,6 +546,7 @@ setup_tty(struct weston_launch *wl, const char *tty)
snprintf(filename, sizeof filename, /dev/tty%d, wl-ttynr);
wl-tty = open(filename, O_RDWR | O_NOCTTY);
+ strcpy(wl-tty_path, filename);
close(tty0);
}
@@ -555,6 +567,10 @@ setup_tty(struct weston_launch *wl, const char *tty)
wl-ttynr = minor(buf.st_rdev);
}
+ /* Activate tty, otherwise tty switches won't work right away. */
+ ioctl(wl-tty, VT_ACTIVATE, wl-ttynr);
+ ioctl(wl-tty, VT_WAITACTIVE, wl-ttynr);
+
if (ioctl(wl-tty, KDGKBMODE, wl-kb_mode))
error(1, errno, failed to get current keyboard mode: %m\n);
@@ -744,8 +760,6 @@ main(int argc, char *argv[])
launch_compositor(wl, argc - optind, argv + optind);
close(wl.sock[1]);
- if (wl.tty != STDIN_FILENO)
- close(wl.tty);
while (1) {
struct pollfd fds[2];
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] evdev: fix inverted mouse normalization
p.s.
I've not pushed the patch attached to:
https://bugs.freedesktop.org/show_bug.cgi?id=89949
As I assume you want to wait for feedback from the bug reporter,
but it looks good to me, so feel free to push it.
On 08-04-15 08:05, Peter Hutterer wrote:
Regression introduced in 9f8edc5fd880e0a9c482b36e6b4120ccc056ee0b where it
changed from delta / (dpi/default) to delta * dpi/default, causing the inverse
effect of what the dpi setting is supposed to achieve.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/evdev.c b/src/evdev.c
index 00450bf..243cd22 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -230,8 +230,8 @@ normalize_delta(struct evdev_device *device,
const struct device_coords *delta,
struct normalized_coords *normalized)
{
- normalized-x = delta-x * (double)device-dpi / DEFAULT_MOUSE_DPI;
- normalized-y = delta-y * (double)device-dpi / DEFAULT_MOUSE_DPI;
+ normalized-x = delta-x * DEFAULT_MOUSE_DPI / (double)device-dpi;
+ normalized-y = delta-y * DEFAULT_MOUSE_DPI / (double)device-dpi;
}
static void
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Xfig menus don't appear - bug?
On Tue, 7 Apr 2015 23:54:17 +0100 Felix E. Klee felix.k...@inka.de wrote: By the way, what also doesn’t work is loading XWayland based on configuration settings in my `~/.config/weston.ini`: [core] modules=xwayland.so What works is specifying XWayland on the command line: $ weston-launch -- --modules=xwayland.so Hi, if you check Weston's output, it says which config file it is reading. Is it reading the one you are editing? Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] weston-launch: Fixed TTY switching
Hi
On Wed, Apr 8, 2015 at 2:03 AM, Bryce Harrington br...@osg.samsung.com wrote:
On Wed, Apr 01, 2015 at 08:10:44AM +0100, mateuszx.potr...@intel.com wrote:
From: Mateusz Polrola mateuszx.potr...@intel.com
After weston-launch is executing weston it cannot close TTY file,
because it is still required to properly handle SIGUSR1 and SIGUSR2
signals that are used for switching TTY.
Additionally after opening TTY it has to be activated, so that user
don't have to manually switch to TTY used by weston first to be able to
switch
to other TTY.
During shutdown TTY file has to be reopened, as device was already
deinitialize by child
process, but it is still required to cleanup VT handling and leave
system in sane state.
Signed-off-by: Mateusz Polrola mateuszx.potr...@intel.com
---
src/weston-launch.c | 20 +---
1 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/src/weston-launch.c b/src/weston-launch.c
index 10c66de..312b955 100644
--- a/src/weston-launch.c
+++ b/src/weston-launch.c
@@ -46,6 +46,7 @@
#include linux/vt.h
#include linux/major.h
#include linux/kd.h
+#include linux/limits.h
#include pwd.h
#include grp.h
@@ -105,6 +106,7 @@ struct weston_launch {
pid_t child;
int verbose;
char *new_user;
+ char tty_path[PATH_MAX];
};
union cmsg_data { unsigned char b[4]; int fd; };
@@ -419,6 +421,13 @@ quit(struct weston_launch *wl, int status)
pam_end(wl-ph, err);
}
+ /* tty will be deinitialized by child process, so it has to be
+ * opened again to correctly cleanup vt handling. */
+ if (wl-tty != STDIN_FILENO) {
+ close(wl-tty);
+ wl-tty = open(wl-tty_path, O_RDWR | O_NOCTTY);
+ }
+
Why this? I don't get why wl-tty is not good enough and you need to
reopen the fd.
if (ioctl(wl-tty, KDSKBMUTE, 0)
ioctl(wl-tty, KDSKBMODE, wl-kb_mode))
fprintf(stderr, failed to restore keyboard mode: %m\n);
@@ -521,8 +530,10 @@ setup_tty(struct weston_launch *wl, const char *tty)
t = ttyname(STDIN_FILENO);
if (t strcmp(t, tty) == 0)
wl-tty = STDIN_FILENO;
- else
+ else {
wl-tty = open(tty, O_RDWR | O_NOCTTY);
+ strcpy(wl-tty_path, tty);
+ }
I'm sure this is not going to ever be a problem since tty filenames and
paths are on the short side, but since the tty string is an input
parameter to this routine, it would be better defensive programming to
use strncpy.
} else {
int tty0 = open(/dev/tty0, O_WRONLY | O_CLOEXEC);
char filename[16];
@@ -535,6 +546,7 @@ setup_tty(struct weston_launch *wl, const char *tty)
snprintf(filename, sizeof filename, /dev/tty%d, wl-ttynr);
wl-tty = open(filename, O_RDWR | O_NOCTTY);
+ strcpy(wl-tty_path, filename);
Since filename is set to a fixed length, I'm less worried about the
strcpy here, but for code consistency you might use strncpy here as
well.
close(tty0);
}
@@ -555,6 +567,10 @@ setup_tty(struct weston_launch *wl, const char *tty)
wl-ttynr = minor(buf.st_rdev);
}
+ /* Activate tty, otherwise tty switches won't work right away. */
+ ioctl(wl-tty, VT_ACTIVATE, wl-ttynr);
+ ioctl(wl-tty, VT_WAITACTIVE, wl-ttynr);
+
Check the ioctl returns and issue perror() on failure.
Googling VT_WAITACTIVE shows misc. reports about it causing hangs in
years past. No idea if that's at all likely here in Wayland. But if it
is, it might be better to use a timed wait, checking the active tty each
cycle, like was done in this fix:
http://permalink.gmane.org/gmane.linux.kernel.suspend.devel/7119
Why do this at all? There is no reason to wait for the VT to become
active. Just issue the VT_ACTIVATE and continue, it's async and that's
good.
Btw., I'm no big fan of activating a VT when starting a compositor.
Xorg requires this as it cannot initialize in background, but new
compositors should really support this. Imagine you're started by a
login-manager. You really want the compositor to initialize in
background and wait to be switched to by the login manager.
Yes, weston-launch is special, as it is its own login-manager. So I'd
be fine with the VT_ACTIVATE. But i cannot see why we'd need
VT_WAITACTIVE?
if (ioctl(wl-tty, KDGKBMODE, wl-kb_mode))
error(1, errno, failed to get current keyboard mode: %m\n);
@@ -744,8 +760,6 @@ main(int argc, char *argv[])
launch_compositor(wl, argc - optind, argv + optind);
close(wl.sock[1]);
- if (wl.tty != STDIN_FILENO)
- close(wl.tty);
I'm having some trouble following the move of the close logic. I trust
what you've done is correct but it's not evident to me why?
This is weird, indeed. No idea why the TTY is
Re: [PATCH weston 1/3] gl-renderer: fix EGL initialization steps
Looks good to me !
2015-04-08 16:02 GMT+02:00 Pekka Paalanen ppaala...@gmail.com:
From: Manuel Bachmann manuel.bachm...@open.eurogiciel.org
Some DRI drivers, including VMware vmwgfx, do not support
calling eglQueryString() with a EGL_NO_DISPLAY parameter.
Just as we do in gl_renderer_supports(), which returns 0
but does not fail in this case, do not fail in
gl_renderer_setup_egl_extensions().
Signed-off-by: Manuel Bachmann manuel.bachm...@open.eurogiciel.org
[Pekka: split the patch]
Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk
---
src/gl-renderer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index b3b2364..5a2ed9f 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -2136,7 +2136,7 @@ gl_renderer_setup_egl_extensions(struct
weston_compositor *ec)
(const char *) eglQueryString(EGL_NO_DISPLAY,
EGL_EXTENSIONS);
if (!extensions) {
weston_log(Retrieving EGL client extension string
failed.\n);
- return -1;
+ return 0;
}
if (strstr(extensions, EGL_EXT_platform_base))
--
2.0.5
--
Regards,
*Manuel BACHMANN Tizen Project VANNES-FR*
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/3] gl-renderer: fix EGL initialization steps
On 8 April 2015 at 17:17, Manuel Bachmann
manuel.bachm...@open.eurogiciel.org wrote:
Looks good to me !
All three R-b and pushed; thanks.
b4deec6..8b69d03 master - master
Cheers,
Daniel
2015-04-08 16:02 GMT+02:00 Pekka Paalanen ppaala...@gmail.com:
From: Manuel Bachmann manuel.bachm...@open.eurogiciel.org
Some DRI drivers, including VMware vmwgfx, do not support
calling eglQueryString() with a EGL_NO_DISPLAY parameter.
Just as we do in gl_renderer_supports(), which returns 0
but does not fail in this case, do not fail in
gl_renderer_setup_egl_extensions().
Signed-off-by: Manuel Bachmann manuel.bachm...@open.eurogiciel.org
[Pekka: split the patch]
Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk
---
src/gl-renderer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index b3b2364..5a2ed9f 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -2136,7 +2136,7 @@ gl_renderer_setup_egl_extensions(struct
weston_compositor *ec)
(const char *) eglQueryString(EGL_NO_DISPLAY,
EGL_EXTENSIONS);
if (!extensions) {
weston_log(Retrieving EGL client extension string
failed.\n);
- return -1;
+ return 0;
}
if (strstr(extensions, EGL_EXT_platform_base))
--
2.0.5
--
Regards,
Manuel BACHMANN
Tizen Project
VANNES-FR
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 3/3] xwm: Test hash_table_lookup() returns
On 08/04/15 07:41 AM, Pekka Paalanen wrote:
On Tue, 7 Apr 2015 12:12:15 -0500
Derek Foreman der...@osg.samsung.com wrote:
Make sure we always test hash_table_lookup()s return to prevent trying to
dereference a NULL window.
Signed-off-by: Derek Foreman der...@osg.samsung.com
---
xwayland/window-manager.c | 103
+-
1 file changed, 74 insertions(+), 29 deletions(-)
diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 3967670..9388d2e 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -446,10 +446,16 @@ weston_wm_window_read_properties(struct
weston_wm_window *window)
strndup(xcb_get_property_value(reply),
xcb_get_property_value_length(reply));
break;
-case XCB_ATOM_WINDOW:
+case XCB_ATOM_WINDOW: {
+int found;
xid = xcb_get_property_value(reply);
-hash_table_lookup(wm-window_hash, *xid, (struct
weston_wm_window **)p);
+found = hash_table_lookup(wm-window_hash, *xid,
+ (struct weston_wm_window **)p);
+if (!found)
+weston_log(XCB_ATOM_WINDOW contains window
+id not found in hash table.\n);
break;
+}
case XCB_ATOM_CARDINAL:
case XCB_ATOM_ATOM:
atom = xcb_get_property_value(reply);
@@ -586,14 +592,18 @@ weston_wm_handle_configure_request(struct weston_wm
*wm, xcb_generic_event_t *ev
(xcb_configure_request_event_t *) event;
struct weston_wm_window *window;
uint32_t mask, values[16];
-int x, y, width, height, i = 0;
+int x, y, width, height, i = 0, found;
wm_log(XCB_CONFIGURE_REQUEST (window %d) %d,%d @ %dx%d\n,
configure_request-window,
configure_request-x, configure_request-y,
configure_request-width, configure_request-height);
-hash_table_lookup(wm-window_hash, configure_request-window, window);
+found = hash_table_lookup(wm-window_hash,
+ configure_request-window,
+ window);
+if (!found)
+return;
if (window-fullscreen) {
weston_wm_window_send_configure_notify(window);
@@ -650,6 +660,8 @@ our_resource(struct weston_wm *wm, uint32_t id)
static void
weston_wm_handle_configure_notify(struct weston_wm *wm, xcb_generic_event_t
*event)
{
+int found;
+
xcb_configure_notify_event_t *configure_notify =
(xcb_configure_notify_event_t *) event;
struct weston_wm_window *window;
@@ -659,7 +671,12 @@ weston_wm_handle_configure_notify(struct weston_wm *wm,
xcb_generic_event_t *eve
configure_notify-x, configure_notify-y,
configure_notify-width, configure_notify-height);
-hash_table_lookup(wm-window_hash, configure_notify-window, window);
+found = hash_table_lookup(wm-window_hash,
+ configure_notify-window,
+ window);
+if (!found)
+return;
Hi,
on a quick look this whole patch seems fine, I'll read it properly the
next time. :-)
Would it save anything to have a helper:
bool
wm_lookup_window(struct weston_wm *wm, uint32_t xid,
struct weston_wm_window **ret);
or something like that? We're always passing wm-window_hash anyway.
Letting you write
found = wm_lookup_window(wm, blargh, window);
which is a bit shorter.
That way you wouldn't need to change the pointer type for
hash_table_lookup() either, leaving it generic if we happen to need a
hash of something else. Hm?
And the shorter form probably keeps most if statements under 80 cols
without a temp var. :)
Sure, I'll do that today.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] gl-renderer: fix EGL initialization steps
On Fri, 3 Apr 2015 07:05:23 +0200
Manuel Bachmann manuel.bachm...@open.eurogiciel.org wrote:
From: Manuel Bachmann manuel.bachm...@open.eurogiciel.org
Some DRI drivers, including VMware vmwgfx, do not support
calling eglQueryString() with a EGL_NO_DISPLAY parameter.
Just as we do in gl_renderer_supports(), which returns 0
but does not fail in this case, do not fail in
gl_renderer_setup_egl_extensions().
With some versions of Mesa, EGL client extensions may
very well be defined without the corresponding platform
extensions (EGL_EXT_platform_x11/wayland/gbm). Do not
fail in this case, but report lack of information.
Signed-off-by: Manuel Bachmann manuel.bachm...@open.eurogiciel.org
---
src/gl-renderer.c | 33 ++---
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index b3b2364..d9b6a66 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -2136,7 +2136,7 @@ gl_renderer_setup_egl_extensions(struct
weston_compositor *ec)
(const char *) eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS);
if (!extensions) {
weston_log(Retrieving EGL client extension string failed.\n);
- return -1;
+ return 0;
Hi,
I think this should be two different patches. This hunk is the first
patch.
}
if (strstr(extensions, EGL_EXT_platform_base))
@@ -2213,23 +2213,26 @@ gl_renderer_supports(struct weston_compositor *ec,
extensions);
}
- snprintf(s, sizeof s, EGL_KHR_platform_%s, extension_suffix);
- if (strstr(extensions, s))
- return 1;
+ if (strstr(extensions, EGL_EXT_client_extensions)) {
Wasn't this supposed to be testing for EGL_EXT_platform_base?
Also, if platform_base is not there, just return 0 right away, no need
to add indentation levels to the rest of the code.
+ snprintf(s, sizeof s, EGL_KHR_platform_%s, extension_suffix);
+ if (strstr(extensions, s))
+ return 1;
- snprintf(s, sizeof s, EGL_EXT_platform_%s, extension_suffix);
- if (strstr(extensions, s))
- return 1;
+ snprintf(s, sizeof s, EGL_EXT_platform_%s, extension_suffix);
+ if (strstr(extensions, s))
+ return 1;
- snprintf(s, sizeof s, EGL_MESA_platform_%s, extension_suffix);
- if (strstr(extensions, s))
- return 1;
+ snprintf(s, sizeof s, EGL_MESA_platform_%s, extension_suffix);
+ if (strstr(extensions, s))
+ return 1;
- /* at this point we definitely have some client extensions but
- * haven't found the supplied client extension, so chances are it's
- * not supported. */
-
- return -1;
+ /* at this point we definitely have some client extensions but
+ * haven't found the supplied client extension, so it may be
+ * supported or not. */
+ return 0;
+ } else {
+ return -1;
For platform_base test, these conditionals are reversed.
+ }
}
static const char *
I think I'll make my own version of this for you, I need to fix some
other damage close by, too. Stay tuned.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: modules=xwayland.so ignored (Was: Xfig menus don't appear - bug?)
On Wed, Apr 8, 2015 at 11:16 AM, Pekka Paalanen ppaala...@gmail.com wrote: try removing that tab from your .ini. I tried with spaces, with tab, and without; But probably when I first tried without indentation, I didn’t save, because: After removing the tab, now it works! Should probably fix that one day... Don’t know. Perhaps without indentation is how ini files have to be formatted. I presume you use some ini parser library. There are various. In the meantime, someone could fix documentation for [XServer][1] which currently reads: weston.ini == Add this to ~/.config/weston.ini (or use the --modules=xwayland.so command line argument): [core] modules=xwayland.so [1]: http://wayland.freedesktop.org/xserver.html ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 3/3] gl-renderer: fix configless_context check
From: Pekka Paalanen pekka.paala...@collabora.co.uk
EGL_MESA_configless_context is a display extension. The query for client
extensions was overwriting the pointer, so it was being searched from
the client extensions instead.
Fix any confusion here by moving all client extension checks into
another function. Drop a useless cast.
Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk
---
src/gl-renderer.c | 33 -
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index 35cd7e7..ae3122f 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -2086,6 +2086,24 @@ gl_renderer_destroy(struct weston_compositor *ec)
free(gr);
}
+static void
+renderer_setup_egl_client_extensions(struct gl_renderer *gr)
+{
+ const char *extensions;
+
+ extensions = eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS);
+ if (!extensions) {
+ weston_log(Retrieving EGL client extension string failed.\n);
+ return;
+ }
+
+ if (strstr(extensions, EGL_EXT_platform_base))
+ gr-create_platform_window =
+ (void *)
eglGetProcAddress(eglCreatePlatformWindowSurfaceEXT);
+ else
+ weston_log(warning: EGL_EXT_platform_base not supported.\n);
+}
+
static int
gl_renderer_setup_egl_extensions(struct weston_compositor *ec)
{
@@ -2132,24 +2150,13 @@ gl_renderer_setup_egl_extensions(struct
weston_compositor *ec)
supported. Performance could be affected.\n);
#endif
- extensions =
- (const char *) eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS);
- if (!extensions) {
- weston_log(Retrieving EGL client extension string failed.\n);
- return 0;
- }
-
- if (strstr(extensions, EGL_EXT_platform_base))
- gr-create_platform_window =
- (void *)
eglGetProcAddress(eglCreatePlatformWindowSurfaceEXT);
- else
- weston_log(warning: EGL_EXT_platform_base not supported.\n);
-
#ifdef EGL_MESA_configless_context
if (strstr(extensions, EGL_MESA_configless_context))
gr-has_configless_context = 1;
#endif
+ renderer_setup_egl_client_extensions(gr);
+
return 0;
}
--
2.0.5
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 2/3] gl-renderer: check EGL_EXT_platform_base in supports()
From: Pekka Paalanen pekka.paala...@collabora.co.uk An EGL implementation may support client extensions without supporting EGL_EXT_platform_base. In such a case, we should return 0 to fall back to the old eglGetDisplay() way. Cc: Manuel Bachmann manuel.bachm...@open.eurogiciel.org Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk --- src/gl-renderer.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/gl-renderer.c b/src/gl-renderer.c index 5a2ed9f..35cd7e7 100644 --- a/src/gl-renderer.c +++ b/src/gl-renderer.c @@ -2213,6 +2213,9 @@ gl_renderer_supports(struct weston_compositor *ec, extensions); } + if (!strstr(extensions, EGL_EXT_platform_base)) + return 0; + snprintf(s, sizeof s, EGL_KHR_platform_%s, extension_suffix); if (strstr(extensions, s)) return 1; @@ -2225,8 +2228,8 @@ gl_renderer_supports(struct weston_compositor *ec, if (strstr(extensions, s)) return 1; - /* at this point we definitely have some client extensions but -* haven't found the supplied client extension, so chances are it's + /* at this point we definitely have some platform extensions but +* haven't found the supplied platform, so chances are it's * not supported. */ return -1; -- 2.0.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/3] xwm: Make hash_table_lookup use an output parameter
On Tue, 7 Apr 2015 12:12:14 -0500
Derek Foreman der...@osg.samsung.com wrote:
Previously hash_table_lookup returned a pointer which must always be tested
for NULL - but rarely was.
Now hash_table_lookup returns the found data as an out parameter and returns
an integer indicating whether the lookup succeeded or not. This lets us
flag the return value as warn_unused_result so the compiler can stop us from
missing the test.
Signed-off-by: Derek Foreman der...@osg.samsung.com
---
xwayland/hash.c | 15 +--
xwayland/hash.h | 6 +-
xwayland/window-manager.c | 29 ++---
3 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/xwayland/hash.c b/xwayland/hash.c
index 54f3de9..2e5ecfc 100644
--- a/xwayland/hash.c
+++ b/xwayland/hash.c
@@ -201,16 +201,19 @@ hash_table_for_each(struct hash_table *ht,
}
}
-void *
-hash_table_lookup(struct hash_table *ht, uint32_t hash)
+int
+hash_table_lookup(struct hash_table *ht, uint32_t hash,
+ struct weston_wm_window **ret)
int for a boolean? ;-)
Apart from that,
Reviewed-by: Pekka Paalanen pekka.paala...@collabora.co.uk
{
struct hash_entry *entry;
entry = hash_table_search(ht, hash);
- if (entry != NULL)
- return entry-data;
-
- return NULL;
+ if (entry != NULL) {
+ *ret = entry-data;
+ return 1;
+ }
+ *ret = NULL;
+ return 0;
}
static void
diff --git a/xwayland/hash.h b/xwayland/hash.h
index 6e1674e..b181f19 100644
--- a/xwayland/hash.h
+++ b/xwayland/hash.h
@@ -36,11 +36,15 @@
#define HASH_H
struct hash_table;
+struct weston_wm_window;
struct hash_table *hash_table_create(void);
typedef void (*hash_table_iterator_func_t)(void *element, void *data);
void hash_table_destroy(struct hash_table *ht);
-void *hash_table_lookup(struct hash_table *ht, uint32_t hash);
+int hash_table_lookup(struct hash_table *ht, uint32_t hash,
+ struct weston_wm_window **ret)
+ __attribute__ ((warn_unused_result));
I suppose this really isn't used for anything else than
weston_wm_window, so ok for now.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] protocol: Add DnD actions
Hey Bryce, Thanks for the review! I'm attaching new patches fixing the issues you/Marek have seen, the doc blurbs have been reworded in a few places. This v2 also adds a wl_data_offer.source_actions event. This allows drag destinations to adapt their action mask accordingly, instead just being practically able to set a static one, things get a bit more akin to XDnD in this regard. With v2, I've been able to make DnD on par with X in GTK+ (feature- wise, that is): https://git.gnome.org/browse/gtk+/log/?h=wip/wayland-dnd-actions https://git.gnome.org/browse/mutter/log/?h=wip/dnd-actions Cheers, Carlos On mié, 2015-03-18 at 16:56 -0700, Bryce Harrington wrote: On Mon, Feb 16, 2015 at 04:37:34PM +0100, Carlos Garnacho wrote: These 2 requests have been added: - wl_data_source.notify_actions request: Notifies the compositor of the available actions on the data source. - wl_data_offer.notify_actions request: Notifies the compositor of the available actions on the destination side, plus the preferred action. Out of the data from these requests, the compositor can determine the action both parts agree on (and optionally let the user play a role through eg. keyboard modifiers). The chosen option will be notified to both parties through the following two requests: - wl_data_source.action - wl_data_offer.action Compared to the XDND protocol, there is one notable change: XDND lets the source suggest an action, whereas wl_data_device lets the destination prefer a given action. The difference is subtle here, it comes off as convenience because it is the drag destination which receives the motion events (unlike in X) and can perform action updates. The drag destination seems also in a better position to update the preferred action based on things like the data being transferred, the place being dropped, and whether the drag is client-local. Additionally, the wl_data_source.drop_performed and finished events will notify the source of the different termination phases of the DnD operation. Roughly based on previous work by Giulio Camuffo giuliocamu...@gmail.com --- protocol/wayland.xml | 97 +--- 1 file changed, 92 insertions(+), 5 deletions(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index 2a49805..110804e 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -408,7 +408,7 @@ /interface - interface name=wl_data_offer version=1 + interface name=wl_data_offer version=3 description summary=offer to transfer data A wl_data_offer represents a piece of data offered for transfer by another client (the source client). It is used by the @@ -461,9 +461,37 @@ arg name=mime_type type=string/ /event + +!-- Version 3 additions -- Watch your spaces and tabs, looks like there's a mix... +event name=action since=3 + description summary=notify the selected action + This event notifies the offer of the action selected by the compositor. The phrasing of this first sentence seems cumbersome. Maybe: This event indicates the action selected by the compositor from the offered set of actions. +The action will be determined after matching the options offered by the +source side (through data_source.set_actions) and the destination side +(through data_offer.notify_actions). + +This event can be emitted multiple times during the lifetime of a +data_offer, the most recent action received is always the valid one. + /description + arg name=dnd_action type=uint/ +/event + +request name=notify_actions since=3 + description summary=the data has been dropped + Sets the actions that the client supports for this operation. This +request may trigger a data_offer.action event if the compositor needs +changing the selected option after the destination-side change. + + Compositors wishing to stay compatible with earlier data_device versions + should set the copy action by default. Perhaps do you mean? copy action *as the* default + /description + arg name=dnd_actions type=uint/ + arg name=preferred_action type=uint/ +/request /interface - interface name=wl_data_source version=1 + interface name=wl_data_source version=3 description summary=offer to transfer data The wl_data_source object is the source side of a wl_data_offer. It is created by the source client in a data transfer and @@ -510,14 +538,61 @@ event name=cancelled description summary=selection was cancelled - This data source has been replaced by another data source. + This data source has been replaced by another
[PATCH weston 1/3] gl-renderer: fix EGL initialization steps
From: Manuel Bachmann manuel.bachm...@open.eurogiciel.org
Some DRI drivers, including VMware vmwgfx, do not support
calling eglQueryString() with a EGL_NO_DISPLAY parameter.
Just as we do in gl_renderer_supports(), which returns 0
but does not fail in this case, do not fail in
gl_renderer_setup_egl_extensions().
Signed-off-by: Manuel Bachmann manuel.bachm...@open.eurogiciel.org
[Pekka: split the patch]
Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk
---
src/gl-renderer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index b3b2364..5a2ed9f 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -2136,7 +2136,7 @@ gl_renderer_setup_egl_extensions(struct weston_compositor
*ec)
(const char *) eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS);
if (!extensions) {
weston_log(Retrieving EGL client extension string failed.\n);
- return -1;
+ return 0;
}
if (strstr(extensions, EGL_EXT_platform_base))
--
2.0.5
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] desktop-shell: Fix a crash when right-clicking the panel
On Mon, 6 Apr 2015 13:27:23 -0700
Dima Ryazanov d...@gmail.com wrote:
Yeah, the logic is pretty sketchy now - if it's a shell surface, do the
error checking; otherwise, do nothing - but I don't understand the code
well enough to know if this is the expected behavior.
Should the panel just be a shell surface? Then we could require that the
popup's parent is a shell surface and simplify the error check.
No, that won't work. panel is a special surface role, it cannot also
be a shell_surface.
I suppose the answer to this whole problem is to remove the whole panel
popup thing. It never had anything useful, right?
Thanks,
pq
On Mon, Apr 6, 2015 at 12:33 PM, Derek Foreman der...@osg.samsung.com
wrote:
This bug was introduced in commits da6ecd0cc52 and 24185e2561
I guess nobody right clicked on the panel for over a month. :)
I've CC'd Jasper and Jonas in case they haven't noticed this yet...
On 06/04/15 01:52 AM, Dima Ryazanov wrote:
It looks like the error-checking code assumes the popup's parent is
a shell surface - but that's not always the case.
Signed-off-by: Dima Ryazanov d...@gmail.com
---
desktop-shell/shell.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index f7c928e..0159547 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -3392,7 +3392,7 @@ add_popup_grab(struct shell_surface *shsurf,
parent = get_shell_surface(shsurf-parent);
top_surface = get_top_popup(shseat);
- if (shell_surface_is_xdg_popup(shsurf)
+ if (parent shell_surface_is_xdg_popup(shsurf)
This part broke in da6ecd0cc52
Your patch definitely stops the crash for me, which is good. :)
I think this part's ok, but I'm not 100% sure we're still going to send
the new not-top-level-popup error when we're supposed to now...
((top_surface == NULL
!shell_surface_is_xdg_surface(parent)) ||
(top_surface != NULL parent != top_surface))) {
wl_resource_post_error(shsurf-owner-resource,
@@ -4098,12 +4098,13 @@ create_xdg_popup(struct shell_client *owner,
void *shell,
{
struct shell_surface *shsurf, *parent_shsurf;
- /* Verify that we are creating the top most popup when mapping,
- * as its not until then we know whether it was mapped as most
+ /* Verify that we are creating the topmost popup when mapping,
+ * as it's not until then we know whether it was mapped as most
* top level or not. */
The grammar fix-ups look good to me.
parent_shsurf = get_shell_surface(parent);
- if (!shell_surface_is_xdg_popup(parent_shsurf)
+ if (parent_shsurf
This part broke in 24185e2561...
I wonder if we should be doing a more comprehensive test here - it looks
like some invalid parent resources could get past this test?
+ !shell_surface_is_xdg_popup(parent_shsurf)
!shell_surface_is_xdg_surface(parent_shsurf)) {
wl_resource_post_error(owner-resource,
XDG_POPUP_ERROR_INVALID_PARENT,
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 3/3] xwm: Test hash_table_lookup() returns
On Tue, 7 Apr 2015 12:12:15 -0500
Derek Foreman der...@osg.samsung.com wrote:
Make sure we always test hash_table_lookup()s return to prevent trying to
dereference a NULL window.
Signed-off-by: Derek Foreman der...@osg.samsung.com
---
xwayland/window-manager.c | 103
+-
1 file changed, 74 insertions(+), 29 deletions(-)
diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 3967670..9388d2e 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -446,10 +446,16 @@ weston_wm_window_read_properties(struct
weston_wm_window *window)
strndup(xcb_get_property_value(reply),
xcb_get_property_value_length(reply));
break;
- case XCB_ATOM_WINDOW:
+ case XCB_ATOM_WINDOW: {
+ int found;
xid = xcb_get_property_value(reply);
- hash_table_lookup(wm-window_hash, *xid, (struct
weston_wm_window **)p);
+ found = hash_table_lookup(wm-window_hash, *xid,
+ (struct weston_wm_window **)p);
+ if (!found)
+ weston_log(XCB_ATOM_WINDOW contains window
+ id not found in hash table.\n);
break;
+ }
case XCB_ATOM_CARDINAL:
case XCB_ATOM_ATOM:
atom = xcb_get_property_value(reply);
@@ -586,14 +592,18 @@ weston_wm_handle_configure_request(struct weston_wm
*wm, xcb_generic_event_t *ev
(xcb_configure_request_event_t *) event;
struct weston_wm_window *window;
uint32_t mask, values[16];
- int x, y, width, height, i = 0;
+ int x, y, width, height, i = 0, found;
wm_log(XCB_CONFIGURE_REQUEST (window %d) %d,%d @ %dx%d\n,
configure_request-window,
configure_request-x, configure_request-y,
configure_request-width, configure_request-height);
- hash_table_lookup(wm-window_hash, configure_request-window, window);
+ found = hash_table_lookup(wm-window_hash,
+ configure_request-window,
+ window);
+ if (!found)
+ return;
if (window-fullscreen) {
weston_wm_window_send_configure_notify(window);
@@ -650,6 +660,8 @@ our_resource(struct weston_wm *wm, uint32_t id)
static void
weston_wm_handle_configure_notify(struct weston_wm *wm, xcb_generic_event_t
*event)
{
+ int found;
+
xcb_configure_notify_event_t *configure_notify =
(xcb_configure_notify_event_t *) event;
struct weston_wm_window *window;
@@ -659,7 +671,12 @@ weston_wm_handle_configure_notify(struct weston_wm *wm,
xcb_generic_event_t *eve
configure_notify-x, configure_notify-y,
configure_notify-width, configure_notify-height);
- hash_table_lookup(wm-window_hash, configure_notify-window, window);
+ found = hash_table_lookup(wm-window_hash,
+ configure_notify-window,
+ window);
+ if (!found)
+ return;
Hi,
on a quick look this whole patch seems fine, I'll read it properly the
next time. :-)
Would it save anything to have a helper:
bool
wm_lookup_window(struct weston_wm *wm, uint32_t xid,
struct weston_wm_window **ret);
or something like that? We're always passing wm-window_hash anyway.
Letting you write
found = wm_lookup_window(wm, blargh, window);
which is a bit shorter.
That way you wouldn't need to change the pointer type for
hash_table_lookup() either, leaving it generic if we happen to need a
hash of something else. Hm?
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: modules=xwayland.so ignored (Was: Xfig menus don't appear - bug?)
On Wed, 8 Apr 2015 14:41:10 +0100 Felix E. Klee felix.k...@inka.de wrote: On Wed, Apr 8, 2015 at 11:16 AM, Pekka Paalanen ppaala...@gmail.com wrote: try removing that tab from your .ini. I tried with spaces, with tab, and without; But probably when I first tried without indentation, I didn’t save, because: After removing the tab, now it works! Should probably fix that one day... Don’t know. Perhaps without indentation is how ini files have to be formatted. I presume you use some ini parser library. There are various. No, we don't. :-P And it's probably not worth the trouble to add a new library dependency at this point. In the meantime, someone could fix documentation for [XServer][1] which currently reads: weston.ini == Add this to ~/.config/weston.ini (or use the --modules=xwayland.so command line argument): [core] modules=xwayland.so [1]: http://wayland.freedesktop.org/xserver.html Ah ah. Yes. Fixed! Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v2] protocol: Add DnD actions
These 2 requests have been added: - wl_data_source.notify_actions request: Notifies the compositor of the available actions on the data source. - wl_data_offer.notify_actions request: Notifies the compositor of the available actions on the destination side, plus the preferred action. Out of the data from these requests, the compositor can determine the action both parts agree on (and optionally let the user play a role through eg. keyboard modifiers). The chosen option will be notified to both parties through the following three requests: - wl_data_source.action - wl_data_offer.source_actions - wl_data_offer.action Compared to the XDND protocol, there is one notable change: XDND lets the source suggest an action, whereas wl_data_device lets the destination prefer a given action. The difference is subtle here, it comes off as convenience because it is the drag destination which receives the motion events (unlike in X) and can perform action updates. The drag destination seems also in a better position to update the preferred action based on things like the data being transferred, the place being dropped, and whether the drag is client-local. Additionally, the wl_data_source.drop_performed and finished events will notify the source of the different termination phases of the DnD operation. Roughly based on previous work by Giulio Camuffo giuliocamu...@gmail.com Changes since v1: - Added wl_data_offer.source_actions to let know of the actions offered by a data source. - Renamed wl_data_source.finished to drag_finished for clarity - Improved wording as suggested by Bryce Signed-off-by: Carlos Garnacho carl...@gnome.org --- protocol/wayland.xml | 113 --- 1 file changed, 107 insertions(+), 6 deletions(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index f52677f..ebe9061 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -408,7 +408,7 @@ /interface - interface name=wl_data_offer version=1 + interface name=wl_data_offer version=3 description summary=offer to transfer data A wl_data_offer represents a piece of data offered for transfer by another client (the source client). It is used by the @@ -461,9 +461,47 @@ arg name=mime_type type=string/ /event + +!-- Version 3 additions -- + +event name=source_actions since=3 + description summary=notify the source-side available actions +This event indicates the actions offered by the data source. +source. This event will be sent right after data_device.enter, +or anytime the source changes its offered actions. + /description + arg name=source_actions type=uint/ +/event + +event name=action since=3 + description summary=notify the selected action +This event indicates the action selected by the compositor from the +offered set of actions. The action will be determined after matching +the options offered by the source side (through data_source.set_actions) +and the destination side (through data_offer.notify_actions). + +This event can be emitted multiple times during the lifetime of a +data_offer, the most recent action received is always the valid one. + /description + arg name=dnd_action type=uint/ +/event + +request name=notify_actions since=3 + description summary=the data has been dropped +Sets the actions that the client supports for this operation. This +request may trigger a data_offer.action event if the compositor needs +changing the selected option after the destination-side change. + +Compositors wishing to allow interoperability between data_device v3 +and earlier versions should automatically enable on behalf of the older +client the copy action as the single/preferred action. + /description + arg name=dnd_actions type=uint/ + arg name=preferred_action type=uint/ +/request /interface - interface name=wl_data_source version=1 + interface name=wl_data_source version=3 description summary=offer to transfer data The wl_data_source object is the source side of a wl_data_offer. It is created by the source client in a data transfer and @@ -510,14 +548,65 @@ event name=cancelled description summary=selection was cancelled - This data source has been replaced by another data source. - The client should clean up and destroy this data source. +This data source is no longer valid. There are several reasons why this could +happen: + +- The data source has been replaced by another data source. +- The drop operation finished, but the drop destination did not accept any + of the mimetypes offered through data_source.target. +- The drop operation finished, but the drop destination did not select any + action present in the mask offered
Re: [PATCH] protocol: Add DnD actions
Hey Bill, On mié, 2015-03-18 at 19:40 -0700, Bill Spitzak wrote: This is still bothering me as being much too complicated. The bad news is, DnD is complex, there's plenty of prior usecases here that won't be fair/possible to have simplified. I think a list of actions can be sent from the DnD source to the target, and the target selects one. There is no need for the compositor to do any intersection of the sets and there is no need to communicate the set of target actions to the compositor. Your concerns about shift state being handled by the source are misplaced. This would not change at all under what I am proposing, the source could still use the shift state to change the list of actions. I see several issues with this: * How are actions conveyed? do we encode these in the mimetype string? how do we standardize on the actions? how do we make that backwards compatible? * How do drag destinations react to unhandled options? Say a drag source appends ?action=ask when you press Alt and the drag destination only knows copy/move, how does this fallback to an action that both parts recognize? * Additionally to modifier state, there's other keyboard/accessibility features as DnD is done in GNOME/GTK+ (eg. DnD driven by cursor keys), these must be implemented on the compositor, this sounds like conflicting with the expectation in your proposal to have the drag source receive key events [1]. My proposal is basically to take yours, and remove the ability for the target to send a set of actions that the compositor then interesect with the source list. Instead this intersection is done by the target, and the target sends *one* action (or none) indicating the result of the intersection. Unless you want the compositor to draw user interface to allow the user to choose the action, which seems very much a bad idea, I cannot see what your proposal will allow to happen that this simplified version would not. IMO, it kind of misses the point that DnD is a negotiation. I was suggesting that the ask action were implemented completely by the drag destination BTW, that wouldn't change much compared to XDnD. I do believe any kind of popup (like a menu for choosing move or copy) would have to be done by the target. This is because the target may have extra actions that the source does not care about or does not know about, such as insert verses replace. The popup would grab the keyboard focus but when dismissed it may go back to a different client than the target. Agreed about the grabbing behavior, I'm unclear though on how would the actions in such popup work in your proposal: Say you start a drag with the special ask modifier, and the drag source changes its action list to convey ask (how exactly? is this the only option exposed?). When the destination shows the popup, how does it tell the source of the chosen action, so that eg. the selection is deleted after move? Cheers, Carlos [1] I'm actually meaning to propose some doc updates with more consistent guidelines for grabbing behavior, IMO how do keyboard/touch devices behave during the various pointer grabs is somewhat underdocumented... ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v2] data-device: Implement DnD actions
The policy in weston in order to determine the chosen DnD action is
deliberately simple, and is probably the minimals that any compositor
should be doing here.
Besides honoring the notify_actions requests on both wl_data_source
and wl_data_offer, weston now emits the newly added events notifying
both source and dest sides of the chosen action and operation progress.
The dnd client has been updated too (although minimally), so it
notifies the compositor of a move action on both sides.
Changes since v1:
- Updated to v2 of DnD actions protocol changes, implement
wl_data_offer.source_actions.
- Fixed coding style issues.
Signed-off-by: Carlos Garnacho carl...@gnome.org
---
clients/dnd.c | 26 ++-
clients/window.c | 27 +++-
src/compositor.h | 5 +++
src/data-device.c | 97 +++
4 files changed, 146 insertions(+), 9 deletions(-)
diff --git a/clients/dnd.c b/clients/dnd.c
index e893d36..df84538 100644
--- a/clients/dnd.c
+++ b/clients/dnd.c
@@ -70,6 +70,7 @@ struct dnd_drag {
struct item *item;
int x_offset, y_offset;
int width, height;
+ uint32_t dnd_action;
const char *mime_type;
struct wl_surface *drag_surface;
@@ -337,10 +338,31 @@ data_source_cancelled(void *data, struct wl_data_source
*source)
free(dnd_drag);
}
+static void
+data_source_action(void *data, struct wl_data_source *source, uint32_t
dnd_action)
+{
+ struct dnd_drag *dnd_drag = data;
+
+ dnd_drag-dnd_action = dnd_action;
+}
+
+static void
+data_source_drop_performed(void *data, struct wl_data_source *source)
+{
+}
+
+static void
+data_source_finished(void *data, struct wl_data_source *source)
+{
+}
+
static const struct wl_data_source_listener data_source_listener = {
data_source_target,
data_source_send,
- data_source_cancelled
+ data_source_cancelled,
+ data_source_action,
+ data_source_drop_performed,
+ data_source_finished
};
static cairo_surface_t *
@@ -436,6 +458,8 @@ create_drag_source(struct dnd *dnd,
window_get_wl_surface(dnd-window),
dnd_drag-drag_surface,
serial);
+ wl_data_source_notify_actions(dnd_drag-data_source,
+
WL_DATA_DEVICE_MANAGER_DND_ACTION_MOVE);
dnd_drag-opaque =
create_drag_icon(dnd_drag, item, x, y, 1);
diff --git a/clients/window.c b/clients/window.c
index 81e007b..954210a 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -3323,6 +3323,8 @@ struct data_offer {
int fd;
data_func_t func;
int32_t x, y;
+ uint32_t dnd_action;
+ uint32_t source_actions;
void *user_data;
};
@@ -3336,8 +3338,26 @@ data_offer_offer(void *data, struct wl_data_offer
*wl_data_offer, const char *ty
*p = strdup(type);
}
+static void
+data_offer_source_actions(void *data, struct wl_data_offer *wl_data_offer,
uint32_t source_actions)
+{
+ struct data_offer *offer = data;
+
+ offer-source_actions = source_actions;
+}
+
+static void
+data_offer_action(void *data, struct wl_data_offer *wl_data_offer, uint32_t
dnd_action)
+{
+ struct data_offer *offer = data;
+
+ offer-dnd_action = dnd_action;
+}
+
static const struct wl_data_offer_listener data_offer_listener = {
data_offer_offer,
+ data_offer_source_actions,
+ data_offer_action
};
static void
@@ -3401,6 +3421,11 @@ data_device_enter(void *data, struct wl_data_device
*data_device,
*p = NULL;
types_data = input-drag_offer-types.data;
+ wl_data_offer_notify_actions(offer,
+
WL_DATA_DEVICE_MANAGER_DND_ACTION_COPY |
+
WL_DATA_DEVICE_MANAGER_DND_ACTION_MOVE,
+
WL_DATA_DEVICE_MANAGER_DND_ACTION_COPY);
+
} else {
input-drag_offer = NULL;
types_data = NULL;
@@ -5289,7 +5314,7 @@ registry_handle_global(void *data, struct wl_registry
*registry, uint32_t id,
d-shm = wl_registry_bind(registry, id, wl_shm_interface, 1);
wl_shm_add_listener(d-shm, shm_listener, d);
} else if (strcmp(interface, wl_data_device_manager) == 0) {
- d-data_device_manager_version = MIN(version, 2);
+ d-data_device_manager_version = MIN(version, 3);
d-data_device_manager =
wl_registry_bind(registry, id,
wl_data_device_manager_interface,
diff --git a/src/compositor.h b/src/compositor.h
index 5f49237..2c932a6 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -304,12 +304,17 @@ struct weston_data_offer {
struct
[PATCH weston] xwm: Add and use helper function for looking up windows in the hash table
This lets us verify that all callers are actually testing for a
successful hash lookup at compile time.
All current users of hash_table_lookup are converted to the new
wm_lookup_window() and the appropriate success check is added.
This fixes any call sites that used to assume a successful return
and dereference a NULL pointer.
This closes http://bugs.freedesktop.org/show_bug.cgi?id=83994
The xwayland test has been failing because weston crashes due to
a hash lookup failure and a subsequent dereference of the returned
NULL pointer.
Signed-off-by: Derek Foreman der...@osg.samsung.com
---
This is a rewrite of two previous patches, differences are:
Use a helper function instead of changing hash_table_lookup
combine the two patches into one
try to limit the use of temporary variables to places where if
statements become ugly (due to 80 col constraints).
xwayland/window-manager.c | 91 ---
1 file changed, 70 insertions(+), 21 deletions(-)
diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 83ebfae..966962e 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -195,6 +195,15 @@ wm_log_continue(const char *fmt, ...)
#endif
}
+static bool __attribute__ ((warn_unused_result))
+wm_lookup_window(struct hash_table *ht, uint32_t hash,
+ struct weston_wm_window **window)
+{
+ *window = hash_table_lookup(ht, hash);
+ if (*window)
+ return true;
+ return false;
+}
const char *
get_atom_name(xcb_connection_t *c, xcb_atom_t atom)
@@ -448,8 +457,10 @@ weston_wm_window_read_properties(struct weston_wm_window
*window)
break;
case XCB_ATOM_WINDOW:
xid = xcb_get_property_value(reply);
- *(struct weston_wm_window **) p =
- hash_table_lookup(wm-window_hash, *xid);
+ if (!wm_lookup_window(wm-window_hash, *xid,
+ (struct weston_wm_window **)p))
+ weston_log(XCB_ATOM_WINDOW contains window
+ id not found in hash table.\n);
break;
case XCB_ATOM_CARDINAL:
case XCB_ATOM_ATOM:
@@ -588,13 +599,18 @@ weston_wm_handle_configure_request(struct weston_wm *wm,
xcb_generic_event_t *ev
struct weston_wm_window *window;
uint32_t mask, values[16];
int x, y, width, height, i = 0;
+ bool found;
wm_log(XCB_CONFIGURE_REQUEST (window %d) %d,%d @ %dx%d\n,
configure_request-window,
configure_request-x, configure_request-y,
configure_request-width, configure_request-height);
- window = hash_table_lookup(wm-window_hash, configure_request-window);
+ found = wm_lookup_window(wm-window_hash,
+configure_request-window,
+window);
+ if (!found)
+ return;
if (window-fullscreen) {
weston_wm_window_send_configure_notify(window);
@@ -654,13 +670,19 @@ weston_wm_handle_configure_notify(struct weston_wm *wm,
xcb_generic_event_t *eve
xcb_configure_notify_event_t *configure_notify =
(xcb_configure_notify_event_t *) event;
struct weston_wm_window *window;
+ bool found;
wm_log(XCB_CONFIGURE_NOTIFY (window %d) %d,%d @ %dx%d\n,
configure_notify-window,
configure_notify-x, configure_notify-y,
configure_notify-width, configure_notify-height);
- window = hash_table_lookup(wm-window_hash, configure_notify-window);
+ found = wm_lookup_window(wm-window_hash,
+configure_notify-window,
+window);
+ if (!found)
+ return;
+
window-x = configure_notify-x;
window-y = configure_notify-y;
if (window-override_redirect) {
@@ -925,6 +947,7 @@ weston_wm_handle_map_request(struct weston_wm *wm,
xcb_generic_event_t *event)
xcb_map_request_event_t *map_request =
(xcb_map_request_event_t *) event;
struct weston_wm_window *window;
+ bool found;
if (our_resource(wm, map_request-window)) {
wm_log(XCB_MAP_REQUEST (window %d, ours)\n,
@@ -932,7 +955,11 @@ weston_wm_handle_map_request(struct weston_wm *wm,
xcb_generic_event_t *event)
return;
}
- window = hash_table_lookup(wm-window_hash, map_request-window);
+ found = wm_lookup_window(wm-window_hash,
+map_request-window,
+window);
+ if (!found)
+ return;
weston_wm_window_read_properties(window);
@@ -970,6 +997,7 @@ weston_wm_handle_unmap_notify(struct weston_wm *wm,
[PATCH weston 2/2] desktop-shell: Remove the panel popup
It doesn't work anymore, and it never did anything useful.
Signed-off-by: Dima Ryazanov d...@gmail.com
---
clients/desktop-shell.c | 33 -
1 file changed, 33 deletions(-)
diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index ac2928f..e2f9f80 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -145,26 +145,6 @@ sigchild_handler(int s)
fprintf(stderr, child %d exited\n, pid);
}
-static void
-menu_func(void *data, struct input *input, int index)
-{
- printf(Selected index %d from a panel menu.\n, index);
-}
-
-static void
-show_menu(struct panel *panel, struct input *input, uint32_t time)
-{
- int32_t x, y;
- static const char *entries[] = {
- Roy, Pris, Leon, Zhora
- };
-
- input_get_position(input, x, y);
- window_show_menu(window_get_display(panel-window),
-input, time, panel-window,
-x - 10, y - 10, menu_func, entries, 4);
-}
-
static int
is_desktop_painted(struct desktop *desktop)
{
@@ -454,18 +434,6 @@ panel_add_clock(struct panel *panel)
}
static void
-panel_button_handler(struct widget *widget,
-struct input *input, uint32_t time,
-uint32_t button,
-enum wl_pointer_button_state state, void *data)
-{
- struct panel *panel = data;
-
- if (button == BTN_RIGHT state == WL_POINTER_BUTTON_STATE_PRESSED)
- show_menu(panel, input, time);
-}
-
-static void
panel_resize_handler(struct widget *widget,
int32_t width, int32_t height, void *data)
{
@@ -553,7 +521,6 @@ panel_create(struct desktop *desktop)
widget_set_redraw_handler(panel-widget, panel_redraw_handler);
widget_set_resize_handler(panel-widget, panel_resize_handler);
- widget_set_button_handler(panel-widget, panel_button_handler);
panel_add_clock(panel);
--
2.1.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 1/2] desktop-shell: Require a popup parent to be a shell surface
Currently, the shell crashes if the parent is not a shell surface. Instead,
send an error to the client.
Signed-off-by: Dima Ryazanov d...@gmail.com
---
desktop-shell/shell.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index f7c928e..96aa8f3 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -3393,7 +3393,8 @@ add_popup_grab(struct shell_surface *shsurf,
parent = get_shell_surface(shsurf-parent);
top_surface = get_top_popup(shseat);
if (shell_surface_is_xdg_popup(shsurf)
- ((top_surface == NULL !shell_surface_is_xdg_surface(parent)) ||
+ (!parent ||
+(top_surface == NULL !shell_surface_is_xdg_surface(parent)) ||
(top_surface != NULL parent != top_surface))) {
wl_resource_post_error(shsurf-owner-resource,
XDG_POPUP_ERROR_NOT_THE_TOPMOST_POPUP,
@@ -4098,13 +4099,14 @@ create_xdg_popup(struct shell_client *owner, void
*shell,
{
struct shell_surface *shsurf, *parent_shsurf;
- /* Verify that we are creating the top most popup when mapping,
-* as its not until then we know whether it was mapped as most
+ /* Verify that we are creating the topmost popup when mapping,
+* as it's not until then we know whether it was mapped as most
* top level or not. */
parent_shsurf = get_shell_surface(parent);
- if (!shell_surface_is_xdg_popup(parent_shsurf)
- !shell_surface_is_xdg_surface(parent_shsurf)) {
+ if (!parent_shsurf ||
+ (!shell_surface_is_xdg_popup(parent_shsurf)
+!shell_surface_is_xdg_surface(parent_shsurf))) {
wl_resource_post_error(owner-resource,
XDG_POPUP_ERROR_INVALID_PARENT,
xdg_popup parent was invalid);
--
2.1.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 02/17] xdg-shell: Require proper object tree destruction
Will these windows then unmap atomically? I actually expected the opposite to be allowed and encouraged, as destroying a parent surface could then guarantee the children disappear atomically. On 04/07/2015 05:25 PM, Bryce Harrington wrote: On Tue, Apr 07, 2015 at 05:01:17PM +0800, Jonas Ådahl wrote: Require all child objects to be destroyed before the parent. In other words, all popups and surfaces created by one xdg_shell instance needs to be destroyed before the xdg_shell object, otherwise a protocol error is raised. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/3] gl-renderer: check EGL_EXT_platform_base in supports()
Looks good to me ! 2015-04-08 16:02 GMT+02:00 Pekka Paalanen ppaala...@gmail.com: From: Pekka Paalanen pekka.paala...@collabora.co.uk An EGL implementation may support client extensions without supporting EGL_EXT_platform_base. In such a case, we should return 0 to fall back to the old eglGetDisplay() way. Cc: Manuel Bachmann manuel.bachm...@open.eurogiciel.org Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk --- src/gl-renderer.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/gl-renderer.c b/src/gl-renderer.c index 5a2ed9f..35cd7e7 100644 --- a/src/gl-renderer.c +++ b/src/gl-renderer.c @@ -2213,6 +2213,9 @@ gl_renderer_supports(struct weston_compositor *ec, extensions); } + if (!strstr(extensions, EGL_EXT_platform_base)) + return 0; + snprintf(s, sizeof s, EGL_KHR_platform_%s, extension_suffix); if (strstr(extensions, s)) return 1; @@ -2225,8 +2228,8 @@ gl_renderer_supports(struct weston_compositor *ec, if (strstr(extensions, s)) return 1; - /* at this point we definitely have some client extensions but -* haven't found the supplied client extension, so chances are it's + /* at this point we definitely have some platform extensions but +* haven't found the supplied platform, so chances are it's * not supported. */ return -1; -- 2.0.5 -- Regards, *Manuel BACHMANN Tizen Project VANNES-FR* ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] touchpad: count the tapping fingers separately from the main touchpad code
On 4/7/2015 8:54 PM, Peter Hutterer wrote:
tp-nfingers_down gives us the current state of the touchpad but in the case
of the tapping state we need the touchpoints separately. If all touchpoints
end in the same SYN_REPORT frame, tp-nfingers_down is 0 when we handle the
touch releases. This changes the tap state to IDLE on the first release and
then logs a bug when the remaining touches are released while the touchpad is
in IDLE.
Avoid this by counting the fingers separately for the tap state, this way we
can count up/down with the down/up events as we process them for the tapping
state machine.
This also adds tests for 4 and 5-finger tapping which is how the bug was
discovered in the first place.
https://bugs.freedesktop.org/show_bug.cgi?id=89800
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
The summary doesn't seem to jive with what I see. In particular, the
scenario it describes doesn't seem to apply: I can have four touches go
up across two frames and see the error, or three touches all disappear
in a single frame and not see it. Additionally, it doesn't seem like
tp-nfingers_down gives us the current state of the touchpad since the
value is reset to 0 if five or more fingers are physically present (due
to the behavior of 'tp_unhover_touches' mentioned in the bug report).
That said, the patch *does* prevent the error messages from appearing,
so at the very least:
Tested-by: Jason Gerecke jason.gere...@wacom.com
Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours
---
src/evdev-mt-touchpad-tap.c | 5 +-
src/evdev-mt-touchpad.h | 1 +
test/touchpad.c | 233
3 files changed, 238 insertions(+), 1 deletion(-)
diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index 4ba4ad2..78d4a0f 100644
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -452,7 +452,7 @@ tp_tap_dead_handle_event(struct tp_dispatch *tp,
switch (event) {
case TAP_EVENT_RELEASE:
- if (tp-nfingers_down == 0)
+ if (tp-tap.tap_finger_count == 0)
tp-tap.state = TAP_STATE_IDLE;
break;
case TAP_EVENT_TOUCH:
@@ -567,10 +567,12 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time)
t-tap.state = TAP_TOUCH_STATE_DEAD;
if (t-state == TOUCH_BEGIN) {
+ tp-tap.tap_finger_count++;
t-tap.state = TAP_TOUCH_STATE_TOUCH;
t-tap.initial = t-point;
tp_tap_handle_event(tp, t, TAP_EVENT_TOUCH, time);
} else if (t-state == TOUCH_END) {
+ tp-tap.tap_finger_count--;
tp_tap_handle_event(tp, t, TAP_EVENT_RELEASE, time);
t-tap.state = TAP_TOUCH_STATE_IDLE;
} else if (tp-tap.state != TAP_STATE_IDLE
@@ -754,6 +756,7 @@ tp_release_all_taps(struct tp_dispatch *tp, uint64_t now)
}
tp-tap.state = tp-nfingers_down ? TAP_STATE_DEAD : TAP_STATE_IDLE;
+ tp-tap.tap_finger_count = 0;
}
void
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 19a262e..b88dadd 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -255,6 +255,7 @@ struct tp_dispatch {
struct libinput_timer timer;
enum tp_tap_state state;
uint32_t buttons_pressed;
+ unsigned int tap_finger_count;
} tap;
struct {
diff --git a/test/touchpad.c b/test/touchpad.c
index 6fa2301..b06e00d 100644
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -369,6 +369,38 @@ START_TEST(touchpad_2fg_tap_inverted)
}
END_TEST
+START_TEST(touchpad_2fg_tap_quickrelease)
+{
+ struct litest_device *dev = litest_current_device();
+ struct libinput *li = dev-libinput;
+
+ libinput_device_config_tap_set_enabled(dev-libinput_device,
+ LIBINPUT_CONFIG_TAP_ENABLED);
+
+ litest_drain_events(dev-libinput);
+
+ litest_touch_down(dev, 0, 50, 50);
+ litest_touch_down(dev, 1, 70, 70);
+ litest_event(dev, EV_ABS, ABS_MT_SLOT, 0);
+ litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
+ litest_event(dev, EV_ABS, ABS_MT_SLOT, 1);
+ litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
+ litest_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 0);
+ litest_event(dev, EV_KEY, BTN_TOUCH, 0);
+ litest_event(dev, EV_SYN, SYN_REPORT, 0);
+
+ libinput_dispatch(li);
+
+ litest_assert_button_event(li, BTN_RIGHT,
+ LIBINPUT_BUTTON_STATE_PRESSED);
+ litest_timeout_tap();
+ litest_assert_button_event(li, BTN_RIGHT,
+
Re: [PATCH weston] desktop-shell: Fix a crash when right-clicking the panel
Sure, I'll remove it then. (I was going to remove it originally - but
figured, it was useful for testing since it exposed this bug.)
On Wed, Apr 8, 2015 at 7:00 AM, Pekka Paalanen ppaala...@gmail.com wrote:
On Mon, 6 Apr 2015 13:27:23 -0700
Dima Ryazanov d...@gmail.com wrote:
Yeah, the logic is pretty sketchy now - if it's a shell surface, do the
error checking; otherwise, do nothing - but I don't understand the code
well enough to know if this is the expected behavior.
Should the panel just be a shell surface? Then we could require that the
popup's parent is a shell surface and simplify the error check.
No, that won't work. panel is a special surface role, it cannot also
be a shell_surface.
I suppose the answer to this whole problem is to remove the whole panel
popup thing. It never had anything useful, right?
Thanks,
pq
On Mon, Apr 6, 2015 at 12:33 PM, Derek Foreman der...@osg.samsung.com
wrote:
This bug was introduced in commits da6ecd0cc52 and 24185e2561
I guess nobody right clicked on the panel for over a month. :)
I've CC'd Jasper and Jonas in case they haven't noticed this yet...
On 06/04/15 01:52 AM, Dima Ryazanov wrote:
It looks like the error-checking code assumes the popup's parent is
a shell surface - but that's not always the case.
Signed-off-by: Dima Ryazanov d...@gmail.com
---
desktop-shell/shell.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index f7c928e..0159547 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -3392,7 +3392,7 @@ add_popup_grab(struct shell_surface *shsurf,
parent = get_shell_surface(shsurf-parent);
top_surface = get_top_popup(shseat);
- if (shell_surface_is_xdg_popup(shsurf)
+ if (parent shell_surface_is_xdg_popup(shsurf)
This part broke in da6ecd0cc52
Your patch definitely stops the crash for me, which is good. :)
I think this part's ok, but I'm not 100% sure we're still going to send
the new not-top-level-popup error when we're supposed to now...
((top_surface == NULL
!shell_surface_is_xdg_surface(parent)) ||
(top_surface != NULL parent != top_surface))) {
wl_resource_post_error(shsurf-owner-resource,
@@ -4098,12 +4098,13 @@ create_xdg_popup(struct shell_client *owner,
void *shell,
{
struct shell_surface *shsurf, *parent_shsurf;
- /* Verify that we are creating the top most popup when mapping,
- * as its not until then we know whether it was mapped as most
+ /* Verify that we are creating the topmost popup when mapping,
+ * as it's not until then we know whether it was mapped as most
* top level or not. */
The grammar fix-ups look good to me.
parent_shsurf = get_shell_surface(parent);
- if (!shell_surface_is_xdg_popup(parent_shsurf)
+ if (parent_shsurf
This part broke in 24185e2561...
I wonder if we should be doing a more comprehensive test here - it
looks
like some invalid parent resources could get past this test?
+ !shell_surface_is_xdg_popup(parent_shsurf)
!shell_surface_is_xdg_surface(parent_shsurf)) {
wl_resource_post_error(owner-resource,
XDG_POPUP_ERROR_INVALID_PARENT,
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 16/17] xdg-shell: Specify fullscreen size-mismatch handling
I think the compositor is allowed to choose a color other than black, right? On 04/07/2015 07:52 PM, Bryce Harrington wrote: +If the surface doesn't cover the whole output, the compositor will +position the surface in the center of the output and compensate with +black borders filling the rest of the output. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] config: use simpler regexp syntax to get dot version
On 04/08/2015 12:15 AM, Pekka Paalanen wrote: + dot_version=`$DOT -V 21|$GREP -o ['[0-9]*\.[0-9]*\.[0-9]*']` AS_VERSION_COMPARE([$dot_version], [2.26.0], [AC_MSG_RESULT([no]) AC_MSG_ERROR([Graphviz dot $dot_version too old. Graphviz 2.26+ required for documentation build. Install required graphviz version or disable the documentation using --disable-documentation])], For the record, I think those @:@ and @:@ are square brackets. They are just an M4 escape thing to prevent M4 from interpreting those square brackets and possibly removing them. Oh yeah, quadrigraphs: http://stackoverflow.com/questions/2308721/how-do-i-escape-text-in-autoconf-m4 It's just another way to protect the square brackets. Your way should work fine here too, since these are not M4 macro arguments. Egad, M4 is a mess. Anyway it is possible the following is better: $EGREP -o ['[0-9]+\.[0-9]+\.[0-9]+'] This will prevent it from matching the text .. if for some reason dot prints that before the version number. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] weston-launch: Fixed TTY switching
On 04/07/2015 05:03 PM, Bryce Harrington wrote: I'm sure this is not going to ever be a problem since tty filenames and paths are on the short side, but since the tty string is an input parameter to this routine, it would be better defensive programming to use strncpy. strncpy is not a fix and should never be used. That is because the proper invocation is: strncpy(dest, len-1, src); dest[len-1] = 0; This is almost always done incorrectly, and even when correct it is hard to read and it wastes time filling the buffer with 0 when only the first 0 needs to be written. The best solution is to use strlcpy. If politics make that impossible, use snprintf(dest, len, %s, src) which is exactly the same as strlcpy, including the return value! (imagine that...) ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 1/2] touchpad: a touch in TOUCH_NONE doesn't need to be ended
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev-mt-touchpad.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index 19ec99e..0627056 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -547,7 +547,8 @@ tp_unhover_touches(struct tp_dispatch *tp, uint64_t time)
for (i = tp-ntouches - 1; i = 0; i--) {
t = tp_get_touch(tp, i);
- if (t-state == TOUCH_HOVERING)
+ if (t-state == TOUCH_HOVERING ||
+ t-state == TOUCH_NONE)
continue;
tp_end_touch(tp, t, time);
--
2.3.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] weston-launch: Fixed TTY switching
On 04/08/2015 02:35 PM, Daniel Stone wrote: The best solution is to use strlcpy. If politics make that impossible, use snprintf(dest, len, %s, src) which is exactly the same as strlcpy, including the return value! (imagine that...) It's not the politics, it's that silently truncating a filename you're hoping to use will at best pick a non-existent file, and at worst pick a totally different/unrelated file. Except strncpy and snprintf also silently truncate the filename so it is politics because those functions exist. And strlcpy and snprintf are not really silent: you can check if the return value is greater than the buffer size and know if truncation happened. To be honest though, I'd prefer the library entrypoint existed so the intention was clear, making it easier to audit for and spot this horrendous anti-pattern. Maybe something like fopen(filename(a,b,c,NULL),...). But in C this is going to have to return a TLS buffer, I guess. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] weston-launch: Fixed TTY switching
On Wednesday, April 8, 2015, Bill Spitzak spit...@gmail.com wrote: On 04/07/2015 05:03 PM, Bryce Harrington wrote: I'm sure this is not going to ever be a problem since tty filenames and paths are on the short side, but since the tty string is an input parameter to this routine, it would be better defensive programming to use strncpy. strncpy is not a fix and should never be used. That is because the proper invocation is: strncpy(dest, len-1, src); dest[len-1] = 0; This is almost always done incorrectly, and even when correct it is hard to read and it wastes time filling the buffer with 0 when only the first 0 needs to be written. The best solution is to use strlcpy. If politics make that impossible, use snprintf(dest, len, %s, src) which is exactly the same as strlcpy, including the return value! (imagine that...) It's not the politics, it's that silently truncating a filename you're hoping to use will at best pick a non-existent file, and at worst pick a totally different/unrelated file. Unless truncation is explicitly acceptable (indicative/non-authoritative strings in UI), strlcpy and other silently-truncating copies are actively harmful, and a potential security risk. To be honest though, I'd prefer the library entrypoint existed so the intention was clear, making it easier to audit for and spot this horrendous anti-pattern. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput] touchpad: detect fake finger overflow after BTN_TOOL_QUINTTAB
Up to QUINTTAP, we count fake fingers through the BTN_TOOL_*TAP kernel defines.
Once we exceed QUINTTAP, the nfake_finger count returns to 0 and
tp_unhover_touches terminates all touch sequences. The most visible effect of
this was stopped in 591a41f but the problem remained.
Since we're not using 5 fingers for anything, use that to set the
overflow flag. The kernel gives us either a BTN_TOUCH 0 (all
released) or a lower BTN_TOOL_*TAP to unset the flag when we go below 5
fingers again.
And if we overflow, we can skip the unhovering of touch points since we a)
have a decent touchpad that gives us real touchpoints and b) hovering isn't
supported for 5 touches anyway.
https://bugs.freedesktop.org/show_bug.cgi?id=89800
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev-mt-touchpad.c | 29 +
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index 724fd32..19ec99e 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -34,6 +34,7 @@
#define DEFAULT_ACCEL_NUMERATOR 3000.0
#define DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR 700.0
#define DEFAULT_TRACKPOINT_ACTIVITY_TIMEOUT 500 /* ms */
+#define FAKE_FINGER_OVERFLOW (1 7)
static inline int
tp_hysteresis(int in, int center, int margin)
@@ -127,8 +128,10 @@ tp_get_touch(struct tp_dispatch *tp, unsigned int slot)
static inline unsigned int
tp_fake_finger_count(struct tp_dispatch *tp)
{
- /* don't count BTN_TOUCH */
- return ffs(tp-fake_touches 1);
+ if (tp-fake_touches FAKE_FINGER_OVERFLOW)
+ return FAKE_FINGER_OVERFLOW;
+ else /* don't count BTN_TOUCH */
+ return ffs(tp-fake_touches 1);
}
static inline bool
@@ -146,6 +149,8 @@ tp_fake_finger_set(struct tp_dispatch *tp,
switch (code) {
case BTN_TOUCH:
+ if (!is_press)
+ tp-fake_touches = ~FAKE_FINGER_OVERFLOW;
shift = 0;
break;
case BTN_TOOL_FINGER:
@@ -156,14 +161,24 @@ tp_fake_finger_set(struct tp_dispatch *tp,
case BTN_TOOL_QUADTAP:
shift = code - BTN_TOOL_DOUBLETAP + 2;
break;
+ /* when QUINTTAP is released we're either switching to 6 fingers
+ (flag stays in place until BTN_TOUCH is released) or
+ one of DOUBLE/TRIPLE/QUADTAP (will clear the flag on press) */
+ case BTN_TOOL_QUINTTAP:
+ if (is_press)
+ tp-fake_touches |= FAKE_FINGER_OVERFLOW;
+ return;
default:
return;
}
- if (is_press)
+ if (is_press) {
+ tp-fake_touches = ~FAKE_FINGER_OVERFLOW;
tp-fake_touches |= 1 shift;
- else
+
+ } else {
tp-fake_touches = ~(0x1 shift);
+ }
}
static inline void
@@ -325,6 +340,8 @@ tp_process_fake_touches(struct tp_dispatch *tp,
unsigned int i, start;
nfake_touches = tp_fake_finger_count(tp);
+ if (nfake_touches == FAKE_FINGER_OVERFLOW)
+ return;
start = tp-has_mt ? tp-real_touches : 0;
for (i = start; i tp-ntouches; i++) {
@@ -387,6 +404,7 @@ tp_process_key(struct tp_dispatch *tp,
case BTN_TOOL_DOUBLETAP:
case BTN_TOOL_TRIPLETAP:
case BTN_TOOL_QUADTAP:
+ case BTN_TOOL_QUINTTAP:
tp_fake_finger_set(tp, e-code, !!e-value);
break;
case BTN_0:
@@ -494,6 +512,9 @@ tp_unhover_touches(struct tp_dispatch *tp, uint64_t time)
return;
nfake_touches = tp_fake_finger_count(tp);
+ if (nfake_touches == FAKE_FINGER_OVERFLOW)
+ return;
+
if (tp-nfingers_down == nfake_touches
((tp-nfingers_down == 0 !tp_fake_finger_is_touching(tp)) ||
(tp-nfingers_down 0 tp_fake_finger_is_touching(tp
--
2.3.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput v3] evdev: Add support for POINTINGSTICK_CONST_ACCEL udev property
On Wed, Apr 08, 2015 at 12:11:58PM +0200, Hans de Goede wrote:
There is quite a wide spread in the delta events generated by trackpoints,
some generate deltas of 1-2 under normal use, while others generate deltas
from 1-20.
It is desirable to normalize trackpoint deltas just like we are normalizing
mouse deltas to 1000 dpi, so as to give different model laptops aprox.
the same trackpoint cursor speed ootb.
Recent versions of udev + hwdb set a POINTINGSTICK_CONST_ACCEL udev property
which can be used to adjust trackpoints which are too slow / too fast
ootb, this commit implements support for that property.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
Changes in v3:
-Fix a couple of typos
-Document TRACKPOINT_CONST_ACCEL udev property in
doc/device-configuration-via-udev.dox
-Added a test for parse_trackpoint_accel_property
Changes in v2:
-Use POINTINGSTICK_CONST_ACCEL instead of TRACKPOINT_CONST_ACCEL to match
changes to the udev patchset
foo
---
doc/device-configuration-via-udev.dox | 4
src/evdev.c | 33 +
src/evdev.h | 7 +++
src/libinput-util.c | 31 +++
src/libinput-util.h | 1 +
test/misc.c | 27 +++
6 files changed, 103 insertions(+)
diff --git a/doc/device-configuration-via-udev.dox
b/doc/device-configuration-via-udev.dox
index fc1c0af..6579041 100644
--- a/doc/device-configuration-via-udev.dox
+++ b/doc/device-configuration-via-udev.dox
@@ -57,6 +57,10 @@ See @ref motion_normalization for details.
ddThe angle in degrees for each click on a mouse wheel. See
libinput_pointer_get_axis_source() for details.
/dd
+dtPOINTINGSTICK_CONST_ACCEL/dt
+ddA constant (linear) acceleration factor to apply to pointingstick deltas
+to normalize them. See evdev_get_trackpoint_dpi() for details.
You can drop the see ... sentence since doxygen won't resolve it anyway.
Anyone interested enough in the code will (have to) be able to git grep.
Reviewed-by: Peter Hutterer peter.hutte...@who-t.net
otherwise, feel free to push this once all the udev changes have landed.
Cheers,
Peter
+/dd
/dl
Below is an example udev rule to assign seat1 to a device from vendor
diff --git a/src/evdev.c b/src/evdev.c
index 4205c2d..1730dae 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -1319,6 +1319,31 @@ evdev_read_wheel_click_prop(struct evdev_device
*device)
return angle;
}
+
+static inline int
+evdev_get_trackpoint_dpi(struct evdev_device *device)
+{
+ struct libinput *libinput = device-base.seat-libinput;
+ const char *trackpoint_accel;
+ double accel = DEFAULT_TRACKPOINT_ACCEL;
+
+ trackpoint_accel = udev_device_get_property_value(
+ device-udev_device,
POINTINGSTICK_CONST_ACCEL);
+ if (trackpoint_accel) {
+ accel = parse_trackpoint_accel_property(trackpoint_accel);
+ if (accel == 0.0) {
+ log_error(libinput, Trackpoint accel property for
+ '%s' is present but invalid,
+ using %.2f instead\n,
+ device-devname,
+ DEFAULT_TRACKPOINT_ACCEL);
+ accel = DEFAULT_TRACKPOINT_ACCEL;
+ }
+ }
+
+ return DEFAULT_MOUSE_DPI / accel;
+}
+
static inline int
evdev_read_dpi_prop(struct evdev_device *device)
{
@@ -1326,6 +1351,14 @@ evdev_read_dpi_prop(struct evdev_device *device)
const char *mouse_dpi;
int dpi = DEFAULT_MOUSE_DPI;
+ /*
+ * Trackpoints do not have dpi, instead hwdb may contain a
+ * POINTINGSTICK_CONST_ACCEL value to compensate for sensitivity
+ * differences between models, we translate this to a fake dpi.
+ */
+ if (libevdev_has_property(device-evdev, INPUT_PROP_POINTING_STICK))
+ return evdev_get_trackpoint_dpi(device);
+
mouse_dpi = udev_device_get_property_value(device-udev_device,
MOUSE_DPI);
if (mouse_dpi) {
diff --git a/src/evdev.h b/src/evdev.h
index c7a9e75..b63ae86 100644
--- a/src/evdev.h
+++ b/src/evdev.h
@@ -36,6 +36,13 @@
/* The HW DPI rate we normalize to before calculating pointer acceleration */
#define DEFAULT_MOUSE_DPI 1000
+
+/*
+ * The constant (linear) acceleration factor we use to normalize trackpoint
+ * deltas before calculating pointer acceleration.
+ */
+#define DEFAULT_TRACKPOINT_ACCEL 1.0
+
/* The fake resolution value for abs devices without resolution */
#define EVDEV_FAKE_RESOLUTION 1
diff --git a/src/libinput-util.c b/src/libinput-util.c
index 49e297a..4857435 100644
--- a/src/libinput-util.c
+++ b/src/libinput-util.c
@@
[PATCH libinput 2/2] touchpad: rename real_touches to num_slots
Less ambiguous since real_touches can be interpreted to current number of
real touches as opposed to fake touches. Which it isn't, this variable holds
the number of slots.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev-mt-touchpad-gestures.c | 2 +-
src/evdev-mt-touchpad.c | 10 +-
src/evdev-mt-touchpad.h | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/evdev-mt-touchpad-gestures.c b/src/evdev-mt-touchpad-gestures.c
index 7dbf4c4..e01b921 100644
--- a/src/evdev-mt-touchpad-gestures.c
+++ b/src/evdev-mt-touchpad-gestures.c
@@ -39,7 +39,7 @@ tp_get_touches_delta(struct tp_dispatch *tp, bool average)
struct normalized_coords normalized;
struct normalized_coords delta = {0.0, 0.0};
- for (i = 0; i tp-real_touches; i++) {
+ for (i = 0; i tp-num_slots; i++) {
t = tp-touches[i];
if (tp_touch_active(tp, t) t-dirty) {
diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index 0627056..68070c2 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -343,7 +343,7 @@ tp_process_fake_touches(struct tp_dispatch *tp,
if (nfake_touches == FAKE_FINGER_OVERFLOW)
return;
- start = tp-has_mt ? tp-real_touches : 0;
+ start = tp-has_mt ? tp-num_slots : 0;
for (i = start; i tp-ntouches; i++) {
t = tp_get_touch(tp, i);
if (i nfake_touches)
@@ -577,7 +577,7 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time)
if (tp-semi_mt tp-nfingers_down != tp-old_nfingers_down)
tp_motion_history_reset(t);
- if (i = tp-real_touches t-state != TOUCH_NONE) {
+ if (i = tp-num_slots t-state != TOUCH_NONE) {
t-point = first-point;
if (!t-dirty)
t-dirty = first-dirty;
@@ -938,11 +938,11 @@ tp_init_slots(struct tp_dispatch *tp,
absinfo = libevdev_get_abs_info(device-evdev, ABS_MT_SLOT);
if (absinfo) {
- tp-real_touches = absinfo-maximum + 1;
+ tp-num_slots = absinfo-maximum + 1;
tp-slot = absinfo-value;
tp-has_mt = true;
} else {
- tp-real_touches = 1;
+ tp-num_slots = 1;
tp-slot = 0;
tp-has_mt = false;
}
@@ -958,7 +958,7 @@ tp_init_slots(struct tp_dispatch *tp,
}
}
- tp-ntouches = max(tp-real_touches, n_btn_tool_touches);
+ tp-ntouches = max(tp-num_slots, n_btn_tool_touches);
tp-touches = calloc(tp-ntouches, sizeof(struct tp_touch));
if (!tp-touches)
return -1;
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index b88dadd..6ab0981 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -183,7 +183,7 @@ struct tp_dispatch {
bool semi_mt;
enum touchpad_model model;
- unsigned int real_touches; /* number of slots */
+ unsigned int num_slots; /* number of slots */
unsigned int ntouches; /* no slots inc. fakes */
struct tp_touch *touches; /* len == ntouches */
/* bit 0: BTN_TOUCH
--
2.3.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] protocol: Add DnD actions
On 04/08/2015 07:46 AM, Carlos Garnacho wrote: I may be missing something. As far as I can tell your proposal is that the source and destination send sets of actions to the compositor, the compositor then intersects the sets, and sends the result of the intersection to the source and destination. My proposal is that only the source sends a list of actions, and the compositor sends that list to the destination. The destination intersects with it's list internally and sends the result of the intersection to the compositor, which then sends it back to the source. Otherwise everything is identical to what you are proposing. * How are actions conveyed? do we encode these in the mimetype string? how do we standardize on the actions? how do we make that backwards compatible? The exact same way they are being conveyed by your proposal. You have both the source and destination sending a list of actions to the compositor, use the same api here. * How do drag destinations react to unhandled options? Say a drag source appends ?action=ask when you press Alt and the drag destination only knows copy/move, how does this fallback to an action that both parts recognize? Since the destination figures out which of the offered actions to use, it would not select this one. It would select another one. If it does not like any of them then we are in the exact same state as your proposal when the intersection is empty. * Additionally to modifier state, there's other keyboard/accessibility features as DnD is done in GNOME/GTK+ (eg. DnD driven by cursor keys), these must be implemented on the compositor, this sounds like conflicting with the expectation in your proposal to have the drag source receive key events [1]. I don't understand at all. Your proposal also requires the drag source to receive key events, since it can use those to change the set of actions (for instance to add the ask action like you said in the previous question). ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput] evdev: reject devices with only one of x/y resolution
This is a kernel bug, reject such devices outright. This saves us from a bunch
of extra double checks to make sure that the resolutions are always set up.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev.c | 32 +-
test/device.c | 87 +++
2 files changed, 112 insertions(+), 7 deletions(-)
diff --git a/src/evdev.c b/src/evdev.c
index a6d6fae..243cd22 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -1377,13 +1377,6 @@ evdev_fix_abs_resolution(struct evdev_device *device,
absx = libevdev_get_abs_info(evdev, xcode);
absy = libevdev_get_abs_info(evdev, ycode);
- if ((absx-resolution == 0 absy-resolution != 0) ||
- (absx-resolution != 0 absy-resolution == 0)) {
- log_bug_kernel(libinput,
- Kernel has only x or y resolution, not
both.\n);
- return 0;
- }
-
if (absx-resolution == 0 || absx-resolution == EVDEV_FAKE_RESOLUTION)
{
fixed = *absx;
fixed.resolution = xresolution;
@@ -1487,8 +1480,10 @@ evdev_check_min_max(struct evdev_device *device,
unsigned int code)
static int
evdev_reject_device(struct evdev_device *device)
{
+ struct libinput *libinput = device-base.seat-libinput;
struct libevdev *evdev = device-evdev;
unsigned int code;
+ const struct input_absinfo *absx, *absy;
if (libevdev_has_event_code(evdev, EV_ABS, ABS_X) ^
libevdev_has_event_code(evdev, EV_ABS, ABS_Y))
@@ -1498,6 +1493,29 @@ evdev_reject_device(struct evdev_device *device)
libevdev_has_event_code(evdev, EV_ABS, ABS_MT_POSITION_Y))
return -1;
+ if (libevdev_has_event_code(evdev, EV_ABS, ABS_X)) {
+ absx = libevdev_get_abs_info(evdev, ABS_X);
+ absy = libevdev_get_abs_info(evdev, ABS_Y);
+ if ((absx-resolution == 0 absy-resolution != 0) ||
+ (absx-resolution != 0 absy-resolution == 0)) {
+ log_bug_kernel(libinput,
+ Kernel has only x or y resolution, not
both.\n);
+ return -1;
+ }
+ }
+
+ if (!evdev_is_fake_mt_device(device)
+ libevdev_has_event_code(evdev, EV_ABS, ABS_MT_POSITION_X)) {
+ absx = libevdev_get_abs_info(evdev, ABS_MT_POSITION_X);
+ absy = libevdev_get_abs_info(evdev, ABS_MT_POSITION_Y);
+ if ((absx-resolution == 0 absy-resolution != 0) ||
+ (absx-resolution != 0 absy-resolution == 0)) {
+ log_bug_kernel(libinput,
+ Kernel has only x or y MT resolution,
not both.\n);
+ return -1;
+ }
+ }
+
for (code = 0; code ABS_CNT; code++) {
switch (code) {
case ABS_MISC:
diff --git a/test/device.c b/test/device.c
index c9d5255..cf9885a 100644
--- a/test/device.c
+++ b/test/device.c
@@ -894,6 +894,91 @@ START_TEST(abs_mt_device_no_range)
}
END_TEST
+START_TEST(abs_device_missing_res)
+{
+ struct libevdev_uinput *uinput;
+ struct libinput *li;
+ struct libinput_device *device;
+ struct input_absinfo absinfo[] = {
+ { ABS_X, 0, 10, 0, 0, 10 },
+ { ABS_Y, 0, 10, 0, 0, 0 },
+ { -1, -1, -1, -1, -1, -1 }
+ };
+
+ li = litest_create_context();
+ litest_disable_log_handler(li);
+ uinput = litest_create_uinput_abs_device(test device, NULL,
+absinfo,
+EV_KEY, BTN_LEFT,
+EV_KEY, BTN_RIGHT,
+-1);
+ device = libinput_path_add_device(li,
+ libevdev_uinput_get_devnode(uinput));
+ ck_assert(device == NULL);
+ libevdev_uinput_destroy(uinput);
+
+ absinfo[0].resolution = 0;
+ absinfo[1].resolution = 20;
+ uinput = litest_create_uinput_abs_device(test device, NULL,
+absinfo,
+EV_KEY, BTN_LEFT,
+EV_KEY, BTN_RIGHT,
+-1);
+ device = libinput_path_add_device(li,
+ libevdev_uinput_get_devnode(uinput));
+ ck_assert(device == NULL);
+ libevdev_uinput_destroy(uinput);
+
+ litest_restore_log_handler(li);
+ libinput_unref(li);
+
+}
+END_TEST
+
+START_TEST(abs_mt_device_missing_res)
+{
+ struct libevdev_uinput *uinput;
+ struct libinput *li;
+ struct libinput_device *device;
+ struct input_absinfo absinfo[] = {
+ {
Re: [PATCH libinput v2 4/4] touchpad: Implement pinch gesture support
On Thu, Mar 26, 2015 at 10:04:40AM +0100, Hans de Goede wrote:
Implement touchpad pinch (and rotate) gesture support.
Note that two two-finger scrolling tests are slightly tweaked to assure that
there is enough touch movement to allow the scroll-or-pinch detect code to do
its works.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
src/evdev-mt-touchpad-gestures.c | 268
++-
src/evdev-mt-touchpad.h | 17 +++
src/libinput-private.h | 33 +
test/touchpad.c | 4 +-
4 files changed, 315 insertions(+), 7 deletions(-)
diff --git a/src/evdev-mt-touchpad-gestures.c
b/src/evdev-mt-touchpad-gestures.c
index 017354c..3d3dcaf 100644
--- a/src/evdev-mt-touchpad-gestures.c
+++ b/src/evdev-mt-touchpad-gestures.c
@@ -22,7 +22,6 @@
#include config.h
-#include assert.h
#include math.h
#include stdbool.h
#include limits.h
@@ -30,6 +29,7 @@
#include evdev-mt-touchpad.h
#define DEFAULT_GESTURE_SWITCH_TIMEOUT 100 /* ms */
+#define DEFAULT_GESTURE_2FG_SCROLL_TIMEOUT 1000 /* ms */
static struct normalized_coords
tp_get_touches_delta(struct tp_dispatch *tp, bool average)
@@ -75,6 +75,7 @@ tp_get_average_touches_delta(struct tp_dispatch *tp)
static void
tp_gesture_start(struct tp_dispatch *tp, uint64_t time)
{
+ struct libinput *libinput = tp-device-base.seat-libinput;
const struct normalized_coords zero = { 0.0, 0.0 };
if (tp-gesture.started)
@@ -82,7 +83,21 @@ tp_gesture_start(struct tp_dispatch *tp, uint64_t time)
switch (tp-gesture.finger_count) {
case 2:
- /* NOP */
+ switch (tp-gesture.twofinger_state) {
+ case GESTURE_2FG_STATE_NONE:
+ case GESTURE_2FG_STATE_UNKNOWN:
+ log_bug_libinput(libinput,
+ %s in unknown gesture mode\n, __func__);
nitpick: for multi-line calls please line up the arguments with the opening
parenthesis where possible and use one arg per line. So this would become:
log_bug_libinput(libinput,
%s in unknown gesture mode\n,
__func__);
+ break;
+ case GESTURE_2FG_STATE_SCROLL:
+ /* NOP */
+ break;
+ case GESTURE_2FG_STATE_PINCH:
+ gesture_notify_pinch(tp-device-base, time,
+ LIBINPUT_EVENT_GESTURE_PINCH_START,
+ zero, zero, 0.0, 0.0);
+ break;
+ }
break;
case 3:
case 4:
@@ -114,8 +129,172 @@ tp_gesture_post_pointer_motion(struct tp_dispatch *tp,
uint64_t time)
}
}
+static unsigned int
+tp_gesture_get_active_touches(struct tp_dispatch *tp,
+ struct tp_touch **touches,
+ unsigned int count)
+{
+ unsigned int i, n = 0;
+ struct tp_touch *t;
+
+ memset(touches, 0, count * sizeof(struct tp_touch *));
+
+ for (i = 0; i tp-real_touches; i++) {
+ t = tp-touches[i];
+ if (tp_touch_active(tp, t)) {
+ touches[n++] = t;
+ if (n == count)
+ return count;
+ }
+ }
+
+ /*
+ * This can happen when the user does .e.g:
+ * 1) Put down 1st finger in center (so active)
+ * 2) Put down 2nd finger in a button area (so inactive)
+ * 3) Put down 3th finger somewhere, gets reported as a fake finger,
+ *so gets same coordinates as 1st - active
+ *
+ * We could avoid this by looking at all touches, be we really only
+ * want to look at real touches.
+ */
+ return n;
+}
+
+static int
+tp_gesture_get_direction(struct tp_dispatch *tp, int touch)
+{
+ struct normalized_coords normalized;
+ double move_threshold;
+
+ /*
+ * Semi-mt touchpads have somewhat inaccurate coordinates when
+ * 2 fingers are down, so use a slightly larger threshold.
+ */
+ if (tp-semi_mt)
+ move_threshold = TP_MM_TO_DPI_NORMALIZED(4);
+ else
+ move_threshold = TP_MM_TO_DPI_NORMALIZED(3);
+
+ normalized = tp_normalize_delta(tp,
+ device_delta(tp-gesture.touches[touch]-point,
+ tp-gesture.touches[touch]-gesture.initial));
temp. variable please, with the indentation it's not immediately obvious
what calls what, so this would be better as:
struct tp_touch *t = tp-gesture.touches[touch];
struct device_float_coords delta;
delta = device_delta(t-point, t-gesture.initial);
normalized = tp_normalize_delta(tp, delta);
also - any reason you're passing in the touch index rather than a
struct tp_touch directly?
[PATCH weston] input: Don't use uninitialized variables in default_grab_pointer_focus()
If we have no pointer focus and weston_compositor_pick_view() returns NULL default_grab_pointer_focus() will test the unset sx, sy output parameters from weston_compositor_pick_view(). Instead, assume that since both pointers are NULL focus hasn't changed and we don't need to update pointer focus. Signed-off-by: Derek Foreman der...@osg.samsung.com --- This fixes an error reported by valgrind. I think the patch is correct but it does stop pointer-focus_signal from being emitted when focus switches from NULL to NULL. I think that shouldn't have happened anyway? Initializing sx, sy to wl_fixed_from_int(0) would silence the warning too, but I think this way is more correct... src/input.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/input.c b/src/input.c index 142c670..18d1262 100644 --- a/src/input.c +++ b/src/input.c @@ -157,6 +157,9 @@ default_grab_pointer_focus(struct weston_pointer_grab *grab) pointer-x, pointer-y, sx, sy); + if (!pointer-focus !view) + return; + if (pointer-focus != view || pointer-sx != sx || pointer-sy != sy) weston_pointer_set_focus(pointer, view, sx, sy); } -- 2.1.4 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
