Re: [PATCH weston v5 12/14] compositor: Add scene-graph debug scope

2018-08-06 Thread Pekka Paalanen
On Fri, 20 Jul 2018 20:03:33 +0100
Daniel Stone  wrote:

> Add a 'scene-graph' debug scope which will dump out the current set of
> outputs, layers, and views and as much information as possible about how
> they are rendered and composited.
> 
> Signed-off-by: Daniel Stone 
> ---
>  libweston/compositor.c | 219 +
>  libweston/compositor.h |   4 +
>  2 files changed, 223 insertions(+)

Hi Daniel,

this looks nice, just a few minor issues below.


> 
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index 0c147d4a6..dea91d173 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -55,6 +55,8 @@
>  #include "timeline.h"
>  
>  #include "compositor.h"
> +#include "weston-debug.h"
> +#include "linux-dmabuf.h"
>  #include "viewporter-server-protocol.h"
>  #include "presentation-time-server-protocol.h"
>  #include "shared/helpers.h"
> @@ -6306,6 +6308,216 @@ timeline_key_binding_handler(struct weston_keyboard 
> *keyboard,
>   weston_timeline_open(compositor);
>  }
>  
> +static const char *
> +output_repaint_status_text(struct weston_output *output)
> +{
> + switch (output->repaint_status) {
> + case REPAINT_NOT_SCHEDULED:
> + return "no repaint";
> + case REPAINT_BEGIN_FROM_IDLE:
> + return "start_repaint_loop scheduled";
> + case REPAINT_SCHEDULED:
> + return "repaint scheduled";
> + case REPAINT_AWAITING_COMPLETION:
> + return "awaiting completion";
> + }
> +
> + assert(!"output_repaint_status_text missing enum");
> + return NULL;
> +}
> +
> +static void
> +debug_scene_view_print_buffer(FILE *fp, struct weston_view *view)
> +{
> + struct weston_buffer *buffer = view->surface->buffer_ref.buffer;
> + struct wl_shm_buffer *shm;
> + struct linux_dmabuf_buffer *dmabuf;
> +
> + if (!buffer) {
> + fprintf(fp, "\t\tsolid-colour surface\n");

This is incorrect. Under GL-renderer, it will claim all wl_shm surfaces
to be solid color surfaces, e.g. background, panel, and a
weston-terminal window.

> + return;
> + }
> +
> + shm = wl_shm_buffer_get(buffer->resource);
> + if (shm) {
> + fprintf(fp, "\t\tSHM buffer\n");
> + fprintf(fp, "\t\t\tformat: 0x%lx\n",
> + (unsigned long) wl_shm_buffer_get_format(shm));
> + return;
> + }
> +
> + dmabuf = linux_dmabuf_buffer_get(buffer->resource);
> + if (dmabuf) {
> + fprintf(fp, "\t\tdmabuf buffer\n");
> + fprintf(fp, "\t\t\tformat: 0x%lx\n",
> + (unsigned long) dmabuf->attributes.format);
> + fprintf(fp, "\t\t\tmodifier: 0x%llx\n",
> + (unsigned long long) dmabuf->attributes.modifier[0]);
> + return;
> + }
> +
> + fprintf(fp, "\t\tEGL buffer");
> +}
> +
> +static void
> +debug_scene_view_print(FILE *fp, struct weston_view *view, int view_idx)
> +{
> + struct weston_compositor *ec = view->surface->compositor;
> + struct weston_output *output;
> + pixman_box32_t *box;
> + uint32_t surface_id = 0;
> + pid_t pid = 0;
> +
> + if (view->surface->resource) {
> + struct wl_resource *resource = view->surface->resource;
> + wl_client_get_credentials(wl_resource_get_client(resource),
> +   , NULL, NULL);
> + surface_id = wl_resource_get_id(view->surface->resource);
> + }
> +
> + fprintf(fp, "\tView %d (role %s, PID %d, surface ID %u, %p):\n",
> + view_idx, view->surface->role_name, pid, surface_id, view);

It might be useful here to print the surface description using
weston_surface::get_label() vfunc. For an example, see
check_weston_surface_description() in timeline.c.

Printing role_name is nice, it pointed out that desktop-shell is not
calling weston_surface_set_role() backgrounds or panels. I don't see
why it shouldn't.

> +
> + box = pixman_region32_extents(>transform.boundingbox);
> + fprintf(fp, "\t\tposition: (%d, %d) -> (%d, %d)\n",
> + box->x1, box->y1, box->x2, box->y2);
> + box = pixman_region32_extents(>transform.opaque);
> +
> + if (pixman_region32_equal(>transform.opaque,
> +   >transform.boundingbox)) {
> + fprintf(fp, "\t\t[fully opaque]\n");
> + } else if (!pixman_region32_not_empty(>transform.opaque)) {
> + fprintf(fp, "\t\t[not opaque]\n");

This case could also mean transformed with a matrix, but I suppose we're
interested in how the renderer is handling the surface, not how a
client set it up?

> + } else {
> + fprintf(fp, "\t\t[opaque: (%d, %d) -> (%d, %d)]\n",
> + box->x1, box->y1, box->x2, box->y2);
> + }
> +
> + if (view->alpha < 1.0)
> + fprintf(fp, "\t\talpha: %f\n", view->alpha);
> +
> + if (view->output_mask != 0) {
> + 

Re: [PATCH weston v5 11/14] compositor: Add weston_layer_mask_is_infinite

2018-08-06 Thread Pekka Paalanen
On Fri, 20 Jul 2018 20:03:32 +0100
Daniel Stone  wrote:

> As a counterpart to weston_layer_set_mask_infinite(), returning if the
> mask is the same as what is set.
> 
> Signed-off-by: Daniel Stone 
> ---
>  libweston/compositor.c | 9 +
>  libweston/compositor.h | 3 +++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index 8768f67a0..0c147d4a6 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -2746,6 +2746,15 @@ weston_layer_set_mask_infinite(struct weston_layer 
> *layer)
>UINT32_MAX, UINT32_MAX);
>  }
>  
> +WL_EXPORT bool
> +weston_layer_mask_is_infinite(struct weston_layer *layer)
> +{
> + return layer->mask.x1 == INT32_MIN &&
> +layer->mask.y1 == INT32_MIN &&
> +layer->mask.x2 == INT32_MIN + UINT32_MAX &&
> +layer->mask.y2 == INT32_MIN + UINT32_MAX;
> +}

Hi Daniel,

I suppose this repeats what weston_layer_set_mask_infinite() does, so

Reviewed-by: Pekka Paalanen 

They are both overflowing int or maybe this one is implicitly
converting a negative to unsigned, but I believe it works in practice.
Wonder if Clang would disagree.


Thanks,
pq

> +
>  WL_EXPORT void
>  weston_output_schedule_repaint(struct weston_output *output)
>  {
> diff --git a/libweston/compositor.h b/libweston/compositor.h
> index 83448b70f..67a835713 100644
> --- a/libweston/compositor.h
> +++ b/libweston/compositor.h
> @@ -1686,6 +1686,9 @@ weston_layer_set_mask(struct weston_layer *layer, int 
> x, int y, int width, int h
>  void
>  weston_layer_set_mask_infinite(struct weston_layer *layer);
>  
> +bool
> +weston_layer_mask_is_infinite(struct weston_layer *layer);
> +
>  void
>  weston_plane_init(struct weston_plane *plane,
>   struct weston_compositor *ec,



pgpxLo6eMlatT.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 v5 03/14] libweston: add weston_debug API and implementation

2018-08-06 Thread Pekka Paalanen
On Fri, 20 Jul 2018 20:03:24 +0100
Daniel Stone  wrote:

> From: Pekka Paalanen 
> 
> weston_debug is both a libweston API for relaying debugging messages,
> and the compositor-debug wayland protocol implementation for accessing those
> debug messages from a Wayland client.
> 
> weston_debug_compositor_{create,destroy}() are private API, hence not
> exported.
> 
> Signed-off-by: Pekka Paalanen 
> 
> append the debug scope name along with the timestamp in
> weston_debug_scope_timestamp API
> 
> Signed-off-by: Maniraj Devadoss 
> Reviewed-by: Pekka Paalanen 
> 
> Add explicit advertisement of debug scope names.
> 
> Signed-off-by: Daniel Stone 
> ---
>  Makefile.am  |   2 +
>  libweston/compositor.c   |   6 +
>  libweston/compositor.h   |   9 +
>  libweston/weston-debug.c | 723 +++
>  libweston/weston-debug.h | 107 ++
>  5 files changed, 847 insertions(+)
>  create mode 100644 libweston/weston-debug.c
>  create mode 100644 libweston/weston-debug.h
> 

Hi,

yet another thing I noticed.

> +/** Write data into a specific debug stream
> + *
> + * \param stream The debug stream to write into.
> + * \param data[in] Pointer to the data to write.
> + * \param len Number of bytes to write.
> + *
> + * Writes the given data (binary verbatim) into the debug stream.
> + * If \c len is zero or negative, the write is silently dropped.
> + *
> + * Writing is continued until all data has been written or
> + * a write fails. If the write fails due to a signal, it is re-tried.
> + * Otherwise on failure, the stream is closed and
> + * \c weston_debug_stream_v1.failure event is sent to the client.
> + *
> + * \memberof weston_debug_stream
> + */
> +WL_EXPORT void
> +weston_debug_stream_write(struct weston_debug_stream *stream,
> +   const char *data, size_t len)
> +{
> + ssize_t len_ = len;
> + ssize_t ret;
> + int e;
> +
> + if (stream->fd == -1)
> + return;
> +
> + while (len_ > 0) {
> + ret = write(stream->fd, data, len_);
> + e = errno;
> + if (ret < 0) {
> + if (e == EINTR)
> + continue;
> +
> + stream_close_on_failure(stream,
> + "Error writing %zd bytes: %s (%d)",
> + len_, strerror(e), e);
> + break;
> + }
> +
> + len_ -= ret;
> + data += ret;

If write() starts returning zero, we might be in for a very long loop.

> + }
> +}


Thanks,
pq


pgp6vsSyGLS8b.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 v5 10/14] compositor: protocol logger

2018-08-06 Thread Pekka Paalanen
On Mon, 30 Jul 2018 09:17:27 +
"Ucan, Emre (ADITG/ESB)"  wrote:

> Hi Daniel,
> 
> We found an issue with this patch. I am adding a patch to this email to fix 
> this issue. Please check:
> 
> Subject: [PATCH] main: copy va_list for second use
> 
> we are using va_list once for debug protocol and
> once for local logging to stdout. After first use
> the list is invalidated. Therefore, we have to copy
> it for the second use.
> 
> Signed-off-by: Emre Ucan 
> ---
>  compositor/main.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/compositor/main.c b/compositor/main.c
> index 147..7975af0 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -193,6 +193,9 @@ vlog(const char *fmt, va_list ap)
>  {
>   int l;
>   char timestr[128];
> + va_list ap2;
> +
> + va_copy(ap2, ap);
>  
>   if (weston_debug_scope_is_enabled(log_scope)) {
>   weston_debug_scope_printf(log_scope, "%s ",
> @@ -202,7 +205,8 @@ vlog(const char *fmt, va_list ap)
>   }
>  
>   l = weston_log_timestamp();
> - l += vfprintf(weston_logfile, fmt, ap);
> + l += vfprintf(weston_logfile, fmt, ap2);
> + va_end(ap2);
>  
>   return l;
>  }

Hi Emre,

nice, I wonder if this was the thing causing those crashes that IIRC
Maniraj was seeing and worked around them by having
weston_debug_scope_timestamp() return void.

Reading the manual, you are correct and this is needed.

Squashing this into the appropriate commit (patch 6) retains:
Reviewed-by: Pekka Paalanen 

custom_handler() and vlog_continue() seem to need the same fix.


Thanks,
pq


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


[RFC wayland-protocols] input-method: Add zwp-input-method-v2

2018-08-06 Thread Dorota Czaplejewicz
This protocol is based on v1, and current text-input-v3.

The pieces passing data relevant to the application on the other side of the 
compositor are a mirror copy of text-input-v3 events and requests.

Compared to input-method-v1:
- assumes that once preedit is displayed, no selection can be active, removing 
some selection handling
- follow text-input and removes language indicators
- explicitly attaches to seats
- removes "commit" text which would replace the preedit string automatically in 
case it wasn't "confirmed" (whatever it means)
- adds double-buffering in the same places as text-input-v3
- drops input_method_context and places its functionality directly in 
input_method
- removes the ability to move the cursor position outside of preedit. It still 
allows to delete a larger chunk of text and replace it with a preedit
- doesn't allow for sending of keyboard events to the compositor
- Doesn't define any surfaces except for a special compositor-positioned popup
---
Hi,

the third item on the path to well-supported virtual keyboard experience under 
Wayland comes from Purism. This one allows the compositor to delegate text 
input and composition duties to an application instead of handling it itself. 
Input-method-delegating compositor would typically become only a broker between 
the input method client and other applications.

I took the existing input-method protocol, and after dissecting it with the 
help of wlroots developers, I came up with an improved version, which is 
attached for your (re)viewing pleasure.

A large part of this protocol was taken from recent text-input-v3, and some 
description text was improved. Still, the protocol has a couple of rough edges, 
which is why it's titled RFC and not a patch.

The issues I have had most trouble with are related to the handling of keyboard 
grabs. These are meant to allow traditional, keyboard-based input methods, to 
take over keyboard input in order to compose text in a different way.

First, keyboard grabs are defined as unreliable. I'm not sure whether this is 
the right choice, but given that making them more reliable seems rather 
complicated, this is the default one.

In addition, handling the keyboard events themselves is troublesome, because an 
input method, even if it has a surface, is not meant to have keyboard focus 
while it's in operation. Returning wl_keyboard as new_id seems not to be 
possible due to versioning. Should the request make an existing wl_keyboard 
instance change behaviour? Or perhaps there should be a new 
zwp_input_method_keyboard mimicking the wl_keyboard interface?

I'm interested in your opinions.

Thank you,
Dorota Czaplejewicz 

 Makefile.am|   1 +
 unstable/input-method/input-method-unstable-v2.xml | 403 +
 2 files changed, 404 insertions(+)
 create mode 100644 unstable/input-method/input-method-unstable-v2.xml

diff --git a/Makefile.am b/Makefile.am
index 6394e26..f3b9f80 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -7,6 +7,7 @@ unstable_protocols =
\
unstable/text-input/text-input-unstable-v1.xml  
\
unstable/text-input/text-input-unstable-v3.xml  
\
unstable/input-method/input-method-unstable-v1.xml  
\
+   unstable/input-method/input-method-unstable-v2.xml  
\
unstable/xdg-shell/xdg-shell-unstable-v5.xml
\
unstable/xdg-shell/xdg-shell-unstable-v6.xml
\
unstable/relative-pointer/relative-pointer-unstable-v1.xml  
\
diff --git a/unstable/input-method/input-method-unstable-v2.xml 
b/unstable/input-method/input-method-unstable-v2.xml
new file mode 100644
index 000..8cf8e29
--- /dev/null
+++ b/unstable/input-method/input-method-unstable-v2.xml
@@ -0,0 +1,403 @@
+
+
+
+  
+Copyright © 2012, 2013 Intel Corporation
+Copyright © 2015, 2016 Jan Arne Petersen
+Copyright © 2017, 2018 Red Hat, Inc.
+Copyright © 2018   Purism SPC
+
+Permission is hereby granted, free of charge, to any person obtaining a
+copy of this software and associated documentation files (the "Software"),
+to deal in the Software without restriction, including without limitation
+the rights to use, copy, modify, merge, publish, distribute, sublicense,
+and/or sell copies of the Software, and to permit persons to whom the
+Software is furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice (including the next
+paragraph) shall be included in all copies or substantial portions of the
+Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND 

Re: [PATCH weston v5 06/14] compositor: offer logs via weston-debug

2018-08-06 Thread Pekka Paalanen
On Fri, 20 Jul 2018 20:03:27 +0100
Daniel Stone  wrote:

> From: Pekka Paalanen 
> 
> This registers a new weston-debug scope "log" through which one can get
> live log output interspersed with possible other debugging prints.
> 
> Signed-off-by: Pekka Paalanen 
> 
> pass the log_scope to weston_debug_scope_timestamp API to append
> the scope name to the timestamp
> 
> Signed-off-by: Maniraj Devadoss 
> Reviewed-by: Pekka Paalanen 
> Reviewed-by: Daniel Stone 
> ---
>  compositor/main.c | 22 ++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/compositor/main.c b/compositor/main.c
> index 2f34e1115..eaf4cf381 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c

> @@ -2490,6 +2508,9 @@ int main(int argc, char *argv[])
>   }
>   segv_compositor = wet.compositor;
>  
> + log_scope = weston_compositor_add_debug_scope(wet.compositor, "log",
> + "Weston and Wayland log\n", NULL, NULL);
> +
>   if (debug_protocol)
>   weston_compositor_enable_debug_protocol(wet.compositor);
>  
> @@ -2602,6 +2623,7 @@ out:
>   /* free(NULL) is valid, and it won't be NULL if it's used */
>   free(wet.parsed_options);
>  
> + weston_debug_scope_destroy(log_scope);

Maybe log_scope should be set to NULL again, in case
weston_compositor_destroy() logs something.

>   weston_compositor_destroy(wet.compositor);
>  
>  out_signals:

Otherwise still good.


Thanks,
pq


pgpNbPCx2MNEn.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 v5 03/14] libweston: add weston_debug API and implementation

2018-08-06 Thread Pekka Paalanen
On Fri, 20 Jul 2018 20:03:24 +0100
Daniel Stone  wrote:

> From: Pekka Paalanen 
> 
> weston_debug is both a libweston API for relaying debugging messages,
> and the compositor-debug wayland protocol implementation for accessing those
> debug messages from a Wayland client.
> 
> weston_debug_compositor_{create,destroy}() are private API, hence not
> exported.
> 
> Signed-off-by: Pekka Paalanen 
> 
> append the debug scope name along with the timestamp in
> weston_debug_scope_timestamp API
> 
> Signed-off-by: Maniraj Devadoss 
> Reviewed-by: Pekka Paalanen 
> 
> Add explicit advertisement of debug scope names.
> 
> Signed-off-by: Daniel Stone 
> ---
>  Makefile.am  |   2 +
>  libweston/compositor.c   |   6 +
>  libweston/compositor.h   |   9 +
>  libweston/weston-debug.c | 723 +++
>  libweston/weston-debug.h | 107 ++
>  5 files changed, 847 insertions(+)
>  create mode 100644 libweston/weston-debug.c
>  create mode 100644 libweston/weston-debug.h

...

> +/** Format current time as a string
> + * and append the debug scope name to it
> + *
> + * \param scope[in] debug scope.
> + * \param buf[out] Buffer to store the string.
> + * \param len Available size in the buffer in bytes.
> + * \return \c buf
> + *
> + * Reads the current local wall-clock time and formats it into a string.
> + * and append the debug scope name to it.
> + * The string is nul-terminated, even if truncated.
> + */
> +WL_EXPORT char *
> +weston_debug_scope_timestamp(struct weston_debug_scope *scope,
> +  char *buf, size_t len)
> +{
> + struct timeval tv;
> + struct tm *bdt;
> + char string[128];
> + size_t ret = 0;
> +
> + gettimeofday(, NULL);
> +
> + bdt = localtime(_sec);
> + if (bdt)
> + ret = strftime(string, sizeof string,
> +"%Y-%m-%d %H:%M:%S", bdt);
> +
> + if (ret > 0)
> + snprintf(buf, len, "[%s.%03ld][%s]", string,
> +  tv.tv_usec / 1000, scope->name);
> + else
> + snprintf(buf, len, "[?][%s]", scope->name);
> +
> + return buf;
> +}

Hi,

realized something when looking at the "log" debug scope patch:
weston_debug_scope_timestamp() should probably be resilient against
scope == NULL, all the other functions already are.

weston_log gets initialized early, but the log debug scope gets
initialized after the compositor. If something logs something between
those two...


Thanks,
pq


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


[PATCH weston 2/5] doc: Update CONTRIBUTING for Weston

2018-08-06 Thread Daniel Stone
Change some Wayland-specific references to instead refer to Weston.

Signed-off-by: Daniel Stone 
Reviewed-by: Quentin Glidic 
Reviewed-by: Pekka Paalanen 
---
 CONTRIBUTING.md | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 4273d99d4..91b3fffe9 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -1,4 +1,4 @@
-Contributing to Wayland
+Contributing to Weston
 ===
 
 Sending patches
@@ -11,11 +11,11 @@ The first line of a commit message should contain a prefix 
indicating
 what part is affected by the patch followed by one sentence that
 describes the change. For examples:
 
-protocol: Support scaled outputs and surfaces
+compositor-drm: Support modifiers for drm_fb
 
 and
 
-doc: generate server documentation from XML too
+input: do not forward unmatched touch-ups
 
 If in doubt what prefix to use, look at other commits that change the
 same file(s) as the patch being sent.
@@ -193,9 +193,9 @@ or bullying behaviour is not tolerated by the project.
 Licensing
 =
 
-Wayland is licensed with the intention to be usable anywhere X.org is.
+Weston is licensed with the intention to be usable anywhere X.org is.
 Originally, X.org was covered under the MIT X11 license, but changed to
-the MIT Expat license.  Similarly, Wayland was covered initially as MIT
+the MIT Expat license.  Similarly, Weston was covered initially as MIT
 X11 licensed, but changed to the MIT Expat license, following in X.org's
 footsteps.  Other than wording, the two licenses are substantially the
 same, with the exception of a no-advertising clause in X11 not included
-- 
2.17.1

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


[PATCH weston 4/5] doc: Use GitLab MRs for patches, not the list

2018-08-06 Thread Daniel Stone
Though Wayland and the protocols still use mail-based patch review,
Weston can now move to GitLab MRs with review through that system.

Add some documentation on how to submit patches through GitLab,
specifically targeted at people who may be familiar with GitLab review,
but not familiar with our rebasing microcommit workflow.

Signed-off-by: Daniel Stone 
Reviewed-by: Quentin Glidic 
Reviewed-by: Pekka Paalanen 
---
 CONTRIBUTING.md | 143 
 1 file changed, 73 insertions(+), 70 deletions(-)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 91b3fffe9..f2e4750df 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -4,8 +4,48 @@ Contributing to Weston
 Sending patches
 ---
 
-Patches should be sent to **wayland-devel@lists.freedesktop.org**, using
-`git send-email`. See [git documentation] for help.
+Patches should be sent via
+[GitLab merge 
requests](https://docs.gitlab.com/ce/gitlab-basics/add-merge-request.html).
+Weston is
+[hosted on freedesktop.org's 
GitLab](https://gitlab.freedesktop.org/wayland/weston/):
+in order to submit code, you should create an account on this GitLab instance,
+fork the core Weston repository, push your changes to a branch in your new
+repository, and then submit these patches for review through a merge request.
+
+Weston formerly accepted patches via `git-send-email`, sent to
+**wayland-devel@lists.freedesktop.org**; these were
+[tracked using Patchwork](https://patchwork.freedesktop.org/projects/wayland/).
+Some old patches continue to be sent this way, and we may accept small new
+patches sent to the list, but please send all new patches through GitLab merge
+requests.
+
+Formatting and separating commits
+-
+
+Unlike many projects using GitHub and GitLab, Weston has a
+[linear, 'recipe' style 
history](http://www.bitsnbites.eu/git-history-work-log-vs-recipe/).
+This means that every commit should be small, digestible, stand-alone, and
+functional. Rather than a purely chronological commit history like this:
+
+doc: final docs for view transforms
+fix tests when disabled, redo broken doc formatting
+better transformed-view iteration (thanks Hannah!)
+try to catch more cases in tests
+tests: add new spline test
+fix compilation on splines
+doc: notes on reticulating splines
+compositor: add spline reticulation for view transforms
+
+we aim to have a clean history which only reflects the final state, broken up
+into functional groupings:
+
+compositor: add spline reticulation for view transforms
+compositor: new iterator for view transforms
+tests: add view-transform correctness tests
+doc: fix Doxygen formatting for view transforms
+
+This ensures that the final patch series only contains the final state,
+without the changes and missteps taken along the development process.
 
 The first line of a commit message should contain a prefix indicating
 what part is affected by the patch followed by one sentence that
@@ -54,74 +94,37 @@ deserve, and the patches may cause redundant review effort.
 Tracking patches and following up
 -
 
-[Wayland Patchwork](http://patchwork.freedesktop.org/project/wayland/list/) is
-used for tracking patches to Wayland and Weston. Xwayland patches are tracked
-with the [Xorg project](https://patchwork.freedesktop.org/project/Xorg/list/)
-instead. Libinput patches, even though they use the same mailing list as
-Wayland, are not tracked in the Wayland Patchwork.
-
-The following applies only to Wayland and Weston.
-
-If a patch is not found in Patchwork, there is a high possibility for it to be
-forgotten. Patches attached to bug reports or not arriving to the mailing list
-because of e.g. subscription issues will not be in Patchwork because Patchwork
-only collects patches sent to the list.
-
-When you send a revised version of a patch, it would be very nice to mark your
-old patch as superseded (or rejected, if that is applicable). You can change
-the status of your own patches by registering to Patchwork - ownership is
-identified by email address you use to register. Updating your patch status
-appropriately will help maintainer work.
-
-The following patch states are found in Patchwork:
-
-- **New**:
-Patches under discussion or not yet processed.
-
-- **Under review**:
-Mostly unused state.
-
-- **Accepted**:
-The patch is merged in the master branch upstream, as is or slightly
-modified.
-
-- **Rejected**:
-The idea or approach is rejected and cannot be fixed by revising
-the patch.
-
-- **RFC**:
-Request for comments, not meant to be merged as is.
-
-- **Not applicable**:
-The email was not actually a patch, or the patch is not for Wayland or
-Weston. Libinput patches are usually automatically ignored by Wayland
-Patchwork, but if they get through, they will be marked as Not
-applicable.
-
-- **Changes requested**:
-

[PATCH wayland 5/5] contributing: Weston now uses GitLab MRs

2018-08-06 Thread Daniel Stone
Note that Weston uses GitLab MRs for review, not mail.

Signed-off-by: Daniel Stone 
---
 CONTRIBUTING.md | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 4273d99d..686ed63f 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -55,12 +55,16 @@ Tracking patches and following up
 -
 
 [Wayland Patchwork](http://patchwork.freedesktop.org/project/wayland/list/) is
-used for tracking patches to Wayland and Weston. Xwayland patches are tracked
-with the [Xorg project](https://patchwork.freedesktop.org/project/Xorg/list/)
-instead. Libinput patches, even though they use the same mailing list as
+used for tracking patches to Wayland. Xwayland patches are tracked with the
+[Xorg project](https://patchwork.freedesktop.org/project/Xorg/list/)
+instead. Weston uses
+[GitLab merge 
requests](https://gitlab.freedesktop.org/wayland/weston/merge_requests)
+for code review, and does not use mailing list review at all.
+
+Libinput patches, even though they use the same mailing list as
 Wayland, are not tracked in the Wayland Patchwork.
 
-The following applies only to Wayland and Weston.
+The following applies only to Wayland.
 
 If a patch is not found in Patchwork, there is a high possibility for it to be
 forgotten. Patches attached to bug reports or not arriving to the mailing list
@@ -93,8 +97,8 @@ The following patch states are found in Patchwork:
 Request for comments, not meant to be merged as is.
 
 - **Not applicable**:
-The email was not actually a patch, or the patch is not for Wayland or
-Weston. Libinput patches are usually automatically ignored by Wayland
+The email was not actually a patch, or the patch is not for Wayland.
+Libinput patches are usually automatically ignored by Wayland
 Patchwork, but if they get through, they will be marked as Not
 applicable.
 
@@ -121,7 +125,7 @@ shown.
 
 There is also a command line interface to Patchwork called `pwclient`, see
 http://patchwork.freedesktop.org/project/wayland/
-for links where to get it and the sample `.pwclientrc` for Wayland/Weston.
+for links where to get it and the sample `.pwclientrc` for Wayland.
 
 
 Coding style
-- 
2.17.1

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


[PATCH weston 1/5] Add CONTRIBUTING.md document

2018-08-06 Thread Daniel Stone
From: Pekka Paalanen 

Taken from Pekka's wayland/wayland@630c25f4c160 and follow-ups, use
Wayland's CONTRIBUTING document as a basis for Weston.

Signed-off-by: Daniel Stone 
Reviewed-by: Quentin Glidic 
Reviewed-by: Pekka Paalanen 
---
 CONTRIBUTING.md | 343 
 Makefile.am |   1 +
 2 files changed, 344 insertions(+)
 create mode 100644 CONTRIBUTING.md

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
new file mode 100644
index 0..4273d99d4
--- /dev/null
+++ b/CONTRIBUTING.md
@@ -0,0 +1,343 @@
+Contributing to Wayland
+===
+
+Sending patches
+---
+
+Patches should be sent to **wayland-devel@lists.freedesktop.org**, using
+`git send-email`. See [git documentation] for help.
+
+The first line of a commit message should contain a prefix indicating
+what part is affected by the patch followed by one sentence that
+describes the change. For examples:
+
+protocol: Support scaled outputs and surfaces
+
+and
+
+doc: generate server documentation from XML too
+
+If in doubt what prefix to use, look at other commits that change the
+same file(s) as the patch being sent.
+
+The body of the commit message should describe what the patch changes
+and why, and also note any particular side effects. This shouldn't be
+empty on most of the cases. It shouldn't take a lot of effort to write
+a commit message for an obvious change, so an empty commit message
+body is only acceptable if the questions "What?" and "Why?" are already
+answered on the one-line summary.
+
+The lines of the commit message should have at most 76 characters, to
+cope with the way git log presents them.
+
+See [notes on commit messages] for a recommended reading on writing commit
+messages.
+
+Your patches should also include a Signed-off-by line with your name and
+email address.  If you're not the patch's original author, you should
+also gather S-o-b's by them (and/or whomever gave the patch to you.) The
+significance of this is that it certifies that you created the patch,
+that it was created under an appropriate open source license, or
+provided to you under those terms.  This lets us indicate a chain of
+responsibility for the copyright status of the code.
+
+We won't reject patches that lack S-o-b, but it is strongly recommended.
+
+When you re-send patches, revised or not, it would be very good to document the
+changes compared to the previous revision in the commit message and/or the
+cover letter. If you have already received Reviewed-by or Acked-by tags, you
+should evaluate whether they still apply and include them in the respective
+commit messages. Otherwise the tags may be lost, reviewers miss the credit they
+deserve, and the patches may cause redundant review effort.
+
+
+Tracking patches and following up
+-
+
+[Wayland Patchwork](http://patchwork.freedesktop.org/project/wayland/list/) is
+used for tracking patches to Wayland and Weston. Xwayland patches are tracked
+with the [Xorg project](https://patchwork.freedesktop.org/project/Xorg/list/)
+instead. Libinput patches, even though they use the same mailing list as
+Wayland, are not tracked in the Wayland Patchwork.
+
+The following applies only to Wayland and Weston.
+
+If a patch is not found in Patchwork, there is a high possibility for it to be
+forgotten. Patches attached to bug reports or not arriving to the mailing list
+because of e.g. subscription issues will not be in Patchwork because Patchwork
+only collects patches sent to the list.
+
+When you send a revised version of a patch, it would be very nice to mark your
+old patch as superseded (or rejected, if that is applicable). You can change
+the status of your own patches by registering to Patchwork - ownership is
+identified by email address you use to register. Updating your patch status
+appropriately will help maintainer work.
+
+The following patch states are found in Patchwork:
+
+- **New**:
+Patches under discussion or not yet processed.
+
+- **Under review**:
+Mostly unused state.
+
+- **Accepted**:
+The patch is merged in the master branch upstream, as is or slightly
+modified.
+
+- **Rejected**:
+The idea or approach is rejected and cannot be fixed by revising
+the patch.
+
+- **RFC**:
+Request for comments, not meant to be merged as is.
+
+- **Not applicable**:
+The email was not actually a patch, or the patch is not for Wayland or
+Weston. Libinput patches are usually automatically ignored by Wayland
+Patchwork, but if they get through, they will be marked as Not
+applicable.
+
+- **Changes requested**:
+Reviewers determined that changes to the patch are needed. The
+submitter is expected to send a revised version. (You should
+not wait for your patch to be set to this state before revising,
+though.)
+
+- **Awaiting upstream**:
+Mostly unused as the patch is waiting for upstream actions but
+is not shown in the 

[PATCH 0/5] Weston contributing doc, GitLab MRs

2018-08-06 Thread Daniel Stone
Hi,
Thanks to Pekka and Quentin for the review: this fixes some errors in
the original README/CONTRIBUTING doc submission for Weston, retaining
the GitLab MR move. It also adds a patch to Wayland's CONTRIBUTING.md,
severing the two sets of instructions.

Cheers,
Daniel


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


Re: [PATCH weston 4/4] doc: Use GitLab MRs for patches, not the list

2018-08-06 Thread Daniel Stone
Hi Pekka,

On Fri, 3 Aug 2018 at 12:05, Pekka Paalanen  wrote:
> I'm happy to move to MR-based work flow immediately.

\o/

> From the above link, I found this:
> http://www.bitsnbites.eu/git-history-work-log-vs-recipe/
> which explain the below in more words. Maybe include this link as well?
>
> Linear history doesn't exactly imply "recipe" history, we want both. I
> only came to think of the differences after reading both of the above
> links.

Yeah, good point. I've just changed it to include the recipe link:
linear history is a far less important detail than the recipe side.

> Patches 1 and 2:
> Reviewed-by: Pekka Paalanen 
>
> Patch 4 also R-b with fixes, and patch 3 practically as well.
>
> Oh wait, the rename of README to README.md will probably drop the file
> from the dist tar-ball, since it's not auto-magic by autotools anymore.
> CONTRIBUTING.md should also be added to dist. And the screenshot.jpg.
>
> Should we start with Gitlab MRs already, but merge the doc patches only
> after the final release, as per our documented release process?

Thanks, I'll resubmit now. I am keen to avoid a situation where we
have a mismatch between what we do (GitLab MRs) and the contribution
documentation (mailing list). I think it would be great to get these
in the release, since they carry no risk of code regression, and mean
that anyone reading from the release will immediately see how to
contribute properly.

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


Re: [PATCH weston v5 05/14] clients: add weston-debug

2018-08-06 Thread Pekka Paalanen
On Fri, 20 Jul 2018 20:03:26 +0100
Daniel Stone  wrote:

> From: Pekka Paalanen 
> 
> A tool for accessing the zcompositor_debug_v1 interface features.
> 
> Installed along weston-info, because it should be potentially useful for
> people running libweston-based compositors.
> 
> Signed-off-by: Pekka Paalanen 
> 
> Added a man page for weston-debug client
> 
> Signed-off-by: Maniraj Devadoss 
> [Pekka: fixed 'missing braces aroudn initializer' warning]
> 
> Add --list and --all arguments, using interface advertisement.
> 
> Signed-off-by: Daniel Stone 
> ---
>  Makefile.am|  16 +-
>  clients/weston-debug.c | 453 +
>  man/weston-debug.man   |  46 +
>  3 files changed, 513 insertions(+), 2 deletions(-)
>  create mode 100644 clients/weston-debug.c
>  create mode 100644 man/weston-debug.man

...

> diff --git a/clients/weston-debug.c b/clients/weston-debug.c
> new file mode 100644
> index 0..59dacd269
> --- /dev/null
> +++ b/clients/weston-debug.c
> @@ -0,0 +1,453 @@
> +/*
> + * Copyright © 2017 Pekka Paalanen 
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> + * a copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sublicense, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial
> + * portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include "config.h"
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#include "shared/helpers.h"
> +#include "shared/zalloc.h"
> +#include "weston-debug-client-protocol.h"
> +
> +struct debug_app {
> + struct {
> + bool help;
> + bool list;
> + bool bind_all;
> + char *output;
> + char *outfd;
> + } opt;
> +
> + int out_fd;
> + struct wl_display *dpy;
> + struct wl_registry *registry;
> + struct weston_debug_v1 *debug_iface;
> + struct wl_list stream_list;
> +};
> +
> +struct debug_stream {
> + struct wl_list link;
> + bool should_bind;
> + char *name;
> + struct weston_debug_stream_v1 *obj;
> +};

...

> +static void
> +debug_advertise(void *data, struct weston_debug_v1 *debug, const char *name)
> +{
> + struct debug_app *app = data;

Add empty line.

> + (void) stream_find(app, name);
> +}
> +
> +static const struct weston_debug_v1_listener debug_listener = {
> + debug_advertise,
> +};

...

> +static int
> +parse_cmdline(struct debug_app *app, int argc, char **argv)
> +{
> + static const struct option opts[] = {
> + { "help", no_argument, NULL, 'h' },
> + { "list", no_argument, NULL, 'l' },
> + { "all-streams", no_argument, NULL, 'a' },
> + { "output", required_argument, NULL, 'o' },
> + { "outfd", required_argument, NULL, 'f' },
> + { 0 }
> + };
> + static const char optstr[] = "hlao:f:";
> + int c;
> + bool failed = false;
> +
> + while (1) {
> + c = getopt_long(argc, argv, optstr, opts, NULL);
> + if (c == -1)
> + break;
> +
> + switch (c) {
> + case 'h':
> + app->opt.help = true;
> + break;
> + case 'l':
> + app->opt.list = true;
> + break;
> + case 'a':
> + app->opt.bind_all = true;
> + break;
> + case 'o':
> + free(app->opt.output);
> + app->opt.output = strdup(optarg);
> + break;
> + case 'f':
> + free(app->opt.outfd);
> + app->opt.outfd = strdup(optarg);
> + break;
> + case '?':
> + failed = true;
> + break;
> + default:
> + fprintf(stderr, "huh? getopt => %c (%d)\n", c, c);
> +  

Re: How to release the weston_buffer after weston_view is destroyed.

2018-08-06 Thread Pekka Paalanen
On Fri, 3 Aug 2018 18:04:36 -0400
Sichem Zhou  wrote:

> Hi Pekka,
> 
> Thanks for your reply. Yes, preferably if I can hide the map and unmap from
> client, in this case I cannot because it is part of a desktop shell. After
> I unmap it and remap the view next time, it is not expected the shell to
> continue from last draw. I would want the shell be drawing total different
> things instead. If I simply remap, I shall see the last frame that did get
> drawed.
> 
> It can be seen as a problem in the application, designing smarter clients
> which is aware of such details, by not sending the last frame for example,
> but I found an easier solution by unmap the view in the `frame_signal`. It
> worked quite well.

Hi,

if you need the unmap to happen immediately without round-tripping to
the helper client, I think a good solution for your case would be for
the desktop-shell to unmap the surface and send an event to the helper
client telling that the surface is unmapped. That way the client can
destroy the wl_surface, which will make the compositor automatically
release buffers it might have held, and there is no chance of showing
outdated content on the next time.

In my opinion, the frame_signal solution is a hack. It is not really
meant for what you are doing it with it, and it does not allow the
compositor to release buffers it would not have released anyway.

It also does not guarantee that the client would not block inside
eglSwapBuffers, you have to prevent that in the client anyway. Usually
unmapping a window permanently is initiated by the client by e.g.
destroying the wl_surface, which naturally avoids an indefinite wait
for a frame callback. That could be a response to a specific event.

With the frame_signal hack, even if you unmap the view after sending
out the frame callbacks, what will guarantee that the client will not
post another frame as a response to the frame callback you just sent?
And then another frame after that would again hit the indefinite
blocking inside eglSwapBuffers(). I suspect your design is quite
fragile as is.


Thanks,
pq


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