Re: [PATCH weston v5] input: Don't test keyboard/pointer/touch pointers

2015-05-20 Thread Jonas Ådahl
On Mon, May 04, 2015 at 10:59:10AM -0500, Derek Foreman wrote:
> Keyboards and pointers aren't freed when devices are removed,
> so we should really be testing keyboard_device_count and
> pointer_device_count in most cases, not the actual pointers.
> Otherwise we end up with different behaviour after removing a
> device than we had before it was inserted.
> 
> This commit renames the touch/keyboard/pointer pointers and adds
> helper functions to get them that hide this complexity and return
> NULL when *_device_count is 0.
> 
> Signed-off-by: Derek Foreman 
> ---
> 
> Fixed a few bugs that turned up while running devices-test - now
> seat_get_pointer, seat_get_touch and seat_get_keyboard use the resource
> pointers directly.
> 
> Added a couple of null pointer checks.
> 
> Moved the helper functions to input.c

Hey, some issues and nits inlined below. Another general comment is, in
various places, you use the helper and temporary variabes when it is
guaranteed that it'll always return for example seat->pointer (in grabs,
keybindings and the like). Are those changes really necessary? Being
able to go directly from seat to pointer is awfully convenient and very
often reliable (in grabs, keybindings etc).

> 
>  desktop-shell/exposay.c |  27 ++--
>  desktop-shell/input-panel.c |   7 +-
>  desktop-shell/shell.c   | 266 +++-
>  fullscreen-shell/fullscreen-shell.c |  21 ++-
>  ivi-shell/hmi-controller.c  |  35 +++--
>  ivi-shell/input-panel-ivi.c |   7 +-
>  src/bindings.c  |  28 ++--
>  src/compositor-drm.c|  16 +-
>  src/compositor-wayland.c|   8 +-
>  src/compositor-x11.c|  20 ++-
>  src/compositor.c|  32 ++--
>  src/compositor.h|  15 +-
>  src/data-device.c   |  41 ++---
>  src/input.c | 294 
> +++-
>  src/libinput-device.c   |   3 +-
>  src/libinput-seat.c |  15 +-
>  src/screen-share.c  |   3 +-
>  src/screenshooter.c |   7 +-
>  src/text-backend.c  |  19 ++-
>  src/zoom.c  |   8 +-
>  tests/surface-screenshot.c  |   7 +-
>  tests/weston-test.c |  12 +-
>  xwayland/dnd.c  |   3 +-
>  xwayland/window-manager.c   |  21 ++-
>  24 files changed, 583 insertions(+), 332 deletions(-)
> 
> diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c
> index 4b65cbd..e3b1c78 100644
> --- a/desktop-shell/exposay.c
> +++ b/desktop-shell/exposay.c
> @@ -517,11 +517,13 @@ static enum exposay_layout_state
>  exposay_set_inactive(struct desktop_shell *shell)
>  {
>   struct weston_seat *seat = shell->exposay.seat;
> + struct weston_keyboard *keyboard = weston_seat_get_keyboard(seat);
> + struct weston_pointer *pointer = weston_seat_get_pointer(seat);
>  
> - weston_keyboard_end_grab(seat->keyboard);
> - weston_pointer_end_grab(seat->pointer);
> - if (seat->keyboard->input_method_resource)
> - seat->keyboard->grab = &seat->keyboard->input_method_grab;
> + weston_keyboard_end_grab(keyboard);
> + weston_pointer_end_grab(pointer);
> + if (keyboard->input_method_resource)
> + keyboard->grab = &keyboard->input_method_grab;

This may introduce segfaults as seat->keyboard/seat->pointer would
previously always be non-NULL, even when the last keyboard/pointer was
disconnected.

>  
>   return EXPOSAY_LAYOUT_INACTIVE;
>  }
> @@ -554,26 +556,27 @@ static enum exposay_layout_state
>  exposay_transition_active(struct desktop_shell *shell)
>  {
>   struct weston_seat *seat = shell->exposay.seat;
> + struct weston_pointer *pointer = weston_seat_get_pointer(seat);
> + struct weston_keyboard *keyboard = weston_seat_get_keyboard(seat);
>   struct shell_output *shell_output;
>   bool animate = false;
>  
>   shell->exposay.workspace = get_current_workspace(shell);
> - shell->exposay.focus_prev = get_default_view (seat->keyboard->focus);
> - shell->exposay.focus_current = get_default_view (seat->keyboard->focus);
> + shell->exposay.focus_prev = get_default_view(keyboard->focus);
> + shell->exposay.focus_current = get_default_view(keyboard->focus);

Same issue here regarding pointe/keyboard can now be NULL when before
they couldn't.

>   shell->exposay.clicked = NULL;
>   wl_list_init(&shell->exposay.surface_list);
>  
>   lower_fullscreen_layer(shell);
>   shell->exposay.grab_kbd.interface = &exposay_kbd_grab;
> - weston_keyboard_start_grab(seat->keyboard,
> -&shell->exposay.grab_kbd);
> - weston_keyboard_set_focus(seat->keyboard, NULL);
> + weston_keyboard_start_grab(keyboard, &shell->exposay.grab_kbd);
> + weston_keyboard_set_focus(keyboard, NULL);
>  
>   shell->exposay.g

Re: [PATCH weston 4/4] input: Detect keyboard capabilities

2015-05-20 Thread Jonas Ådahl
On Wed, May 20, 2015 at 12:32:46PM -0500, Derek Foreman wrote:
> Thanks for taking a look!
> 
> On 20/05/15 05:04 AM, Jonas Ådahl wrote:
> > On Tue, May 05, 2015 at 03:01:54PM -0500, Derek Foreman wrote:
> >> Some devices (power buttons, acpi video bus driver) are considered
> >> keyboards but you can't type with them.
> >>
> >> As of libinput 0.15 we can query a keyboard to see which key events it
> >> can generate.  We use this to detect if a keyboard can type letters or
> >> digits.
> >>
> >> The wayland protocol doesn't propagate these capabilites, so
> >> weston_seat_send_dirty_caps() will differentiate between dirty keyboards
> >> (which generate the weston signal) and dirty seats (which generate a
> >> wayland event) and only send the appropriate updates.
> >>
> >> Functions are provided for backends that don't use libinput to force these
> >> capabilities to a sensible state.
> >>
> >> Signed-off-by: Derek Foreman 
> >> ---
> >>  configure.ac  |  2 +-
> >>  src/compositor-headless.c |  1 +
> >>  src/compositor-rdp.c  |  1 +
> >>  src/compositor-wayland.c  |  1 +
> >>  src/compositor-x11.c  |  1 +
> >>  src/compositor.h  |  8 
> >>  src/input.c   | 29 +++--
> >>  src/libinput-device.c | 32 +++-
> >>  src/libinput-device.h |  4 +++-
> >>  src/libinput-seat.c   | 27 +++
> >>  src/screen-share.c|  1 +
> >>  11 files changed, 102 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/configure.ac b/configure.ac
> >> index d9d8d8f..4e9ae20 100644
> >> --- a/configure.ac
> >> +++ b/configure.ac
> >> @@ -155,7 +155,7 @@ if test x$enable_drm_compositor = xyes; then
> >>  fi
> >>  
> >>  
> >> -PKG_CHECK_MODULES(LIBINPUT_BACKEND, [libinput >= 0.8.0])
> >> +PKG_CHECK_MODULES(LIBINPUT_BACKEND, [libinput >= 0.15.0])
> >>  PKG_CHECK_MODULES(COMPOSITOR, [$COMPOSITOR_MODULES])
> >>  
> >>  AC_ARG_ENABLE(wayland-compositor, [  --enable-wayland-compositor],,
> >> diff --git a/src/compositor-headless.c b/src/compositor-headless.c
> >> index 0ddb26e..0b461c6 100644
> >> --- a/src/compositor-headless.c
> >> +++ b/src/compositor-headless.c
> >> @@ -184,6 +184,7 @@ headless_input_create(struct headless_compositor *c)
> >>if (weston_seat_init_keyboard(&c->fake_seat, NULL) < 0)
> >>return -1;
> >>  
> >> +  weston_seat_override_keyboard_caps(&c->fake_seat, true, true);
> >>weston_seat_send_dirty_caps(&c->fake_seat);
> >>  
> >>return 0;
> >> diff --git a/src/compositor-rdp.c b/src/compositor-rdp.c
> >> index 5b50382..b6e3876 100644
> >> --- a/src/compositor-rdp.c
> >> +++ b/src/compositor-rdp.c
> >> @@ -842,6 +842,7 @@ xf_peer_post_connect(freerdp_peer* client)
> >>}
> >>weston_seat_init_keyboard(&peerCtx->item.seat, keymap);
> >>weston_seat_init_pointer(&peerCtx->item.seat);
> >> +  weston_seat_override_keyboard_caps(&peerCtx->item.seat, true, true);
> >>weston_seat_send_dirty_caps(&peerCtx->item.seat);
> >>  
> >>peerCtx->item.flags |= RDP_PEER_ACTIVATED;
> >> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> >> index 53159c4..1e33697 100644
> >> --- a/src/compositor-wayland.c
> >> +++ b/src/compositor-wayland.c
> >> @@ -1463,6 +1463,7 @@ input_handle_keymap(void *data, struct wl_keyboard 
> >> *keyboard, uint32_t format,
> >>else
> >>weston_seat_init_keyboard(&input->base, keymap);
> >>  
> >> +  weston_seat_override_keyboard_caps(&input->base, true, true);
> >>weston_seat_send_dirty_caps(&input->base);
> >>  
> >>xkb_keymap_unref(keymap);
> >> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
> >> index 4a3b10e..69e3a14 100644
> >> --- a/src/compositor-x11.c
> >> +++ b/src/compositor-x11.c
> >> @@ -336,6 +336,7 @@ x11_input_create(struct x11_compositor *c, int 
> >> no_input)
> >>return -1;
> >>xkb_keymap_unref(keymap);
> >>  
> >> +  weston_seat_override_keyboard_caps(&c->core_seat, true, true);
> >>weston_seat_send_dirty_caps(&c->core_seat);
> >>  
> >>x11_compositor_setup_xkb(c);
> >> diff --git a/src/compositor.h b/src/compositor.h
> >> index e05b262..da6d126 100644
> >> --- a/src/compositor.h
> >> +++ b/src/compositor.h
> >> @@ -451,6 +451,10 @@ weston_touch_start_drag(struct weston_touch *touch,
> >>struct wl_client *client);
> >>  void
> >>  weston_seat_send_dirty_caps(struct weston_seat *seat);
> >> +void
> >> +weston_seat_override_keyboard_caps(struct weston_seat *seat,
> >> + bool has_letters,
> >> + bool has_digits);
> >>  
> >>  struct weston_xkb_info {
> >>struct xkb_keymap *keymap;
> >> @@ -505,6 +509,10 @@ struct weston_keyboard {
> >>enum weston_led leds;
> >>} xkb_state;
> >>struct xkb_keymap *pending_keymap;
> >> +
> >> +  bool has_digits;
> >> +  bool has_letters;
> >> +  bool caps_dirty;
> >>  };
> >>  
> >>  struct weston_seat {
> >> diff --git a/

[PATCH] desktop-shell: support an option to use 16 bit color or not, instead of using it by default

2015-05-20 Thread nerdopolis
---
 clients/desktop-shell.c | 13 +++--
 man/weston.ini.man  |  4 
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index e2f9f80..d4ba127 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -94,6 +94,7 @@ struct background {
char *image;
int type;
uint32_t color;
+   int32_t low_bpp;
 };
 
 struct output {
@@ -1015,10 +1016,18 @@ background_create(struct desktop *desktop)
window_set_user_data(background->window, background);
widget_set_redraw_handler(background->widget, background_draw);
widget_set_transparent(background->widget, 0);
-   window_set_preferred_format(background->window,
-   WINDOW_PREFERRED_FORMAT_RGB565);
 
s = weston_config_get_section(desktop->config, "shell", NULL, NULL);
+   weston_config_section_get_bool(s, "background-low-bpp",
+&background->low_bpp, NULL);
+   if (!background->low_bpp) {
+ window_set_preferred_format(background->window,
+ WINDOW_PREFERRED_FORMAT_NONE);
+   } else {
+ window_set_preferred_format(background->window,
+ WINDOW_PREFERRED_FORMAT_RGB565);
+   }
+
weston_config_section_get_string(s, "background-image",
 &background->image, NULL);
weston_config_section_get_uint(s, "background-color",
diff --git a/man/weston.ini.man b/man/weston.ini.man
index fe86bb6..3a8b2a4 100644
--- a/man/weston.ini.man
+++ b/man/weston.ini.man
@@ -216,6 +216,10 @@ output. Tile repeats the background image to fill the 
output.
 sets the color of the background (unsigned integer). The hexadecimal
 digit pairs are in order alpha, red, green, and blue.
 .TP 7
+.BI "background-low-bpp=" true
+specify to reduce the background to 16 bit color (boolean). This option is
+useful for low memory platforms. This only affects the background.
+.TP 7
 .BI "panel-color=" 0xAARRGGBB
 sets the color of the panel (unsigned integer). The hexadecimal
 digit pairs are in order transparency, red, green, and blue. Examples:
-- 
2.1.0

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


Re: [PATCH wayland] tests: add an headers test

2015-05-20 Thread Bill Spitzak

Please don't duplicate the functions in multiple headers!

You could achieve the same result this way so that there is only one 
copy of each function in the headers:


wayland-client-protocol-core.h =
  (the same as you have it, generated by wayland-scanner -c)

wayland-client-protocol.h =
  #include 
  #include 

However this really is a mess and these extra header files do nothing 
except confuse anybody trying to learn wayland. Would vastly prefer 
making -c the default and requiring the *VERY FEW* programs that no 
longer compile to be patched.


On 05/20/2015 12:38 PM, Giulio Camuffo wrote:


@@ -85,6 +87,12 @@ protocol/%-server-protocol.h : $(top_srcdir)/protocol/%.xml
  protocol/%-client-protocol.h : $(top_srcdir)/protocol/%.xml
$(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(wayland_scanner) client-header < $< 
> $@

+protocol/%-server-protocol-core.h : $(top_srcdir)/protocol/%.xml
+   $(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(wayland_scanner) server-header -c < 
$< > $@
+
+protocol/%-client-protocol-core.h : $(top_srcdir)/protocol/%.xml
+   $(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(wayland_scanner) client-header -c < 
$< > $@
+


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


Re: [PATCH] desktop-shell: support an option to use 16 bit color or not, instead of using it by default

2015-05-20 Thread nerdopolis
On Wednesday, May 20, 2015 12:17:42 PM Bill Spitzak wrote:
> That name is pretty confusing and not what anybody would guess. It 
> certainly should start with "background-" so that it matches all the 
> other switches that control the desktop image.
> 
> The fact that it does not always work does NOT have to be part of the name.
> 
> Some ideas I prefer:
> 
> background-depth=16
> background-format=RGB565
> 
> The fact that it takes a number or name does not mean you have to worry 
> about any values other than the one you are interested in. In particular 
> if a depth is used just use 'background_depth <= 16' to choose whether 
> to try RGB565. If a format name is used I think you can insist the only 
> ones that do not produce an error are "RGB565" and "". Future versions 
> can then enhance this with obvious additions, rather than being forced 
> to add even more switches.
> 
> I am also annoyed that the keywords have dashes in them (thus requiring 
> the C variable that stores the value to have a different name) but that 
> seems to be well-established here...
> 
> On 05/20/2015 04:55 AM, nerdopolis wrote:
> 
> > s = weston_config_get_section(desktop->config, "shell", NULL, NULL);
> > +   weston_config_section_get_bool(s, "background-low-bitrate",
> > +&background->low_bitrate, NULL);
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Hi.

I submitted a patch changing it to background-low-bpp

While the option might be slighty harder to guess, I think a boolean is better.
Specifying the format will cause users to look up the names of 
the formats. 
I don't know how many people would remember RGB565 

Doing an integer for number of pixels might lead to attempts to 
set it at
4, or 8, when it only supports 16 or 32 bit... 

It is unlikely that more bpp options will be used for just the weston 
background... 
It was only at low bpp to reduce memory usage on the Raspberry Pi...
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH libinput 7/7] touchpad: sync the initial touch state

2015-05-20 Thread Benjamin Tissoires
On Tue, May 19, 2015 at 6:52 PM, Peter Hutterer
 wrote:
> Unlikely, but there's the odd chance of the first touch coming in with the
> same X or Y coordinate the kernel already has internally. This would
> generate a bogus delta on the second event when the touch coordinate jumps
> from 0/y or x/0 to the real coordinates.
>
> For touchpads with distance support this is a real issue since the default
> value for a touch distance is > 0.
>
> Signed-off-by: Peter Hutterer 
> ---

The series is:
Tested-by: Benjamin Tissoires 

Cheers,
Benjamin

>  src/evdev-mt-touchpad.c | 28 ++
>  test/touchpad.c | 62 
> +
>  2 files changed, 90 insertions(+)
>
> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> index 026f9ee..409d81e 100644
> --- a/src/evdev-mt-touchpad.c
> +++ b/src/evdev-mt-touchpad.c
> @@ -1087,6 +1087,28 @@ tp_init_touch(struct tp_dispatch *tp,
> t->has_ended = true;
>  }
>
> +static void
> +tp_sync_touch(struct tp_dispatch *tp,
> + struct evdev_device *device,
> + struct tp_touch *t,
> + int slot)
> +{
> +   struct libevdev *evdev = device->evdev;
> +
> +   if (!libevdev_fetch_slot_value(evdev,
> +  slot,
> +  ABS_MT_POSITION_X,
> +  &t->point.x))
> +   t->point.x = libevdev_get_event_value(evdev, EV_ABS, ABS_X);
> +   if (!libevdev_fetch_slot_value(evdev,
> +  slot,
> +  ABS_MT_POSITION_Y,
> +  &t->point.y))
> +   t->point.y = libevdev_get_event_value(evdev, EV_ABS, ABS_Y);
> +
> +   libevdev_fetch_slot_value(evdev, slot, ABS_MT_DISTANCE, &t->distance);
> +}
> +
>  static int
>  tp_init_slots(struct tp_dispatch *tp,
>   struct evdev_device *device)
> @@ -1134,6 +1156,12 @@ tp_init_slots(struct tp_dispatch *tp,
> for (i = 0; i < tp->ntouches; i++)
> tp_init_touch(tp, &tp->touches[i]);
>
> +   /* Always sync the first touch so we get ABS_X/Y synced on
> +* single-touch touchpads */
> +   tp_sync_touch(tp, device, &tp->touches[0], 0);
> +   for (i = 1; i < tp->num_slots; i++)
> +   tp_sync_touch(tp, device, &tp->touches[i], i);
> +
> return 0;
>  }
>
> diff --git a/test/touchpad.c b/test/touchpad.c
> index f7f9dd5..3bdcc2b 100644
> --- a/test/touchpad.c
> +++ b/test/touchpad.c
> @@ -4530,9 +4530,69 @@ START_TEST(touchpad_trackpoint_no_trackpoint)
>  }
>  END_TEST
>
> +START_TEST(touchpad_initial_state)
> +{
> +   struct litest_device *dev;
> +   struct libinput *libinput1, *libinput2;
> +   struct libinput_event *ev1, *ev2;
> +   struct libinput_event_pointer *p1, *p2;
> +   int axis = _i; /* looped test */
> +   int x = 40, y = 60;
> +
> +   dev = litest_current_device();
> +   libinput1 = dev->libinput;
> +
> +   libinput_device_config_tap_set_enabled(dev->libinput_device,
> +  LIBINPUT_CONFIG_TAP_DISABLED);
> +
> +   litest_touch_down(dev, 0, x, y);
> +   litest_touch_up(dev, 0);
> +
> +   /* device is now on some x/y value */
> +   litest_drain_events(libinput1);
> +
> +   libinput2 = litest_create_context();
> +   libinput_path_add_device(libinput2,
> +libevdev_uinput_get_devnode(dev->uinput));
> +   litest_drain_events(libinput2);
> +
> +   if (axis == ABS_X)
> +   x = 30;
> +   else
> +   y = 30;
> +   litest_touch_down(dev, 0, x, y);
> +   litest_touch_move_to(dev, 0, x, y, 80, 80, 10, 1);
> +   litest_touch_up(dev, 0);
> +
> +   litest_wait_for_event(libinput1);
> +   litest_wait_for_event(libinput2);
> +
> +   while (libinput_next_event_type(libinput1)) {
> +   ev1 = libinput_get_event(libinput1);
> +   ev2 = libinput_get_event(libinput2);
> +
> +   p1 = litest_is_motion_event(ev1);
> +   p2 = litest_is_motion_event(ev2);
> +
> +   ck_assert_int_eq(libinput_event_get_type(ev1),
> +libinput_event_get_type(ev2));
> +
> +   ck_assert_int_eq(libinput_event_pointer_get_dx(p1),
> +libinput_event_pointer_get_dx(p2));
> +   ck_assert_int_eq(libinput_event_pointer_get_dy(p1),
> +libinput_event_pointer_get_dy(p2));
> +   libinput_event_destroy(ev1);
> +   libinput_event_destroy(ev2);
> +   }
> +
> +   libinput_unref(libinput2);
> +}
> +END_TEST
> +
>  int main(int argc, char **argv)
>  {
> struct range multitap_range = {3, 8};
> +   struct range axis_range = {ABS_X, ABS_Y + 1};
>
> litest_add("touchpad:motion", touchpad_1fg_mot

[PATCH weston] gl-renderer: Make the error logging a little nicer

2015-05-20 Thread Derek Foreman
Signed-off-by: Derek Foreman 
---
 src/gl-renderer.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index 772c6a8..d18e124 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -1925,7 +1925,6 @@ match_config_to_visual(EGLDisplay egl_display,
return i;
}
 
-   weston_log("Unable to find an appropriate EGL config.\n");
return -1;
 }
 
@@ -1939,16 +1938,19 @@ egl_choose_config(struct gl_renderer *gr, const EGLint 
*attribs,
EGLConfig *configs;
int i, config_index = -1;
 
-   if (!eglGetConfigs(gr->egl_display, NULL, 0, &count) || count < 1)
+   if (!eglGetConfigs(gr->egl_display, NULL, 0, &count) || count < 1) {
+   weston_log("No EGL configs to choose from.\n");
return -1;
-
+   }
configs = calloc(count, sizeof *configs);
if (!configs)
return -1;
 
if (!eglChooseConfig(gr->egl_display, attribs, configs,
- count, &matched) || !matched)
+ count, &matched) || !matched) {
+   weston_log("No EGL configs with appropriate attributes.\n");
goto out;
+   }
 
if (!visual_id)
config_index = 0;
@@ -1967,6 +1969,10 @@ out:
if (config_index == -1)
return -1;
 
+   if (i > 1)
+   weston_log("Unable to use first choice EGL config with id"
+  " 0x%x, succeeded with alternate id 0x%x.\n",
+  visual_id[0], visual_id[i - 1]);
return 0;
 }
 
@@ -2356,7 +2362,7 @@ gl_renderer_create(struct weston_compositor *ec, EGLenum 
platform,
if (egl_choose_config(gr, attribs, visual_id,
  n_ids, &gr->egl_config) < 0) {
weston_log("failed to choose EGL config\n");
-   goto err_egl;
+   goto err_config;
}
 
ec->renderer = &gr->base;
@@ -2375,6 +2381,7 @@ gl_renderer_create(struct weston_compositor *ec, EGLenum 
platform,
 
 err_egl:
gl_renderer_print_egl_error_state();
+err_config:
free(gr);
return -1;
 }
-- 
2.1.4

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


[PATCH wayland] tests: add an headers test

2015-05-20 Thread Giulio Camuffo
This test checks that the protocol and library headers include only what
they are supposed to include. That is, that the core headers do not
include the protocol headers and that the core protocol headers do not
include the non core library headers.
The build process now generates core protocol headers, but they are only
used in the test and don't get installed.
---
 Makefile.am| 15 +++-
 tests/headers-protocol-core-test.c | 31 +++
 tests/headers-protocol-test.c  | 31 +++
 tests/headers-test.c   | 50 ++
 4 files changed, 126 insertions(+), 1 deletion(-)
 create mode 100644 tests/headers-protocol-core-test.c
 create mode 100644 tests/headers-protocol-test.c
 create mode 100644 tests/headers-test.c

diff --git a/Makefile.am b/Makefile.am
index 5b44d6f..0f13fb3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -50,6 +50,7 @@ libwayland_server_la_SOURCES =\
 
 nodist_libwayland_server_la_SOURCES =  \
protocol/wayland-server-protocol.h  \
+   protocol/wayland-server-protocol-core.h \
protocol/wayland-protocol.c
 
 libwayland_client_la_CFLAGS = $(FFI_CFLAGS) $(GCC_CFLAGS) -pthread
@@ -60,6 +61,7 @@ libwayland_client_la_SOURCES =\
 
 nodist_libwayland_client_la_SOURCES =  \
protocol/wayland-client-protocol.h  \
+   protocol/wayland-client-protocol-core.h \
protocol/wayland-protocol.c
 
 pkgconfig_DATA += src/wayland-client.pc src/wayland-server.pc
@@ -85,6 +87,12 @@ protocol/%-server-protocol.h : $(top_srcdir)/protocol/%.xml
 protocol/%-client-protocol.h : $(top_srcdir)/protocol/%.xml
$(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(wayland_scanner) client-header < 
$< > $@
 
+protocol/%-server-protocol-core.h : $(top_srcdir)/protocol/%.xml
+   $(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(wayland_scanner) server-header -c 
< $< > $@
+
+protocol/%-client-protocol-core.h : $(top_srcdir)/protocol/%.xml
+   $(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(wayland_scanner) client-header -c 
< $< > $@
+
 BUILT_SOURCES =\
$(nodist_libwayland_server_la_SOURCES)  \
$(nodist_libwayland_client_la_SOURCES)
@@ -132,7 +140,8 @@ TESTS = \
queue-test  \
signal-test \
resources-test  \
-   message-test
+   message-test\
+   headers-test
 
 if ENABLE_CPP_TEST
 TESTS += cpp-compile-test
@@ -188,6 +197,10 @@ resources_test_SOURCES = tests/resources-test.c
 resources_test_LDADD = libtest-runner.la
 message_test_SOURCES = tests/message-test.c
 message_test_LDADD = libtest-runner.la
+headers_test_SOURCES = tests/headers-test.c \
+  tests/headers-protocol-test.c \
+  tests/headers-protocol-core-test.c \
+headers_test_LDADD = libtest-runner.la
 
 if ENABLE_CPP_TEST
 cpp_compile_test_SOURCES = tests/cpp-compile-test.cpp
diff --git a/tests/headers-protocol-core-test.c 
b/tests/headers-protocol-core-test.c
new file mode 100644
index 000..60a8a62
--- /dev/null
+++ b/tests/headers-protocol-core-test.c
@@ -0,0 +1,31 @@
+/*
+ * Copyright © 2015 Giulio Camuffo
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#include "wayland-client-protocol-core.h"
+#include "wayland-server-protocol-core.h"
+
+#ifdef WAYLAND_CLIENT_H
+#error including wayland-client-protocol-core.h included wayland-client.h!
+#endif
+#ifdef WAYLAND_SERVER_H
+#error including wayland-server-protocol-core.h included wayland-server.h!
+#endif
diff --git a/tests/headers-protocol-test.c b/tests/headers-protocol-test.c
new file mode 100644
index 000..f8726ba
--- /dev/null
+++ b/test

Re: [PATCH] desktop-shell: support an option to use 16 bit color or not, instead of using it by default

2015-05-20 Thread Bill Spitzak
That name is pretty confusing and not what anybody would guess. It 
certainly should start with "background-" so that it matches all the 
other switches that control the desktop image.


The fact that it does not always work does NOT have to be part of the name.

Some ideas I prefer:

background-depth=16
background-format=RGB565

The fact that it takes a number or name does not mean you have to worry 
about any values other than the one you are interested in. In particular 
if a depth is used just use 'background_depth <= 16' to choose whether 
to try RGB565. If a format name is used I think you can insist the only 
ones that do not produce an error are "RGB565" and "". Future versions 
can then enhance this with obvious additions, rather than being forced 
to add even more switches.


I am also annoyed that the keywords have dashes in them (thus requiring 
the C variable that stores the value to have a different name) but that 
seems to be well-established here...


On 05/20/2015 04:55 AM, nerdopolis wrote:


s = weston_config_get_section(desktop->config, "shell", NULL, NULL);
+   weston_config_section_get_bool(s, "background-low-bitrate",
+&background->low_bitrate, NULL);


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


Re: [PATCH weston 4/4] input: Detect keyboard capabilities

2015-05-20 Thread Derek Foreman
Thanks for taking a look!

On 20/05/15 05:04 AM, Jonas Ådahl wrote:
> On Tue, May 05, 2015 at 03:01:54PM -0500, Derek Foreman wrote:
>> Some devices (power buttons, acpi video bus driver) are considered
>> keyboards but you can't type with them.
>>
>> As of libinput 0.15 we can query a keyboard to see which key events it
>> can generate.  We use this to detect if a keyboard can type letters or
>> digits.
>>
>> The wayland protocol doesn't propagate these capabilites, so
>> weston_seat_send_dirty_caps() will differentiate between dirty keyboards
>> (which generate the weston signal) and dirty seats (which generate a
>> wayland event) and only send the appropriate updates.
>>
>> Functions are provided for backends that don't use libinput to force these
>> capabilities to a sensible state.
>>
>> Signed-off-by: Derek Foreman 
>> ---
>>  configure.ac  |  2 +-
>>  src/compositor-headless.c |  1 +
>>  src/compositor-rdp.c  |  1 +
>>  src/compositor-wayland.c  |  1 +
>>  src/compositor-x11.c  |  1 +
>>  src/compositor.h  |  8 
>>  src/input.c   | 29 +++--
>>  src/libinput-device.c | 32 +++-
>>  src/libinput-device.h |  4 +++-
>>  src/libinput-seat.c   | 27 +++
>>  src/screen-share.c|  1 +
>>  11 files changed, 102 insertions(+), 5 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index d9d8d8f..4e9ae20 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -155,7 +155,7 @@ if test x$enable_drm_compositor = xyes; then
>>  fi
>>  
>>  
>> -PKG_CHECK_MODULES(LIBINPUT_BACKEND, [libinput >= 0.8.0])
>> +PKG_CHECK_MODULES(LIBINPUT_BACKEND, [libinput >= 0.15.0])
>>  PKG_CHECK_MODULES(COMPOSITOR, [$COMPOSITOR_MODULES])
>>  
>>  AC_ARG_ENABLE(wayland-compositor, [  --enable-wayland-compositor],,
>> diff --git a/src/compositor-headless.c b/src/compositor-headless.c
>> index 0ddb26e..0b461c6 100644
>> --- a/src/compositor-headless.c
>> +++ b/src/compositor-headless.c
>> @@ -184,6 +184,7 @@ headless_input_create(struct headless_compositor *c)
>>  if (weston_seat_init_keyboard(&c->fake_seat, NULL) < 0)
>>  return -1;
>>  
>> +weston_seat_override_keyboard_caps(&c->fake_seat, true, true);
>>  weston_seat_send_dirty_caps(&c->fake_seat);
>>  
>>  return 0;
>> diff --git a/src/compositor-rdp.c b/src/compositor-rdp.c
>> index 5b50382..b6e3876 100644
>> --- a/src/compositor-rdp.c
>> +++ b/src/compositor-rdp.c
>> @@ -842,6 +842,7 @@ xf_peer_post_connect(freerdp_peer* client)
>>  }
>>  weston_seat_init_keyboard(&peerCtx->item.seat, keymap);
>>  weston_seat_init_pointer(&peerCtx->item.seat);
>> +weston_seat_override_keyboard_caps(&peerCtx->item.seat, true, true);
>>  weston_seat_send_dirty_caps(&peerCtx->item.seat);
>>  
>>  peerCtx->item.flags |= RDP_PEER_ACTIVATED;
>> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
>> index 53159c4..1e33697 100644
>> --- a/src/compositor-wayland.c
>> +++ b/src/compositor-wayland.c
>> @@ -1463,6 +1463,7 @@ input_handle_keymap(void *data, struct wl_keyboard 
>> *keyboard, uint32_t format,
>>  else
>>  weston_seat_init_keyboard(&input->base, keymap);
>>  
>> +weston_seat_override_keyboard_caps(&input->base, true, true);
>>  weston_seat_send_dirty_caps(&input->base);
>>  
>>  xkb_keymap_unref(keymap);
>> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
>> index 4a3b10e..69e3a14 100644
>> --- a/src/compositor-x11.c
>> +++ b/src/compositor-x11.c
>> @@ -336,6 +336,7 @@ x11_input_create(struct x11_compositor *c, int no_input)
>>  return -1;
>>  xkb_keymap_unref(keymap);
>>  
>> +weston_seat_override_keyboard_caps(&c->core_seat, true, true);
>>  weston_seat_send_dirty_caps(&c->core_seat);
>>  
>>  x11_compositor_setup_xkb(c);
>> diff --git a/src/compositor.h b/src/compositor.h
>> index e05b262..da6d126 100644
>> --- a/src/compositor.h
>> +++ b/src/compositor.h
>> @@ -451,6 +451,10 @@ weston_touch_start_drag(struct weston_touch *touch,
>>  struct wl_client *client);
>>  void
>>  weston_seat_send_dirty_caps(struct weston_seat *seat);
>> +void
>> +weston_seat_override_keyboard_caps(struct weston_seat *seat,
>> +   bool has_letters,
>> +   bool has_digits);
>>  
>>  struct weston_xkb_info {
>>  struct xkb_keymap *keymap;
>> @@ -505,6 +509,10 @@ struct weston_keyboard {
>>  enum weston_led leds;
>>  } xkb_state;
>>  struct xkb_keymap *pending_keymap;
>> +
>> +bool has_digits;
>> +bool has_letters;
>> +bool caps_dirty;
>>  };
>>  
>>  struct weston_seat {
>> diff --git a/src/input.c b/src/input.c
>> index c37bd20..3a0adc3 100644
>> --- a/src/input.c
>> +++ b/src/input.c
>> @@ -601,14 +601,16 @@ weston_touch_destroy(struct weston_touch *touch)
>>  
>>  /* Send seat capability updates if necessary
>>   *
>> - * Checks if 

Re: [PATCH] desktop-shell: support an option to use 16 bit color or not, instead of using it by default

2015-05-20 Thread Pekka Paalanen
On Wed, 20 May 2015 07:55:17 -0400
nerdopolis  wrote:

> ---
>  clients/desktop-shell.c | 13 +++--
>  man/weston.ini.man  |  4 
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> index e2f9f80..970b9f0 100644
> --- a/clients/desktop-shell.c
> +++ b/clients/desktop-shell.c
> @@ -94,6 +94,7 @@ struct background {
>   char *image;
>   int type;
>   uint32_t color;
> + int32_t low_bitrate;
>  };
>  
>  struct output {
> @@ -1015,10 +1016,18 @@ background_create(struct desktop *desktop)
>   window_set_user_data(background->window, background);
>   widget_set_redraw_handler(background->widget, background_draw);
>   widget_set_transparent(background->widget, 0);
> - window_set_preferred_format(background->window,
> - WINDOW_PREFERRED_FORMAT_RGB565);
>  
>   s = weston_config_get_section(desktop->config, "shell", NULL, NULL);
> + weston_config_section_get_bool(s, "background-low-bitrate",
> +  &background->low_bitrate, NULL);
> + if (!background->low_bitrate) {
> +   window_set_preferred_format(background->window,
> +   WINDOW_PREFERRED_FORMAT_NONE);

No need to call this at all.

> + } else {
> +   window_set_preferred_format(background->window,
> +   WINDOW_PREFERRED_FORMAT_RGB565);
> + }
> +
>   weston_config_section_get_string(s, "background-image",
>&background->image, NULL);
>   weston_config_section_get_uint(s, "background-color",
> diff --git a/man/weston.ini.man b/man/weston.ini.man
> index fe86bb6..240fbd8 100644
> --- a/man/weston.ini.man
> +++ b/man/weston.ini.man
> @@ -216,6 +216,10 @@ output. Tile repeats the background image to fill the 
> output.
>  sets the color of the background (unsigned integer). The hexadecimal
>  digit pairs are in order alpha, red, green, and blue.
>  .TP 7
> +.BI "background-low-bitrate=" true
> +specify to reduce the background to 16 bit color (boolean). This option is
> +useful for low memory platforms.
> +.TP 7
>  .BI "panel-color=" 0xAARRGGBB
>  sets the color of the panel (unsigned integer). The hexadecimal
>  digit pairs are in order transparency, red, green, and blue. Examples:

Technically the patch is ok.

Bikeshedding on the wording a bit, bitrate is a little far-fetched. It
does reduce used memory bandwidth on the rpi, but not elsewhere without
overlays. Reducing memory consumption is the main point. I'd be happier
with "background-default-bpp" or such choosing between 32 and 16. And a
note that this does not affect Weston's framebuffer depth.


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH] desktop-shell: support an option to use 16 bit color or not, instead of using it by default

2015-05-20 Thread nerdopolis
---
 clients/desktop-shell.c | 13 +++--
 man/weston.ini.man  |  4 
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index e2f9f80..970b9f0 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -94,6 +94,7 @@ struct background {
char *image;
int type;
uint32_t color;
+   int32_t low_bitrate;
 };
 
 struct output {
@@ -1015,10 +1016,18 @@ background_create(struct desktop *desktop)
window_set_user_data(background->window, background);
widget_set_redraw_handler(background->widget, background_draw);
widget_set_transparent(background->widget, 0);
-   window_set_preferred_format(background->window,
-   WINDOW_PREFERRED_FORMAT_RGB565);
 
s = weston_config_get_section(desktop->config, "shell", NULL, NULL);
+   weston_config_section_get_bool(s, "background-low-bitrate",
+&background->low_bitrate, NULL);
+   if (!background->low_bitrate) {
+ window_set_preferred_format(background->window,
+ WINDOW_PREFERRED_FORMAT_NONE);
+   } else {
+ window_set_preferred_format(background->window,
+ WINDOW_PREFERRED_FORMAT_RGB565);
+   }
+
weston_config_section_get_string(s, "background-image",
 &background->image, NULL);
weston_config_section_get_uint(s, "background-color",
diff --git a/man/weston.ini.man b/man/weston.ini.man
index fe86bb6..240fbd8 100644
--- a/man/weston.ini.man
+++ b/man/weston.ini.man
@@ -216,6 +216,10 @@ output. Tile repeats the background image to fill the 
output.
 sets the color of the background (unsigned integer). The hexadecimal
 digit pairs are in order alpha, red, green, and blue.
 .TP 7
+.BI "background-low-bitrate=" true
+specify to reduce the background to 16 bit color (boolean). This option is
+useful for low memory platforms.
+.TP 7
 .BI "panel-color=" 0xAARRGGBB
 sets the color of the panel (unsigned integer). The hexadecimal
 digit pairs are in order transparency, red, green, and blue. Examples:
-- 
2.1.0

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


Re: [PATCH weston 4/4] input: Detect keyboard capabilities

2015-05-20 Thread Jonas Ådahl
On Tue, May 05, 2015 at 03:01:54PM -0500, Derek Foreman wrote:
> Some devices (power buttons, acpi video bus driver) are considered
> keyboards but you can't type with them.
> 
> As of libinput 0.15 we can query a keyboard to see which key events it
> can generate.  We use this to detect if a keyboard can type letters or
> digits.
> 
> The wayland protocol doesn't propagate these capabilites, so
> weston_seat_send_dirty_caps() will differentiate between dirty keyboards
> (which generate the weston signal) and dirty seats (which generate a
> wayland event) and only send the appropriate updates.
> 
> Functions are provided for backends that don't use libinput to force these
> capabilities to a sensible state.
> 
> Signed-off-by: Derek Foreman 
> ---
>  configure.ac  |  2 +-
>  src/compositor-headless.c |  1 +
>  src/compositor-rdp.c  |  1 +
>  src/compositor-wayland.c  |  1 +
>  src/compositor-x11.c  |  1 +
>  src/compositor.h  |  8 
>  src/input.c   | 29 +++--
>  src/libinput-device.c | 32 +++-
>  src/libinput-device.h |  4 +++-
>  src/libinput-seat.c   | 27 +++
>  src/screen-share.c|  1 +
>  11 files changed, 102 insertions(+), 5 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index d9d8d8f..4e9ae20 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -155,7 +155,7 @@ if test x$enable_drm_compositor = xyes; then
>  fi
>  
>  
> -PKG_CHECK_MODULES(LIBINPUT_BACKEND, [libinput >= 0.8.0])
> +PKG_CHECK_MODULES(LIBINPUT_BACKEND, [libinput >= 0.15.0])
>  PKG_CHECK_MODULES(COMPOSITOR, [$COMPOSITOR_MODULES])
>  
>  AC_ARG_ENABLE(wayland-compositor, [  --enable-wayland-compositor],,
> diff --git a/src/compositor-headless.c b/src/compositor-headless.c
> index 0ddb26e..0b461c6 100644
> --- a/src/compositor-headless.c
> +++ b/src/compositor-headless.c
> @@ -184,6 +184,7 @@ headless_input_create(struct headless_compositor *c)
>   if (weston_seat_init_keyboard(&c->fake_seat, NULL) < 0)
>   return -1;
>  
> + weston_seat_override_keyboard_caps(&c->fake_seat, true, true);
>   weston_seat_send_dirty_caps(&c->fake_seat);
>  
>   return 0;
> diff --git a/src/compositor-rdp.c b/src/compositor-rdp.c
> index 5b50382..b6e3876 100644
> --- a/src/compositor-rdp.c
> +++ b/src/compositor-rdp.c
> @@ -842,6 +842,7 @@ xf_peer_post_connect(freerdp_peer* client)
>   }
>   weston_seat_init_keyboard(&peerCtx->item.seat, keymap);
>   weston_seat_init_pointer(&peerCtx->item.seat);
> + weston_seat_override_keyboard_caps(&peerCtx->item.seat, true, true);
>   weston_seat_send_dirty_caps(&peerCtx->item.seat);
>  
>   peerCtx->item.flags |= RDP_PEER_ACTIVATED;
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index 53159c4..1e33697 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -1463,6 +1463,7 @@ input_handle_keymap(void *data, struct wl_keyboard 
> *keyboard, uint32_t format,
>   else
>   weston_seat_init_keyboard(&input->base, keymap);
>  
> + weston_seat_override_keyboard_caps(&input->base, true, true);
>   weston_seat_send_dirty_caps(&input->base);
>  
>   xkb_keymap_unref(keymap);
> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
> index 4a3b10e..69e3a14 100644
> --- a/src/compositor-x11.c
> +++ b/src/compositor-x11.c
> @@ -336,6 +336,7 @@ x11_input_create(struct x11_compositor *c, int no_input)
>   return -1;
>   xkb_keymap_unref(keymap);
>  
> + weston_seat_override_keyboard_caps(&c->core_seat, true, true);
>   weston_seat_send_dirty_caps(&c->core_seat);
>  
>   x11_compositor_setup_xkb(c);
> diff --git a/src/compositor.h b/src/compositor.h
> index e05b262..da6d126 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -451,6 +451,10 @@ weston_touch_start_drag(struct weston_touch *touch,
>   struct wl_client *client);
>  void
>  weston_seat_send_dirty_caps(struct weston_seat *seat);
> +void
> +weston_seat_override_keyboard_caps(struct weston_seat *seat,
> +bool has_letters,
> +bool has_digits);
>  
>  struct weston_xkb_info {
>   struct xkb_keymap *keymap;
> @@ -505,6 +509,10 @@ struct weston_keyboard {
>   enum weston_led leds;
>   } xkb_state;
>   struct xkb_keymap *pending_keymap;
> +
> + bool has_digits;
> + bool has_letters;
> + bool caps_dirty;
>  };
>  
>  struct weston_seat {
> diff --git a/src/input.c b/src/input.c
> index c37bd20..3a0adc3 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -601,14 +601,16 @@ weston_touch_destroy(struct weston_touch *touch)
>  
>  /* Send seat capability updates if necessary
>   *
> - * Checks if the seat capabilities (WL_SEAT_CAPABILITY_*) have changed
> - * and propagates updates appropriately.
> + * Checks if the seat capabilities (WL_SEAT_CAPABILITY_*

Re: [PATCH] drm/plane-helper: Adapt cursor hack to transitional helpers

2015-05-20 Thread Pekka Paalanen
On Wed, 20 May 2015 10:36:32 +0200
Daniel Vetter  wrote:

> In
> 
> commit f02ad907cd9e7fe3a6405d2d005840912f1ed258
> Author: Daniel Vetter 
> Date:   Thu Jan 22 16:36:23 2015 +0100
> 
> drm/atomic-helpers: Recover full cursor plane behaviour
> 
> we've added a hack to atomic helpers to never to vblank waits for
> cursor updates through the legacy apis since that's what X expects.
> Unfortunately we've (again) forgotten to adjust the transitional
> helpers. Do this now.
> 
> This fixes regressions for drivers only partially converted over to
> atomic (like i915).
> 
> Reported-by: Pekka Paalanen 
> Cc: Pekka Paalanen 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_plane_helper.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_plane_helper.c 
> b/drivers/gpu/drm/drm_plane_helper.c
> index 40c1db9ad7c3..2f0ed11024eb 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -465,6 +465,9 @@ int drm_plane_helper_commit(struct drm_plane *plane,
>   if (!crtc[i])
>   continue;
>  
> + if (crtc[i]->cursor == plane)
> + continue;
> +
>   /* There's no other way to figure out whether the crtc is 
> running. */
>   ret = drm_crtc_vblank_get(crtc[i]);
>   if (ret == 0) {

Hi,

just adding more people to CC who might want to test this.

When you test this, please make sure your Weston does *NOT* have this
patch:
http://cgit.freedesktop.org/wayland/weston/commit/?id=6858383d51b12632481370fdc7d886a1e6bb4ebd

That is, use Weston 1.7.92 or earlier.


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH] drm/plane-helper: Adapt cursor hack to transitional helpers

2015-05-20 Thread Daniel Vetter
In

commit f02ad907cd9e7fe3a6405d2d005840912f1ed258
Author: Daniel Vetter 
Date:   Thu Jan 22 16:36:23 2015 +0100

drm/atomic-helpers: Recover full cursor plane behaviour

we've added a hack to atomic helpers to never to vblank waits for
cursor updates through the legacy apis since that's what X expects.
Unfortunately we've (again) forgotten to adjust the transitional
helpers. Do this now.

This fixes regressions for drivers only partially converted over to
atomic (like i915).

Reported-by: Pekka Paalanen 
Cc: Pekka Paalanen 
Cc: sta...@vger.kernel.org
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_plane_helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_plane_helper.c 
b/drivers/gpu/drm/drm_plane_helper.c
index 40c1db9ad7c3..2f0ed11024eb 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -465,6 +465,9 @@ int drm_plane_helper_commit(struct drm_plane *plane,
if (!crtc[i])
continue;
 
+   if (crtc[i]->cursor == plane)
+   continue;
+
/* There's no other way to figure out whether the crtc is 
running. */
ret = drm_crtc_vblank_get(crtc[i]);
if (ret == 0) {
-- 
2.1.4

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


Re: [PATCH weston] compositor-wayland: Code cleanup

2015-05-20 Thread Pekka Paalanen
On Wed, 20 May 2015 01:03:53 -0700
Dima Ryazanov  wrote:

> Don't do multi-assignments.
> 
> Signed-off-by: Dima Ryazanov 
> ---
>  src/compositor-wayland.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index aaf205b..935701a 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -1395,7 +1395,8 @@ input_handle_button(void *data, struct wl_pointer 
> *pointer,
>  
>   if (frame_status(input->output->frame) & FRAME_STATUS_CLOSE) {
>   wayland_output_destroy(&input->output->base);
> - input->output = input->keyboard_focus = NULL;
> + input->output = NULL;
> + input->keyboard_focus = NULL;
>  
>   if (wl_list_empty(&input->compositor->base.output_list))
>   
> wl_display_terminate(input->compositor->base.wl_display);

Pushed.
   01d5c02..b7e70af  master -> master


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston] compositor-wayland: Code cleanup

2015-05-20 Thread Dima Ryazanov
Don't do multi-assignments.

Signed-off-by: Dima Ryazanov 
---
 src/compositor-wayland.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
index aaf205b..935701a 100644
--- a/src/compositor-wayland.c
+++ b/src/compositor-wayland.c
@@ -1395,7 +1395,8 @@ input_handle_button(void *data, struct wl_pointer 
*pointer,
 
if (frame_status(input->output->frame) & FRAME_STATUS_CLOSE) {
wayland_output_destroy(&input->output->base);
-   input->output = input->keyboard_focus = NULL;
+   input->output = NULL;
+   input->keyboard_focus = NULL;
 
if (wl_list_empty(&input->compositor->base.output_list))

wl_display_terminate(input->compositor->base.wl_display);
-- 
2.4.1

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


Re: [PATCH] drm-backend: for now, on the egl backend, force gl cursors to be used instead of hardware cursors

2015-05-20 Thread Daniel Vetter
On Wed, May 20, 2015 at 10:23:19AM +0300, Pekka Paalanen wrote:
> On Wed, 20 May 2015 09:04:52 +0200
> Daniel Vetter  wrote:
> 
> > On Mon, May 18, 2015 at 10:10:27PM -0400, nerdopolis wrote:
> > > Hardware cursors have been causing some problems with some drivers, 
> > > mostly i915
> > 
> > What exactly goes wrong with cursor on i915? Working around kernel bugs in
> > userspace is imo bad since it'll perpetuate the bug ...
> 
> Hi,
> 
> the other Daniel explained to me, and I summarized the discussion in
> the this patch that already landed:
> http://cgit.freedesktop.org/wayland/weston/commit/?id=6858383d51b12632481370fdc7d886a1e6bb4ebd

Ah right seen that one too. Yes if you change the bo all the time the
legacy plane interfaces aren't useful since with the default helpers
there'll be a vblank stall when you switch the bo. X generally doesn't do
that.

For cursors specifically we did add a hack though to cut out all vblank
waits unconditionally, so there shouldn't be a reason to blacklist them.
See

commit f02ad907cd9e7fe3a6405d2d005840912f1ed258
Author: Daniel Vetter 
Date:   Thu Jan 22 16:36:23 2015 +0100

drm/atomic-helpers: Recover full cursor plane behaviour

Cursor plane updates have historically been fully async and mutliple
updates batched together for the next vsync. And userspace relies upon
that. Since implementing a full queue of async atomic updates is a bit
of work lets just recover the cursor specific behaviour with a hint
flag and some hacks to drop the vblank wait.

v2: Fix kerneldoc, reported by Wu Fengguang.

Reviewed-by: Thierry Reding 
Signed-off-by: Daniel Vetter 

And since drivers are switching over to atomic helpers you can even expect
this to be the default behaviour. But ofc it's only for cursors because of
X.

I realized that we've failed to cc: stable this patch, can be fixed if you
want that.
-Daniel

> 
> 
> Cheers,
> pq
> 
> > > 
> > > This will probably be changed once Atomic Mode Setting arrives, to 
> > > probably only force gl cursors
> > > to always be on when Atomic Mode Setting isn't supported by the driver, 
> > > or kernel version
> > > ---
> > >  src/compositor-drm.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > > index 0cdb8f4..0636a37 100644
> > > --- a/src/compositor-drm.c
> > > +++ b/src/compositor-drm.c
> > > @@ -1641,6 +1641,9 @@ drm_output_init_egl(struct drm_output *output, 
> > > struct drm_compositor *ec)
> > >   weston_log("cursor buffers unavailable, using gl cursors\n");
> > >   ec->cursors_are_broken = 1;
> > >   }
> > > + /* TODO Remove when atomic mode setting is merged into the mainline 
> > > kernel, and detect if the
> > > +  * running kernel supports atomic mode setting instead. */
> > > + ec->cursors_are_broken = 1;
> > >  
> > >   return 0;
> > >  }

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] compositor-wayland: Handle window close events more gracefully

2015-05-20 Thread Dima Ryazanov
On Tue, May 19, 2015 at 11:35 PM, Pekka Paalanen 
wrote:

> On Tue, 19 May 2015 16:41:19 -0700
> Bryce Harrington  wrote:
>
> > On Mon, May 18, 2015 at 11:14:16PM -0700, Dima Ryazanov wrote:
> > > When a compositor window is closed, remove the output instead of just
> exiting.
> > >
> > > (The "if (!input->output)" checks are kind of ugly - but I couldn't
> find
> > > a better way to handle the output going away.)
> > >
> > > Signed-off-by: Dima Ryazanov 
> > Reviewed-by: Bryce Harrington 
>
> Acked-by: Pekka Paalanen 
>
> If someone tests this patch (maybe Bryce already did?) that it doesn't
> cause any harm wrt. closing compositor windows in single and multi
> window cases, then I think this is suitable to go in before RC2. After
> RC2 not so much.
>
> > > @@ -1384,8 +1393,15 @@ input_handle_button(void *data, struct
> wl_pointer *pointer,
> > > return;
> > > }
> > >
> > > -   if (frame_status(input->output->frame) &
> FRAME_STATUS_CLOSE)
> > > -
>  wl_display_terminate(input->compositor->base.wl_display);
> > > +   if (frame_status(input->output->frame) &
> FRAME_STATUS_CLOSE) {
> > > +   wayland_output_destroy(&input->output->base);
> > > +   input->output = input->keyboard_focus = NULL;
>
> Please don't do multi-assignments like this, it's too easy to miss one
> of them when reading. Also setting output to keyboard_focus is strange
> to begin with, which is how I'd carelessly read this and go wtf.
>

Haha ok, I'll send out a fix.

Thanks,
> pq
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] compositor-wayland: Handle window close events more gracefully

2015-05-20 Thread Dima Ryazanov
On Wed, May 20, 2015 at 12:00 AM, Bryce Harrington 
wrote:

> On Wed, May 20, 2015 at 09:39:54AM +0300, Pekka Paalanen wrote:
> > On Wed, 20 May 2015 08:34:32 +0200
> > Hardening  wrote:
> >
> > > Le 20/05/2015 01:41, Bryce Harrington a écrit :
> > > > On Mon, May 18, 2015 at 11:14:16PM -0700, Dima Ryazanov wrote:
> > > >> When a compositor window is closed, remove the output instead of
> just exiting.
> > > >>
> > > >> (The "if (!input->output)" checks are kind of ugly - but I couldn't
> find
> > > >> a better way to handle the output going away.)
> > > >>
> > > >> Signed-off-by: Dima Ryazanov 
> > > > Reviewed-by: Bryce Harrington 
> > > >
> > > >> ---
> > > >>  src/compositor-wayland.c | 22 +++---
> > > >>  1 file changed, 19 insertions(+), 3 deletions(-)
> > > >>
> > > [...]
> > >
> > > >>
> > > >>  if (frame_status(input->output->frame) &
> FRAME_STATUS_REPAINT)
> > > >>
> weston_output_schedule_repaint(&input->output->base);
> > > >> @@ -1521,7 +1537,7 @@ input_handle_keyboard_leave(void *data,
> > > >>
> > > >>  focus = input->keyboard_focus;
> > > >>  if (!focus)
> > > >> -return; /* This shouldn't happen */
> > > >> +return;
> > >
> > > Just a remark, if it's something that shouldn't happen it's a failing
> > > assert not a silent return, isn't it ?
> >
> > I'm assuming that after this patch, this actually can legally happen,
> > and in that case we should just ignore it, because we are getting
> > keyboard leave for something we already left by actively destroying it
> > to begin with. Right?
>
> That's how I was understanding it.
>

Yeah, exactly.


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


Re: Landing priorities for 1.8

2015-05-20 Thread Bryce Harrington
With just a few days left until RC2, we're shifting review focus to
bugfixes, tests, documentation, and low-risk changes.

After RC2, we'll be limiting changes to just release critical fixes.

Bryce

On Wed, May 13, 2015 at 02:53:58PM -0700, Bryce Harrington wrote:
> In patchwork, ideally I'd like to see all the patchsets marked 'Under
> Review' dealt with, since those are all ones we postponed for 1.7.  I
> see there's been good progress at processing most of those, and there's
> really just Guulio's libweston patchset and Derek's zoom series that are
> left.
> 
> As to libweston, I imagine it at least needs rebased, and for that
> reason maybe not a candidate for 1.8 inclusion at this point.  But since
> it sounds like we're now generally in consensus that this is a direction
> weston should go, it would probably be worthwhile to identify the next
> steps and a plan for getting it landed as soon as the 1.9 tree opens.
> 
> I'm not sure what the status is with Derek's zoom series but looks like
> it only got some cursory review back in January.  The code looks
> straightforward enough, and the patches are all pretty short and
> concise; is there anything of concern conceptually regarding the
> functionality itself?  Essentially it's just fixing a crash condition.
> Mightn't we just land it?
> 
> Beyond that, there's a number of one-off patches, many of which are
> legitimate bug fixes that would be nice to have in the release.
> Particularly all the input fixups.
> 
> I don't know that I'll have tons of time to do in depth reviews myself
> next week, but I will try and land things that get R-B's by others.  And
> I know reviewing can be kind of a thankless job... I'll try and focus my
> review time to stuff from people who are themselves doing review work.
> 
> Bryce
> ___
> 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] drm-backend: for now, on the egl backend, force gl cursors to be used instead of hardware cursors

2015-05-20 Thread Pekka Paalanen
On Wed, 20 May 2015 09:04:52 +0200
Daniel Vetter  wrote:

> On Mon, May 18, 2015 at 10:10:27PM -0400, nerdopolis wrote:
> > Hardware cursors have been causing some problems with some drivers, mostly 
> > i915
> 
> What exactly goes wrong with cursor on i915? Working around kernel bugs in
> userspace is imo bad since it'll perpetuate the bug ...

Hi,

the other Daniel explained to me, and I summarized the discussion in
the this patch that already landed:
http://cgit.freedesktop.org/wayland/weston/commit/?id=6858383d51b12632481370fdc7d886a1e6bb4ebd


Cheers,
pq

> > 
> > This will probably be changed once Atomic Mode Setting arrives, to probably 
> > only force gl cursors
> > to always be on when Atomic Mode Setting isn't supported by the driver, or 
> > kernel version
> > ---
> >  src/compositor-drm.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > index 0cdb8f4..0636a37 100644
> > --- a/src/compositor-drm.c
> > +++ b/src/compositor-drm.c
> > @@ -1641,6 +1641,9 @@ drm_output_init_egl(struct drm_output *output, struct 
> > drm_compositor *ec)
> > weston_log("cursor buffers unavailable, using gl cursors\n");
> > ec->cursors_are_broken = 1;
> > }
> > +   /* TODO Remove when atomic mode setting is merged into the mainline 
> > kernel, and detect if the
> > +* running kernel supports atomic mode setting instead. */
> > +   ec->cursors_are_broken = 1;
> >  
> > return 0;
> >  }
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] drm-backend: for now, on the egl backend, force gl cursors to be used instead of hardware cursors

2015-05-20 Thread Daniel Vetter
On Mon, May 18, 2015 at 10:10:27PM -0400, nerdopolis wrote:
> Hardware cursors have been causing some problems with some drivers, mostly 
> i915

What exactly goes wrong with cursor on i915? Working around kernel bugs in
userspace is imo bad since it'll perpetuate the bug ...
-Daniel

> 
> This will probably be changed once Atomic Mode Setting arrives, to probably 
> only force gl cursors
> to always be on when Atomic Mode Setting isn't supported by the driver, or 
> kernel version
> ---
>  src/compositor-drm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 0cdb8f4..0636a37 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -1641,6 +1641,9 @@ drm_output_init_egl(struct drm_output *output, struct 
> drm_compositor *ec)
>   weston_log("cursor buffers unavailable, using gl cursors\n");
>   ec->cursors_are_broken = 1;
>   }
> + /* TODO Remove when atomic mode setting is merged into the mainline 
> kernel, and detect if the
> +  * running kernel supports atomic mode setting instead. */
> + ec->cursors_are_broken = 1;
>  
>   return 0;
>  }
> -- 
> 2.1.0
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] compositor-wayland: Handle window close events more gracefully

2015-05-20 Thread Bryce Harrington
On Wed, May 20, 2015 at 09:39:54AM +0300, Pekka Paalanen wrote:
> On Wed, 20 May 2015 08:34:32 +0200
> Hardening  wrote:
> 
> > Le 20/05/2015 01:41, Bryce Harrington a écrit :
> > > On Mon, May 18, 2015 at 11:14:16PM -0700, Dima Ryazanov wrote:
> > >> When a compositor window is closed, remove the output instead of just 
> > >> exiting.
> > >>
> > >> (The "if (!input->output)" checks are kind of ugly - but I couldn't find
> > >> a better way to handle the output going away.)
> > >>
> > >> Signed-off-by: Dima Ryazanov 
> > > Reviewed-by: Bryce Harrington 
> > > 
> > >> ---
> > >>  src/compositor-wayland.c | 22 +++---
> > >>  1 file changed, 19 insertions(+), 3 deletions(-)
> > >>
> > [...]
> > 
> > >>  
> > >>  if (frame_status(input->output->frame) & 
> > >> FRAME_STATUS_REPAINT)
> > >>  
> > >> weston_output_schedule_repaint(&input->output->base);
> > >> @@ -1521,7 +1537,7 @@ input_handle_keyboard_leave(void *data,
> > >>  
> > >>  focus = input->keyboard_focus;
> > >>  if (!focus)
> > >> -return; /* This shouldn't happen */
> > >> +return;
> > 
> > Just a remark, if it's something that shouldn't happen it's a failing
> > assert not a silent return, isn't it ?
> 
> I'm assuming that after this patch, this actually can legally happen,
> and in that case we should just ignore it, because we are getting
> keyboard leave for something we already left by actively destroying it
> to begin with. Right?

That's how I was understanding it.

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