date:20140612

Re: [PATCH] shell: Destroy the shell_surface when the xdg_shell interface gets destroyed

2014-06-12 Thread Sjoerd Simons

On Wed, 2014-06-04 at 13:44 +0200, Sjoerd Simons wrote:
 When running gtk3-demo under weston opening comboboxes a second time
 causes the program to bail due to weston returning an error. The
 relevant client trace in this case is:
  - xdg_shell@15.get_xdg_popup(new id xdg_popup@12, wl_surface@28, 
  - xdg_popup@12.destroy()
  - xdg_shell@15.get_xdg_popup(new id xdg_popup@19, wl_surface@28, 
   wl_display@1.error(wl_surface@28, 0,
 xdg_shell::get_xdg_popup already requested)
 
 However, according to the xdg_shell protocol this should be valid as
 calling destroy should remove the xdg_popup interface from the
 wl_surface object.
 
 Fix this by ensuring the internal shell surface also gets destroyed when
 the relevant xdg interface is destroyed and not just when the underlying
 wl_surface gets destroyed.
 
 This patch applies both to Weston 1.5 and master, please apply to both
 if correct.

Turns out this patch is, in fact, not correct. The desktop-shell code
seems to have a bigger assumption on the shell surface having a lifetime
bound to the underlying wl_surface then i first thought. The patch
causes weston to crash if the fade-out animation is running when the xdg
shell surface is destroyed.

One solution here is to rework the code so the shell surface can be
destroyed  recreated seperately. Another one would be to get xdg_shell
more in line with wl_shell when it comes to life-time rules. 

I've put together an initial patch for GTK+ such that it doesn't try to
re-create xdg_shell surfaces anymore and essentially assumes the xdg
surfaces are once-only per wl_surface:
  https://bugzilla.gnome.org/show_bug.cgi?id=727021#c3



 ---
  desktop-shell/shell.c | 2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
 index 46c6e18..45a3321 100644
 --- a/desktop-shell/shell.c
 +++ b/desktop-shell/shell.c
 @@ -3153,6 +3153,7 @@ destroy_shell_surface(struct shell_surface *shsurf)
   /* As destroy_resource() use wl_list_for_each_safe(),
* we can always remove the listener.
*/
 + wl_list_remove(shsurf-resource_destroy_listener.link);
   wl_list_remove(shsurf-surface_destroy_listener.link);
   shsurf-surface-configure = NULL;
   free(shsurf-title);
 @@ -3175,6 +3176,7 @@ shell_destroy_shell_surface(struct wl_resource 
 *resource)
   if (!wl_list_empty(shsurf-popup.grab_link))
   remove_popup_grab(shsurf);
   shsurf-resource = NULL;
 + destroy_shell_surface(shsurf);
  }
  
  static void




smime.p7s
Description: S/MIME cryptographic signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Wayland generic dmabuf protocol

2014-06-12 Thread Rob Clark

On Wed, Jun 11, 2014 at 2:26 AM, Thomas Hellstrom thellst...@vmware.com wrote:
On 06/09/2014 01:23 PM, Daniel Stone wrote:
Hi,

On 9 June 2014 12:06, Pekka Paalanen pekka.paala...@collabora.co.uk
mailto:pekka.paala...@collabora.co.uk wrote:

On Mon, 9 Jun 2014 11:00:04 +0200
Benjamin Gaignard benjamin.gaign...@linaro.org
mailto:benjamin.gaign...@linaro.org wrote:
One of the main comment on the latest patches was that wl_dmabuf use
DRM for buffer allocation.
This appear to be an issue since wayland doesn't want to rely on one
specific framework (DRM, or V4L2) for buffer allocation, so we have
start working on a central dmabuf allocation on kernel side. The
goal is provide some as generic as possible to make it acceptable by
wayland.

Why would Wayland need a central allocator for dmabuf?

I think you've just answered your own question further below:

On my hardware the patches you have (+ this one on gstwaylandsink
https://bugzilla.gnome.org/show_bug.cgi?id=711155

https://urldefense.proofpoint.com/v1/url?u=https://bugzilla.gnome.org/show_bug.cgi?id%3D711155k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0Ar=l5Ago9ekmVFZ3c4M6eauqrJWGwjf6fTb%2BP3CxbBFkVM%3D%0Am=NHSSDNdXVt2oCxexwhcqQAQSZ3K6Xa8Zvuv0jMYfS2c%3D%0As=f6d8c24d7fa4ccdacae79aeaa8221773d892350a191f96cb3bab4f542fb3ad1c)
allow me to do zero
copy between the hardware video decoder and the display engine. I
don't have implemented GPU yet because my hardware is able to do
compose few video overlays planes and it was enough for my tests.

Right.

What I have been thinking is, that the compositor must be able to use
the new wl_buffer and we need to guarantee that before-hand. If the
compositor fails to use a wl_buffer when the client has already
attached it to a wl_surface and it is time to repaint, it is too late
and the user will see a glitch. Recovering from that requires asking
the client to provide a new wl_buffer of a different kind, which might
take time. Or a very rude compositor would just send a protocol error,
and then we'd get bug reports like the video player just disappears
when I try to play (and ps. I have an old kernel that doesn't support
importing whatever).

I believe we must allow the compositor to test the wl_buffer before it
is usable for the client. That is the reason for the roundtrippy
design
of the below proposal.

A central allocator would solve these issues, by having everyone agree
on the restrictions upfront, instead of working out which of the media
decode engine, camera, GPU, or display controller is the lowest common
denominator, and forcing all allocations through there.

One such solution was discussed a while back WRT ION:
https://lwn.net/Articles/565469/
https://urldefense.proofpoint.com/v1/url?u=https://lwn.net/Articles/565469/k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0Ar=l5Ago9ekmVFZ3c4M6eauqrJWGwjf6fTb%2BP3CxbBFkVM%3D%0Am=NHSSDNdXVt2oCxexwhcqQAQSZ3K6Xa8Zvuv0jMYfS2c%3D%0As=36b71b58f7fc6f3d594529cc674364a23718e5cbb38a439e4a9c628848a13b3a

See the 'possible solutions' part for a way for people to agree on
restrictions wrt tiling, stride, contiguousness, etc.

Hi!

I think before deciding on something like this, one needs also to
account for the virtual drivers like vmwgfx.

Here, a dma-buf internally holds an opaque handle to an object on the
host / hypervisor, and the actual memory buffer is only temporarily
allocated for dma-buf operations that strictly need it. Not to hold the
data while transferring it between devices or applications.

Let's say you'd want to use a USB display controller in a virtual
machine with the vmwgfx exported prime objects, for example. There's no
common denominator. The vmwgfx driver would need to read the dma-buf
data from the host object at sg-table export (dma-buf map) time.

Whereas if you just want to share data between a wayland server and
client, no pages are ever allocated and the only thing passed
around is in effect the opaque handle to the host / hypervisor object.

I'm currently having trouble seeing how a central allocator would be
able to deal with this?

well, central standalone allocator device would need to know about all
possible devices. Works ok in a cell phone. I don't really see it
scaling well to desktop, so it isn't really an option.

BR,
-R

/Thomas

Cheers,
Daniel

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/wayland-develk=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0Ar=l5Ago9ekmVFZ3c4M6eauqrJWGwjf6fTb%2BP3CxbBFkVM%3D%0Am=NHSSDNdXVt2oCxexwhcqQAQSZ3K6Xa8Zvuv0jMYfS2c%3D%0As=c4f94901274570868164b5aac297336d544bdba1bc12bda6c09b915fa5495423
___
wayland-devel mailing list

[PATCH libinput 1/3] test: move the interface declaration down

2014-06-12 Thread Peter Hutterer

No functional changes, just some prep work.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 test/litest.c | 35 +--
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/test/litest.c b/test/litest.c
index 0a9cc72..d3f8f0d 100644
--- a/test/litest.c
+++ b/test/litest.c
@@ -267,6 +267,23 @@ litest_log_handler(enum libinput_log_priority pri,
vfprintf(stderr, format, args);
 }
 
+static int
+open_restricted(const char *path, int flags, void *userdata)
+{
+   return open(path, flags);
+}
+
+static void
+close_restricted(int fd, void *userdata)
+{
+   close(fd);
+}
+
+struct libinput_interface interface = {
+   .open_restricted = open_restricted,
+   .close_restricted = close_restricted,
+};
+
 static const struct option opts[] = {
{ list, 0, 0, 'l' },
{ verbose, 0, 0, 'v' },
@@ -335,24 +352,6 @@ litest_run(int argc, char **argv) {
return failed;
 }
 
-static int
-open_restricted(const char *path, int flags, void *userdata)
-{
-   return open(path, flags);
-}
-
-static void
-close_restricted(int fd, void *userdata)
-{
-   close(fd);
-}
-
-const struct libinput_interface interface = {
-   .open_restricted = open_restricted,
-   .close_restricted = close_restricted,
-};
-
-
 static struct input_absinfo *
 merge_absinfo(const struct input_absinfo *orig,
  const struct input_absinfo *override)
-- 
1.9.3

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

[PATCH libinput 3/3] Change the logging system to be per-context

2014-06-12 Thread Peter Hutterer

Rather than a single global logging function, make the logging dependent on
the individual context. This way we won't stomp on each other's feet in the
(admittedly unusual) case of having multiple libinput contexts.

The log handler and the log priority is now a part of the libinput interface.
We can drop the various setters and getters, the caller owns the struct anyway
so we don't need functions to give it those values.

The userdata argument to the log handler was dropped. The caller has a ref to
the libinput context now, any userdata can be attached to that context
instead.

There is no need for a default log function anymore. Any serious caller should
hook into it anyway, those that don't care can just use NULL.

There is no default log priority anymore, a caller must set the desired
priority in the interface.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
There's a side-effect to this that I'm not sure is intended. We don't copy
the interface into libinput, we merely keep a reference. The caller is
already able to change open_restricted/close_restricted at runtime, though
we can't do this ourselves (it's const).

Given that, I figured we can leave the log handler and priority up to the
caller as well then, switching at runtime. That's the main reason for
dropping the set/get priority calls. If that side effect wasn't intended,
then we'll have rework a few things. Jonas?

 src/evdev-mt-touchpad-buttons.c |  15 +++--
 src/evdev-mt-touchpad-tap.c |  13 -
 src/evdev.c |  23 +---
 src/libinput-private.h  |  20 ---
 src/libinput.c  |  73 ++-
 src/libinput.h  |  99 +--
 src/path.c  |  20 +--
 src/timer.c |   4 +-
 src/udev-seat.c |  17 +++---
 test/litest.c   |   9 +--
 test/log.c  | 126 ++--
 test/misc.c |   2 +
 test/path.c |   2 +
 test/udev.c |   7 ++-
 tools/event-debug.c |  28 -
 15 files changed, 194 insertions(+), 264 deletions(-)

diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
index ce48ed0..b86f344 100644
--- a/src/evdev-mt-touchpad-buttons.c
+++ b/src/evdev-mt-touchpad-buttons.c
@@ -452,6 +452,7 @@ tp_button_handle_event(struct tp_dispatch *tp,
   enum button_event event,
   uint64_t time)
 {
+   struct libinput *libinput = tp-device-base.seat-libinput;
enum button_state current = t-button.state;
 
switch(t-button.state) {
@@ -485,7 +486,8 @@ tp_button_handle_event(struct tp_dispatch *tp,
}
 
if (current != t-button.state)
-   log_debug(button state: from %s, event %s to %s\n,
+   log_debug(libinput,
+ button state: from %s, event %s to %s\n,
  button_state_to_str(current),
  button_event_to_str(event),
  button_state_to_str(t-button.state));
@@ -538,11 +540,13 @@ tp_process_button(struct tp_dispatch *tp,
  const struct input_event *e,
  uint64_t time)
 {
+   struct libinput *libinput = tp-device-base.seat-libinput;
uint32_t mask = 1  (e-code - BTN_LEFT);
 
/* Ignore other buttons on clickpads */
if (tp-buttons.is_clickpad  e-code != BTN_LEFT) {
-   log_bug_kernel(received %s button event on a clickpad\n,
+   log_bug_kernel(libinput,
+  received %s button event on a clickpad\n,
   libevdev_event_code_get_name(EV_KEY, e-code));
return 0;
}
@@ -562,6 +566,7 @@ int
 tp_init_buttons(struct tp_dispatch *tp,
struct evdev_device *device)
 {
+   struct libinput *libinput = tp-device-base.seat-libinput;
struct tp_touch *t;
int width, height;
double diagonal;
@@ -574,10 +579,12 @@ tp_init_buttons(struct tp_dispatch *tp,
if (libevdev_has_event_code(device-evdev, EV_KEY, BTN_MIDDLE) ||
libevdev_has_event_code(device-evdev, EV_KEY, BTN_RIGHT)) {
if (tp-buttons.is_clickpad)
-   log_bug_kernel(clickpad advertising right button\n);
+   log_bug_kernel(libinput,
+  clickpad advertising right button\n);
} else {
if (!tp-buttons.is_clickpad)
-   log_bug_kernel(non clickpad without right button?\n);
+   log_bug_kernel(libinput,
+  non clickpad without right button?\n);
}
 
width = abs(device-abs.max_x - device-abs.min_x);
diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index 34bb0d0..2541218 100644

[PATCH libinput 2/3] udev: split libinput_udev context init into two functions

2014-06-12 Thread Peter Hutterer

This is preparation work for context-specific log handlers.

Callers are now encouraged to first initialize the context with
libinput_udev_create_context() and then set the seat for this context with
libinput_udev_set_seat().

In the upcoming patch to support context-specific log handlers this enables a
caller to set the log handler for a context before any devices are
initialized. Otherwise, a log message generated by a new device may pass a
libinput context that the caller is not yet aware of.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
 doc/libinput.doxygen.in |  3 ++-
 src/libinput.h  | 49 +++-
 src/udev-seat.c | 54 +
 test/udev.c | 48 ---
 tools/event-debug.c |  8 +++-
 5 files changed, 130 insertions(+), 32 deletions(-)

diff --git a/doc/libinput.doxygen.in b/doc/libinput.doxygen.in
index f78b7cf..9f931c3 100644
--- a/doc/libinput.doxygen.in
+++ b/doc/libinput.doxygen.in
@@ -1570,7 +1570,8 @@ INCLUDE_FILE_PATTERNS  =
 # undefined via #undef or recursively expanded use the := operator
 # instead of the = operator.
 
-PREDEFINED = LIBINPUT_ATTRIBUTE_PRINTF(f,a)=
+PREDEFINED = LIBINPUT_ATTRIBUTE_PRINTF(f, a)= \
+LIBINPUT_ATTRIBUTE_DEPRECATED
 
 # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then
 # this tag can be used to specify a list of macro names that should be 
expanded.
diff --git a/src/libinput.h b/src/libinput.h
index 54c96e5..4501c66 100644
--- a/src/libinput.h
+++ b/src/libinput.h
@@ -33,6 +33,7 @@ extern C {
 
 #define LIBINPUT_ATTRIBUTE_PRINTF(_format, _args) \
__attribute__ ((format (printf, _format, _args)))
+#define LIBINPUT_ATTRIBUTE_DEPRECATED __attribute__ ((deprecated))
 
 /**
  * @mainpage
@@ -784,6 +785,47 @@ struct libinput_interface {
 /**
  * @ingroup base
  *
+ * Create a new libinput context from udev. This context is inactive until
+ * assigned a seat ID with libinput_udev_set_seat().
+ *
+ * @param interface The callback interface
+ * @param user_data Caller-specific data passed to the various callback
+ * interfaces.
+ * @param udev An already initialized udev context
+ *
+ * @return An initialized, but inactive libinput context or NULL on error
+ */
+struct libinput *
+libinput_udev_create_context(const struct libinput_interface *interface,
+void *user_data,
+struct udev *udev);
+
+/**
+ * @ingroup base
+ *
+ * Assign a seat to this libinput context. New devices or the removal of
+ * existing devices will appear as events during libinput_dispatch().
+ *
+ * libinput_udev_set_seat() succeeds even if no input devices are currently
+ * available on this seat, or if devices are available but fail to open in
+ * @ref libinput_interface::open_restricted. Devices that do not have the
+ * minimum capabilities to be recognized as pointer, keyboard or touch
+ * device are ignored. Such devices and those that failed to open
+ * ignored until the next call to libinput_resume().
+ *
+ * @param libinput A libinput context initialized with
+ * libinput_udev_create_context()
+ * @param seat_id A seat identifier. This string must not be NULL.
+ *
+ * @return 0 on success or -1 on failure.
+ */
+int
+libinput_udev_set_seat(struct libinput *libinput,
+  const char *seat_id);
+
+/**
+ * @ingroup base
+ *
  * Create a new libinput context from udev, for input devices matching
  * the given seat ID. New devices or devices removed will appear as events
  * during libinput_dispatch.
@@ -803,12 +845,17 @@ struct libinput_interface {
  *
  * @return An initialized libinput context, ready to handle events or NULL on
  * error.
+ *
+ * @deprecated This function was deprecated in 0.4.0 and will be removed
+ * soon. Use libinput_udev_create_context() and libinput_udev_set_seat()
+ * instead.
  */
 struct libinput *
 libinput_udev_create_for_seat(const struct libinput_interface *interface,
  void *user_data,
  struct udev *udev,
- const char *seat_id);
+ const char *seat_id)
+   LIBINPUT_ATTRIBUTE_DEPRECATED;
 
 /**
  * @ingroup base
diff --git a/src/udev-seat.c b/src/udev-seat.c
index 38a13b7..1e1307b 100644
--- a/src/udev-seat.c
+++ b/src/udev-seat.c
@@ -333,14 +333,13 @@ static const struct libinput_interface_backend 
interface_backend = {
 };
 
 LIBINPUT_EXPORT struct libinput *
-libinput_udev_create_for_seat(const struct libinput_interface *interface,
- void *user_data,
- struct udev *udev,
- const char *seat_id)
+libinput_udev_create_context(const struct libinput_interface *interface,
+void *user_data,
+struct udev

[PATCH libinput 0/3] Per-context log handlers

2014-06-12 Thread Peter Hutterer


This work was motivated by libevdev, which too started out with a global log
   
handler, and that's biting us now. libevdev is lower-level than libinput, so
   
there's a higher chance of two unrelated bits loading libevdev into the same
   
process. Nonetheless, having a global log handler in a library makes me a   
   
bit nervous now.
   

   
This patchset changes over to a per-context log handler and that breaks the 
   
API in a couple of ways:
   
* the libinput context is now passed into the log function  
   
* the user_data pointer was dropped, use the context's user data if needed  
   
* the log handler/priority is now part of the libinput_interface and managed
   
  by the caller, not by libinput (hence all the libinput_set/get_log*   
   
  functions are gone)   
   
* the udev context creation was split up so we can give the client a
   
  context before calling a log function that uses that context. I've left   
   
  the old function there for now for some backwards compatibility but tbh   
   
  given that everything else changes I think we should just drop
   
  libinput_udev_create_for_seat().  
   

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

27 matches

Mail list logo