[PATCH libinput] touchpad: only use the last two coordinates for delta calculation

[Peter Hutterer]

Taking the last 4 points means factoring in a coordinate that may be more than
40ms in the past - or even more when the finger moves slowly and we don't get
events for a while. This makes the pointer more sluggish and slower to catch up
with what the finger is actually doing.

We already have the motion hysteresis as a separate item to prevent jumps (and
thus adds some delay to the movement), the calculation over time doesn't
provide enough benefit to justify the sluggish pointer.

Signed-off-by: Peter Hutterer 
---
I'm leaving the motion history as-is for now even though it's largely unused
now. This can be fixed up later once we know this patch has the desired
effect (it does here, but that could be confirmation bias)

 src/evdev-mt-touchpad.c | 20 +---
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index d72cb19..7b8514c 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -262,29 +262,19 @@ tp_end_sequence(struct tp_dispatch *tp, struct tp_touch 
*t, uint64_t time)
tp_end_touch(tp, t, time);
 }
 
-static double
-tp_estimate_delta(int x0, int x1, int x2, int x3)
-{
-   return (x0 + x1 - x2 - x3) / 4.0;
-}
-
 struct normalized_coords
 tp_get_delta(struct tp_touch *t)
 {
struct device_float_coords delta;
const struct normalized_coords zero = { 0.0, 0.0 };
 
-   if (t->history.count < TOUCHPAD_MIN_SAMPLES)
+   if (t->history.count <= 1)
return zero;
 
-   delta.x = tp_estimate_delta(tp_motion_history_offset(t, 0)->x,
-   tp_motion_history_offset(t, 1)->x,
-   tp_motion_history_offset(t, 2)->x,
-   tp_motion_history_offset(t, 3)->x);
-   delta.y = tp_estimate_delta(tp_motion_history_offset(t, 0)->y,
-   tp_motion_history_offset(t, 1)->y,
-   tp_motion_history_offset(t, 2)->y,
-   tp_motion_history_offset(t, 3)->y);
+   delta.x = tp_motion_history_offset(t, 0)->x -
+ tp_motion_history_offset(t, 1)->x;
+   delta.y = tp_motion_history_offset(t, 0)->y -
+ tp_motion_history_offset(t, 1)->y;
 
return tp_normalize_delta(t->tp, delta);
 }
-- 
2.9.3

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

[PATCH libinput 3/4] test: allow the first event to be a short one during scroll tests

[Peter Hutterer]

The hysteresis cuts the first pointer motion by the hysteresis margin. On some
touchpads this causes the tests to fail when the motion history length is
reduced (future patch). Allow the first event to be smaller than the expected
minimum.

This doesn't trigger in current tests because the hysteresis is per-event and
by the time we get past the minimum 4 events to move the pointer, we're
already flying unaffected by the hysteresis.

Signed-off-by: Peter Hutterer 
---
 test/litest.c | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/test/litest.c b/test/litest.c
index e685d61..b74f905 100644
--- a/test/litest.c
+++ b/test/litest.c
@@ -2806,6 +2806,7 @@ litest_assert_scroll(struct libinput *li,
struct libinput_event *event, *next_event;
struct libinput_event_pointer *ptrev;
int value;
+   int nevents = 0;
 
event = libinput_get_event(li);
next_event = libinput_get_event(li);
@@ -2813,16 +2814,26 @@ litest_assert_scroll(struct libinput *li,
 
while (event) {
ptrev = litest_is_axis_event(event, axis, 0);
+   nevents++;
 
if (next_event) {
+   int min = minimum_movement;
+
value = libinput_event_pointer_get_axis_value(ptrev,
  axis);
+   /* Due to how the hysteresis works on touchpad
+* events, the first event is reduced by the
+* hysteresis margin that can cause the first event
+* go under the minimum we expect for all other
+* events */
+   if (nevents == 1)
+   min = minimum_movement/2;
+
/* Normal scroll event, check dir */
-   if (minimum_movement > 0) {
-   litest_assert_int_ge(value, minimum_movement);
-   } else {
-   litest_assert_int_le(value, minimum_movement);
-   }
+   if (minimum_movement > 0)
+   litest_assert_int_ge(value, min);
+   else
+   litest_assert_int_le(value, min);
} else {
/* Last scroll event, must be 0 */
ck_assert_double_eq(
-- 
2.9.3

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

[PATCH libinput 2/4] test: fix edge-scroll no-motion test

[Peter Hutterer]

The test is supposed to make sure no motion event is sent and that scrolling
continues once leaving the edge. It does so by moving down the edge, into the
touchpad, then down further. The move from the edge into the touchpad had a
vertical component to it though and could cause the scroll minimum test to
fail. This is currently covered up by the delta calculations though, but fix
it anyway.

Signed-off-by: Peter Hutterer 
---
 test/touchpad.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/touchpad.c b/test/touchpad.c
index 9a4aa6c..cdc261b 100644
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -664,9 +664,9 @@ START_TEST(touchpad_edge_scroll_no_motion)
litest_touch_down(dev, 0, 99, 10);
litest_touch_move_to(dev, 0, 99, 10, 99, 70, 12, 0);
/* moving outside -> no motion event */
-   litest_touch_move_to(dev, 0, 99, 70, 20, 80, 12, 0);
+   litest_touch_move_to(dev, 0, 99, 70, 20, 70, 12, 0);
/* moving down outside edge once scrolling had started -> scroll */
-   litest_touch_move_to(dev, 0, 20, 80, 40, 99, 12, 0);
+   litest_touch_move_to(dev, 0, 20, 70, 40, 99, 12, 0);
litest_touch_up(dev, 0);
libinput_dispatch(li);
 
-- 
2.9.3

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

[PATCH libinput 1/4] test: start with the first offset when moving touches

[Peter Hutterer]

This doesn't have an effect in our current tests because the touchpad always
needs 4 motion events to get moving. But for the future, it simplifies the
case of "i want to move between x1/y1 and x2/y2", because it fills in only the
events in between rather than re-using the touch down coordinates and thus not
causing a motion on the first event.

Signed-off-by: Peter Hutterer 
---
 test/litest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/litest.c b/test/litest.c
index 94da85a..e685d61 100644
--- a/test/litest.c
+++ b/test/litest.c
@@ -1603,7 +1603,7 @@ litest_touch_move_to(struct litest_device *d,
 double x_to, double y_to,
 int steps, int sleep_ms)
 {
-   for (int i = 0; i < steps - 1; i++) {
+   for (int i = 1; i < steps - 1; i++) {
litest_touch_move(d, slot,
  x_from + (x_to - x_from)/steps * i,
  y_from + (y_to - y_from)/steps * i);
@@ -1699,7 +1699,7 @@ litest_touch_move_two_touches(struct litest_device *d,
  double dx, double dy,
  int steps, int sleep_ms)
 {
-   for (int i = 0; i < steps - 1; i++) {
+   for (int i = 1; i < steps; i++) {
litest_push_event_frame(d);
litest_touch_move(d, 0, x0 + dx / steps * i,
y0 + dy / steps * i);
-- 
2.9.3

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

[PATCH libinput 4/4] evdev: simplify hysteresis code and document it

[Peter Hutterer]

center + diff is the input coordinate. Simplify the code so it's clear what
we're returning. And document the function to explain what it does.

Signed-off-by: Peter Hutterer 
---
 src/evdev.h | 38 +++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/src/evdev.h b/src/evdev.h
index 888cc28..0888600 100644
--- a/src/evdev.h
+++ b/src/evdev.h
@@ -530,6 +530,38 @@ evdev_to_left_handed(struct evdev_device *device,
return button;
 }
 
+/**
+ * Apply a hysteresis filtering to the coordinate in, based on the current
+ * hystersis center and the margin. If 'in' is within 'margin' of center,
+ * return the center (and thus filter the motion). If 'in' is outside,
+ * return a point on the edge of the new margin. So for a point x in the
+ * space outside c + margin we return r:
+ * +---+   +---+
+ * | c |  x →  | r x
+ * +---+   +---+
+ *
+ * The effect of this is that initial small motions are filtered. Once we
+ * move into one direction we lag the real coordinates by 'margin' but any
+ * movement that continues into that direction will always be just outside
+ * margin - we get responsive movement. Once we move back into the other
+ * direction, the first movements are filtered again.
+ *
+ * Returning the edge rather than the point avoids cursor jumps, as the
+ * first reachable coordinate is the point next to the center (center + 1).
+ * Otherwise, the center has a dead zone of size margin around it and the
+ * first reachable point is the margin edge.
+ *
+ * Hysteresis is handled separately per axis (and the window is thus
+ * rectangular, not circular). It is unkown if that's an issue, but the
+ * calculation to do circular hysteresis are nontrivial, especially since
+ * many touchpads have uneven x/y resolutions.
+ *
+ * @param in The input coordinate
+ * @param center Current center of the hysteresis
+ * @param margin Hysteresis width (on each side)
+ *
+ * @return The new center of the hysteresis
+ */
 static inline int
 evdev_hysteresis(int in, int center, int margin)
 {
@@ -537,10 +569,10 @@ evdev_hysteresis(int in, int center, int margin)
if (abs(diff) <= margin)
return center;
 
-   if (diff > margin)
-   return center + diff - margin;
+   if (diff > 0)
+   return in - margin;
else
-   return center + diff + margin;
+   return in + margin;
 }
 
 static inline struct libinput *
-- 
2.9.3

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

Re: [RFC v2 wayland-protocols] tablet: define our own enum for tablet tool buttons

[Peter Hutterer]

On Mon, Nov 21, 2016 at 12:42:36PM +, Daniel Stone wrote:
> Hi,
> 
> On 20 November 2016 at 05:14, Peter Hutterer  wrote:
> > Rather than relying on input-event-codes, define our own enum that is 
> > tailored
> > towards the tablet interface.
> >
> > Signed-off-by: Peter Hutterer 
> > ---
> > Because it's usually easier to pick holes into a patch proposal than come up
> > with ideas elsewhere, here's a quick-and-dirty patch.
> >
> > Advantage: we control the button names/numbers and clients don't have to
> > know about cases where linux/input.h isn't enough.
> > Obvious drawback: adding new buttons requires a new protocol. Given this
> > hardware hasn't really changed much in quite a while, this may not be much
> > of an issue.
> 
> Conceptually, I don't see why not.
> 
> > @@ -539,6 +539,26 @@
> >
> >  
> >
> > +
> > +  
> > +   Describes the physical button that produced the button event.
> > +  
> > +  
> > +  
> > +  
> > +  
> > +  
> > +  
> > +  
> > +  
> > +  
> > +  
> > +  
> > +  
> > +  
> > +  
> > +
> 
> Concretely though, reusing BTN_* codes where possible would make it
> easier for clients to transition between the two. 

I disagree here. The kernel only has BTN_STYLUS and BTN_STYLUS2, after that
we overlap with DOUBLETAP range and later buttons that are completely
different (e.g. BTN_GEAR_DOWN). I think this would only make it worse.
This protocol is still unstable, every client needs updates once we mark it
stable anyway, making the enums *values* mean something is counterproductive
IMO.

> Also, 9 stylus buttons seems like an oddly specific number.

was supposed to be 10 [0-9], I just have an index offset error here, sorry :)

Cheers,
   Peter

> Anyway, if you switch these values to the current BTN_* equivalents:
> Acked-by: Daniel Stone 
> 
> Cheers,
> Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [weston 1/2] linux-dmabuf: implement immediate dmabuf import

[Daniel Stone]

Hi Varad,

On 11 November 2016 at 11:40, Varad Gautam  wrote:
> handle create_immed() dmabuf import requests and support
> zwp_linux_dmabuf_v1_interface version 2.

Same caveat about holding off on merging applies, but these two patches are:
Reviewed-by: Daniel Stone 

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

[PATCH weston v3 1/2] editor: Use parse_options() from shared for command line options

[Bryce Harrington]

Also add a basic --help option

Signed-off-by: Bryce Harrington 
---
 clients/editor.c | 80 
 1 file changed, 58 insertions(+), 22 deletions(-)

diff --git a/clients/editor.c b/clients/editor.c
index 30bf555..b8fc63a 100644
--- a/clients/editor.c
+++ b/clients/editor.c
@@ -37,6 +37,7 @@
 
 #include 
 
+#include "shared/config-parser.h"
 #include "shared/helpers.h"
 #include "shared/xalloc.h"
 #include "window.h"
@@ -1489,28 +1490,63 @@ global_handler(struct display *display, uint32_t name,
}
 }
 
+/** Display help for command line options, and exit */
+static uint32_t opt_help = 0;
+
+/** Require a distinct click to show the input panel (virtual keyboard) */
+static uint32_t opt_click_to_show = 0;
+
+/** Set a specific (RFC-3066) language.  Used for the virtual keyboard, etc. */
+static const char *opt_preferred_language = NULL;
+
+/**
+ * \brief command line options for editor
+ */
+static const struct weston_option editor_options[] = {
+   { WESTON_OPTION_BOOLEAN, "help", 'h', _help },
+   { WESTON_OPTION_BOOLEAN, "click-to-show", 'C', _click_to_show },
+   { WESTON_OPTION_STRING, "preferred-language", 'L', 
_preferred_language },
+};
+
+static void
+usage(const char *program_name, int exit_code)
+{
+   unsigned k;
+
+   fprintf(stderr, "Usage: %s [OPTIONS]\n\n", program_name);
+   for (k = 0; k < ARRAY_LENGTH(editor_options); k++) {
+   const struct weston_option *p = _options[k];
+   if (p->name) {
+   fprintf(stderr, "  --%s", p->name);
+   if (p->type != WESTON_OPTION_BOOLEAN)
+   fprintf(stderr, "=VALUE");
+   fprintf(stderr, "\n");
+   }
+   if (p->short_name) {
+   fprintf(stderr, "  -%c", p->short_name);
+   if (p->type != WESTON_OPTION_BOOLEAN)
+   fprintf(stderr, "VALUE");
+   fprintf(stderr, "\n");
+   }
+   }
+   exit(exit_code);
+}
+
 int
 main(int argc, char *argv[])
 {
struct editor editor;
-   int i;
-   uint32_t click_to_show = 0;
-   const char *preferred_language = NULL;
-
-   for (i = 1; i < argc; i++) {
-   if (strcmp("--click-to-show", argv[i]) == 0)
-   click_to_show = 1;
-   else if (strcmp("--preferred-language", argv[i]) == 0 &&
-i + 1 < argc) {
-   preferred_language = argv[i + 1];
-   i++;
-   } else {
-   printf("Usage: %s [OPTIONS]\n"
-  "  --click-to-show\n"
-  "  --preferred-language LANGUAGE\n",
-  argv[0]);
-   return 1;
-   }
+   char *text_buffer = NULL;
+
+   parse_options(editor_options, ARRAY_LENGTH(editor_options),
+ , argv);
+   if (opt_help)
+   usage(argv[0], EXIT_SUCCESS);
+
+   if (argc > 1) {
+   usage(argv[0], EXIT_FAILURE);
+   /* FIXME: Use remaining arguments as a path/filename to load */
+   return 0;
}
 
memset(, 0, sizeof editor);
@@ -1537,12 +1573,12 @@ main(int argc, char *argv[])
editor.widget = window_frame_create(editor.window, );
 
editor.entry = text_entry_create(, "Entry");
-   editor.entry->click_to_show = click_to_show;
-   if (preferred_language)
-   editor.entry->preferred_language = strdup(preferred_language);
+   editor.entry->click_to_show = opt_click_to_show;
+   if (opt_preferred_language)
+   editor.entry->preferred_language = 
strdup(opt_preferred_language);
editor.editor = text_entry_create(, "Numeric");
editor.editor->content_purpose = 
ZWP_TEXT_INPUT_V1_CONTENT_PURPOSE_NUMBER;
-   editor.editor->click_to_show = click_to_show;
+   editor.editor->click_to_show = opt_click_to_show;
editor.selection = NULL;
editor.selected_text = NULL;
 
-- 
1.9.1

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

Re: [PATCH weston v2 1/2] editor: Use parse_options() from shared for command line options

[Bryce Harrington]

On Mon, Nov 21, 2016 at 09:08:38PM +0100, Silvan Jegen wrote:
> Hi
> 
> >  int
> >  main(int argc, char *argv[])
> >  {
> > struct editor editor;
> > int i;
> 
> This is still unused (as pointed out by Daniel) and should be removed.

Oh, thanks, missed that.

Bryce

> Cheers,
> 
> Silvan
> 
> > -   uint32_t click_to_show = 0;
> > -   const char *preferred_language = NULL;
> > -
> > -   for (i = 1; i < argc; i++) {
> > -   if (strcmp("--click-to-show", argv[i]) == 0)
> > -   click_to_show = 1;
> > -   else if (strcmp("--preferred-language", argv[i]) == 0 &&
> > -i + 1 < argc) {
> > -   preferred_language = argv[i + 1];
> > -   i++;
> > -   } else {
> > -   printf("Usage: %s [OPTIONS]\n"
> > -  "  --click-to-show\n"
> > -  "  --preferred-language LANGUAGE\n",
> > -  argv[0]);
> > -   return 1;
> > -   }
> > +
> > +   parse_options(editor_options, ARRAY_LENGTH(editor_options),
> > + , argv);
> > +   if (opt_help)
> > +   usage(argv[0], EXIT_SUCCESS);
> > +
> > +   if (argc > 1) {
> > +   usage(argv[0], EXIT_FAILURE);
> > +   /* FIXME: Use remaining arguments as a path/filename to load */
> > +   return 0;
> > }
> >  
> > memset(, 0, sizeof editor);
> > @@ -1537,12 +1573,12 @@ main(int argc, char *argv[])
> > editor.widget = window_frame_create(editor.window, );
> >  
> > editor.entry = text_entry_create(, "Entry");
> > -   editor.entry->click_to_show = click_to_show;
> > -   if (preferred_language)
> > -   editor.entry->preferred_language = strdup(preferred_language);
> > +   editor.entry->click_to_show = opt_click_to_show;
> > +   if (opt_preferred_language)
> > +   editor.entry->preferred_language = 
> > strdup(opt_preferred_language);
> > editor.editor = text_entry_create(, "Numeric");
> > editor.editor->content_purpose = 
> > ZWP_TEXT_INPUT_V1_CONTENT_PURPOSE_NUMBER;
> > -   editor.editor->click_to_show = click_to_show;
> > +   editor.editor->click_to_show = opt_click_to_show;
> > editor.selection = NULL;
> > editor.selected_text = NULL;
> >  
> > -- 
> > 1.9.1
> > 
> > ___
> > wayland-devel mailing list
> > wayland-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston v3 2/2] editor: Load a file if specified on command line

[Bryce Harrington]

Add support for basic text file loading, to facilitate more expansive
testing of its UTF-8 text editing support.

Signed-off-by: Bryce Harrington 
---
 clients/editor.c | 67 +++-
 1 file changed, 62 insertions(+), 5 deletions(-)

diff --git a/clients/editor.c b/clients/editor.c
index b8fc63a..42c7f52 100644
--- a/clients/editor.c
+++ b/clients/editor.c
@@ -25,6 +25,7 @@
 #include "config.h"
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1513,7 +1514,7 @@ usage(const char *program_name, int exit_code)
 {
unsigned k;
 
-   fprintf(stderr, "Usage: %s [OPTIONS]\n\n", program_name);
+   fprintf(stderr, "Usage: %s [OPTIONS] [FILENAME]\n\n", program_name);
for (k = 0; k < ARRAY_LENGTH(editor_options); k++) {
const struct weston_option *p = _options[k];
if (p->name) {
@@ -1532,6 +1533,53 @@ usage(const char *program_name, int exit_code)
exit(exit_code);
 }
 
+/* Load the contents of a file into a UTF-8 text buffer and return it.
+ *
+ * Caller is responsible for freeing the buffer when done.
+ * On error, returns NULL.
+ */
+static char *
+read_file(char *filename)
+{
+   char *buffer = NULL;
+   int buf_size, read_size;
+   FILE *fin;
+   int errsv;
+
+   fin = fopen(filename, "r");
+   if (fin == NULL)
+   goto error;
+
+   /* Determine required buffer size */
+   if (fseek(fin, 0, SEEK_END) != 0)
+   goto error;
+   buf_size = ftell(fin);
+   if (buf_size < 0)
+   goto error;
+   rewind(fin);
+
+   /* Create buffer and read in the text */
+   buffer = (char*) malloc(sizeof(char) * (buf_size + 1));
+   if (buffer == NULL)
+   goto error;
+   read_size = fread(buffer, sizeof(char), buf_size, fin);
+   fclose(fin);
+   if (buf_size != read_size)
+   goto error;
+   buffer[buf_size] = '\0';
+
+   return buffer;
+
+error:
+   errsv = errno;
+   if (fin)
+   fclose(fin);
+   free(buffer);
+   errno = errsv || EINVAL;
+
+   return NULL;
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -1544,9 +1592,14 @@ main(int argc, char *argv[])
usage(argv[0], EXIT_SUCCESS);
 
if (argc > 1) {
-   usage(argv[0], EXIT_FAILURE);
-   /* FIXME: Use remaining arguments as a path/filename to load */
-   return 0;
+   if (argv[1][0] == '-')
+   usage(argv[0], EXIT_FAILURE);
+
+   text_buffer = read_file(argv[1]);
+   if (text_buffer == NULL) {
+   fprintf(stderr, "could not read file '%s': %m\n", 
argv[1]);
+   return -1;
+   }
}
 
memset(, 0, sizeof editor);
@@ -1572,7 +1625,10 @@ main(int argc, char *argv[])
editor.window = window_create(editor.display);
editor.widget = window_frame_create(editor.window, );
 
-   editor.entry = text_entry_create(, "Entry");
+   if (text_buffer)
+   editor.entry = text_entry_create(, text_buffer);
+   else
+   editor.entry = text_entry_create(, "Entry");
editor.entry->click_to_show = opt_click_to_show;
if (opt_preferred_language)
editor.entry->preferred_language = 
strdup(opt_preferred_language);
@@ -1606,6 +1662,7 @@ main(int argc, char *argv[])
widget_destroy(editor.widget);
window_destroy(editor.window);
display_destroy(editor.display);
+   free(text_buffer);
 
return 0;
 }
-- 
1.9.1

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

Re: [wayland-protocols] linux-dmabuf: add immediate dmabuf import path

[Daniel Stone]

Hi Varad,

On 11 November 2016 at 11:40, Varad Gautam  wrote:
> provide a mechanism that allows clients to import the added dmabufs
> and immediately receive the newly created wl_buffer handle. this is
> useful to clients that are sure of their import request succeeding,
> and wish to avoid the wl_buffer communication roundtrip.
>
> bump zwp_linux_dmabuf_v1, zwp_linux_buffer_params_v1 interface
> versions.

As with the others, I'd like to wait a little bit for more feedback
before merging, but this is:
Reviewed-by: Daniel Stone 

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

Re: [PATCH weston] simple-egl: Do not set EGL up until XDG setup is complete

[Miguel Angel Vico]

Well, I agree this fix would allow non-compliant implementations to
keep misusing wl_surface.commit, and that could potentially remove any
motivation to find an appropriate resolution for the bug below.

I'm fine with deferring this patch, but I still think applications
should not start setting EGL surfaces up before they have been set up
by the compositor.

Thanks.

On Mon, 21 Nov 2016 09:52:40 +
Daniel Stone  wrote:

> Hi Miguel,
> 
> On 15 November 2016 at 04:49, Miguel A. Vico 
> wrote:
> > There is nothing that prohibits the underlying EGL_PLATFORM_WAYLAND
> > implementation to attach a buffer or commit surfaces right after the
> > Wayland EGLSurface has been created.
> >
> > Since XDG Shell v6 imposes that no buffer attachments or surface
> > commits must be done before a configure is complete, Wayland
> > clients shouldn't start setting EGL up until XDG setup is complete.
> >
> > Related bug:
> >
> > https://bugs.freedesktop.org/show_bug.cgi?id=98731  
> 
> Per discussion on that bug, I'd like to defer this patch until we
> reach a resolution. However, given the pretty firm opinions in this
> bug, I'd definitely lean towards retaining the current behaviour, and
> doing what we need with EGL implementations (Vivante's does the same
> thing, in the opposite direction: it commits too late, rather than too
> early!) rather than allow commits to happen outside the user's
> control.
> 
> I guess you already know this, but figured I'd better document it for
> the list. :)
> 
> Cheers,
> Daniel
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel


-- 
Miguel


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

Re: [PATCH wayland v2] util: Clarify documentation of wl_dispatcher_func_t

[Daniel Stone]

Hi,

On 21 November 2016 at 20:59, Yong Bakos  wrote:
> On Nov 21, 2016, at 9:14 AM, Daniel Stone  wrote:
>> I think removing the final word 'the dispatcher-specific
>> implementation data' is a loss of precision/accuracy. If you don't
>> mind reinstating that final word, I'll merge with my R-b.
>
> I have no problem with this. The reason why I changed it from "implementation 
> data"
> to just "implementation" is because wl_closure_dispatch[1] passes an 
> implementation
> as the first argument. I assumed this use was the specific intent, making the
> description vague; but perhaps this is just one intent, and the description 
> should
> be more general ("implementation data").
>
> I'm fine either way (you know wy better than I do).

OK, fair enough! Pushed in its original form, thanks:

To ssh://git.freedesktop.org/git/wayland/wayland
   a26ed09..0242007  master -> master

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

Re: [PATCH wayland v2] util: Clarify documentation of wl_dispatcher_func_t

[Yong Bakos]

Hi Daniel,

> On Nov 21, 2016, at 9:14 AM, Daniel Stone  wrote:
> 
> Hi Yong,
> 
> On 21 November 2016 at 13:44, Yong Bakos  wrote:
>> - * A dispatcher takes five arguments:  The first is the dispatcher-specific
>> - * implementation data associated with the target object.  The second is the
>> - * object on which the callback is being invoked (either wl_proxy or
>> - * wl_resource).  The third and fourth arguments are the opcode the 
>> wl_message
>> - * structure corresponding to the callback being emitted.  The final 
>> argument
>> - * is an array of arguments received from the other process via the wire
>> - * protocol.
>> + * A dispatcher takes five arguments: The first is the dispatcher-specific
>> + * implementation associated with the target object. The second is the 
>> object
>> + * upon which the callback is being invoked (either wl_proxy or 
>> wl_resource).
>> + * The third and fourth arguments are the opcode and the wl_message
>> + * corresponding to the callback. The final argument is an array of 
>> arguments
>> + * received from the other process via the wire protocol.
> 
> I think removing the final word 'the dispatcher-specific
> implementation data' is a loss of precision/accuracy. If you don't
> mind reinstating that final word, I'll merge with my R-b.

I have no problem with this. The reason why I changed it from "implementation 
data"
to just "implementation" is because wl_closure_dispatch[1] passes an 
implementation
as the first argument. I assumed this use was the specific intent, making the
description vague; but perhaps this is just one intent, and the description 
should
be more general ("implementation data").

I'm fine either way (you know wy better than I do).

Thank you,
yong

[1] connection.c:939:

void
wl_closure_dispatch(struct wl_closure *closure, wl_dispatcher_func_t dispatcher,
struct wl_object *target, uint32_t opcode)
{
dispatcher(target->implementation, target, opcode, closure->message,
   closure->args);
}



> 
> Cheers,
> Daniel
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

Re: [PATCH weston 9/9] clients/simple-dmabuf-drm: use tiled frame data with modifiers

[Daniel Stone]

Hi,

On 17 November 2016 at 11:56, Varad Gautam  wrote:
> fill the dmabuf with valid DRM_FORMAT_NV12 +
> DRM_FORMAT_MOD_SAMSUNG_64_32_TILE frame data before importing to display
> a non-gibberish pattern when importing with modifiers.

Just squash this into the previous patch, which I've only just now
realised needs to also include the header in weston_simple_drm_SOURCES
inside Makefile.am. My Acked-by stands.

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

Re: [PATCH weston 8/9] clients/simple-dmabuf-drm: add valid frame data to use with modifiers

[Daniel Stone]

On 17 November 2016 at 11:56, Varad Gautam  wrote:
> From: Varad Gautam 
>
> raw SMPTE color bar pattern in drm_fourcc.h format DRM_FORMAT_NV12 with
> modifier DRM_FORMAT_MOD_SAMSUNG_64_32_TILE to be imported as a dmabuf.
>
> Signed-off-by: Varad Gautam 

Acked-by: Daniel Stone 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 7/9] clients/simple-dmabuf-drm: import with dmabuf modifiers

[Daniel Stone]

Hi Varad,

On 17 November 2016 at 11:56, Varad Gautam  wrote:
> From: Varad Gautam 
>
> mesa's freedreno driver supports importing dmabufs with format
> DRM_FORMAT_NV12 and DRM_FORMAT_MOD_SAMSUNG_64_32_TILE modifier.
> demonstrate weston modifier advertising and import path using this
> combination when run with --import-format=NV12.

Acked-by: Daniel Stone 

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

Re: [PATCH weston 6/9] clients/simple-dmabuf-drm: add freedreno support alongside intel

[Daniel Stone]

Hi Varad,

On 17 November 2016 at 11:56, Varad Gautam  wrote:
> diff --git a/configure.ac b/configure.ac
> index d084d32..b959637 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -392,12 +392,21 @@ AC_ARG_ENABLE(simple-dmabuf-drm-client,
>   [do not build the simple dmabuf drm client]),,
>enable_simple_dmabuf_drm_client="auto")
>  if ! test "x$enable_simple_dmabuf_drm_client" = "xno"; then
> -  PKG_CHECK_MODULES(SIMPLE_DMABUF_DRM_CLIENT, [wayland-client libdrm 
> libdrm_intel],
> -   have_simple_dmabuf_drm_client=yes, 
> have_simple_dmabuf_drm_client=no)
> -  if test "x$have_simple_dmabuf_drm_client" = "xno" -a 
> "x$enable_simple_dmabuf_drm_client" = "xyes"; then
> -AC_MSG_ERROR([DRM dmabuf client explicitly enabled, but libdrm_intel 
> couldn't be found])
> +  PKG_CHECK_MODULES(SIMPLE_DMABUF_DRM_CLIENT, [wayland-client libdrm],
> +[PKG_CHECK_MODULES(LIBDRM_PLATFORM, [libdrm_freedreno],
> +  AC_DEFINE([HAVE_LIBDRM_FREEDRENO], [1], [Build freedreno dmabuf 
> client]) have_simple_dmabuf_drm_client=freedreno,
> +[PKG_CHECK_MODULES(LIBDRM_PLATFORM, [libdrm_intel],
> +  AC_DEFINE([HAVE_LIBDRM_INTEL], [1], [Build intel dmabuf client]) 
> have_simple_dmabuf_drm_client=intel,
> +have_simple_dmabuf_drm_client=unsupported)])],
> +  have_simple_dmabuf_drm_client=unsupported)
> +
> +  if test "x$have_simple_dmabuf_drm_client" = "xunsupported" -a 
> "x$enable_simple_dmabuf_drm_client" = "xyes"; then
> +AC_MSG_ERROR([DRM dmabuf client explicitly enabled, but libdrm_intel or 
> libdrm_freedreno not found])
> +  fi
> +
> +  if test "x$have_simple_dmabuf_drm_client" = "xfreedreno" -o 
> "x$have_simple_dmabuf_drm_client" = "xintel"; then
> +enable_simple_dmabuf_drm_client="yes"
>fi
> -  enable_simple_dmabuf_drm_client="$have_simple_dmabuf_drm_client"
>  fi
>  AM_CONDITIONAL(BUILD_SIMPLE_DMABUF_DRM_CLIENT, test 
> "x$enable_simple_dmabuf_drm_client" = "xyes")

This conditional ladder pains my eys, but I can't think of anything
better at this point.

Acked-by: Daniel Stone 

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

Re: [PATCH weston 5/9] clients/simple-dmabuf-intel: rename to simple-dmabuf-drm

[Daniel Stone]

Hi Varad,

On 17 November 2016 at 11:55, Varad Gautam  wrote:
> From: Varad Gautam 
>
> this will allow adding other drm backends later.

It's still not a particularly pretty client, but no worse than what came before.

Reviewed-by: Daniel Stone 

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

Re: [PATCH weston 4/9] gl-renderer: allow importing fourth dmabuf plane

[Daniel Stone]

Hi Varad,

On 17 November 2016 at 11:55, Varad Gautam  wrote:
> @@ -1621,6 +1621,21 @@ import_simple_dmabuf(struct gl_renderer *gr,
> }
> }
>
> +   if (gr->has_dmabuf_import_modifiers) {
> +   if (attributes->n_planes > 3) {
> +   attribs[atti++] = EGL_DMA_BUF_PLANE3_FD_EXT;
> +   attribs[atti++] = attributes->fd[3];
> +   attribs[atti++] = EGL_DMA_BUF_PLANE3_OFFSET_EXT;
> +   attribs[atti++] = attributes->offset[3];
> +   attribs[atti++] = EGL_DMA_BUF_PLANE2_PITCH_EXT;

PLANE3!

With that fixed:
Reviewed-by: Daniel Stone 

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

Re: [PATCH weston 3/9] gl-renderer: allow importing dmabufs with format modifiers

[Daniel Stone]

Hi Varad,

On 17 November 2016 at 11:55, Varad Gautam  wrote:
> attribs[atti++] = EGL_NONE;
> @@ -1924,9 +1941,10 @@ gl_renderer_import_dmabuf(struct weston_compositor *ec,
> assert(gr->has_dmabuf_import);
>
> for (i = 0; i < dmabuf->attributes.n_planes; i++) {
> -   /* EGL import does not have modifiers */
> +   /* return if EGL doesn't support import modifiers */
> if (dmabuf->attributes.modifier[i] != 0)
> -   return false;
> +   if (!gr->has_dmabuf_import_modifiers)
> +   return false;

Given the discussion which only kicked off on dri-devel@ when this
extension landed, please also fail the import if modifier[i] !=
modifier[0], i.e. ensure that the modifier passed is the same for all
planes.

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

Re: [PATCH weston 2/9] linux-dmabuf: advertise supported formats and modifiers

[Daniel Stone]

Hi Varad,

On 17 November 2016 at 11:55, Varad Gautam  wrote:
> implement 'format' and 'modifier' events to communicate available
> formats and modifiers to the client and support zwp_linux_dmabuf_v1
> interface version 3.

When this lands, this will need a wayland-protocols dependency bump in
configure.ac. That being said:
Reviewed-by: Daniel Stone 

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

Re: [PATCH weston 1/9] gl-renderer: support format and modifier queries

[Daniel Stone]

Hi Varad,

On 17 November 2016 at 11:55, Varad Gautam  wrote:
> @@ -1847,6 +1851,69 @@ import_dmabuf(struct gl_renderer *gr,
>  }
>
>  static bool
> +gl_renderer_query_dmabuf_formats(struct weston_compositor *wc,
> +   int **formats, int *num_formats)
> +{
> +   struct gl_renderer *gr = get_renderer(wc);
> +   EGLint num;
> +
> +   assert(gr->has_dmabuf_import);
> +
> +   if (!gr->query_dmabuf_formats(gr->egl_display, 0, NULL, )) {
> +   *num_formats = 0;
> +   return false;
> +   }
> +
> +   *formats = calloc(1, num * sizeof(int));

calloc(num, sizeof(int))

> +static bool
> +gl_renderer_query_dmabuf_modifiers(struct weston_compositor *wc, int format,
> +   uint64_t **modifiers,
> +   int *num_modifiers)
> +{
> +   struct gl_renderer *gr = get_renderer(wc);
> +   int num;
> +
> +   assert(gr->has_dmabuf_import);
> +
> +   if (!gr->has_dmabuf_import_modifiers ||
> +   !gr->query_dmabuf_modifiers(gr->egl_display, format, 0, NULL,
> +   NULL, )) {

The test can just be removed here, as there's none for query_dmabuf_formats.

With those fixed:
Reviewed-by: Daniel Stone 

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

Re: [PATCH weston 1/9] gl-renderer: support format and modifier queries

[Daniel Stone]

On 21 November 2016 at 20:07, Daniel Stone  wrote:
> On 17 November 2016 at 11:55, Varad Gautam  wrote:
>> +static bool
>> +gl_renderer_query_dmabuf_modifiers(struct weston_compositor *wc, int format,
>> +   uint64_t **modifiers,
>> +   int *num_modifiers)
>> +{
>> +   struct gl_renderer *gr = get_renderer(wc);
>> +   int num;
>> +
>> +   assert(gr->has_dmabuf_import);
>> +
>> +   if (!gr->has_dmabuf_import_modifiers ||
>> +   !gr->query_dmabuf_modifiers(gr->egl_display, format, 0, NULL,
>> +   NULL, )) {
>
> The test can just be removed here, as there's none for query_dmabuf_formats.

Sorry, I can't read, and it's the other way around:
query_dmabuf_formats needs a check here, and also the
eglGetProcAddress should be moved under the same if
(EGL_EXT_...modifiers) branch as the GetProcAddress for the modifier
query.

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

Re: [PATCH weston v2 1/2] editor: Use parse_options() from shared for command line options

[Silvan Jegen]

Hi

One comment below.

On Mon, Nov 21, 2016 at 10:34:33AM -0800, Bryce Harrington wrote:
> Also add a basic --help option
> 
> Signed-off-by: Bryce Harrington 
> ---
>  clients/editor.c | 78 
> +---
>  1 file changed, 57 insertions(+), 21 deletions(-)
> 
> diff --git a/clients/editor.c b/clients/editor.c
> index 30bf555..33b43d2 100644
> --- a/clients/editor.c
> +++ b/clients/editor.c
> @@ -37,6 +37,7 @@
>  
>  #include 
>  
> +#include "shared/config-parser.h"
>  #include "shared/helpers.h"
>  #include "shared/xalloc.h"
>  #include "window.h"
> @@ -1489,28 +1490,63 @@ global_handler(struct display *display, uint32_t name,
>   }
>  }
>  
> +/** Display help for command line options, and exit */
> +static uint32_t opt_help = 0;
> +
> +/** Require a distinct click to show the input panel (virtual keyboard) */
> +static uint32_t opt_click_to_show = 0;
> +
> +/** Set a specific (RFC-3066) language.  Used for the virtual keyboard, etc. 
> */
> +static const char *opt_preferred_language = NULL;
> +
> +/**
> + * \brief command line options for editor
> + */
> +static const struct weston_option editor_options[] = {
> + { WESTON_OPTION_BOOLEAN, "help", 'h', _help },
> + { WESTON_OPTION_BOOLEAN, "click-to-show", 'C', _click_to_show },
> + { WESTON_OPTION_STRING, "preferred-language", 'L', 
> _preferred_language },
> +};
> +
> +static void
> +usage(const char *program_name, int exit_code)
> +{
> + unsigned k;
> +
> + fprintf(stderr, "Usage: %s [OPTIONS]\n\n", program_name);
> + for (k = 0; k < ARRAY_LENGTH(editor_options); k++) {
> + const struct weston_option *p = _options[k];
> + if (p->name) {
> + fprintf(stderr, "  --%s", p->name);
> + if (p->type != WESTON_OPTION_BOOLEAN)
> + fprintf(stderr, "=VALUE");
> + fprintf(stderr, "\n");
> + }
> + if (p->short_name) {
> + fprintf(stderr, "  -%c", p->short_name);
> + if (p->type != WESTON_OPTION_BOOLEAN)
> + fprintf(stderr, "VALUE");
> + fprintf(stderr, "\n");
> + }
> + }
> + exit(exit_code);
> +}
> +
>  int
>  main(int argc, char *argv[])
>  {
>   struct editor editor;
>   int i;

This is still unused (as pointed out by Daniel) and should be removed.


Cheers,

Silvan

> - uint32_t click_to_show = 0;
> - const char *preferred_language = NULL;
> -
> - for (i = 1; i < argc; i++) {
> - if (strcmp("--click-to-show", argv[i]) == 0)
> - click_to_show = 1;
> - else if (strcmp("--preferred-language", argv[i]) == 0 &&
> -  i + 1 < argc) {
> - preferred_language = argv[i + 1];
> - i++;
> - } else {
> - printf("Usage: %s [OPTIONS]\n"
> -"  --click-to-show\n"
> -"  --preferred-language LANGUAGE\n",
> -argv[0]);
> - return 1;
> - }
> +
> + parse_options(editor_options, ARRAY_LENGTH(editor_options),
> +   , argv);
> + if (opt_help)
> + usage(argv[0], EXIT_SUCCESS);
> +
> + if (argc > 1) {
> + usage(argv[0], EXIT_FAILURE);
> + /* FIXME: Use remaining arguments as a path/filename to load */
> + return 0;
>   }
>  
>   memset(, 0, sizeof editor);
> @@ -1537,12 +1573,12 @@ main(int argc, char *argv[])
>   editor.widget = window_frame_create(editor.window, );
>  
>   editor.entry = text_entry_create(, "Entry");
> - editor.entry->click_to_show = click_to_show;
> - if (preferred_language)
> - editor.entry->preferred_language = strdup(preferred_language);
> + editor.entry->click_to_show = opt_click_to_show;
> + if (opt_preferred_language)
> + editor.entry->preferred_language = 
> strdup(opt_preferred_language);
>   editor.editor = text_entry_create(, "Numeric");
>   editor.editor->content_purpose = 
> ZWP_TEXT_INPUT_V1_CONTENT_PURPOSE_NUMBER;
> - editor.editor->click_to_show = click_to_show;
> + editor.editor->click_to_show = opt_click_to_show;
>   editor.selection = NULL;
>   editor.selected_text = NULL;
>  
> -- 
> 1.9.1
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [wayland-protocols v3] linux-dmabuf: advertise format modifiers with modifier event

[Daniel Stone]

Hi Varad,

On 21 November 2016 at 10:17, Varad Gautam  wrote:
> advertise the supported fourcc format modifiers along with supported
> formats to the client.
>
> bump zwp_linux_dmabuf_v1, zwp_linux_buffer_params_v1 interface
> versions to 3.

Reviewed-by: Daniel Stone 

I'll wait a little while with this series to see if more review pops
up, and then merge it if not.

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

Re: [wayland-protocols v2] linux-dmabuf: clarify format event description

[Daniel Stone]

Hi Varad,

On 21 November 2016 at 10:17, Varad Gautam  wrote:
> clearly state the request name in format event to avoid abmiguous
> interpretation between 'zwp_linux_buffer_params_v1::create' and
> 'zwp_linux_dmabuf_v1::create_params' requests.

Sorry, I forgot to note that I pushed this earlier.

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

Re: [PATCH weston v3] libweston: Position layers in an absolute way

[Daniel Stone]

Hi Quentin,

On 11 July 2016 at 10:29, Quentin Glidic
 wrote:
> Currently, layers’ order depends on the module loading order and it does
> not survive runtime modifications (like shell locking/unlocking).
> With this patch, modules can safely add their own layer at the expected
> position in the stack, with runtime persistence.
>
> Signed-off-by: Quentin Glidic 
> Acked-by: Pekka Paalanen 

Yes, this API is a hell of a lot more pleasant than what we had before.

Unfortunately for you though, I'm not exactly a desktop developer, so
would like to see Giulio ack this before it lands. With that being
said:
Reviewed-by: Daniel Stone 

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

Re: [PATCH weston 6/8] compositor-drm: Make some functions take a weston_output directly

[Daniel Stone]

Hi Emmanuel,

On 2 May 2016 at 22:40, Emmanuel Gil Peyrot
 wrote:
> Makes it clearer that they don’t use any field specific to drm_output,
> and reduce the amount of churn from the following commits.

I forgot to mention that I think patches 5 and 6 from this look good,
and I'll try to remember to pull them on top of my atomic patchset.

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

Re: [PATCH weston 7/8] compositor-drm: Implement clone mode, refactor output into logical ones

[Daniel Stone]

Hi Emmanuel,

On 2 May 2016 at 22:40, Emmanuel Gil Peyrot
 wrote:
> Introduces a “same-as” configuration option for each output, which
> bypasses the rest of the output configuration (mode, scale, transform
> and seat) and instead makes it a clone of the specified output.
>
> This is implemented by splitting the drm_output struct into the
> per-connector drm_output and the per-weston_output drm_logical_output,
> with the latter containing one or more of the former in a wl_list.

Hmm. Let me stop you there ...

> +struct drm_logical_output {
> +   struct weston_output   base;
> +   struct wl_list output_list;
> +
> +   int page_flip_refcount;

This to me is a red flag. I think you've built this patch at the wrong level.

There are really two kinds of clone mode: same-CRTC and different
CRTC. Same-CRTC can share planes, and they share timing as well. I
think same-CRTC clones should be ganged together within the backend,
and different-CRTC clones should appear entirely disjoint from the
backend's point of view.

With the caveat that my review of Armin's drm_keep_current_mode patch
shows that I comprehensively don't yet understand the new output
configuration API - so I'd like his input on this as well - I'd
suggest the following approach.

Firstly, keep every connector as a separate weston_output, as it is today.

When you parse an outputB same-as outputA configuration, call a new
backend vfunc: outputA->chain_output(outputB). If this returns
success, mark output B as chained from output A: not appearing in a
surface's output_mask[0], not having dpms/repaint/... called on it,
etc. If this returns failure, then continue to consider them as
totally separate outputs. In the DRM backend, this would simply mean
that we passed multiple connectors to drmModeSetCrtc and multiple
connector_states bound to the same CRTC.

This would remove a lot of complexity from the DRM backend, and also
allow planes to work when we can clone multiple connectors on the same
CRTC. It would also mean we avoid a possible drop to 30fps, if the two
CRTCs end up clocked half the repaint cycle apart, and thus didn't
call weston_output_finish_frame until too late. Plus it has the added
benefit of working better when we have a constrained number of CRTCs,
or constrained CRTC capability, or whatever.

The core already has to deal with surfaces overlapping multiple 'real'
outputs - i.e. running on different paint cycles - today, so I don't
see that punting clones running on disjoint CRTCs makes that problem
worse in terms of complexity for the core. We could certainly do
better with repaint-cycle time reporting[1] in the core, but just
punting down to the backend only makes that worse rather than better,
I think.

Doing it this way would make also make the patchset a lot less
invasive, particularly if rebased on top of the
atomic/plane_state/output_state patchset ... ;)

Sorry to be the bearer of bad news.

Cheers,
Daniel

[0]: Well. The client should still see the output for
wl_surface::{enter,leave} events, but the backend should only see the
'real' outputs, not the chained ones. Maybe we need a
client_output_mask, and a backend_output_mask ... ?
[1]: Have a surface span two weston_outputs with disjoint paint
clocks. Watch the surface's frame-event timing bounce between the two
outputs, with resultant jittery framerate. Presumably we should pick
one 'master' surface to clock from, and stick to that.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] gl: Don't declare variables in for loop

[Daniel Stone]

Hi,

On 21 November 2016 at 19:05, Bryce Harrington  wrote:
> On Mon, Nov 21, 2016 at 06:06:23PM +, Daniel Stone wrote:
>> On 21 November 2016 at 18:02, Bryce Harrington  wrote:
>> >   libweston/gl-renderer.c:2862:2: error: ‘for’ loop initial declarations
>> >   are only allowed in C99 mode
>> > for (unsigned i = 0; i < ARRAY_LENGTH(swap_damage_ext_to_entrypoint);
>> > i++) {
>> > ^
>>
>> Sorry, no idea how I didn't spot that during review. Out of interest,
>> what exotic compiler/CFLAGS are you using to have caught this?
>
> Nothing particularly fancy, I think most of this is just stock compiler
> settings for my distro:
>
> GCC_CFLAGS='-Wall -Wextra -Wno-unused-parameter -Wno-shift-negative-value 
> -Wno-missing-field-initializers \
> -g -fvisibility=hidden -Wstrict-prototypes -Wmissing-prototypes 
> -Wsign-compare'
>
> Here's the full warning for context:
> [...]
>
> So, looks more like -std=c99 just isn't set by default by the compiler
> on my system (which it shouldn't be, for wayland anyway.)  I'm running
> gcc 4.8.4 which is on the old side; perhaps things have changed wrt c99
> defaults used with newer compilers?

Oh, yeah. I'm on 6.2.1, and I'm quite sure the default standard
flipped to gnu99 since then.

> I haven't sorted out the -Wno-shift-negative-value warning but
> suspecting it's bogus.

Well, it's bogus to the extent that it doesn't appear with newer
toolchains, but still it'd be nice to get that sorted out. I'm pretty
sure one of the other projects (xserver? Mesa?) tries to filter out
options which aren't supported by your compiler version, which might
be nice to have.

>> Pushed
>> with review in any case:
>> To ssh://git.freedesktop.org/git/wayland/weston
>>3447619..fe0410b  upstream -> master
>
> Thanks for reviewing and pushing,

No problem, sorry I missed it in the first.

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

Re: [PATCH weston] gl: Don't declare variables in for loop

[Bryce Harrington]

On Mon, Nov 21, 2016 at 06:06:23PM +, Daniel Stone wrote:
> Hi,
>
> On 21 November 2016 at 18:02, Bryce Harrington  wrote:
> > Fixes compilation error introduced by 43cea54c:
> >
> >   libweston/gl-renderer.c:2862:2: error: ‘for’ loop initial declarations
> >   are only allowed in C99 mode
> > for (unsigned i = 0; i < ARRAY_LENGTH(swap_damage_ext_to_entrypoint);
> > i++) {
> > ^
>
> Sorry, no idea how I didn't spot that during review. Out of interest,
> what exotic compiler/CFLAGS are you using to have caught this?

Nothing particularly fancy, I think most of this is just stock compiler
settings for my distro:

GCC_CFLAGS='-Wall -Wextra -Wno-unused-parameter -Wno-shift-negative-value 
-Wno-missing-field-initializers \
-g -fvisibility=hidden -Wstrict-prototypes -Wmissing-prototypes 
-Wsign-compare'

Here's the full warning for context:

  CCLD libweston-desktop-2.la
  CC   libweston/gl_renderer_la-gl-renderer.lo
libweston/gl-renderer.c: In function ‘gl_renderer_setup_egl_extensions’:
libweston/gl-renderer.c:2862:2: error: ‘for’ loop initial declarations are only 
allowed in C99 mode
  for (unsigned i = 0; i < ARRAY_LENGTH(swap_damage_ext_to_entrypoint); i++) {
  ^
libweston/gl-renderer.c:2862:2: note: use option -std=c99 or -std=gnu99 to 
compile your code
libweston/gl-renderer.c: At top level:
cc1: warning: unrecognized command line option "-Wno-shift-negative-value" 
[enabled by default]
make[1]: *** [libweston/gl_renderer_la-gl-renderer.lo] Error 1
make[1]: Leaving directory `/home/bryce/src/Wayland/weston'
make: *** [all] Error 2

So, looks more like -std=c99 just isn't set by default by the compiler
on my system (which it shouldn't be, for wayland anyway.)  I'm running
gcc 4.8.4 which is on the old side; perhaps things have changed wrt c99
defaults used with newer compilers?

I haven't sorted out the -Wno-shift-negative-value warning but
suspecting it's bogus.

> Pushed
> with review in any case:
> To ssh://git.freedesktop.org/git/wayland/weston
>3447619..fe0410b  upstream -> master

Thanks for reviewing and pushing,
Bryce

> Cheers,
> Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston v2 2/2] editor: Load a file if specified on command line

[Bryce Harrington]

Add support for basic text file loading, to facilitate more expansive
testing of its UTF-8 text editing support.

Signed-off-by: Bryce Harrington 
---
 clients/editor.c | 68 +++-
 1 file changed, 63 insertions(+), 5 deletions(-)

diff --git a/clients/editor.c b/clients/editor.c
index 33b43d2..f386a8b 100644
--- a/clients/editor.c
+++ b/clients/editor.c
@@ -25,6 +25,7 @@
 #include "config.h"
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1513,7 +1514,7 @@ usage(const char *program_name, int exit_code)
 {
unsigned k;
 
-   fprintf(stderr, "Usage: %s [OPTIONS]\n\n", program_name);
+   fprintf(stderr, "Usage: %s [OPTIONS] [FILENAME]\n\n", program_name);
for (k = 0; k < ARRAY_LENGTH(editor_options); k++) {
const struct weston_option *p = _options[k];
if (p->name) {
@@ -1532,10 +1533,58 @@ usage(const char *program_name, int exit_code)
exit(exit_code);
 }
 
+/* Load the contents of a file into a UTF-8 text buffer and return it.
+ *
+ * Caller is responsible for freeing the buffer when done.
+ * On error, returns NULL.
+ */
+static char *
+read_file(char *filename)
+{
+   char *buffer = NULL;
+   int buf_size, read_size;
+   FILE *fin;
+   int errsv;
+
+   fin = fopen(filename, "r");
+   if (fin == NULL)
+   goto error;
+
+   /* Determine required buffer size */
+   if (fseek(fin, 0, SEEK_END) != 0)
+   goto error;
+   buf_size = ftell(fin);
+   if (buf_size < 0)
+   goto error;
+   rewind(fin);
+
+   /* Create buffer and read in the text */
+   buffer = (char*) malloc(sizeof(char) * (buf_size + 1));
+   if (buffer == NULL)
+   goto error;
+   read_size = fread(buffer, sizeof(char), buf_size, fin);
+   fclose(fin);
+   if (buf_size != read_size)
+   goto error;
+   buffer[buf_size] = '\0';
+
+   return buffer;
+
+error:
+   errsv = errno;
+   if (fin)
+   fclose(fin);
+   free(buffer);
+   errno = errsv || EINVAL;
+
+   return NULL;
+}
+
 int
 main(int argc, char *argv[])
 {
struct editor editor;
+   char *text_buffer = NULL;
int i;
 
parse_options(editor_options, ARRAY_LENGTH(editor_options),
@@ -1544,9 +1593,14 @@ main(int argc, char *argv[])
usage(argv[0], EXIT_SUCCESS);
 
if (argc > 1) {
-   usage(argv[0], EXIT_FAILURE);
-   /* FIXME: Use remaining arguments as a path/filename to load */
-   return 0;
+   if (argv[1][0] == '-')
+   usage(argv[0], EXIT_FAILURE);
+
+   text_buffer = read_file(argv[1]);
+   if (text_buffer == NULL) {
+   fprintf(stderr, "could not read file '%s': %m\n", 
argv[1]);
+   return -1;
+   }
}
 
memset(, 0, sizeof editor);
@@ -1572,7 +1626,10 @@ main(int argc, char *argv[])
editor.window = window_create(editor.display);
editor.widget = window_frame_create(editor.window, );
 
-   editor.entry = text_entry_create(, "Entry");
+   if (text_buffer)
+   editor.entry = text_entry_create(, text_buffer);
+   else
+   editor.entry = text_entry_create(, "Entry");
editor.entry->click_to_show = opt_click_to_show;
if (opt_preferred_language)
editor.entry->preferred_language = 
strdup(opt_preferred_language);
@@ -1606,6 +1663,7 @@ main(int argc, char *argv[])
widget_destroy(editor.widget);
window_destroy(editor.window);
display_destroy(editor.display);
+   free(text_buffer);
 
return 0;
 }
-- 
1.9.1

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

[PATCH weston v2 1/2] editor: Use parse_options() from shared for command line options

[Bryce Harrington]

Also add a basic --help option

Signed-off-by: Bryce Harrington 
---
 clients/editor.c | 78 +---
 1 file changed, 57 insertions(+), 21 deletions(-)

diff --git a/clients/editor.c b/clients/editor.c
index 30bf555..33b43d2 100644
--- a/clients/editor.c
+++ b/clients/editor.c
@@ -37,6 +37,7 @@
 
 #include 
 
+#include "shared/config-parser.h"
 #include "shared/helpers.h"
 #include "shared/xalloc.h"
 #include "window.h"
@@ -1489,28 +1490,63 @@ global_handler(struct display *display, uint32_t name,
}
 }
 
+/** Display help for command line options, and exit */
+static uint32_t opt_help = 0;
+
+/** Require a distinct click to show the input panel (virtual keyboard) */
+static uint32_t opt_click_to_show = 0;
+
+/** Set a specific (RFC-3066) language.  Used for the virtual keyboard, etc. */
+static const char *opt_preferred_language = NULL;
+
+/**
+ * \brief command line options for editor
+ */
+static const struct weston_option editor_options[] = {
+   { WESTON_OPTION_BOOLEAN, "help", 'h', _help },
+   { WESTON_OPTION_BOOLEAN, "click-to-show", 'C', _click_to_show },
+   { WESTON_OPTION_STRING, "preferred-language", 'L', 
_preferred_language },
+};
+
+static void
+usage(const char *program_name, int exit_code)
+{
+   unsigned k;
+
+   fprintf(stderr, "Usage: %s [OPTIONS]\n\n", program_name);
+   for (k = 0; k < ARRAY_LENGTH(editor_options); k++) {
+   const struct weston_option *p = _options[k];
+   if (p->name) {
+   fprintf(stderr, "  --%s", p->name);
+   if (p->type != WESTON_OPTION_BOOLEAN)
+   fprintf(stderr, "=VALUE");
+   fprintf(stderr, "\n");
+   }
+   if (p->short_name) {
+   fprintf(stderr, "  -%c", p->short_name);
+   if (p->type != WESTON_OPTION_BOOLEAN)
+   fprintf(stderr, "VALUE");
+   fprintf(stderr, "\n");
+   }
+   }
+   exit(exit_code);
+}
+
 int
 main(int argc, char *argv[])
 {
struct editor editor;
int i;
-   uint32_t click_to_show = 0;
-   const char *preferred_language = NULL;
-
-   for (i = 1; i < argc; i++) {
-   if (strcmp("--click-to-show", argv[i]) == 0)
-   click_to_show = 1;
-   else if (strcmp("--preferred-language", argv[i]) == 0 &&
-i + 1 < argc) {
-   preferred_language = argv[i + 1];
-   i++;
-   } else {
-   printf("Usage: %s [OPTIONS]\n"
-  "  --click-to-show\n"
-  "  --preferred-language LANGUAGE\n",
-  argv[0]);
-   return 1;
-   }
+
+   parse_options(editor_options, ARRAY_LENGTH(editor_options),
+ , argv);
+   if (opt_help)
+   usage(argv[0], EXIT_SUCCESS);
+
+   if (argc > 1) {
+   usage(argv[0], EXIT_FAILURE);
+   /* FIXME: Use remaining arguments as a path/filename to load */
+   return 0;
}
 
memset(, 0, sizeof editor);
@@ -1537,12 +1573,12 @@ main(int argc, char *argv[])
editor.widget = window_frame_create(editor.window, );
 
editor.entry = text_entry_create(, "Entry");
-   editor.entry->click_to_show = click_to_show;
-   if (preferred_language)
-   editor.entry->preferred_language = strdup(preferred_language);
+   editor.entry->click_to_show = opt_click_to_show;
+   if (opt_preferred_language)
+   editor.entry->preferred_language = 
strdup(opt_preferred_language);
editor.editor = text_entry_create(, "Numeric");
editor.editor->content_purpose = 
ZWP_TEXT_INPUT_V1_CONTENT_PURPOSE_NUMBER;
-   editor.editor->click_to_show = click_to_show;
+   editor.editor->click_to_show = opt_click_to_show;
editor.selection = NULL;
editor.selected_text = NULL;
 
-- 
1.9.1

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

Re: [PATCH weston 3/3] editor: Load a file if specified on command line

[Bryce Harrington]

On Mon, Nov 21, 2016 at 12:55:14PM +, Daniel Stone wrote:
> Hi Bryce,
> 
> On 20 November 2016 at 22:00, Bryce Harrington  wrote:
> > Add support for basic text file loading, to facilitate more expansive
> > testing of its UTF-8 text editing support.
> 
> Honestly, I question the value of turning the editor into something
> 'real': as soon as we're adding a --version argument to something,

Fair enough I can drop the --version, didn't expect that would be
controversial.  I have local and system installations of weston, so
having tools report which weston they came from seemed useful, but not a
big deal, there's plenty of other ways to determine that.

I do think that an editor actually loading a file to edit is
extraordinarily basic functionality; little risk there of making
weston-editor 'real'.  This small feature would have been moderately
handy while working on the compose editing support, although I was more
focused on input than display.  I figure future developers working on
text support might find use of being able to load up a file with test
UTF-8 text.

Remaining review comments incorporated, thanks.

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

Re: [PATCH] Introduce keyboard grabbing protocol

[Daniel Stone]

Hi,

On 30 August 2016 at 14:05, Olivier Fourdan  wrote:
>> Xwayland should probably use a private protocol, like EGL, ideally
>> completely hidden and pid-restricted.
>
> That's the point, I initially thought of a private protocol, but then 
> realized it could be useful outside of the Xwayland use case as well.
>
> If not, I'd be happy to drop this patch and return to the private approach :)
>
> Speaking of which, that was the idea behind those patches:
>
> https://patchwork.freedesktop.org/patch/105319/
> https://patchwork.freedesktop.org/patch/103815/
> https://patchwork.freedesktop.org/patch/105421/

H. I really, desperately, want to avoid keyboard grabs as much as
possible. But I guess Alt-Tab in a VM is the one usecase which can't
route around this. Out of interest, do you know if that works on
VMWare/VirtualBox/etc on OS X and Windows? If they don't, then we
don't have to either, and we can just make this private to XWayland.
If they do, then I guess we probably need to follow suit as well.

If we do though, I'd prefer to see two protocols: one for native
clients which demands a matching wl_seat serial, and another for
XWayland which doesn't have that restriction.

Note that this is basically just about the concept; I haven't looked
at the exact detail of the wording yet.

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

Re: [PATCH weston] libweston: Add move (without scale) animation

[Daniel Stone]

Hi,

On 10 August 2016 at 14:53, Quentin Glidic
 wrote:
> From: Quentin Glidic 
>
> Signed-off-by: Quentin Glidic 
> ---
>  libweston/animation.c  | 35 +--
>  libweston/compositor.h |  7 ++-
>  2 files changed, 35 insertions(+), 7 deletions(-)

Why not.

To ssh://git.freedesktop.org/git/wayland/weston
   e7ed60f..24d306c  upstream -> master

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

Re: [PATCH weston] gl: Don't declare variables in for loop

[Daniel Stone]

Hi,

On 21 November 2016 at 18:02, Bryce Harrington  wrote:
> Fixes compilation error introduced by 43cea54c:
>
>   libweston/gl-renderer.c:2862:2: error: ‘for’ loop initial declarations
>   are only allowed in C99 mode
> for (unsigned i = 0; i < ARRAY_LENGTH(swap_damage_ext_to_entrypoint);
> i++) {
> ^

Sorry, no idea how I didn't spot that during review. Out of interest,
what exotic compiler/CFLAGS are you using to have caught this? Pushed
with review in any case:
To ssh://git.freedesktop.org/git/wayland/weston
   3447619..fe0410b  upstream -> master

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

Re: [PATCH weston v2] compositor-wayland: Port to xdg-shell-v6

[Daniel Stone]

Hi,

On 21 November 2016 at 18:01, Armin Krezović  wrote:
> On 21.11.2016 18:42, Armin Krezović wrote:
>> v2:
>>
>>  - Keep wl_shell code around until xdg_shell is declared stable.
>>
>> Signed-off-by: Armin Krezović 
>
> I hereby agree that the following changes can be merged as part of this patch:
>
> https://phabricator.freedesktop.org/P6

Brilliant, thanks Armin! R-b me and pushed with those changes:
To ssh://git.freedesktop.org/git/wayland/weston
   e5732c7..3447619  upstream -> master

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

[PATCH weston] gl: Don't declare variables in for loop

[Bryce Harrington]

Fixes compilation error introduced by 43cea54c:

  libweston/gl-renderer.c:2862:2: error: ‘for’ loop initial declarations
  are only allowed in C99 mode
for (unsigned i = 0; i < ARRAY_LENGTH(swap_damage_ext_to_entrypoint);
i++) {
^

Signed-off-by: Bryce Harrington 
---
 libweston/gl-renderer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
index 1a8cf67..4077c62 100644
--- a/libweston/gl-renderer.c
+++ b/libweston/gl-renderer.c
@@ -2828,6 +2828,7 @@ gl_renderer_setup_egl_extensions(struct weston_compositor 
*ec)
struct gl_renderer *gr = get_renderer(ec);
const char *extensions;
EGLBoolean ret;
+   unsigned i;
 
gr->create_image = (void *) eglGetProcAddress("eglCreateImageKHR");
gr->destroy_image = (void *) eglGetProcAddress("eglDestroyImageKHR");
@@ -2859,7 +2860,7 @@ gl_renderer_setup_egl_extensions(struct weston_compositor 
*ec)
weston_log("warning: EGL_EXT_buffer_age not supported. "
   "Performance could be affected.\n");
 
-   for (unsigned i = 0; i < ARRAY_LENGTH(swap_damage_ext_to_entrypoint); 
i++) {
+   for (i = 0; i < ARRAY_LENGTH(swap_damage_ext_to_entrypoint); i++) {
if (weston_check_egl_extension(extensions,
swap_damage_ext_to_entrypoint[i].extension)) {
gr->swap_buffers_with_damage =
-- 
1.9.1

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

Re: [PATCH weston v2] compositor-wayland: Port to xdg-shell-v6

[Armin Krezović]

On 21.11.2016 18:42, Armin Krezović wrote:
> v2:
> 
>  - Keep wl_shell code around until xdg_shell is declared stable.
> 
> Signed-off-by: Armin Krezović 

I hereby agree that the following changes can be merged as part of this patch:

https://phabricator.freedesktop.org/P6

> ---
>  Makefile.am|   8 ++-
>  libweston/compositor-wayland.c | 146 
> +
>  2 files changed, 139 insertions(+), 15 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index b08ce71..b90c4c8 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -391,9 +391,11 @@ wayland_backend_la_CFLAGS =  \
>   $(CAIRO_CFLAGS) \
>   $(WAYLAND_COMPOSITOR_CFLAGS)\
>   $(AM_CFLAGS)
> -wayland_backend_la_SOURCES = \
> - libweston/compositor-wayland.c  \
> - libweston/compositor-wayland.h  \
> +wayland_backend_la_SOURCES = \
> + libweston/compositor-wayland.c  \
> + libweston/compositor-wayland.h  \
> + protocol/xdg-shell-unstable-v6-protocol.c   \
> + protocol/xdg-shell-unstable-v6-client-protocol.h\
>   shared/helpers.h
>  nodist_wayland_backend_la_SOURCES =  \
>   protocol/fullscreen-shell-unstable-v1-protocol.c\
> diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
> index 775da25..6ac11eb 100644
> --- a/libweston/compositor-wayland.c
> +++ b/libweston/compositor-wayland.c
> @@ -51,6 +51,7 @@
>  #include "shared/os-compatibility.h"
>  #include "shared/cairo-util.h"
>  #include "fullscreen-shell-unstable-v1-client-protocol.h"
> +#include "xdg-shell-unstable-v6-client-protocol.h"
>  #include "presentation-time-server-protocol.h"
>  #include "linux-dmabuf.h"
>  #include "windowed-output-api.h"
> @@ -66,6 +67,7 @@ struct wayland_backend {
>   struct wl_registry *registry;
>   struct wl_compositor *compositor;
>   struct wl_shell *shell;
> + struct zxdg_shell_v6 *xdg_shell;
>   struct zwp_fullscreen_shell_v1 *fshell;
>   struct wl_shm *shm;
>  
> @@ -98,7 +100,10 @@ struct wayland_output {
>   uint32_t global_id;
>  
>   struct wl_shell_surface *shell_surface;
> + struct zxdg_surface_v6 *xdg_surface;
> + struct zxdg_toplevel_v6 *xdg_toplevel;
>   int configure_width, configure_height;
> + bool wait_for_configure;
>   } parent;
>  
>   int keyboard_count;
> @@ -623,6 +628,12 @@ wayland_output_repaint_pixman(struct weston_output 
> *output_base,
>  static void
>  wayland_backend_destroy_output_surface(struct wayland_output *output)
>  {
> + if (output->parent.xdg_toplevel)
> + zxdg_toplevel_v6_destroy(output->parent.xdg_toplevel);
> +
> + if (output->parent.xdg_surface)
> + zxdg_surface_v6_destroy(output->parent.xdg_surface);
> +
>   if (output->parent.shell_surface)
>   wl_shell_surface_destroy(output->parent.shell_surface);
>  
> @@ -822,6 +833,9 @@ wayland_output_set_windowed(struct wayland_output *output)
>   title = strdup(WINDOW_TITLE);
>   }
>  
> + if (output->parent.xdg_toplevel)
> + zxdg_toplevel_v6_set_title(output->parent.xdg_toplevel, title);
> +
>   if (!b->theme) {
>   b->theme = theme_create();
>   if (!b->theme) {
> @@ -840,7 +854,8 @@ wayland_output_set_windowed(struct wayland_output *output)
>  
>   wayland_output_resize_surface(output);
>  
> - wl_shell_surface_set_toplevel(output->parent.shell_surface);
> + if (output->parent.shell_surface)
> + wl_shell_surface_set_toplevel(output->parent.shell_surface);
>  
>   return 0;
>  }
> @@ -860,7 +875,9 @@ wayland_output_set_fullscreen(struct wayland_output 
> *output,
>  
>   wayland_output_resize_surface(output);
>  
> - if (output->parent.shell_surface) {
> + if (output->parent.xdg_toplevel) {
> + zxdg_toplevel_v6_set_fullscreen(output->parent.xdg_toplevel, 
> target);
> + } else if (output->parent.shell_surface) {
>   wl_shell_surface_set_fullscreen(output->parent.shell_surface,
>   method, framerate, target);
>   } else if (b->parent.fshell) {
> @@ -961,7 +978,7 @@ wayland_output_switch_mode(struct weston_output 
> *output_base,
>  
>   b = to_wayland_backend(output_base->compositor);
>  
> - if (output->parent.shell_surface || !b->parent.fshell)
> + if (output->parent.xdg_surface || output->parent.shell_surface || 
> !b->parent.fshell)
>   return -1;
>  
>   mode = wayland_output_choose_mode(output, mode);
> @@ -1033,6 +1050,41 @@ err_output:
>   return -1;
>  }
>  
> +static void
> 

Re: [weston] linux-dmabuf: align DMABUF exposed formats with EGL supported formats

[Daniel Stone]

Hi Vincent,

On 7 October 2016 at 16:08, Vincent Abriou  wrote:
> @@ -72,6 +72,9 @@ install-libweston_moduleLTLIBRARIES 
> install-moduleLTLIBRARIES: install-libLTLIBR
>  lib_LTLIBRARIES = libweston-@LIBWESTON_MAJOR@.la
>  libweston_@LIBWESTON_MAJOR@_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
>  libweston_@LIBWESTON_MAJOR@_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) 
> $(LIBUNWIND_CFLAGS)
> +if ENABLE_DRM_COMPOSITOR
> +libweston_@LIBWESTON_MAJOR@_la_CFLAGS += $(DRM_COMPOSITOR_CFLAGS)
> +endif
>  libweston_@LIBWESTON_MAJOR@_la_LIBADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) 
> \
> $(DLOPEN_LIBS) -lm $(CLOCK_GETTIME_LIBS) \
> $(LIBINPUT_BACKEND_LIBS) libshared.la

So, given that the set of formats is defined unconditionally in
gl-renderer, this would break compilation with --disable-drm-backend.
The options that come to mind are either: using wl_shm enums for these
formats, shipping a local copy of drm_fourcc.h, or mandating the DRM
backend for dmabuf. Of those, I'd prefer the first, I think.

I did merge your YUYV patch, though: thanks!

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

[PATCH weston v2] compositor-wayland: Port to xdg-shell-v6

[Armin Krezović]

v2:

 - Keep wl_shell code around until xdg_shell is declared stable.

Signed-off-by: Armin Krezović 
---
 Makefile.am|   8 ++-
 libweston/compositor-wayland.c | 146 +
 2 files changed, 139 insertions(+), 15 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index b08ce71..b90c4c8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -391,9 +391,11 @@ wayland_backend_la_CFLAGS =\
$(CAIRO_CFLAGS) \
$(WAYLAND_COMPOSITOR_CFLAGS)\
$(AM_CFLAGS)
-wayland_backend_la_SOURCES =   \
-   libweston/compositor-wayland.c  \
-   libweston/compositor-wayland.h  \
+wayland_backend_la_SOURCES =   \
+   libweston/compositor-wayland.c  \
+   libweston/compositor-wayland.h  \
+   protocol/xdg-shell-unstable-v6-protocol.c   \
+   protocol/xdg-shell-unstable-v6-client-protocol.h\
shared/helpers.h
 nodist_wayland_backend_la_SOURCES =\
protocol/fullscreen-shell-unstable-v1-protocol.c\
diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index 775da25..6ac11eb 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -51,6 +51,7 @@
 #include "shared/os-compatibility.h"
 #include "shared/cairo-util.h"
 #include "fullscreen-shell-unstable-v1-client-protocol.h"
+#include "xdg-shell-unstable-v6-client-protocol.h"
 #include "presentation-time-server-protocol.h"
 #include "linux-dmabuf.h"
 #include "windowed-output-api.h"
@@ -66,6 +67,7 @@ struct wayland_backend {
struct wl_registry *registry;
struct wl_compositor *compositor;
struct wl_shell *shell;
+   struct zxdg_shell_v6 *xdg_shell;
struct zwp_fullscreen_shell_v1 *fshell;
struct wl_shm *shm;
 
@@ -98,7 +100,10 @@ struct wayland_output {
uint32_t global_id;
 
struct wl_shell_surface *shell_surface;
+   struct zxdg_surface_v6 *xdg_surface;
+   struct zxdg_toplevel_v6 *xdg_toplevel;
int configure_width, configure_height;
+   bool wait_for_configure;
} parent;
 
int keyboard_count;
@@ -623,6 +628,12 @@ wayland_output_repaint_pixman(struct weston_output 
*output_base,
 static void
 wayland_backend_destroy_output_surface(struct wayland_output *output)
 {
+   if (output->parent.xdg_toplevel)
+   zxdg_toplevel_v6_destroy(output->parent.xdg_toplevel);
+
+   if (output->parent.xdg_surface)
+   zxdg_surface_v6_destroy(output->parent.xdg_surface);
+
if (output->parent.shell_surface)
wl_shell_surface_destroy(output->parent.shell_surface);
 
@@ -822,6 +833,9 @@ wayland_output_set_windowed(struct wayland_output *output)
title = strdup(WINDOW_TITLE);
}
 
+   if (output->parent.xdg_toplevel)
+   zxdg_toplevel_v6_set_title(output->parent.xdg_toplevel, title);
+
if (!b->theme) {
b->theme = theme_create();
if (!b->theme) {
@@ -840,7 +854,8 @@ wayland_output_set_windowed(struct wayland_output *output)
 
wayland_output_resize_surface(output);
 
-   wl_shell_surface_set_toplevel(output->parent.shell_surface);
+   if (output->parent.shell_surface)
+   wl_shell_surface_set_toplevel(output->parent.shell_surface);
 
return 0;
 }
@@ -860,7 +875,9 @@ wayland_output_set_fullscreen(struct wayland_output *output,
 
wayland_output_resize_surface(output);
 
-   if (output->parent.shell_surface) {
+   if (output->parent.xdg_toplevel) {
+   zxdg_toplevel_v6_set_fullscreen(output->parent.xdg_toplevel, 
target);
+   } else if (output->parent.shell_surface) {
wl_shell_surface_set_fullscreen(output->parent.shell_surface,
method, framerate, target);
} else if (b->parent.fshell) {
@@ -961,7 +978,7 @@ wayland_output_switch_mode(struct weston_output 
*output_base,
 
b = to_wayland_backend(output_base->compositor);
 
-   if (output->parent.shell_surface || !b->parent.fshell)
+   if (output->parent.xdg_surface || output->parent.shell_surface || 
!b->parent.fshell)
return -1;
 
mode = wayland_output_choose_mode(output, mode);
@@ -1033,6 +1050,41 @@ err_output:
return -1;
 }
 
+static void
+handle_xdg_surface_configure(void *data, struct zxdg_surface_v6 *surface,
+uint32_t serial)
+{
+   zxdg_surface_v6_ack_configure(surface, serial);
+}
+
+static const struct zxdg_surface_v6_listener xdg_surface_listener = {
+   handle_xdg_surface_configure
+};
+
+static void

Re: [PATCH wayland 1/4] wayland-util: do not export the wl_map_* API

[Daniel Stone]

Hi Emil,

On 30 August 2016 at 18:24, Emil Velikov  wrote:
> From: Emil Velikov 
>
> Use only internally and explicitly marked as such with commit
> cf04b0a18f2 ("Move private definitions and prototypes to new
> zwayland-private.h")
>
> Signed-off-by: Emil Velikov 
> ---
> I could not find any users of the API and I doubt there was ever one. If
> people feel nervous about this, we can keep it.

For the actual series, detaching the discussion from the
wayland-scanner bits in 0/4:

I think this one is fine, but I'd prefer to merge it at the same time
as the wayland-util split, if and when that happens.

Patch 2/4 (to move to the -private file) no longer applies, because we
let this series bitrot for so long. Sorry.

Patch 4/4 removes whitespace from the other -uninstalled.pc.in files,
but you add the same whitespace into the wayland-util file introduced
in 3/4 and don't fix it up in 4/4.

As for the actual libwayland-util split, I'm very much on the fence as
to whether it's a good idea. Broadly speaking I do like the idea and
sympathise with the aims, but am not sure how happy distro packagers
would be with an extra binary package to track. What really worries me
though, is transient symbol dependencies: at least with the pkg-config
modifications as-is in 3/4, with wayland-util dropping back to
Requires.private, I believe we'd see the compiler/linker complaining
that a project directly using symbols from wayland-util does not
directly link to it, only transiently via libwayland-{client,server}.
They could fix that by requiring wayland-util, but then they'd need
versioned fallbacks, and we've just made it a fair bit harder for
people to properly link to it.

Did you test with something that only has
wayland-client/wayland-server (and/or -uninstalled variants) in the
pkg-config file, directly using wl_list/wl_array/etc, and see if they
generated any warnings?

If we could be sure that was not the case, that would soothe my concerns a bit.

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

Re: [PATCH wayland v2] util: Clarify documentation of wl_dispatcher_func_t

[Daniel Stone]

Hi Yong,

On 21 November 2016 at 13:44, Yong Bakos  wrote:
> - * A dispatcher takes five arguments:  The first is the dispatcher-specific
> - * implementation data associated with the target object.  The second is the
> - * object on which the callback is being invoked (either wl_proxy or
> - * wl_resource).  The third and fourth arguments are the opcode the 
> wl_message
> - * structure corresponding to the callback being emitted.  The final argument
> - * is an array of arguments received from the other process via the wire
> - * protocol.
> + * A dispatcher takes five arguments: The first is the dispatcher-specific
> + * implementation associated with the target object. The second is the object
> + * upon which the callback is being invoked (either wl_proxy or wl_resource).
> + * The third and fourth arguments are the opcode and the wl_message
> + * corresponding to the callback. The final argument is an array of arguments
> + * received from the other process via the wire protocol.

I think removing the final word 'the dispatcher-specific
implementation data' is a loss of precision/accuracy. If you don't
mind reinstating that final word, I'll merge with my R-b.

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

Re: [PATCH weston 09/10] compositor-wayland: Port to xdg-shell-v6

[Armin Krezović]

On 21.11.2016 17:37, Daniel Stone wrote:
> Hi Armin,
> 
> On 9 October 2016 at 16:30, Armin Krezović  wrote:
>> Signed-off-by: Armin Krezović 
> 
> Could you please spin another version which doesn't jettison wl_shell
> support? I know it's lame, but it's the only thing which is actually
> stable enough to predictably run across a wide range of versions.
> Until xdg_shell has managed to ship one stable version for a little
> while, I think we need to keep wl_shell for our Wayland backend.
> (Ideally also for simple-shm/simple-egl: when the initial xdg_shell
> conversion landed and wl_shell was removed, I think everyone was under
> the impression it would stabilise far sooner than it has ...)
> 
> Cheers,
> Daniel
> 

Alright, fair enough.



signature.asc
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 1/2] config-parser: add support for more color formats

[Daniel Stone]

Hi Eric,

On 26 October 2016 at 23:53, Eric Engestrom  wrote:
> Supported colour formats are:
> - (0x)(AA)RRGGBB
> - #RGB(A)
> - #RRGGBB(AA)

If I'm honest, I don't entirely see the value in these. Accepting
12345678 as well as 0x12345678 implies, to me at least, that the
former will be interpreted as decimal and the latter as hexadecimal.
Obviously it would be better if we'd only accepted 0x for hex in the
first place, but here we are. I'd still rather not increase the
confusion by accepting both at this point.

I similarly don't think the # adds that much value (people can figure
it out easily enough), and the half-precision variant is rare enough
that I'm not sure it's worth supporting.

So, exceedingly long lines notwithstanding, it's a nice enough patch,
but I'd rather just leave it be, suboptimal though the current
situation is. Thanks and sorry.

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

Re: [PATCH weston 1/2] compositor-x11: Move vfunc setting from set_size to enable

[Daniel Stone]

Hi Armin,

On 28 October 2016 at 23:26, Armin Krezović  wrote:
> Signed-off-by: Armin Krezović 

I've pushed this one, but need to think some more about the resizing.

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

Re: [PATCH weston 08/10] compositor-wayland: Destroy shm buffers on output disable

[Daniel Stone]

Hi,

On 9 October 2016 at 16:30, Armin Krezović  wrote:
> Signed-off-by: Armin Krezović 

Meanwhile, I've reviewed and pushed the series up to this point. Thanks!

To ssh://git.freedesktop.org/git/wayland/weston
   e31d95f..2e66252  upstream -> master

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

Re: [PATCH weston 10/10] compositor-wayland: Simplify fullscreening for sprawl use case

[Daniel Stone]

Hi Armin,

On 9 October 2016 at 16:30, Armin Krezović  wrote:
> wayland_output_set_fullscreen() already takes care of
> everything and xdg_shell doesn't use any present methods
> so a single call to wayland_output_set_fullscreen() is
> sufficient.

Ha, clever. I kind of wish I'd looked at this closer before.

The fullscreen-shell says that a client (should) use 'either the
present_surface or the present_surface_for_mode requests', but this
actually uses both. It ignores the present_surface_for_mode feedback
object, so if a mode switch fails, it relies on the normal
present_surface request having made fullscreen live. I don't really
see anything in the protocol text which suggests that this is
something which should work portably.

Maybe a better approach, whilst you're here, would be to not destroy
the result of present_surface_for_mode, and then fall back to
present_surface if the feedback object returns failed? Of course just
directly calling present_surface if we're not sprawling.

Sorry, I know this isn't your doing, but ...

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

Re: [PATCH weston 09/10] compositor-wayland: Port to xdg-shell-v6

[Daniel Stone]

Hi Armin,

On 9 October 2016 at 16:30, Armin Krezović  wrote:
> Signed-off-by: Armin Krezović 

Could you please spin another version which doesn't jettison wl_shell
support? I know it's lame, but it's the only thing which is actually
stable enough to predictably run across a wide range of versions.
Until xdg_shell has managed to ship one stable version for a little
while, I think we need to keep wl_shell for our Wayland backend.
(Ideally also for simple-shm/simple-egl: when the initial xdg_shell
conversion landed and wl_shell was removed, I think everyone was under
the impression it would stabilise far sooner than it has ...)

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

Re: [PATCH weston v3] build: add -uninstalled.pc.in files for libweston and libweston-desktop

[Daniel Stone]

Hey Derek, Reynaldo,

On 7 September 2016 at 18:58, Derek Foreman  wrote:
> diff --git a/libweston-desktop/libweston-desktop-uninstalled.pc.in 
> b/libweston-desktop/libweston-desktop-uninstalled.pc.in
> new file mode 100644
> index 000..04af505
> --- /dev/null
> +++ b/libweston-desktop/libweston-desktop-uninstalled.pc.in
> @@ -0,0 +1,9 @@
> +libdir=@abs_builddir@/.libs
> +includedir=@abs_srcdir@
> +
> +Name: libweston-desktop
> +Description: Desktop shells abstraction library for libweston compositors 
> (not installed)
> +Version: @WESTON_VERSION@
> +Requires.private: libweston-@LIBWESTON_MAJOR@ wayland-server

This should be libweston-uninstalled, so we don't pick up the wrong version.

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

Re: [PATCH] weston-editor: Free preferred_language in text_entry_destroy

[Daniel Stone]

Hi,

On 17 November 2016 at 23:01, Bryce Harrington  wrote:
> On Thu, Nov 17, 2016 at 09:43:06PM +0100, Silvan Jegen wrote:
>> Signed-off-by: Silvan Jegen 
>
> Yep, the value is set via a strdup in main(), so needs free'd.
>
> Reviewed-by: Bryce Harrington 

Merged with review, thanks!

To ssh://git.freedesktop.org/git/wayland/weston
   5e60408..e31d95f  upstream -> master

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

Re: [PATCH weston v2 5/8] clients/window: use weston_platform_destroy_egl_surface wrapper

[Daniel Stone]

Hi Emil,

On 18 November 2016 at 19:12, Emil Velikov  wrote:
> From: Emil Velikov 
>
> v2: Use correct (destroy) API (Dan)

All merged with review now, thanks!

To ssh://git.freedesktop.org/git/wayland/weston
   43cea54..5e60408  upstream -> master

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

Re: [PATCH weston v5 31/42] compositor-drm: Introduce drm_plane_state structure

[Fabien DESSENNE]

Hi Daniel


Below are two remarks regarding (m/z/c)alloc.

By the way, as a general remark (and maybe a personal opinion) zalloc(x)
is a bit more readable than calloc(1,x)



On 11/16/2016 03:25 PM, Daniel Stone wrote:
> Track dynamic plane state (CRTC, FB, position) in separate structures,
> rather than as part of the plane. This will make it easier to handle
> state management later, and much more closely tracks what the kernel
> does with atomic modesets.
>
> The fb_last pointer previously used in drm_plane now becomes part of
> output->state_last, and is not directly visible from the plane itself.
>
> Signed-off-by: Daniel Stone 
>
> Differential Revision: https://phabricator.freedesktop.org/D1412
> ---
>   libweston/compositor-drm.c | 319 
> -
>   1 file changed, 258 insertions(+), 61 deletions(-)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 1089d77..e80bd71 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -226,6 +226,29 @@ struct drm_edid {
>*/
>   struct drm_output_state {
>  struct drm_output *output;
> +
> +   struct wl_list plane_list;
> +};
> +
> +/**
> + * Plane state holds the dynamic state for a plane: where it is positioned,
> + * and which buffer it is currently displaying.
> + */
> +struct drm_plane_state {
> +   struct drm_plane *plane;
> +   struct drm_output *output;
> +   struct drm_output_state *output_state;
> +
> +   struct drm_fb *fb;
> +
> +   int32_t src_x, src_y;
> +   uint32_t src_w, src_h;
> +   uint32_t dest_x, dest_y;
> +   uint32_t dest_w, dest_h;
> +
> +   bool complete;
> +
> +   struct wl_list link; /* drm_output_state::plane_list */
>   };
>
>   /**
> @@ -244,14 +267,12 @@ struct drm_output_state {
>* are referred to as 'sprites'.
>*/
>   struct drm_plane {
> -   struct wl_list link;
> -
>  struct weston_plane base;
>
> -   struct drm_fb *fb_current, *fb_pending, *fb_last;
> -   struct drm_output *output;
>  struct drm_backend *backend;
>
> +   struct drm_plane_state *state_cur;
> +
>  enum wdrm_plane_type type;
>  struct plane_properties props;
>
> @@ -259,10 +280,7 @@ struct drm_plane {
>  uint32_t plane_id;
>  uint32_t count_formats;
>
> -   int32_t src_x, src_y;
> -   uint32_t src_w, src_h;
> -   uint32_t dest_x, dest_y;
> -   uint32_t dest_w, dest_h;
> +   struct wl_list link;
>
>  uint32_t formats[];
>   };
> @@ -921,6 +939,128 @@ drm_fb_unref(struct drm_fb *fb)
>   }
>
>   /**
> + * Allocate a new, empty, plane state.
> + */
> +static struct drm_plane_state *
> +drm_plane_state_alloc(struct drm_output_state *state_output,
> + struct drm_plane *plane)
> +{
> +   struct drm_plane_state *state = calloc(1, sizeof(*state));
> +
> +   assert(state);
> +   memset(state, 0, sizeof(*state));

Remove this memset (0-allocated memory)


> +   state->output_state = state_output;
> +   state->plane = plane;
> +
> +   /* Here we only add the plane state to the desired link, and not
> +* set the member. Having an output pointer set means that the
> +* plane will be displayed on the output; this won't be the case
> +* when we go to disable a plane. In this case, it must be part of
> +* the commit (and thus the output state), but the member must be
> +* NULL, as it will not be on any output when the state takes
> +* effect.
> +*/
> +   if (state_output)
> +   wl_list_insert(_output->plane_list, >link);
> +   else
> +   wl_list_init(>link);
> +
> +   return state;
> +}
> +
> +/**
> + * Duplicate an existing plane state into a new output state.
> + */
> +static struct drm_plane_state *
> +drm_plane_state_duplicate(struct drm_output_state *state_output,
> + struct drm_plane_state *src)
> +{
> +   struct drm_plane_state *dst = calloc(1, sizeof(*dst));

No need for 0-memory here (memcpy follows), so use malloc() instead of
calloc()

> +
> +   assert(src);
> +   assert(dst);
> +   memcpy(dst, src, sizeof(*dst));
> +   wl_list_insert(_output->plane_list, >link);
> +   if (src->fb)
> +   dst->fb = drm_fb_ref(src->fb);
> +   dst->output_state = state_output;
> +   dst->complete = false;
> +
> +   return dst;
> +}
> +
> +/**
> + * Free an existing plane state. As a special case, the state will not
> + * normally be freed if it is the current state; see drm_plane_set_state.
> + */
> +static void
> +drm_plane_state_free(struct drm_plane_state *state, bool force)
> +{
> +   if (!state)
> +   return;
> +
> +   wl_list_remove(>link);
> +   wl_list_init(>link);
> +   state->output_state = NULL;
> +
> +   if (force || state != state->plane->state_cur) {
> +   

Re: [PATCH wayland] doc: start documenting Xwayland

[Pekka Paalanen]

On Mon, 21 Nov 2016 14:31:43 +0200
Pekka Paalanen  wrote:

> From: Pekka Paalanen 
> 
> This is a rough intro to what Xwayland is and does, with just one
> implementation detail so far (Window identification).
> 
> I paid no attention to formatting details, those can be polished in
> follow-ups. I just want the prose out.
> 
> I also just quickly whacked up the diagram, would be happy to see
> someone replace it with a nicer one. I just didn't have time to learn
> dot for now.
> 
> Cc: Olivier Fourdan 
> Cc: Jonas Ådahl 
> Cc: Daniel Stone 
> Signed-off-by: Pekka Paalanen 
> ---
>  doc/publican/Makefile.am   |   5 +-
>  doc/publican/sources/Wayland.xml   |   1 +
>  doc/publican/sources/Xwayland.xml  | 172 
> +
>  .../sources/images/xwayland-architecture.png   | Bin 0 -> 7611 bytes
>  4 files changed, 177 insertions(+), 1 deletion(-)
>  create mode 100644 doc/publican/sources/Xwayland.xml
>  create mode 100644 doc/publican/sources/images/xwayland-architecture.png
> 

> diff --git a/doc/publican/sources/Xwayland.xml 
> b/doc/publican/sources/Xwayland.xml
> new file mode 100644
> index 000..ca4025a
> --- /dev/null
> +++ b/doc/publican/sources/Xwayland.xml
> @@ -0,0 +1,172 @@
> +
> + "http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd; [
> +
> +%BOOK_ENTITIES;
> +]>  
> +
> +  X11 Application Support
> +  
> +Introduction
> +
> +  Being able to run existing X11 applications is crucial for the adoption
> +  of Wayland, especially on desktops, as there will always be X11
> +  applications that have not been or cannot be converted into Wayland
> +  applications, and throwing them all away would be prohibitive.
> +  Therefore a Wayland compositor often needs to support running X11
> +  applications.
> +
> +
> +  X11 and Wayland are different enough that there is no "simple" way to
> +  translate between them. Most of X11 is uninteresting to a Wayland
> +  compositor. That combined with the gigantic implementation effort 
> needed
> +  to support X11 makes it untractable to just write X11 support directly 
> in
> +  a Wayland compositor. The implementation would be nothing short from a
> +  real X11 server.
> +
> +
> +  Therefore Wayland compositors should use Xwayland, the X11 server that
> +  lives in the Xorg server source code repository and shares most of the
> +  implementation with the Xorg server. Xwayland is a complete X11 server,
> +  just like Xorg is, but instead of talking to hardware and device 
> drivers,
> +  it acts as a Wayland client. The rest of this chapter talks about how
> +  Xwayland works.
> +
> +  
> +  
> +Architecture
> +
> +  A Wayland compositor usually takes care of launching Xwayland.
> +  Xwayland works in cooperation with a Wayland compositor as follows:
> +
> +
> +  Xwayland architecture diagram
> +  
> +
> +  
> + format="PNG" />
> +  
> +
> +  
> +
> +
> +  An X11 application connects to Xwayland just like it would connect to 
> any
> +  X server. Xwayland processes all the X11 requests. On the other end,
> +  Xwayland is a Wayland client that connects to the Wayland compositor.
> +
> +
> +  The X11 window manager (XWM) is an integral part of the Wayland
> +  compositor. XWM uses the usual X11 window management protocol to manage
> +  all X11 windows in Xwayland. Most importantly, XWM acts as a bridge
> +  between Xwayland window state and the Wayland compositor's window 
> manager
> +  (WWM). This way WWM can manage all windows, both native Wayland and X11
> +  (Xwayland) windows. This is very important for a coherent user
> +  experience.
> +
> +
> +  Xwayland uses Wayland core protocol and some extensions for input and
> +  output. The used protocol is all generic and public, there are no
> +  Xwayland-specific or private Wayland extensions being used. Xwayland is
> +  just a regular Wayland client, except it is specially handled by the 
> WWM.
> +  The special handling is due to XWM replacing all shell protocol
> +  extensions that native Wayland applications use.
> +
> +
> +  Since Xwayland uses Wayland for input and output, it does not have any
> +  use for the device drivers that Xorg uses. None of the xf86-video-* or
> +  xf86-input-* modules are used. There also is no configuration file for
> +  the Xwayland server. For optional hardware accelerated rendering,
> +  Xwayland uses GLAMOR.
> +
> +
> +  A Wayland compositor usually spawns only one Xwayland instance. This is
> +  because many X11 applications assume they can communicate 

Re: [PATCH weston v5 30/42] compositor-drm: Introduce drm_output_state structure

[Fabien DESSENNE]

Hi Daniel



On 11/16/2016 03:25 PM, Daniel Stone wrote:
> Currently this doesn't actually really do anything, but will be used in
> the future to track the state for both modeset and repaint requests.
> Completion of the request gives us a single request-completion path for
> both pageflip and vblank events.
>
> Signed-off-by: Daniel Stone 
>
> Differential Revision: https://phabricator.freedesktop.org/D1497
> ---
>   libweston/compositor-drm.c | 230 
> ++---
>   1 file changed, 197 insertions(+), 33 deletions(-)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index c24129b..1089d77 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -120,6 +120,19 @@ struct plane_properties {
>  uint64_t value;
>   };
>
> +/**
> + * Mode for drm_output_state_duplicate.
> + */
> +enum drm_output_state_duplicate_mode {
> +   DRM_OUTPUT_STATE_CLEAR_PLANES, /**< reset all planes to off */
> +   DRM_OUTPUT_STATE_PRESERVE_PLANES, /**< preserve plane state */
> +};
> +
> +enum drm_output_state_update_mode {
> +   DRM_OUTPUT_STATE_UPDATE_SYNCHRONOUS, /**< state already applied */
> +   DRM_OUTPUT_STATE_UPDATE_ASYNCHRONOUS, /**< pending event delivery */
> +};
> +
>   struct drm_backend {
>  struct weston_backend base;
>  struct weston_compositor *compositor;
> @@ -206,6 +219,16 @@ struct drm_edid {
>   };
>
>   /**
> + * Output state holds the dynamic state for one Weston output, i.e. a KMS 
> CRTC,
> + * plus >= 1 each of encoder/connector/plane. Since everything but the planes
> + * is currently statically assigned per-output, we mainly use this to track
> + * plane state.
> + */
> +struct drm_output_state {
> +   struct drm_output *output;
> +};
> +
> +/**
>* A plane represents one buffer, positioned within a CRTC, and stacked
>* relative to other planes on the same CRTC.
>*
> @@ -274,6 +297,10 @@ struct drm_output {
>  struct drm_fb *fb_current, *fb_pending, *fb_last;
>  struct backlight *backlight;
>
> +   struct drm_output_state *state_last;
> +   struct drm_output_state *state_cur;
> +   struct drm_output_state *state_pending;
> +
>  struct drm_fb *dumb[2];
>  pixman_image_t *image[2];
>  int current_image;
> @@ -641,6 +668,9 @@ drm_output_set_cursor(struct drm_output *output);
>   static void
>   drm_output_update_msc(struct drm_output *output, unsigned int seq);
>
> +static void
> +drm_output_destroy(struct weston_output *output_base);
> +
>   static int
>   drm_plane_crtc_supported(struct drm_output *output, struct drm_plane *plane)
>   {
> @@ -890,6 +920,113 @@ drm_fb_unref(struct drm_fb *fb)
>  }
>   }
>
> +/**
> + * Allocate a new, empty drm_output_state. This should not generally be used
> + * in the repaint cycle; see drm_output_state_duplicate.
> + */
> +static struct drm_output_state *
> +drm_output_state_alloc(struct drm_output *output)
> +{
> +   struct drm_output_state *state = calloc(1, sizeof(*state));
> +
> +   state->output = output;
> +
> +   return state;
> +}
> +
> +/**
> + * Duplicate an existing drm_output_state into a new one. This is generally
> + * used during the repaint cycle, to capture the existing state of an output
> + * and modify it to create a new state to be used.
> + *
> + * The mode determines whether the output will be reset to an a blank state,
> + * or an exact mirror of the current state.
> + */
> +static struct drm_output_state *
> +drm_output_state_duplicate(struct drm_output_state *src,
> +  enum drm_output_state_duplicate_mode plane_mode)
> +{
> +   struct drm_output_state *dst = malloc(sizeof(*dst));
> +
> +   assert(dst);
> +   memcpy(dst, src, sizeof(*dst));
> +
> +   return dst;
> +}
> +
> +/**
> + * Free an unused drm_output_state.
> + */
> +static void
> +drm_output_state_free(struct drm_output_state *state)
> +{
> +   if (!state)
> +   return;
> +
> +   free(state);
> +}
> +
> +/**
> + * Mark a drm_output_state (the output's last state) as complete. This 
> handles
> + * any post-completion actions such as updating the repaint timer, disabling 
> the
> + * output, and finally freeing the state.
> + */
> +static void
> +drm_output_update_complete(struct drm_output *output, uint32_t flags,
> +  unsigned int sec, unsigned int usec)
> +{
> +   struct timespec ts;
> +
> +   drm_output_state_free(output->state_last);
> +   output->state_last = NULL;
> +
> +   if (output->destroy_pending) {
> +   drm_output_destroy(>base);
> +   goto out;
> +   }
> +   else if (output->disable_pending) {

Remove 'else'

> +   weston_output_disable(>base);
> +   goto out;
> +   }
> +
> +   ts.tv_sec = sec;
> +   ts.tv_nsec = usec * 1000;
> +   weston_output_finish_frame(>base, , 

Re: [PATCH wayland] util: Clarify documentation of wl_dispatcher_func_t

[Yong Bakos]

Verified that the intent of the return type of wl_dispatcher_func_t is
for an error code. I'm including a response from J Ekstrand, author
of wl_dispatcher_func_t inline below, for the record.

> On Nov 17, 2016, at 8:28 AM, Yong Bakos  wrote:
> 
> On Nov 17, 2016, at 8:23 AM, Yong Bakos  wrote:
>> 
>> From: Yong Bakos 
>> 
>> Adjust the brief, clarify the behavior and arguments, correct a grammar
>> error, and document the parameters.
>> 
>> Signed-off-by: Yong Bakos 
> 
> Eh - what is the int return value for?
> connection.c doesn't use it when invoking a wl_dispatcher_func_t.
> 
> yong

Here...

>> Begin forwarded message:
>> 
>> From: Jason Ekstrand <...>
>> Subject: Re: [PATCH wayland] util: Clarify documentation of 
>> wl_dispatcher_func_t
>> Date: November 20, 2016 at 7:39:04 PM PST
>> To: Yong Bakos <...>
>> 
>> On Sat, Nov 19, 2016 at 10:55 PM, Yong Bakos <...> wrote:
>>> Hi Jason,
>>> I've been working on documentation for Wayland and was wondering if you
>>> might be able to tell me what the intent for the `int` return value of
>>> wl_dispatcher_func_t is for?
>> 
>> First off, I should say that any answers I give are from a very rusty 
>> memory.  It's been about 4 years since I worked on that.  I believe the 
>> intention (which may not have been fully implemented) was for error 
>> handling.  If the dispatcher had an error, it would get propagated up and 
>> returned out of the event loop dispatch function.  Ideally, if working with 
>> a higher-level language such as Java or Python, an exception would get 
>> turned into an error and then back to an exception thrown from the 
>> language-wrapped queue dispatch function.  That was the intention but I'm 
>> not sure if it ever worked quite as well as all that.  It's entirely 
>> possible that libwayland simply ignores it which would make it useless.
>> 
>> --Jason
>>  
>>> Thank you,
>>> yong
>>> 
>>> typedef int (*wl_dispatcher_func_t)(const void *, void *, uint32_t, ...
>> 

Will send a v2 with the return type amendment.

yong


> ---
>> src/wayland-util.h | 25 +++--
>> 1 file changed, 15 insertions(+), 10 deletions(-)
>> 
>> diff --git a/src/wayland-util.h b/src/wayland-util.h
>> index 50f3372..17908fd 100644
>> --- a/src/wayland-util.h
>> +++ b/src/wayland-util.h
>> @@ -607,21 +607,26 @@ union wl_argument {
>> };
>> 
>> /**
>> - * \brief A function pointer type for a dispatcher.
>> + * Dispatcher function type alias
>> *
>> * A dispatcher is a function that handles the emitting of callbacks in client
>> - * code.  For programs directly using the C library, this is done by using
>> - * libffi to call function pointers.  When binding to languages other than 
>> C,
>> + * code. For programs directly using the C library, this is done by using
>> + * libffi to call function pointers. When binding to languages other than C,
>> * dispatchers provide a way to abstract the function calling process to be
>> * friendlier to other function calling systems.
>> *
>> - * A dispatcher takes five arguments:  The first is the dispatcher-specific
>> - * implementation data associated with the target object.  The second is the
>> - * object on which the callback is being invoked (either wl_proxy or
>> - * wl_resource).  The third and fourth arguments are the opcode the 
>> wl_message
>> - * structure corresponding to the callback being emitted.  The final 
>> argument
>> - * is an array of arguments received from the other process via the wire
>> - * protocol.
>> + * A dispatcher takes five arguments: The first is the dispatcher-specific
>> + * implementation associated with the target object. The second is the 
>> object
>> + * upon which the callback is being invoked (either wl_proxy or 
>> wl_resource).
>> + * The third and fourth arguments are the opcode and the wl_message
>> + * corresponding to the callback. The final argument is an array of 
>> arguments
>> + * received from the other process via the wire protocol.
>> + *
>> + * \param "const void *" Dispatcher-specific implementation data
>> + * \param "void *" Callback invocation target (wl_proxy or `wl_resource`)
>> + * \param uint32_t Callback opcode
>> + * \param "const struct wl_message *" Callback message signature
>> + * \param "union wl_argument *" Array of received arguments
>> */
>> typedef int (*wl_dispatcher_func_t)(const void *, void *, uint32_t,
>>  const struct wl_message *,
>> -- 
>> 2.7.2
>> 
>> ___
>> wayland-devel mailing list
>> wayland-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel

___
wayland-devel mailing list

Re: [PATCH] server: Fix crash when accessing client which is already freed

[Daniel Stone]

Hi Pekka,

On 21 November 2016 at 13:30, Pekka Paalanen  wrote:
> On Mon, 21 Nov 2016 12:19:43 + Daniel Stone  wrote:
>> I'm would suggest we document that invoking wl_display_flush_clients() /
>> wl_connection_flush() from a request handler may cause undefined behaviour,
>> and that it must only be called outside of dispatch.
>
> FWIW, I agree with Daniel here. There are several things that are not
> quite defined or expected to be done inside dispatch calls, and
> flushing all clients is one of them.
>
> I could see, IIRC, how sending lots of events from inside dispatch
> could cause the one client to be flushed. I think flush is done
> automatically if the send buffer fills up. If the flush at that point
> fails, there is no way to signal back that "oops, the compositor should
> put that thing aside and do something else" until the connection
> unblocks. We don't want to be checking and handling every send-event
> possibly failing, so killing the client can happen. ISTR we don't
> dynamically grow the send buffers, do we? But even the growing would
> need to stop somewhere.

Two paths, depending on whether the event is queued or immediately
posted. If the latter, then the event will be flushed immediately. If
the former, _and_ the queue is already full, then the queue will be
flushed immediately before the event is written to the queue. In
either case, if the flush fails, then client->error will be set. Since
we work exclusively on NOWAIT connections, this includes if the kernel
buffers are full because the client hasn't yet read.

Either way, this is not related to the above patch, because it takes a
different codepath (wl_resource_queue_event_array -> wl_closure_queue
-> wl_connection_queue -> wl_connection_flush /
wl_resource_post_event_array -> wl_closure_send -> wl_connection_write
-> wl_connection_flush), not involving calling
wl_display_flush_clients(). This implicit mid-dispatch flush is
completely fine: the patch only fixes the compositor _explicitly_
flushing _all_ clients in the middle of request dispatch.

(Side note: I suspect there might be some scope for increasing our -
currently static - buffer limit, but making that limit too high just
shifts the problem of unresponsive clients. But this also does not
have any bearing on this patch.)

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

[PATCH wayland v2] util: Clarify documentation of wl_dispatcher_func_t

[Yong Bakos]

From: Yong Bakos 

Adjust the brief, clarify the behavior and arguments, correct a grammar
error, document the parameters, and document the return type.

Signed-off-by: Yong Bakos 
Reviewed-by: Bryce Harrington 
---
v2: Document intent of return type.

 src/wayland-util.h | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/src/wayland-util.h b/src/wayland-util.h
index 170bf86..f0a4508 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -662,21 +662,28 @@ union wl_argument {
 };

 /**
- * \brief A function pointer type for a dispatcher.
+ * Dispatcher function type alias
  *
  * A dispatcher is a function that handles the emitting of callbacks in client
- * code.  For programs directly using the C library, this is done by using
- * libffi to call function pointers.  When binding to languages other than C,
+ * code. For programs directly using the C library, this is done by using
+ * libffi to call function pointers. When binding to languages other than C,
  * dispatchers provide a way to abstract the function calling process to be
  * friendlier to other function calling systems.
  *
- * A dispatcher takes five arguments:  The first is the dispatcher-specific
- * implementation data associated with the target object.  The second is the
- * object on which the callback is being invoked (either wl_proxy or
- * wl_resource).  The third and fourth arguments are the opcode the wl_message
- * structure corresponding to the callback being emitted.  The final argument
- * is an array of arguments received from the other process via the wire
- * protocol.
+ * A dispatcher takes five arguments: The first is the dispatcher-specific
+ * implementation associated with the target object. The second is the object
+ * upon which the callback is being invoked (either wl_proxy or wl_resource).
+ * The third and fourth arguments are the opcode and the wl_message
+ * corresponding to the callback. The final argument is an array of arguments
+ * received from the other process via the wire protocol.
+ *
+ * \param "const void *" Dispatcher-specific implementation data
+ * \param "void *" Callback invocation target (wl_proxy or `wl_resource`)
+ * \param uint32_t Callback opcode
+ * \param "const struct wl_message *" Callback message signature
+ * \param "union wl_argument *" Array of received arguments
+ *
+ * \return 0 on success, or -1 on failure
  */
 typedef int (*wl_dispatcher_func_t)(const void *, void *, uint32_t,
const struct wl_message *,
--
2.7.2

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

Re: [PATCH wayland] util: Clarify documentation of wl_dispatcher_func_t

[Yong Bakos]

Please disregard, need to annotate the v2. Apologies for the noise.

y


> On Nov 21, 2016, at 5:40 AM, Yong Bakos  wrote:
> 
> From: Yong Bakos 
> 
> Adjust the brief, clarify the behavior and arguments, correct a grammar
> error, document the parameters, and document the return type.
> 
> Signed-off-by: Yong Bakos 
> Reviewed-by: Bryce Harrington 
> ---
> src/wayland-util.h | 27 +--
> 1 file changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/src/wayland-util.h b/src/wayland-util.h
> index 170bf86..f0a4508 100644
> --- a/src/wayland-util.h
> +++ b/src/wayland-util.h
> @@ -662,21 +662,28 @@ union wl_argument {
> };
> 
> /**
> - * \brief A function pointer type for a dispatcher.
> + * Dispatcher function type alias
>  *
>  * A dispatcher is a function that handles the emitting of callbacks in client
> - * code.  For programs directly using the C library, this is done by using
> - * libffi to call function pointers.  When binding to languages other than C,
> + * code. For programs directly using the C library, this is done by using
> + * libffi to call function pointers. When binding to languages other than C,
>  * dispatchers provide a way to abstract the function calling process to be
>  * friendlier to other function calling systems.
>  *
> - * A dispatcher takes five arguments:  The first is the dispatcher-specific
> - * implementation data associated with the target object.  The second is the
> - * object on which the callback is being invoked (either wl_proxy or
> - * wl_resource).  The third and fourth arguments are the opcode the 
> wl_message
> - * structure corresponding to the callback being emitted.  The final argument
> - * is an array of arguments received from the other process via the wire
> - * protocol.
> + * A dispatcher takes five arguments: The first is the dispatcher-specific
> + * implementation associated with the target object. The second is the object
> + * upon which the callback is being invoked (either wl_proxy or wl_resource).
> + * The third and fourth arguments are the opcode and the wl_message
> + * corresponding to the callback. The final argument is an array of arguments
> + * received from the other process via the wire protocol.
> + *
> + * \param "const void *" Dispatcher-specific implementation data
> + * \param "void *" Callback invocation target (wl_proxy or `wl_resource`)
> + * \param uint32_t Callback opcode
> + * \param "const struct wl_message *" Callback message signature
> + * \param "union wl_argument *" Array of received arguments
> + *
> + * \return 0 on success, or -1 on failure
>  */
> typedef int (*wl_dispatcher_func_t)(const void *, void *, uint32_t,
>   const struct wl_message *,
> -- 
> 2.7.2
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

Re: [PATCH weston v5 21/42] compositor-drm: Return FB directly from render

[Fabien DESSENNE]

Hi Daniel,


On 11/16/2016 03:25 PM, Daniel Stone wrote:
> Instead of setting state members directly in the drm_output_render
> functions (to paint using Pixman or GL), just return a drm_fb, and let
> the core function place it in state.
>
> Signed-off-by: Daniel Stone 
>
> Differential Revision: https://phabricator.freedesktop.org/D1419
> ---
>   libweston/compositor-drm.c | 30 +++---
>   1 file changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 88a890e..ee66dbe 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -601,11 +601,12 @@ drm_output_prepare_scanout_view(struct drm_output 
> *output,
>   return >fb_plane;
>   }
>   
> -static void
> +static struct drm_fb *
>   drm_output_render_gl(struct drm_output *output, pixman_region32_t *damage)
>   {
>   struct drm_backend *b = to_drm_backend(output->base.compositor);
>   struct gbm_bo *bo;
> + struct drm_fb *ret;
>   
>   output->base.compositor->renderer->repaint_output(>base,
> damage);
> @@ -613,20 +614,21 @@ drm_output_render_gl(struct drm_output *output, 
> pixman_region32_t *damage)
>   bo = gbm_surface_lock_front_buffer(output->gbm_surface);
>   if (!bo) {
>   weston_log("failed to lock front buffer: %m\n");
> - return;
> + return NULL;
>   }
>   
> - output->fb_pending = drm_fb_get_from_bo(bo, b, output->gbm_format);
> - if (!output->fb_pending) {
> + ret = drm_fb_get_from_bo(bo, b, output->gbm_format);
> + if (!ret) {
>   weston_log("failed to get drm_fb for bo\n");
>   gbm_surface_release_buffer(output->gbm_surface, bo);
> - return;
return NULL missing
>   }
> - output->fb_pending->type = BUFFER_GBM_SURFACE;
> - output->fb_pending->gbm_surface = output->gbm_surface;
> + ret->type = BUFFER_GBM_SURFACE;
> + ret->gbm_surface = output->gbm_surface;
> +
> + return ret;
>   }
>   
> -static void
> +static struct drm_fb *
>   drm_output_render_pixman(struct drm_output *output, pixman_region32_t 
> *damage)
>   {
>   struct weston_compositor *ec = output->base.compositor;
> @@ -642,7 +644,6 @@ drm_output_render_pixman(struct drm_output *output, 
> pixman_region32_t *damage)
>   
>   output->current_image ^= 1;
>   
> - output->fb_pending = drm_fb_ref(output->dumb[output->current_image]);
>   pixman_renderer_output_set_buffer(>base,
> output->image[output->current_image]);
>   
> @@ -650,6 +651,8 @@ drm_output_render_pixman(struct drm_output *output, 
> pixman_region32_t *damage)
>   
>   pixman_region32_fini(_damage);
>   pixman_region32_fini(_damage);
> +
> + return drm_fb_ref(output->dumb[output->current_image]);
>   }
>   
>   static void
> @@ -657,6 +660,7 @@ drm_output_render(struct drm_output *output, 
> pixman_region32_t *damage)
>   {
>   struct weston_compositor *c = output->base.compositor;
>   struct drm_backend *b = to_drm_backend(c);
> + struct drm_fb *fb;
>   
>   /* If we already have a client buffer promoted to scanout, then we don't
>* want to render. */
> @@ -664,9 +668,13 @@ drm_output_render(struct drm_output *output, 
> pixman_region32_t *damage)
>   return;
>   
>   if (b->use_pixman)
> - drm_output_render_pixman(output, damage);
> + fb = drm_output_render_pixman(output, damage);
>   else
> - drm_output_render_gl(output, damage);
> + fb = drm_output_render_gl(output, damage);
> +
> + if (!fb)
> + return;
> + output->fb_pending = fb;
>   
>   pixman_region32_subtract(>primary_plane.damage,
>>primary_plane.damage, damage);
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland] util: Clarify documentation of wl_dispatcher_func_t

[Yong Bakos]

From: Yong Bakos 

Adjust the brief, clarify the behavior and arguments, correct a grammar
error, document the parameters, and document the return type.

Signed-off-by: Yong Bakos 
Reviewed-by: Bryce Harrington 
---
 src/wayland-util.h | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/src/wayland-util.h b/src/wayland-util.h
index 170bf86..f0a4508 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -662,21 +662,28 @@ union wl_argument {
 };
 
 /**
- * \brief A function pointer type for a dispatcher.
+ * Dispatcher function type alias
  *
  * A dispatcher is a function that handles the emitting of callbacks in client
- * code.  For programs directly using the C library, this is done by using
- * libffi to call function pointers.  When binding to languages other than C,
+ * code. For programs directly using the C library, this is done by using
+ * libffi to call function pointers. When binding to languages other than C,
  * dispatchers provide a way to abstract the function calling process to be
  * friendlier to other function calling systems.
  *
- * A dispatcher takes five arguments:  The first is the dispatcher-specific
- * implementation data associated with the target object.  The second is the
- * object on which the callback is being invoked (either wl_proxy or
- * wl_resource).  The third and fourth arguments are the opcode the wl_message
- * structure corresponding to the callback being emitted.  The final argument
- * is an array of arguments received from the other process via the wire
- * protocol.
+ * A dispatcher takes five arguments: The first is the dispatcher-specific
+ * implementation associated with the target object. The second is the object
+ * upon which the callback is being invoked (either wl_proxy or wl_resource).
+ * The third and fourth arguments are the opcode and the wl_message
+ * corresponding to the callback. The final argument is an array of arguments
+ * received from the other process via the wire protocol.
+ *
+ * \param "const void *" Dispatcher-specific implementation data
+ * \param "void *" Callback invocation target (wl_proxy or `wl_resource`)
+ * \param uint32_t Callback opcode
+ * \param "const struct wl_message *" Callback message signature
+ * \param "union wl_argument *" Array of received arguments
+ *
+ * \return 0 on success, or -1 on failure
  */
 typedef int (*wl_dispatcher_func_t)(const void *, void *, uint32_t,
const struct wl_message *,
-- 
2.7.2

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

Re: [PATCH] server: Fix crash when accessing client which is already freed

[Pekka Paalanen]

On Mon, 21 Nov 2016 12:19:43 +
Daniel Stone  wrote:

> Hi Hyun Kook,
> 
> On 23 September 2016 at 00:40, Hyun Kook Khang 
> wrote:
> 
> > in wl_client_connection_data(),
> > there is no guarantee that client would never be destroyed in the process
> > of handling the pending input.
> >
> > For example,
> > 1. It comes to wl_client_connection_data()
> > 2. wl_closure_invoke() would be invoked, and then wl_display_flush_clients
> > would be invoked anyhow later in the same routine. (wl_closure_invoke() ->
> > ? -> wl_display_flush_clients())
> > Here, client will be destroyed if wl_connection_flush() returns
> > negative value and errno is not EAGAIN.
> >  
> 
> Out of interest, what is the '?' here? I can't find any examples of this
> happening in Wayland, Weston, or Mesa. If you could share your usecase,
> this would be quite interesting. I believe the idea behind deferring the
> flush was to minimise the number of system calls made, and also somewhat
> decrease the latency when multiple clients are involved, by batching all
> the flushes (potentially time-consuming) to a point where we do not have
> any more requests to process (so are not sensitive to latency).
> 
> In general, Wayland compositors should always be responsive: client request
> handling should be lightweight and not time-consuming. Wayland clients
> should also be completely asynchronous, rather than following a
> RPC/method-return pattern of { send_request(); block_for_reply(); }. Given
> that, the only reason to flush inside request processing would be because
> you are sending a lot of data, but again the Wayland protocol itself should
> not be carrying large amounts of data. So I'm not sure what usecase there
> is for this.
> 
> 
> > 3. It will come back to the while loop in wl_client_connection_data().
> > If wl_connection_pending_input() returns any values which is bigger
> > than “size of p”; it will go into the while loop again, and then it will do
> > something with the client which is already freed.
> >
> >
> > The simplest way to solve this is checking the source(wl_event_source)’
> > fd, but for now there is no way to access wl_event_source in
> > wayland-server.c. (wl_event_source will be freed a while later)
> >  
> 
> Hm, this is racy though.
> 
> I'm would suggest we document that invoking wl_display_flush_clients() /
> wl_connection_flush() from a request handler may cause undefined behaviour,
> and that it must only be called outside of dispatch.

Hi,

FWIW, I agree with Daniel here. There are several things that are not
quite defined or expected to be done inside dispatch calls, and
flushing all clients is one of them.

I could see, IIRC, how sending lots of events from inside dispatch
could cause the one client to be flushed. I think flush is done
automatically if the send buffer fills up. If the flush at that point
fails, there is no way to signal back that "oops, the compositor should
put that thing aside and do something else" until the connection
unblocks. We don't want to be checking and handling every send-event
possibly failing, so killing the client can happen. ISTR we don't
dynamically grow the send buffers, do we? But even the growing would
need to stop somewhere.

Before we can review a fix, we need to understand exactly when and why
the problem happens.

Is this a case of the compositor explicitly calling flush, or is it an
automatic flush? What triggers it?


Thanks,
pq


pgpD0A7ggfvxD.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 3/3] editor: Load a file if specified on command line

[Daniel Stone]

Hi Bryce,

On 20 November 2016 at 22:00, Bryce Harrington  wrote:
> Add support for basic text file loading, to facilitate more expansive
> testing of its UTF-8 text editing support.

Honestly, I question the value of turning the editor into something
'real': as soon as we're adding a --version argument to something,
we've definitely bought into supporting it to a level that I don't
feel comfortable with. Text editors, much like terminal emulators
(grr) are a never-ending swamp. Some much more concrete issues with
this patch though:

> +/* Join remaining arguments in argv as a space-delimited string.
> + *
> + * Caller is responsible for freeing the returned string.
> + * If there are no remaining arguments, returns a blank string.
> + * Returns NULL on out of memory error.
> +*/
> +char *
> +join_argv(int argc, char *argv[]) {

Warning: no previous declaration (use static). Brace is also on the wrong line.

> +   for (i = 0; i < argc; i++)
> +   buf_size += strlen(argv[i]) + 1;
> +
> +   filename = (char*) malloc(sizeof(char) * (buf_size + 1));
> +   if (filename == NULL)
> +   return NULL;
> +   filename[0] = '\0';
> +
> +   while (argv++, --argc) {
> +   int num = buf_size - offset;
> +   int written = 0;
> +   char space = ' ';
> +
> +   if (argc == 1)
> +   space = '\0';
> +   written = snprintf(filename+offset, num,
> +  "%s%c", *argv, space);
> +   if (num < written)
> +   break;
> +   offset += written;
> +   }
> +   return filename;
> +}

This is an incredibly novel approach to argument parsing, such that
running 'weston-editor filename with spaces.txt' will attempt to open
'filename with spaces.txt'. I don't know of anything else in UNIX
which does this, nor other text editors. Why not just have the user
quote the arguments, as with everything else?

> +/* Load the contents of a file into a UTF-8 text buffer and return it.
> + *
> + * Caller is responsible for freeing the buffer when done.
> + * On error, returns NULL.
> + */
> +char *
> +read_file(char *filename)
> +{

Another no-previous-declaration warning.

>  int
>  main(int argc, char *argv[])
>  {
> struct editor editor;
> +   char *text_buffer = NULL;
> int i;

Warning here: unused variable (introduced with patch 1/3).

I'd be OK with 1/3 if it didn't introduce the warning, but I'd really
rather steer clear of the rest, sorry.

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

Re: [RFC v2 wayland-protocols] tablet: define our own enum for tablet tool buttons

[Daniel Stone]

Hi,

On 20 November 2016 at 05:14, Peter Hutterer  wrote:
> Rather than relying on input-event-codes, define our own enum that is tailored
> towards the tablet interface.
>
> Signed-off-by: Peter Hutterer 
> ---
> Because it's usually easier to pick holes into a patch proposal than come up
> with ideas elsewhere, here's a quick-and-dirty patch.
>
> Advantage: we control the button names/numbers and clients don't have to
> know about cases where linux/input.h isn't enough.
> Obvious drawback: adding new buttons requires a new protocol. Given this
> hardware hasn't really changed much in quite a while, this may not be much
> of an issue.

Conceptually, I don't see why not.

> @@ -539,6 +539,26 @@
>
>  
>
> +
> +  
> +   Describes the physical button that produced the button event.
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +

Concretely though, reusing BTN_* codes where possible would make it
easier for clients to transition between the two. Also, 9 stylus
buttons seems like an oddly specific number.

Anyway, if you switch these values to the current BTN_* equivalents:
Acked-by: Daniel Stone 

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

[PATCH wayland] doc: start documenting Xwayland

[Pekka Paalanen]

From: Pekka Paalanen 

This is a rough intro to what Xwayland is and does, with just one
implementation detail so far (Window identification).

I paid no attention to formatting details, those can be polished in
follow-ups. I just want the prose out.

I also just quickly whacked up the diagram, would be happy to see
someone replace it with a nicer one. I just didn't have time to learn
dot for now.

Cc: Olivier Fourdan 
Cc: Jonas Ådahl 
Cc: Daniel Stone 
Signed-off-by: Pekka Paalanen 
---
 doc/publican/Makefile.am   |   5 +-
 doc/publican/sources/Wayland.xml   |   1 +
 doc/publican/sources/Xwayland.xml  | 172 +
 .../sources/images/xwayland-architecture.png   | Bin 0 -> 7611 bytes
 4 files changed, 177 insertions(+), 1 deletion(-)
 create mode 100644 doc/publican/sources/Xwayland.xml
 create mode 100644 doc/publican/sources/images/xwayland-architecture.png

diff --git a/doc/publican/Makefile.am b/doc/publican/Makefile.am
index 57728a0..e861fe6 100644
--- a/doc/publican/Makefile.am
+++ b/doc/publican/Makefile.am
@@ -24,9 +24,11 @@ publican_sources = \
$(srcdir)/sources/Preface.xml \
$(srcdir)/sources/Revision_History.xml \
$(srcdir)/sources/Protocol.xml \
+   $(srcdir)/sources/Xwayland.xml \
$(srcdir)/sources/Compositors.xml \
$(srcdir)/sources/images/icon.svg  \
$(srcdir)/sources/images/wayland.png \
+   $(srcdir)/sources/images/xwayland-architecture.png \
$(srcdir)/sources/Client.xml \
$(srcdir)/sources/Server.xml
 
@@ -43,7 +45,8 @@ css_sources = \
 
 img_sources = \
$(srcdir)/sources/images/icon.svg \
-   $(srcdir)/sources/images/wayland.png
+   $(srcdir)/sources/images/wayland.png \
+   $(srcdir)/sources/images/xwayland-architecture.png
 
 doxygen_img_sources := \
$(doxydir)/xml/wayland-architecture.png \
diff --git a/doc/publican/sources/Wayland.xml b/doc/publican/sources/Wayland.xml
index 2f47f13..0457c15 100644
--- a/doc/publican/sources/Wayland.xml
+++ b/doc/publican/sources/Wayland.xml
@@ -11,6 +11,7 @@
   http://www.w3.org/2001/XInclude; />
   http://www.w3.org/2001/XInclude; />
   http://www.w3.org/2001/XInclude; />
+  http://www.w3.org/2001/XInclude; />
   http://www.w3.org/2001/XInclude; />
   http://www.w3.org/2001/XInclude"/>
   http://www.w3.org/2001/XInclude"/>
diff --git a/doc/publican/sources/Xwayland.xml 
b/doc/publican/sources/Xwayland.xml
new file mode 100644
index 000..ca4025a
--- /dev/null
+++ b/doc/publican/sources/Xwayland.xml
@@ -0,0 +1,172 @@
+
+http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd; [
+
+%BOOK_ENTITIES;
+]>
+
+  X11 Application Support
+  
+Introduction
+
+  Being able to run existing X11 applications is crucial for the adoption
+  of Wayland, especially on desktops, as there will always be X11
+  applications that have not been or cannot be converted into Wayland
+  applications, and throwing them all away would be prohibitive.
+  Therefore a Wayland compositor often needs to support running X11
+  applications.
+
+
+  X11 and Wayland are different enough that there is no "simple" way to
+  translate between them. Most of X11 is uninteresting to a Wayland
+  compositor. That combined with the gigantic implementation effort needed
+  to support X11 makes it untractable to just write X11 support directly in
+  a Wayland compositor. The implementation would be nothing short from a
+  real X11 server.
+
+
+  Therefore Wayland compositors should use Xwayland, the X11 server that
+  lives in the Xorg server source code repository and shares most of the
+  implementation with the Xorg server. Xwayland is a complete X11 server,
+  just like Xorg is, but instead of talking to hardware and device drivers,
+  it acts as a Wayland client. The rest of this chapter talks about how
+  Xwayland works.
+
+  
+  
+Architecture
+
+  A Wayland compositor usually takes care of launching Xwayland.
+  Xwayland works in cooperation with a Wayland compositor as follows:
+
+
+  Xwayland architecture diagram
+  
+
+  
+
+  
+
+  
+
+
+  An X11 application connects to Xwayland just like it would connect to any
+  X server. Xwayland processes all the X11 requests. On the other end,
+  Xwayland is a Wayland client that connects to the Wayland compositor.
+
+
+  The X11 window manager (XWM) is an integral part of the Wayland
+  compositor. XWM uses the usual X11 window management protocol to manage
+  all X11 windows in Xwayland. Most importantly, XWM acts as a bridge
+  between Xwayland window state and the Wayland compositor's window manager
+  (WWM). This way 

Re: [PATCH] server: Fix crash when accessing client which is already freed

[Daniel Stone]

Hi Hyun Kook,

On 23 September 2016 at 00:40, Hyun Kook Khang 
wrote:

> in wl_client_connection_data(),
> there is no guarantee that client would never be destroyed in the process
> of handling the pending input.
>
> For example,
> 1. It comes to wl_client_connection_data()
> 2. wl_closure_invoke() would be invoked, and then wl_display_flush_clients
> would be invoked anyhow later in the same routine. (wl_closure_invoke() ->
> ? -> wl_display_flush_clients())
> Here, client will be destroyed if wl_connection_flush() returns
> negative value and errno is not EAGAIN.
>

Out of interest, what is the '?' here? I can't find any examples of this
happening in Wayland, Weston, or Mesa. If you could share your usecase,
this would be quite interesting. I believe the idea behind deferring the
flush was to minimise the number of system calls made, and also somewhat
decrease the latency when multiple clients are involved, by batching all
the flushes (potentially time-consuming) to a point where we do not have
any more requests to process (so are not sensitive to latency).

In general, Wayland compositors should always be responsive: client request
handling should be lightweight and not time-consuming. Wayland clients
should also be completely asynchronous, rather than following a
RPC/method-return pattern of { send_request(); block_for_reply(); }. Given
that, the only reason to flush inside request processing would be because
you are sending a lot of data, but again the Wayland protocol itself should
not be carrying large amounts of data. So I'm not sure what usecase there
is for this.


> 3. It will come back to the while loop in wl_client_connection_data().
> If wl_connection_pending_input() returns any values which is bigger
> than “size of p”; it will go into the while loop again, and then it will do
> something with the client which is already freed.
>
>
> The simplest way to solve this is checking the source(wl_event_source)’
> fd, but for now there is no way to access wl_event_source in
> wayland-server.c. (wl_event_source will be freed a while later)
>

Hm, this is racy though.

I'm would suggest we document that invoking wl_display_flush_clients() /
wl_connection_flush() from a request handler may cause undefined behaviour,
and that it must only be called outside of dispatch.

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

Re: [PATCH] xdg-shell: clarify popup constrain's slide mechanism

[Jonas Ådahl]

On Mon, Nov 21, 2016 at 06:36:49AM -0500, Mike Blumenkrantz wrote:
> On Fri, 18 Nov 2016 09:28:56 +0800
> Jonas Ådahl  wrote:
> 
> > On Wed, Nov 16, 2016 at 10:23:59AM -0500, Mike Blumenkrantz wrote:
> > > some restrictions must be placed on this or else it becomes legal for
> > > the compositor to place popups in unexpected locations when sliding
> > > is allowed
> > > 
> > > Signed-off-by: Mike Blumenkrantz 
> > > ---
> > >  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 6 --
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
> > > b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> > > index 6053e3c..30cdaeb 100644
> > > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> > > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> > > @@ -256,7 +256,8 @@
> > >
> > >
> > >   
> > > -   Slide the surface along the x axis until it is no longer constrained.
> > > +   Slide the surface along the x axis within the toplevel surface until 
> > > it
> > > +   is no longer constrained.  
> > 
> > This would break chained popup menus in GTK+. For example a menu chain
> > where eventually a menu now completely outside of the toplevel window
> > region hits the edge of monitor, and is supposed to slide (see
> > screenshot [0]).
> > 
> > So I think we should just limit the way we can slide in relation to the
> > parent. Maybe we should rather put the limitations somewhere more
> > generic? I.e. a popup can only ever be positioned so that its window
> > geometry "touches" (where touches means that there will be no space in
> > between) its parent window geometry, or something like that?
> > 
> > 
> > Jonas
> > 
> > 
> > [0] 
> > https://people.freedesktop.org/~jadahl/menu-slide-outside-of-toplevel.png
> > 
> 
> Hm, while I strongly disagree with any application design that requires this 
> level of submenu chaining, I can see why we might need to support at least 
> basic submenu positioning outside the toplevel. I think changing "toplevel" 
> to "parent" in my patch would address that sufficiently?

It sounds like the "sliding within the parent" means the whole child
surface must be within the parent, which is not the case. Maybe better
with "Slide the surface along the x/y axis until it is no longer
constrained or doesn't touch the parent surface anymore." or something.


Jonas

> 
> > >  
> > > First try to slide towards the direction of the gravity on the x axis
> > > until either the edge in the opposite direction of the gravity is
> > > @@ -271,7 +272,8 @@
> > >
> > >
> > >   
> > > -   Slide the surface along the y axis until it is no longer constrained.
> > > +   Slide the surface along the y axis within the toplevel surface until 
> > > it
> > > +   is no longer constrained.
> > >  
> > > First try to slide towards the direction of the gravity on the y axis
> > > until either the edge in the opposite direction of the gravity is
> > > -- 
> > > 2.5.5
> > > 
> > > ___
> > > wayland-devel mailing list
> > > wayland-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel  
> 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] xdg-shell: clarify popup constrain's slide mechanism

[Mike Blumenkrantz]

On Fri, 18 Nov 2016 09:28:56 +0800
Jonas Ådahl  wrote:

> On Wed, Nov 16, 2016 at 10:23:59AM -0500, Mike Blumenkrantz wrote:
> > some restrictions must be placed on this or else it becomes legal for
> > the compositor to place popups in unexpected locations when sliding
> > is allowed
> > 
> > Signed-off-by: Mike Blumenkrantz 
> > ---
> >  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
> > b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> > index 6053e3c..30cdaeb 100644
> > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> > @@ -256,7 +256,8 @@
> >
> >
> > 
> > - Slide the surface along the x axis until it is no longer constrained.
> > + Slide the surface along the x axis within the toplevel surface until 
> > it
> > + is no longer constrained.  
> 
> This would break chained popup menus in GTK+. For example a menu chain
> where eventually a menu now completely outside of the toplevel window
> region hits the edge of monitor, and is supposed to slide (see
> screenshot [0]).
> 
> So I think we should just limit the way we can slide in relation to the
> parent. Maybe we should rather put the limitations somewhere more
> generic? I.e. a popup can only ever be positioned so that its window
> geometry "touches" (where touches means that there will be no space in
> between) its parent window geometry, or something like that?
> 
> 
> Jonas
> 
> 
> [0] https://people.freedesktop.org/~jadahl/menu-slide-outside-of-toplevel.png
> 

Hm, while I strongly disagree with any application design that requires this 
level of submenu chaining, I can see why we might need to support at least 
basic submenu positioning outside the toplevel. I think changing "toplevel" to 
"parent" in my patch would address that sufficiently?

> >  
> >   First try to slide towards the direction of the gravity on the x axis
> >   until either the edge in the opposite direction of the gravity is
> > @@ -271,7 +272,8 @@
> >
> >
> > 
> > - Slide the surface along the y axis until it is no longer constrained.
> > + Slide the surface along the y axis within the toplevel surface until 
> > it
> > + is no longer constrained.
> >  
> >   First try to slide towards the direction of the gravity on the y axis
> >   until either the edge in the opposite direction of the gravity is
> > -- 
> > 2.5.5
> > 
> > ___
> > wayland-devel mailing list
> > wayland-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel  

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

Re: [PATCH wayland] protocol: indentation fixes

[Daniel Stone]

Hi Bryce,

On 18 November 2016 at 00:42, Bryce Harrington  wrote:
> The patch isn't applying as of change 66a26aeb (remove inconsistent line
> breaks), but in generating a whitespace patch myself using emacs with
> the Wayland style rules, I am getting a similar looking set of changes.
> While whitespace changes are trivial, having these fixed would make
> automatic style tools a bit better able to detect and fix whitespace
> issues in the future.

How about http://editorconfig.org so it works in every editor?

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

Re: [PATCH wayland] protocol: indentation fixes

[Daniel Stone]

Hi Peter,

On 10 November 2016 at 05:02, Peter Hutterer  wrote:
> 8 spaces changed to one tab

Can't say I'm a massive fan of it personally, but that ship's already
sailed, so have applied this; well, manually reconstructed with sed
and taken your commit message / etc.

To ssh://git.freedesktop.org/git/wayland/wayland
   a2cbdef..a26ed09  master -> master

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

Re: [RFC wayland 0/1] tests: Test wl_argument_from_va_list

[Daniel Stone]

Hi Yong,

On 13 November 2016 at 19:14, Yong Bakos  wrote:
> My ongoing routine of documenting objects and checking for related test 
> coverage
> has recently led me to notice that wl_argument_from_va_list does not have a
> specific test in connection-test.c. The 1/1 patch in this RFC describes a 
> rough
> draft at the testing approach: using a wrapper function to generate a va_list
> for wl_argument_from_va_list, and checking the results.
>
> I have two questions regarding this RFC, and welcome any additional comments.
>
> 1) What message signatures should be tested, and how many?
> 2) I'd like to replace the procedural approach with an iterative one,
>but I can't quite see how this will be possible. Any suggestions?

I don't think there's any real reason to go hugely overboard on this.
The only real thing it even does is demarshal the type names, so a
single check for each type - as well as making sure that multiple
arguments work, which you've done - is fine. Everything else should
already be exercised. So maybe a quick: '?iuf?sonah' in addition to
the two you've got, and that'll close it out.

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

Re: [PATCH] wayland-client: Require base 10 for WAYLAND_SOCKET, explicitly

[Daniel Stone]

Hi,

On 9 July 2016 at 02:36, Yong Bakos  wrote:
> On Jul 8, 2016, at 4:42 PM, Bryce Harrington  wrote:
>> The third arg to strtol() specifies the base to assume for the number.
>> When 0 is passed, as is currently done in wayland-client.c, hexadecimal
>> and octal numbers are permitted and automatically detected and
>> converted.
>>
>> I can find no indication that we would ever expect use of hexadecimal or
>> octal for socket fd's.  So be explicit about what base we're assuming
>> here and avoid any potential surprises.
>>
>> Signed-off-by: Bryce Harrington 
>
> I can think of no drawbacks to this.

Yeah, me neither. Thanks Bryce!

To ssh://git.freedesktop.org/git/wayland/wayland
   b05baa6..a2cbdef  master -> master

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

Re: [PATCH weston 2/2] simple-egl: add support for EGL_KHR_swap_buffers_with_damage

[Daniel Stone]

Hi Emil,

On 3 November 2016 at 22:38, Emil Velikov  wrote:
> @@ -191,14 +205,21 @@ init_egl(struct display *display, struct window *window)
> display->swap_buffers_with_damage = NULL;
> extensions = eglQueryString(display->egl.dpy, EGL_EXTENSIONS);
> if (extensions &&
> -   weston_check_egl_extension(extensions, 
> "EGL_EXT_swap_buffers_with_damage") &&
> -   weston_check_egl_extension(extensions, "EGL_EXT_buffer_age"))
> -   display->swap_buffers_with_damage =
> -   (PFNEGLSWAPBUFFERSWITHDAMAGEEXTPROC)
> -   eglGetProcAddress("eglSwapBuffersWithDamageEXT");
> +   weston_check_egl_extension(extensions, "EGL_EXT_buffer_age")) {
> +   for (i = 0; i < ARRAY_LENGTH(foo); i++) {

This caused a signed vs. unsigned warning for me, so I've trivially
fixed and pushed both patches:
To ssh://git.freedesktop.org/git/wayland/weston
   46dc9b4..43cea54  upstream -> master

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

Re: [PATCH wayland] util: Document wl_log_func_t

[Daniel Stone]

Hi Yong,

On 20 November 2016 at 16:59, Yong Bakos  wrote:
> +/**
> + * Log function type alias
> + *
> + * The C implementation of the Wayland protocol abstracts the details of
> + * logging. Users may customize the logging behavior, with a function 
> conforming
> + * to the `wl_log_func_t` type, via `wl_log_set_handler_client` and
> + * `wl_log_set_handler_server`.
> + *
> + * A `wl_log_func_t` typically conforms to the expectations of `vprintf`, and

Hope you don't mind, but I felt like 'typically' was an invitation
towards disaster, so I strengthened that while pushing.

To ssh://git.freedesktop.org/git/wayland/wayland
   8477664..b05baa6  master -> master

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

Re: [PATCH weston] drm: don't put too big surfaces in the cursor plane

[Daniel Stone]

Hi Giulio,

On 21 November 2016 at 11:23, Giulio Camuffo  wrote:
> When using output scaling a client surface's wicth and height can be
> smaller than the cursor plane's size, even if its buffer is actually
> bigger. So check the buffer size rather than the surface size.

Derek had a similar yet conflicting fix around viewports, which this
series would break. We need to account for the total transformation:
buffer scale, viewport transform, view transform, and finally output
as well. I think doing this piecemeal across cursor/scanout/overlay is
error-prone and inconsistent, so I've tried to account for all this in
the plane_state part of the atomic modesetting series. Would you care
to cast an eye over that and tell me if you think I've got it right?
:)

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

Re: [PATCH wayland-web v2] qt5: fixed bug tracker link

[Samuel Gaist]

> On 21 nov. 2016, at 11:55, Daniel Stone  wrote:
> 
> Hi,
> 
> On 9 October 2016 at 21:04, Samuel Gaist  wrote:
>>> On 31 août 2016, at 17:13, Yong Bakos  wrote:
>>> On Aug 30, 2016, at 11:50 PM, Samuel Gaist  wrote:
 No being used to the by email patch workflow, should I send an updated 
 patch with the “Reviewed-by” field added ?
>>> 
>>> Samuel, not necessary. Committers will add the R-b line when applying your 
>>> patch.
>>> 
>>> Not sure why the delay, just overlooked I guess. I'm cc'ig Darxus, who 
>>> would love a distraction from Cairo.
>>> 
>>> long
>> 
>> Thanks for the info !
>> 
>> And a small ping :)
> 
> Very much overlooked indeed; sorry for the delay. Pushed now.
> 
> Cheers,
> Daniel

Thank you very much !

Can you also take a look at:

https://lists.freedesktop.org/archives/wayland-devel/2016-August/030424.html ?

Best regards
Samuel

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



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston] drm: don't put too big surfaces in the cursor plane

[Giulio Camuffo]

When using output scaling a client surface's wicth and height can be
smaller than the cursor plane's size, even if its buffer is actually
bigger. So check the buffer size rather than the surface size.
---
 libweston/compositor-drm.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index a899213..0913f6c 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1127,6 +1127,7 @@ drm_output_prepare_cursor_view(struct drm_output *output,
struct drm_backend *b = to_drm_backend(output->base.compositor);
struct weston_buffer_viewport *viewport = >surface->buffer_viewport;
struct wl_shm_buffer *shmbuf;
+   struct weston_buffer *buffer = ev->surface->buffer_ref.buffer;
 
if (ev->transform.enabled &&
(ev->transform.matrix.type > WESTON_MATRIX_TRANSFORM_TRANSLATE))
@@ -1147,13 +1148,13 @@ drm_output_prepare_cursor_view(struct drm_output 
*output,
return NULL;
if (ev->surface->buffer_ref.buffer == NULL)
return NULL;
-   shmbuf = wl_shm_buffer_get(ev->surface->buffer_ref.buffer->resource);
+   shmbuf = wl_shm_buffer_get(buffer->resource);
if (!shmbuf)
return NULL;
if (wl_shm_buffer_get_format(shmbuf) != WL_SHM_FORMAT_ARGB)
return NULL;
-   if (ev->surface->width > b->cursor_width ||
-   ev->surface->height > b->cursor_height)
+   if (buffer->width > b->cursor_width ||
+   buffer->height > b->cursor_height)
return NULL;
 
output->cursor_view = ev;
@@ -1180,8 +1181,8 @@ cursor_bo_update(struct drm_backend *b, struct gbm_bo *bo,
 
assert(buffer && buffer->shm_buffer);
assert(buffer->shm_buffer == wl_shm_buffer_get(buffer->resource));
-   assert(ev->surface->width <= b->cursor_width);
-   assert(ev->surface->height <= b->cursor_height);
+   assert(buffer->width <= b->cursor_width);
+   assert(buffer->height <= b->cursor_height);
 
memset(buf, 0, sizeof buf);
stride = wl_shm_buffer_get_stride(buffer->shm_buffer);
@@ -1191,7 +1192,7 @@ cursor_bo_update(struct drm_backend *b, struct gbm_bo *bo,
for (i = 0; i < ev->surface->height; i++)
memcpy(buf + i * b->cursor_width,
   s + i * stride,
-  ev->surface->width * 4);
+  buffer->width * 4);
wl_shm_buffer_end_access(buffer->shm_buffer);
 
if (gbm_bo_write(bo, buf, sizeof buf) < 0)
@@ -1292,8 +1293,8 @@ drm_assign_planes(struct weston_output *output_base)
if (b->use_pixman ||
(es->buffer_ref.buffer &&
(!wl_shm_buffer_get(es->buffer_ref.buffer->resource) ||
-(ev->surface->width <= b->cursor_width &&
- ev->surface->height <= b->cursor_height
+(es->buffer_ref.buffer->width <= b->cursor_width &&
+ es->buffer_ref.buffer->height <= b->cursor_height
es->keep_buffer = true;
else
es->keep_buffer = false;
-- 
2.10.2

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

Re: [PATCH wayland-protocols 1/1] (multiple): Remove inconsistent line breaks

[Daniel Stone]

Hi Yong,

On 31 October 2016 at 13:43, Yong Bakos  wrote:
> From: Yong Bakos 
>
> Enum entries and message arguments are sometimes preceded by a blank line, but
> often aren't.
>
> Standardize the format of the protocol specification by removing blank lines
> preceding a list of message arguments and enum entries.

As a matter of taste, I don't personally like this, but given that
we've already done it to the core protocol, I've pushed this now.

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

Re: [PATCH wayland-web v2] qt5: fixed bug tracker link

[Samuel Gaist]

> On 21 nov. 2016, at 12:11, Daniel Stone  wrote:
> 
> Hi Samuel,
> 
> On 21 November 2016 at 11:08, Samuel Gaist  wrote:
>>> On 21 nov. 2016, at 11:55, Daniel Stone  wrote:
>>> Very much overlooked indeed; sorry for the delay. Pushed now.
>> 
>> Thank you very much !
>> 
>> Can you also take a look at:
>> 
>> https://lists.freedesktop.org/archives/wayland-devel/2016-August/030424.html 
>> ?
> 
> Hm, not sure why that didn't appear on Patchwork; pushed now in any case.
> 
> Cheers,
> Daniel

Good question… I saw it in Patchwork after I posted the patch some months ago 
but I also got the surprise that it disappeared in between when I checked after 
your last email.

In any case, thanks !

Best regards
Samuel


signature.asc
Description: Message signed with OpenPGP using GPGMail
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland 2/2] build: Fix scanner path in uninstalled pkg-config file

[Daniel Stone]

Hi,

On 30 August 2016 at 01:29, Bryce Harrington  wrote:
> On Fri, Aug 26, 2016 at 04:04:03PM -0500, Derek Foreman wrote:
>> this was generating a pkg-config file that said wayland-scanner was
>> wayland/src/wayland-scanner when it's actually wayland/wayland-scanner
>>
>> Signed-off-by: Derek Foreman 
>
> Acked-by: Bryce Harrington 

Makes sense, both applied.

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

Re: [PATCH wayland-web v2] qt5: fixed bug tracker link

[Daniel Stone]

Hi Samuel,

On 21 November 2016 at 11:08, Samuel Gaist  wrote:
>> On 21 nov. 2016, at 11:55, Daniel Stone  wrote:
>> Very much overlooked indeed; sorry for the delay. Pushed now.
>
> Thank you very much !
>
> Can you also take a look at:
>
> https://lists.freedesktop.org/archives/wayland-devel/2016-August/030424.html ?

Hm, not sure why that didn't appear on Patchwork; pushed now in any case.

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

Re: [PATCH wayland] util: Improve documentation of wl_iterator_result

[Daniel Stone]

Hi Yong,

On 20 November 2016 at 17:26, Yong Bakos  wrote:
> Use declarative voice, remove the unnecessary doxygen \enum tag, and add
> two see-also's. This keeps the output the same but makes the comment
> voice consistent, a little more readable, and refers to documented
> functions that use this enum type.

Pushed, thanks.

To ssh://git.freedesktop.org/git/wayland/wayland
   5c48aac..013cfd9  master -> master

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

Re: [PATCH v2 wayland] protocol: spell out that we're using linux/input-event-codes.h button codes

[Daniel Stone]

Hi Peter,

On 18 November 2016 at 02:35, Peter Hutterer  wrote:
> Because we already rely on it in the callers anyway. This is a retrofit, which
> is not ideal but I'm not sure any compositor out there uses anything else.
> Might as well define it.

Thanks for this.

To ssh://git.freedesktop.org/git/wayland/wayland
   5c48aac..013cfd9  master -> master

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

Re: [PATCH weston 2/2] Makefile.am: Link modules to libweston.la

[Jan Engelhardt]

On Monday 2016-11-21 12:00, Daniel Stone wrote:
>On 18 August 2016 at 10:15, Quentin Glidic
> wrote:
>> @@ -116,7 +116,9 @@ libweston_@LIBWESTON_MAJOR@_la_SOURCES = 
>>\
>>  lib_LTLIBRARIES += libweston-desktop-@LIBWESTON_MAJOR@.la
>>  libweston_desktop_@LIBWESTON_MAJOR@_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
>>  libweston_desktop_@LIBWESTON_MAJOR@_la_CFLAGS = $(AM_CFLAGS) 
>> $(COMPOSITOR_CFLAGS)
>> -libweston_desktop_@LIBWESTON_MAJOR@_la_LIBADD = 
>> libweston-@LIBWESTON_MAJOR@.la $(COMPOSITOR_LIBS)
>> +libweston_desktop_@LIBWESTON_MAJOR@_la_LIBADD =\
>> +   libweston-@LIBWESTON_MAJOR@.la  \
>> +   $(COMPOSITOR_LIBS)
>>  libweston_desktop_@LIBWESTON_MAJOR@_la_LDFLAGS = -version-info 
>> $(LT_VERSION_INFO)
>
>COMPOSITOR_LIBS is full of -Lfoo -lsyslib. To the best of my
>knowledge, these belong in LDADD

Negative. _LDADD is ignored for library type outputs, and _LIBADD
is ignored for program type outputs.

Everything is correct here..
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 1/2] libweston: Move text_backend_* to weston.h

[Daniel Stone]

Hi,

On 18 August 2016 at 12:28, Quentin Glidic
 wrote:
> I forgot to explain it, but these are actually defined in Weston, not
> libweston, so any libweston user trying to use them will fail.

Right you are:
To ssh://git.freedesktop.org/git/wayland/weston
   c3b6372..46dc9b4  upstream -> master

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

Re: [PATCH weston 2/2] Makefile.am: Link modules to libweston.la

[Daniel Stone]

Hi Quentin,

On 18 August 2016 at 10:15, Quentin Glidic
 wrote:
> @@ -116,7 +116,9 @@ libweston_@LIBWESTON_MAJOR@_la_SOURCES =  
>   \
>  lib_LTLIBRARIES += libweston-desktop-@LIBWESTON_MAJOR@.la
>  libweston_desktop_@LIBWESTON_MAJOR@_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
>  libweston_desktop_@LIBWESTON_MAJOR@_la_CFLAGS = $(AM_CFLAGS) 
> $(COMPOSITOR_CFLAGS)
> -libweston_desktop_@LIBWESTON_MAJOR@_la_LIBADD = 
> libweston-@LIBWESTON_MAJOR@.la $(COMPOSITOR_LIBS)
> +libweston_desktop_@LIBWESTON_MAJOR@_la_LIBADD =\
> +   libweston-@LIBWESTON_MAJOR@.la  \
> +   $(COMPOSITOR_LIBS)
>  libweston_desktop_@LIBWESTON_MAJOR@_la_LDFLAGS = -version-info 
> $(LT_VERSION_INFO)

COMPOSITOR_LIBS is full of -Lfoo -lsyslib. To the best of my
knowledge, these belong in LDADD, which is things to throw in the ld
command line; LIBADD is local libraries to add, which will be
dependency-tracked and also thrown through the libtool mangling
machinery.

Could you please test with a split that has $(COMPOSITOR_LIBS) et al
being set as LDADD, and only the local paths for LIBADD? Such a patch
would have my R-b, as the rest looks sound enough.

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

Re: [PATCH wayland-web v2] qt5: fixed bug tracker link

[Daniel Stone]

Hi,

On 9 October 2016 at 21:04, Samuel Gaist  wrote:
>> On 31 août 2016, at 17:13, Yong Bakos  wrote:
>> On Aug 30, 2016, at 11:50 PM, Samuel Gaist  wrote:
>>> No being used to the by email patch workflow, should I send an updated 
>>> patch with the “Reviewed-by” field added ?
>>
>> Samuel, not necessary. Committers will add the R-b line when applying your 
>> patch.
>>
>> Not sure why the delay, just overlooked I guess. I'm cc'ig Darxus, who would 
>> love a distraction from Cairo.
>>
>> long
>
> Thanks for the info !
>
> And a small ping :)

Very much overlooked indeed; sorry for the delay. Pushed now.

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

Re: [PATCH wayland-web] Drop the ubuntu 12.04 build directions.

[Daniel Stone]

Hi,

On 16 September 2016 at 22:29, Bryce Harrington  wrote:
> It is unlikely anyone still needs directions on how to install on this
> old distro -- Ubuntu 12.04 is scheduled to hit end-of-life this April.
>
> Further, no developers (to my knowledge) still test on 12.04, so the
> directions have likely bitrotted anyway.  (Bill had a machine running
> 12.04 and was keeping the page updated, but last March indicated in
> 4fa80f28 he is no longer testing on it.  I myself moved off 12.04 some
> time ago as well.)
>
> For the most part, the directions are requiring a nearly full build of
> the stack from source, thus is rather duplicative of the generic Wayland
> build directions (which are more actively maintained).  Only a handful
> of lower level X packages and some compiler tools are used from the
> system.

Good plan; pushed.

To ssh://git.freedesktop.org/git/wayland/wayland-web
   dee9dc8..0e0e195  master -> master

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

[wayland-protocols v3] linux-dmabuf: advertise format modifiers with modifier event

[Varad Gautam]

From: Varad Gautam 

advertise the supported fourcc format modifiers along with supported
formats to the client.

bump zwp_linux_dmabuf_v1, zwp_linux_buffer_params_v1 interface
versions to 3.

v2: specify request name in event description for clarity (Yong Bakos)
v3: grammar fixup (Yong Bakos)

Signed-off-by: Varad Gautam 
Reviewed-by: Yong Bakos 
---
 unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml | 25 +++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml 
b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
index a0aa42e..cc5d604 100644
--- a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
+++ b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
@@ -26,7 +26,7 @@
 THIS SOFTWARE.
   
 
-  
+  
 
   Following the interfaces from:
   
https://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_import.txt
@@ -34,7 +34,8 @@
 
   This interface offers ways to create generic dmabuf-based
   wl_buffers. Immediately after a client binds to this interface,
-  the set of supported formats is sent with 'format' events.
+  the set of supported formats and format modifiers is sent with
+  'format' and 'modifier' events.
 
   The following are required from clients:
 
@@ -110,9 +111,27 @@
   
   
 
+
+
+  
+This event advertises the formats that the server supports, along with
+the modifiers supported for each format. All the supported modifiers 
for
+all the supported formats are advertised once when the client binds to
+this interface. A roundtrip after binding guarantees that the client
+has received all supported format-modifier pairs.
+
+For the definition of the format and modifier codes, see the
+zwp_linux_buffer_params_v1::create request.
+  
+  
+  
+  
+
   
 
-  
+  
 
   This temporary object is a collection of dmabufs and other
   parameters that together form a single logical buffer. The temporary
-- 
2.6.2

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

[wayland-protocols v2] linux-dmabuf: clarify format event description

[Varad Gautam]

From: Varad Gautam 

clearly state the request name in format event to avoid abmiguous
interpretation between 'zwp_linux_buffer_params_v1::create' and
'zwp_linux_dmabuf_v1::create_params' requests.

v2: grammar fixup (Yong Bakos)

Suggested-by: Yong Bakos 
Signed-off-by: Varad Gautam 
Reviewed-by: Yong Bakos 
---
 unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml 
b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
index cc5d604..0b22721 100644
--- a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
+++ b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
@@ -105,7 +105,8 @@
 binds to this interface. A roundtrip after binding guarantees
 that the client has received all supported formats.
 
-For the definition of the format codes, see create request.
+For the definition of the format codes, see the
+zwp_linux_buffer_params_v1::create request.
 
 XXX: Can a compositor ever enumerate them?
   
-- 
2.6.2

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

Re: [PATCH wayland-protocols] input-method: Cleanup some grammar

[Daniel Stone]

Hi Bryce,

On 17 September 2016 at 05:37, Bryce Harrington  wrote:
>  
>
> -   Set the styling information on composing text. The style is applied 
> for
> -   length in bytes from index relative to the beginning of
> -   the composing text (as byte offset). Multiple styles can
> -   be applied to a composing text.
> +   Set the styling information on a section of the composing text
> +   offset index bytes from the beginning and ending at length
> +   bytes.

Ending at offset + length bytes, no? Or 'applied for length bytes from
the offset', or similar.

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

Re: [PATCH wayland] cursor: Remove "weston" from anonymous shm filenames

[Daniel Stone]

On 27 July 2016 at 17:06, Derek Foreman  wrote:
> This mildly confused me during some debugging, so I guess it wouldn't
> hurt to make the filename more indicative of where it was actually
> created.
>
> Signed-off-by: Derek Foreman 

Heh. Branding!

To ssh://git.freedesktop.org/git/wayland/wayland
   9ac70f4..5c48aac  master -> master

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

Re: [PATCH weston] Ignore the OSC code for desktop notifications

[Daniel Stone]

Hi Dima,

On 4 November 2016 at 06:46, Dima Ryazanov  wrote:
> In Fedora, bash is configured to display a desktop notification when a command
> finishes (and the terminal is not focused). weston-terminal complains about 
> it;
> let's silence it.

I have no clue about terminal escape codes, but equally no clue how
this would not be the right thing to do. Thanks!

To ssh://git.freedesktop.org/git/wayland/weston
   08f09e2..c3b6372  upstream -> master

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

Re: [PATCH weston] simple-egl: Do not set EGL up until XDG setup is complete

[Daniel Stone]

Hi Miguel,

On 15 November 2016 at 04:49, Miguel A. Vico  wrote:
> There is nothing that prohibits the underlying EGL_PLATFORM_WAYLAND
> implementation to attach a buffer or commit surfaces right after the
> Wayland EGLSurface has been created.
>
> Since XDG Shell v6 imposes that no buffer attachments or surface commits
> must be done before a configure is complete, Wayland clients shouldn't
> start setting EGL up until XDG setup is complete.
>
> Related bug:
>
> https://bugs.freedesktop.org/show_bug.cgi?id=98731

Per discussion on that bug, I'd like to defer this patch until we
reach a resolution. However, given the pretty firm opinions in this
bug, I'd definitely lean towards retaining the current behaviour, and
doing what we need with EGL implementations (Vivante's does the same
thing, in the opposite direction: it commits too late, rather than too
early!) rather than allow commits to happen outside the user's
control.

I guess you already know this, but figured I'd better document it for
the list. :)

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

Re: [PATCH weston] libweston: remove unused function declaration of weston_compositor_top

[Daniel Stone]

Hi,

On 18 November 2016 at 12:17, Ryo Munakata  wrote:
> Signed-off-by: Ryo Munakata 

Pushed with review, thanks!

To ssh://git.freedesktop.org/git/wayland/weston
   97863d6..08f09e2  upstream -> master

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

Re: [PATCH weston v2] xwayland: Fix X11 lock file size confusion

[Daniel Stone]

Hi,

On 17 November 2016 at 14:41, Pekka Paalanen
 wrote:
> On Thu, 17 Nov 2016 12:17:59 +
> Daniel Stone  wrote:
>> @@ -148,13 +148,19 @@ bind_to_unix_socket(int display)
>>  static int
>>  create_lockfile(int display, char *lockfile, size_t lsize)
>>  {
>> - char pid[16];
>> + /* 10 decimal characters, trailing LF and NUL byte; see comment
>> +  * at end of function. */
>> + char pid[11];
>
> Hi,
>
> um, 11? No room for the terminator-for-sure?
>
> Otherwise looks good.

Oh, too clever by half. I just left your explicit termination at byte
11 and have pushed it so we can all move on with our lives.

Thanks for the thorough review all.

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

Re: [PATCH weston] clients/simple-shm: remove unreachable function call

[Daniel Stone]

Hi Munakata-san,

On 18 November 2016 at 12:17, Ryo Munakata  wrote:
> window->wait_for_configure should be false after dispatching more than once.
> Therefore this redraw() will never be called.
>
> Signed-off-by: Ryo Munakata 
> ---
>  clients/simple-shm.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/clients/simple-shm.c b/clients/simple-shm.c
> index 9fa2e21..04b4eff 100644
> --- a/clients/simple-shm.c
> +++ b/clients/simple-shm.c
> @@ -547,9 +547,6 @@ main(int argc, char **argv)
> wl_surface_damage(window->surface, 0, 0,
>   window->width, window->height);
>
> -   if (!window->wait_for_configure)
> -   redraw(window, NULL, 0);
> -
> while (running && ret != -1)
> ret = wl_display_dispatch(display->display);

Hm, I don't think this is quite correct.

In create_window(), we set wait_for_configure = true, only after
creating an xdg_shell surface and committing. There is no roundtrip or
even flush at this point, so we would expect wait_for_configure to be
true, and this path to not be taken. For ivi_shell and
fullscreen_shell, we do not do this, and wait_for_configure will be
false; at this point we have not committed anything to the surface,
and the dispatch loop will not cause a redraw, as that only comes from
the frame callback. So I think this patch will break those shells.

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