Re: [PATCH wayland-protocols 2/3] tests: Add compile tests

[Daniel Stone]

Hi,
Meant 'patch' rather than 'series' when offering my R-b to the other
patch. Oops.

On 11 October 2017 at 10:00, Jonas Ådahl  wrote:
> +  # Check that header can be included by a pedantic C99 compiler
> +  test_name = 'test-build-pedantic-@0@'.format(protocol.underscorify())
> +  test_name_source = '@0@.c'.format(test_name)
> +  test_source = custom_target(test_name_source,
> +  input: 'build-pedantic.c.in',
> +  output: test_name_source,
> +  command: replace_command)
> +  pedantic_test_executable = executable(test_name,
> +[ test_source,
> +  client_header,
> +  server_header,
> +  code ],
> +dependencies: libwayland,
> +c_args: [ '-std=c99',
> +  '-pedantic',
> +  '-Wall',
> +  '-Werror' ],
> +install: false)
> +  test(test_name, pedantic_test_executable)
> +
> +  # Check that the header
> +  if not protocol.contains('xdg-foreign-unstable-v1')

The comment ends abruptly, the xdg_foreign exclusion is a massive
non-sequitur, and also test_configuration can be deleted. With that,
this is also:
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 wayland-protocols 1/3] Add meson build system support

[Daniel Stone]

Hi Jonas,

On 11 October 2017 at 10:00, Jonas Ådahl  wrote:
> +wayland_scanner = find_program('wayland-scanner')

It would be good to have this follow Quentin's suggestion for how to
find wayland-scanner generally. Apart from that, series 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 v2 wayland 00/11] Stop leaking file descriptors

[Daniel Stone]

Hi Derek,

On 13 April 2017 at 17:51, Derek Foreman  wrote:
> Moved the test cases to the end so they're not introduced in a failed
> state.
>
> Reworked the removal of the global zombie singleton patch - we now
> create a wl_zombie at proxy creation time and store the number of
> fds for each opcode in that.  If no signature requires fds we just
> use a NULL pointer to avoid useless malloc/free.  At client
> disconnect we do a map walk to inter any zombie left behind and
> make sure we don't leak memory.
>
> Thanks to Pekka and Jonas for suggestions on how to handle this
> without breaking API as I'd previously done.
>
> To perform the map walk I added a new patch that changes the
> wl_map_for_each() function to provide the entry flags - we can't
> look up flags from the iterator callback without dereferencing the
> pointer, and with my changes we don't know what the data type stored
> in that map location is.  This only changes internal callers - the
> publicly visible map walk function still acts as before.

Thanks for this. It looks fairly good to me so far. I've pushed 1, 5
and 9 as trivially correct. As said in comments to 4, I feel like a
slightly improved 4 could also replace 2 and 3 at the same time. 6, 8
and 10 all have my R-b, though a comment as to why three is the
correct number of roundtrips wouldn't go astray on 10 (it seems right
to me, mind). Unfortunately I'm not getting far into 11 without my
eyes glazing over, and I haven't given 7 a pass over for actual
lifetime handling yet, but that at least gets my A-b.

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

Re: [PATCH v2 wayland 04/11] connection: Make wl_closure_destroy() close fds of undispatched closures

[Daniel Stone]

Hi Derek,

On 13 April 2017 at 17:51, Derek Foreman  wrote:
>  void
> -wl_closure_destroy(struct wl_closure *closure)
> +wl_closure_destroy(struct wl_closure *closure, bool dispatched)
>  {
> +   /* wl_closure_destroy has free() semantics */
> +   if (!closure)
> +   return;
> +   /* If message is NULL then this closure failed during
> +* creation, and any fd cleanup has been done by the
> +* caller
> +*/
> +   if (!dispatched && closure->message)
> +   wl_closure_close_fds(closure);
> free(closure);
>  }

Maybe there's something I'm missing here, but is there any reason to
not set closure->message unconditionally? If you moved the
closure->message assignment up inside marshal/demarshal, and ran
through initialising all the handle-type arg values to -1 (let's call
this hypothetical function wl_closure_init - take an 'extra' size
param to preserve the single-alloc demarshal semantics as well), then
you could just use this everywhere rather than manual fd counting.

Also, please use an enum rather than true/false: it was only after I
got to like four 'this seems completely backwards?' comments that I
realised true meant 'don't do the new behaviour', and false meant 'do
extra stuff'. I blame the flu medication, but still ...

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

Re: [PATCH wayland] scanner: Add autoconf macro to check for the proper scanner

[Daniel Stone]

Jussi, Tomek, Emil,

On 18 August 2017 at 10:36, Quentin Glidic
 wrote:
> On 8/18/17 11:30 AM, Quentin Glidic wrote:
>> Projects have been using various ways to check for the wayland-scanner,
>> mostly based on their developper own use case, and often not allowing
>> others use cases to work without a workaround.
>>
>> Hopefully this macro will support all use cases without needing user
>> action.
>>
>> Signed-off-by: Quentin Glidic 
>> ---
>>
>> Everyone should test this macro for their own project and use cases.
>> Using the ${WAYLAND_SCANNER} variable should just work (assuming you have
>> the proper
>> wayland package built in the expected env). In very very rare cases,
>> setting the
>> WAYLAND_SCANNER variable as a ./configure argument may be needed, but
>> nothing else.
>>
>> Please let me know with enough details if your use case is not working
>> with it.

Any comments on Quentin's suggestion here? Would they be enough for you?

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

Re: [RFC weston] libweston: Do not include subsurfaces with NULL buffer in bounding box

[Daniel Stone]

Hi Philipp,

On 28 July 2017 at 15:41, Philipp Kerling  wrote:
> I was pondering how to remove the window decorations of my application
> (which live in subsurfaces) when going full screen without flickering.
>
> At first I just destroyed the surfaces, but that lead to flicker
> (window without decoration will appear for one frame before the full
> screen application is displayed) since subsurface destruction is not
> specified to be synchronized to the commit of the main surface.
>
> Then I tried attaching a NULL wl_buffer to the surfaces. This works and
> does not flicker, but it has the problem that weston will disconnect my
> application when going to full screen for violating the configured
> xdg_shell buffer size. The reason seems to be that attaching a NULL
> wl_buffer does not reset the surface size and
> weston_surface_get_bounding_box still thinks that the surface has the
> prior size.
>
> The patch fixes this by checking explicitly for the buffer and is
> confirmed to fix the issue, but I am not sure whether this is the right
> solution. Maybe surface->width and surface->height should be set to 0
> anyway?

Hm. Yes, it does seem like the width/height should be 0, which is
undefined but not disallowed by the subsurface spec. I have no idea if
this would work with Mutter in particular, which had problems
implementing that for top-levels.

In any case, perhaps one way you could do it would be to stack the
decoration subsurfaces below your primary surface before committing
your fullscreen buffer. It's ugly, but does seem like it should work
...

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

Re: [PATCH 2/2] server: add log message when client connection is destroyed due to an error

[Daniel Stone]

Hi Mathias,
Thanks for your patch! The idea seems fine, but I have a few comments.

On 8 June 2017 at 08:39,   wrote:
> @@ -313,16 +326,30 @@ wl_client_connection_data(int fd, uint32_t mask, void 
> *data)
> uint32_t resource_flags;
> int opcode, size, since;
> int len;
> +   int err;
> +   socklen_t errlen;
>
> -   if (mask & (WL_EVENT_ERROR | WL_EVENT_HANGUP)) {
> +   if (mask & WL_EVENT_HANGUP) {
> wl_client_destroy(client);
> return 1;
> }
>
> +   if (mask & WL_EVENT_ERROR) {
> +   // query socket error
> +   errlen = sizeof(err);
> +   if (getsockopt(fd, SOL_SOCKET, SO_ERROR, , )) {
> +   // when getsockopt fails use its error instead
> +   err = errno;

I don't believe that errno is necessarily related in this case, so we
can't pass it to the client.

> @@ -334,7 +361,8 @@ wl_client_connection_data(int fd, uint32_t mask, void 
> *data)
> if (mask & WL_EVENT_READABLE) {
> len = wl_connection_read(connection);
> if (len == 0 || (len < 0 && errno != EAGAIN)) {
> -   wl_client_destroy(client);
> +   destroy_client_with_error(
> +   client, "failed to read client connection", 
> errno);

If (len == 0), errno may be irrelevant.

The same is true in your first patch: I believe there are a few places
where we might leak errno to the client when it is not actually
relevant. It would be nice to change the 'if (client->error)' checks
in that patch to (client->error != 0) as well, just to be more
explicit.

Also, your patch uses // C++ comments like this.

Please use /* Old-style C comments */ instead, but whilst you're at
it, comments like 'query socket error' are obvious enough from reading
the surrounding context that they are not really required.

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

Re: [PATCH weston] editor: unify key handling

[Daniel Stone]

Hi Weng,
Thanks for the patch, and sorry for the horrendous delay in giving you feedback.

In general, your patch looks very good and makes a lot of sense. However:

On 1 February 2017 at 03:22, Weng Xuetian  wrote:
> @@ -395,81 +402,8 @@ text_input_keysym(void *data,
>   uint32_t modifiers)
>  {
> struct text_entry *entry = data;
> -   [...]
> +   handle_key(entry, time, key, state, modifiers);
>  }
>
> @@ -1381,29 +1313,19 @@ handle_bound_key(struct editor *editor,
>  }
>
>  static void
> -key_handler(struct window *window,
> -   struct input *input, uint32_t time,
> -   uint32_t key, uint32_t sym, enum wl_keyboard_key_state state,
> -   void *data)
> -{
> -   struct editor *editor = data;
> -   struct text_entry *entry;
> +handle_key(struct text_entry *entry, uint32_t time,
> +  uint32_t sym, enum wl_keyboard_key_state state,
> +  uint32_t modifiers) {
> +   struct input *input = entry->input;
> const char *new_char;
> char text[16];
> -   uint32_t modifiers;
> -
> -   if (!editor->active_entry)
> -   return;
> -
> -   entry = editor->active_entry;
> -
> -   if (state != WL_KEYBOARD_KEY_STATE_PRESSED)
> +   if (!input || state != WL_KEYBOARD_KEY_STATE_PRESSED)
> return;
>
> modifiers = input_get_modifiers(input);
> if ((modifiers & MOD_CONTROL_MASK) &&
> (modifiers & MOD_SHIFT_MASK) &&

This is now incorrect. It overrides the modifiers passed from the
keysym event, with those from the general keyboard state, which may be
different.

Note that the two sets of modifiers are different. The keysym
modifiers are 'raw', e.g. see how we look up shift_mask from the XKB
keymap and compare against that in the keysym state. In order to use
the same codepath, we would need to transform the modifiers from the
keysym event the way that input_get_modifiers() does, i.e. into
MOD_CONTROL_MASK/MOD_SHIFT_MASK.

Could you please submit another revision correcting this? You may want
to split this into a couple patches: one preparatory patch which
allows clients to do the input_get_modifiers() mapping, another patch
which makes the keysym handler use that (deleting shift_mask in the
process), and then one last patch which merges the two.

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

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

[Daniel Stone]

Hi Vincent,

On 20 March 2017 at 09:50, Vincent ABRIOU  wrote:
> Any feedback on this patch?

Sorry for not replying sooner.

With the modifiers code having landed, this would need some rework to
fit there. When the modifiers query is available, we'd need to drop
the RGB format advertisement. For the YUV formats, we'd need to check
the base formats that each YUV format decomposes to (e.g. for NV12,
check R8 + RG88), take the intersection of available modifiers for
those formats, and send out the intersection with the YUV format.

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

Re: [PATCH weston 2/2] weston: add --device option for DRM-backend

[Daniel Stone]

Hi Pekka,

On 28 March 2017 at 16:26, Pekka Paalanen  wrote:
> Developers with testing rigs having multiple graphics cards plugged in
> often want to test things on a specific card. We have ways to choose a
> card through seat assignments, but configuring that run by run is
> awkward.
>
> Add a command line option for opening a specific DRM device.
>
> [...]
>
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -565,6 +565,7 @@ usage(int error_code)
> "  --connector=ID\tBring up only this connector\n"
> "  --seat=SEAT\t\tThe seat that weston should run on\n"
> "  --tty=TTY\t\tThe tty to use\n"
> +   "  --device=CARD\t\tThe DRM device to use, e.g. \"card0\".\n"
> "  --use-pixman\t\tUse the pixman (CPU) renderer\n"
> "  --current-mode\tPrefer current KMS mode over EDID 
> preferred mode\n\n");
>  #endif
> @@ -1225,6 +1226,7 @@ load_drm_backend(struct weston_compositor *c,
> { WESTON_OPTION_INTEGER, "connector", 0,  },
> { WESTON_OPTION_STRING, "seat", 0, _id },
> { WESTON_OPTION_INTEGER, "tty", 0,  },
> +   { WESTON_OPTION_STRING, "device", 0, _device 
> },

Please can we call it --drm-device instead? With that, the two are:
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/5] gl-renderer: always enable unpack subimage and RG textures in ES3 contexts

[Arnaud Vrac]

On Mon, Dec 4, 2017 at 5:57 PM, Emil Velikov  wrote:
> On 29 November 2017 at 14:25, Arnaud Vrac  wrote:
>> From: Arnaud Vrac 
>>
>> The GL_EXT_unpack_subimage and GL_EXT_texture_rg are part of the core ES
>> 3.0 specification, so also check the GL driver version in addition to
>> the extension string to determine if those features are supported.
>>
> Reviewed-by: Emil Velikov 
>
> Out of curiosity: did you notice a ES 3.x driver that doesn't expose
> these in the extensions string?
>
> -Emil

Yes. I've notified the vendor about those missing extensions in the
extensions string, hopefully it will be fixed.

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

Re: [PATCH weston 5/5] gl-renderer: use correct pixel shader for NV12 format uploaded to RG texture

[Arnaud Vrac]

On Mon, Dec 4, 2017 at 6:08 PM, Emil Velikov  wrote:
> On 29 November 2017 at 14:25, Arnaud Vrac  wrote:
>> Signed-off-by: Arnaud Vrac 
>
> Please mention how you've spotted and/or verified this.
>
> I'm ~90% this is correct, although I would check with the author.
> Vincent, can you double check the patch/series [1]?

I'm also not 100% sure about this one, I found all three drivers I
mentionned on the cover letter displayed the wrong colors for NV12
with R/G textures. This just seemed the logical fix after reading the
pixel shaders code.

I'll mention this in the commit message.

Thanks,
-Arnaud

>
> Fixes: 00a03d2f724 ("gl-renderer: add support of WL_SHM_FORMAT_NV12")
> Cc: Vincent Abriou 
>
> -Emil
>> ---
>>  libweston/gl-renderer.c | 3 ++-
>>  1 file cNphanged, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
>> index 60a7bf06..4fffa78c 100644
>> --- a/libweston/gl-renderer.c
>> +++ b/libweston/gl-renderer.c
>> @@ -1596,7 +1596,6 @@ gl_renderer_attach_shm(struct weston_surface *es, 
>> struct weston_buffer *buffer,
>> }
>> break;
>> case WL_SHM_FORMAT_NV12:
>> -   gs->shader = >texture_shader_y_xuxv;
>> pitch = wl_shm_buffer_get_stride(shm_buffer);
>> gl_pixel_type = GL_UNSIGNED_BYTE;
>> num_planes = 2;
>> @@ -1605,9 +1604,11 @@ gl_renderer_attach_shm(struct weston_surface *es, 
>> struct weston_buffer *buffer,
>> gs->hsub[1] = 2;
>> gs->vsub[1] = 2;
>> if (gr->has_gl_texture_rg) {
>> +   gs->shader = >texture_shader_y_uv;
>> gl_format[0] = GL_R8_EXT;
>> gl_format[1] = GL_RG8_EXT;
>> } else {
>> +   gs->shader = >texture_shader_y_xuxv;
>> gl_format[0] = GL_LUMINANCE;
>> gl_format[1] = GL_LUMINANCE_ALPHA;
>> }
>> --
>> 2.15.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 5/5] gl-renderer: use correct pixel shader for NV12 format uploaded to RG texture

[Emil Velikov]

On 29 November 2017 at 14:25, Arnaud Vrac  wrote:
> Signed-off-by: Arnaud Vrac 

Please mention how you've spotted and/or verified this.

I'm ~90% this is correct, although I would check with the author.
Vincent, can you double check the patch/series [1]?

Fixes: 00a03d2f724 ("gl-renderer: add support of WL_SHM_FORMAT_NV12")
Cc: Vincent Abriou 

-Emil
> ---
>  libweston/gl-renderer.c | 3 ++-
>  1 file cNphanged, 2 insertions(+), 1 deletion(-)
>
> diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
> index 60a7bf06..4fffa78c 100644
> --- a/libweston/gl-renderer.c
> +++ b/libweston/gl-renderer.c
> @@ -1596,7 +1596,6 @@ gl_renderer_attach_shm(struct weston_surface *es, 
> struct weston_buffer *buffer,
> }
> break;
> case WL_SHM_FORMAT_NV12:
> -   gs->shader = >texture_shader_y_xuxv;
> pitch = wl_shm_buffer_get_stride(shm_buffer);
> gl_pixel_type = GL_UNSIGNED_BYTE;
> num_planes = 2;
> @@ -1605,9 +1604,11 @@ gl_renderer_attach_shm(struct weston_surface *es, 
> struct weston_buffer *buffer,
> gs->hsub[1] = 2;
> gs->vsub[1] = 2;
> if (gr->has_gl_texture_rg) {
> +   gs->shader = >texture_shader_y_uv;
> gl_format[0] = GL_R8_EXT;
> gl_format[1] = GL_RG8_EXT;
> } else {
> +   gs->shader = >texture_shader_y_xuxv;
> gl_format[0] = GL_LUMINANCE;
> gl_format[1] = GL_LUMINANCE_ALPHA;
> }
> --
> 2.15.0
>
> ___
> 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 3/5] gl-renderer: always enable unpack subimage and RG textures in ES3 contexts

[Emil Velikov]

On 29 November 2017 at 14:25, Arnaud Vrac  wrote:
> From: Arnaud Vrac 
>
> The GL_EXT_unpack_subimage and GL_EXT_texture_rg are part of the core ES
> 3.0 specification, so also check the GL driver version in addition to
> the extension string to determine if those features are supported.
>
Reviewed-by: Emil Velikov 

Out of curiosity: did you notice a ES 3.x driver that doesn't expose
these in the extensions string?

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

Re: [PATCH weston 4/5] gl-renderer: fix pixel format used in texture uploads when using R/RG textures

[Emil Velikov]

On 29 November 2017 at 14:25, Arnaud Vrac  wrote:
> From: Arnaud Vrac 
>
> In glTexImage2D / glTexSubImage2D calls, the only pixel formats allowed
> for the GL_R8 and GL_RG internal formats are respectively GL_RED and
s/GL_RG/GL_RG8/ ^^

> GL_RG [1].
>
> Make sure we match this requirement, as some drivers will fail with the
> current code.
>
AFAICT the patch is spot on. Trivial suggestion below:
Reviewed-by: Emil Velikov 


I'd suggest using something like the following. A reference to the
offending code is always nice, plus CC-ing the author gives you an
extra reviewer ;-)

Fixes: 00a03d2f724 ("gl-renderer: add support of WL_SHM_FORMAT_NV12")
Cc: Vincent Abriou 

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

Re: [PATCH weston 1/5] gl-renderer: save OpenGL version in renderer context

[Arnaud Vrac]

On Mon, Dec 4, 2017 at 5:31 PM, Emil Velikov  wrote:
> Hi Arnaud,
>
> On 29 November 2017 at 14:25, Arnaud Vrac  wrote:
>> From: Arnaud Vrac 
>>
> Here I'd mention why we care about the version. Esp. since the helper
> itself looks quitie strange.

Hi Emil,

Will do.

>
>> Signed-off-by: Arnaud Vrac 
>> ---
>>  libweston/gl-renderer.c | 26 ++
>>  1 file changed, 26 insertions(+)
>>
>> diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
>> index 94d81ef4..0a7db13f 100644
>> --- a/libweston/gl-renderer.c
>> +++ b/libweston/gl-renderer.c
>> @@ -199,6 +199,9 @@ struct gl_renderer {
>>
>> EGLSurface dummy_surface;
>>
>> +   int gl_major;
>> +   int gl_minor;
> Minor seems unused, document/drop/other?

The minor version might be useful if we ever depend on a GLES 3.1
feature. I don't think it hurts to keep it, I'll document the fields.

>> +
>> struct wl_array vertices;
>> struct wl_array vtxcnt;
>>
>> @@ -3572,6 +3575,27 @@ fan_debug_repaint_binding(struct weston_keyboard 
>> *keyboard,
>> weston_compositor_damage_all(compositor);
>>  }
>>
>> +static bool
>> +get_gl_version(int *major, int *minor)
>> +{
>> +   const char *version;
>> +
>> +   version = (const char *) glGetString(GL_VERSION);
>> +   if (version) {
>> +   if (sscanf(version, "%d.%d", major, minor) == 2)
>> +   return true;
>> +
>> +   if (sscanf(version, "OpenGL ES %d.%d", major, minor) == 2)
>> +   return true;
>> +   }
>> +
>> +   weston_log("failed to get GL version, default to GLES 2.0\n");
>> +   *major = 2;
>> +   *minor = 0;
>> +
>> +   return false;
> Function returns bool, yet nobody checks that. Normally it's
> considered bad idea to write into user provided memory on error.
> I don't think that parsing a desktop GL string or continuing is a good idea.
>
> In either of those cases we're pretty much stuffed, so might as well
> error out ASAP.

I agree, I'll add a check in the caller and move the warning and
fallback to 2.0 there in case of error.

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

Re: [systemd-devel] [PATCH weston] doc/systemd: system service example

[Pekka Paalanen]

On Fri, 1 Dec 2017 18:25:35 +0100
Lennart Poettering  wrote:

> On Fr, 01.12.17 13:42, Pekka Paalanen (ppaala...@gmail.com) wrote:
> 
> > > > > This is racy, as the session ID is not really reliably predictable,
> > > > > and is synthesized in different contexts in different ways, for
> > > > > example depnding on whether audit is enabled in the kernel it might be
> > > > > session-1.scope rather than session-c1.scope.
> >   
> > > > If we could determine the bug doesn't exist anymore, that would be
> > > > awesome and I could just drop this.  
> > 
> > Hi Lennart,
> > 
> > taking a step back, the session-c1.scope directive is definitely not
> > wanted and we should drop it. We should also use a custom PAM name
> > setup. If we do that, is the service file otherwise good for
> > guaranteeing:
> > 
> > - a full user session setup (I presume we want it), specifically
> >   XDG_RUNTIME_DIR being set up
> > 
> > - running exclusively on a VT that is made current  
> 
> This really depends on how weston sets up a VT. I really don't know
> Weston and what it does. 

Weston doesn't set up the VT, we rely on the systemd unit directives to
set it up.

Weston calls sd_pid_get_session(getpid()), sd_session_get_seat(), and wants
sd_session_get_vt() to succeed and give a VT number. Then it connects
to logind, wants TakeControl to succeed, and calls Activate. It uses
TakeDevice to open the DRM KMS device and input devices. I think that's
the start-up sequence, it also listens on signals from logind etc.

> > - access to DRM and input devices via logind  
> 
> So, I can't comment on Weston really.

No worries, that was more of a general question about whether the
example unit file was making any unwarranted assumptions.

> Here are brief (and possibly slightly out-of-date, but probably not)
> notes on how to write display managers with logind:
> 
> https://www.freedesktop.org/wiki/Software/systemd/writing-display-managers/

Thanks, I had a quick read through. We expect the systemd unit to also
set up PAM, Weston itself does not touch PAM.

> > so that all these are already in place by the time the Weston process
> > is started?
> > 
> > As you can see from Martyn below, the first issue that prevented Weston
> > from running was that XDG_RUNTIME_DIR was not set. Furthermore, this
> > failure did not occur always, sometimes things just worked as we
> > expected.  
> 
> So, as long as weston runs from a PAM enabled service (and its PAM
> snippet pulls in pam_systemd) all should be good and race-free: as the
> PAM session is set up XDG_RUNTIME_DIR will be made available and the
> systemd --user instance is invoked in the background.

This is exactly what we attempted with the User and PAMName directive,
and it turned out to be racy somehow.

> What currently is not supported is to run things independently of any
> session, i.e. run weston as systemd --user service with nothing that
> creates a session in the first place. In that case XDG_RUNTIME_DIR
> will not be set up, and no devices are made available to weston...

Weston never was a --user service.

As far as I know, there was also nothing that would manually attempt to
start user@.service, the only trigger for that were the User and PAMName
directives in the system weston.service.

> > > > > > +# Set up a full user session for the user, required by Weston.
> > > > > > +PAMName=login  
> > > > > 
> > > > > Piggy-backing on "login" is a bad idea. "login" is a text tool, and
> > > > > thus the PAM rules for it usually pull in some TTY specific PAM
> > > > > modules. YOu shoudl really use your own PAM fragment here, and
> > > > > configure only the bits you need.
> > > > 
> > > 
> > > Oh, so could it just be that we needed something other than
> > > "PAMName=login"?  
> > 
> > What are they key bits in the PAM configuration we must have, and are
> > there any often used bits we must not have to avoid the race Martyn
> > describes?  
> 
> well, pam_systemd needs to be pulled in from it, that's the most
> important thing. A PAM snippet that pulls in pam_systemd means you get
> a session allcoated in logind, which in turn sets up XDG_RUNTIME_DIR
> for you.

Yes, it was, but apparently somewhere in the PAM stack or something it
calls there was a race, which sometimes let Weston to start before
XDG_RUNTIME_DIR environment variable was set, causing weston to fail.

We all here were quite baffled on what could even be racing, unless it
is possible that the weston process gets started in parallel with the
PAM setup done by the User/PAMName in weston.service. We assumed that
all the setup described in the systemd unit file would be guaranteed to
complete before the actual process gets started.

It seems our and your expectations are aligned. Maybe we should just
forget about that race, remove the hacks that tried to work around it,
and see if anyone ever sees the failure again. Maybe it was something
very special on that one system alone.

Re: Scanner with --no-documentation option (source-code included)

[Daniel Stone]

Hi Felipe,

On 2 December 2017 at 20:17, ferreiradaselva
 wrote:
> I don't know how useful this feature would be for others (it would be for
> me), but I made the wayland-scanner to take a no-documentation option. It
> omits the documentation comments (but still keep the copyright notice). Is
> this something of interest?
>
> I added the source on
> https://gist.github.com/ferreiradaselva/ad04b8c6302a376f4bff0477ea01d129

Thanks very much for preparing this. It's hard to comment on the code
though, without being able to see the differences.

The usual manner patches are prepared is:
  * create a new branch in your Git worktree (say, scanner-no-doc)
  * make the changes you want to your source code
  * commit those changes
  * use git send-email to post the changes to the list

That way we can see just the differences between your code and
mainline, rather than having to read the whole file and try not to
miss anything.

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

[PATCH weston 8/8] tests: Add test for touch event timestamps

[Alexandros Frantzis]

Add test to verify that the server correctly sets the timestamps of
touch events. This requires updating the weston-test protocol with a new
request for touch events.

Signed-off-by: Alexandros Frantzis 
---
 Makefile.am   |  7 +++-
 protocol/weston-test.xml  |  9 +
 tests/touch-test.c| 71 +++
 tests/weston-test-client-helper.c |  3 ++
 tests/weston-test-client-helper.h |  3 ++
 tests/weston-test.c   | 16 +
 6 files changed, 108 insertions(+), 1 deletion(-)
 create mode 100644 tests/touch-test.c

diff --git a/Makefile.am b/Makefile.am
index 47b110df..c7141734 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1231,7 +1231,8 @@ weston_tests =\
roles.weston\
subsurface.weston   \
subsurface-shot.weston  \
-   devices.weston
+   devices.weston  \
+   touch.weston
 
 ivi_tests =
 
@@ -1426,6 +1427,10 @@ nodist_viewporter_weston_SOURCES =   \
 viewporter_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
 viewporter_weston_LDADD = libtest-client.la
 
+touch_weston_SOURCES = tests/touch-test.c
+touch_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
+touch_weston_LDADD = libtest-client.la
+
 if ENABLE_XWAYLAND_TEST
 weston_tests +=xwayland-test.weston
 xwayland_test_weston_SOURCES = tests/xwayland-test.c
diff --git a/protocol/weston-test.xml b/protocol/weston-test.xml
index 37fa221f..00b7185d 100644
--- a/protocol/weston-test.xml
+++ b/protocol/weston-test.xml
@@ -96,6 +96,15 @@
provided buffer.
  
 
+
+  
+  
+  
+  
+  
+  
+  
+
   
 
   
diff --git a/tests/touch-test.c b/tests/touch-test.c
new file mode 100644
index ..374df116
--- /dev/null
+++ b/tests/touch-test.c
@@ -0,0 +1,71 @@
+/*
+ * Copyright © 2017 Collabora, Ltd.
+ *
+ * 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 "shared/timespec-util.h"
+#include "weston-test-client-helper.h"
+#include "wayland-server-protocol.h"
+
+static const struct timespec t1 = { .tv_sec = 1, .tv_nsec = 101 };
+static const struct timespec t2 = { .tv_sec = 2, .tv_nsec = 201 };
+static const struct timespec t3 = { .tv_sec = 3, .tv_nsec = 301 };
+
+static struct client *
+create_touch_test_client(void)
+{
+   struct client *cl = create_client_and_test_surface(0, 0, 100, 100);
+   assert(cl);
+   return cl;
+}
+
+static void
+send_touch(struct client *client, const struct timespec *time,
+  uint32_t touch_type)
+{
+   uint32_t tv_sec_hi, tv_sec_lo, tv_nsec;
+
+   timespec_to_proto(time, _sec_hi, _sec_lo, _nsec);
+   weston_test_send_touch(client->test->weston_test, tv_sec_hi, tv_sec_lo,
+  tv_nsec, 1, 1, 1, touch_type);
+   client_roundtrip(client);
+}
+
+TEST(touch_events)
+{
+   struct client *client = create_touch_test_client();
+   struct touch *touch = client->input->touch;
+
+   send_touch(client, , WL_TOUCH_DOWN);
+   assert(touch->down_time == timespec_to_msec());
+
+   send_touch(client, , WL_TOUCH_MOTION);
+   assert(touch->motion_time == timespec_to_msec());
+
+   send_touch(client, , WL_TOUCH_UP);
+   assert(touch->up_time == timespec_to_msec());
+}
diff --git a/tests/weston-test-client-helper.c 
b/tests/weston-test-client-helper.c
index ef58d77a..7266e028 100644
--- a/tests/weston-test-client-helper.c
+++ b/tests/weston-test-client-helper.c
@@ -334,6 +334,7 @@ touch_handle_down(void *data, struct wl_touch *wl_touch,
touch->down_x = wl_fixed_to_int(x_w);
touch->down_y = wl_fixed_to_int(y_w);
touch->id = id;
+   touch->down_time = time;
 

[PATCH weston 7/8] tests: Add test for keyboard key event timestamps

[Alexandros Frantzis]

Add test to verify that the server correctly sets the timestamps of
keyboard key events. This requires updating the weston-test protocol to
support passing key event timestamps.

Signed-off-by: Alexandros Frantzis 
---
 protocol/weston-test.xml  |  3 ++
 tests/keyboard-test.c | 58 +++
 tests/weston-test-client-helper.c |  1 +
 tests/weston-test-client-helper.h |  1 +
 tests/weston-test.c   |  3 +-
 5 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/protocol/weston-test.xml b/protocol/weston-test.xml
index a4a7ad4e..37fa221f 100644
--- a/protocol/weston-test.xml
+++ b/protocol/weston-test.xml
@@ -64,6 +64,9 @@
   
 
 
+  
+  
+  
   
   
 
diff --git a/tests/keyboard-test.c b/tests/keyboard-test.c
index 6b4ba19d..df1940f8 100644
--- a/tests/keyboard-test.c
+++ b/tests/keyboard-test.c
@@ -27,21 +27,45 @@
 
 #include 
 
+#include "shared/timespec-util.h"
 #include "weston-test-client-helper.h"
 
+static const struct timespec t1 = { .tv_sec = 1, .tv_nsec = 101 };
+static const struct timespec t2 = { .tv_sec = 2, .tv_nsec = 201 };
+
+static struct client *
+create_client_with_keyboard_focus(void)
+{
+   struct client *cl = create_client_and_test_surface(10, 10, 1, 1);
+   assert(cl);
+
+   weston_test_activate_surface(cl->test->weston_test,
+cl->surface->wl_surface);
+   client_roundtrip(cl);
+
+   return cl;
+}
+
+static void
+send_key(struct client *client, const struct timespec *time,
+uint32_t key, uint32_t state)
+{
+   uint32_t tv_sec_hi, tv_sec_lo, tv_nsec;
+
+   timespec_to_proto(time, _sec_hi, _sec_lo, _nsec);
+   weston_test_send_key(client->test->weston_test, tv_sec_hi, tv_sec_lo,
+tv_nsec, key, state);
+   client_roundtrip(client);
+}
+
 TEST(simple_keyboard_test)
 {
-   struct client *client;
-   struct surface *expect_focus = NULL;
-   struct keyboard *keyboard;
+   struct client *client = create_client_with_keyboard_focus();
+   struct keyboard *keyboard = client->input->keyboard;
+   struct surface *expect_focus = client->surface;
uint32_t expect_key = 0;
uint32_t expect_state = 0;
 
-   client = create_client_and_test_surface(10, 10, 1, 1);
-   assert(client);
-
-   keyboard = client->input->keyboard;
-
while (1) {
assert(keyboard->key == expect_key);
assert(keyboard->state == expect_state);
@@ -49,8 +73,7 @@ TEST(simple_keyboard_test)
 
if (keyboard->state == WL_KEYBOARD_KEY_STATE_PRESSED) {
expect_state = WL_KEYBOARD_KEY_STATE_RELEASED;
-   weston_test_send_key(client->test->weston_test,
-expect_key, expect_state);
+   send_key(client, , expect_key, expect_state);
} else if (keyboard->focus) {
expect_focus = NULL;
weston_test_activate_surface(
@@ -62,8 +85,7 @@ TEST(simple_keyboard_test)
weston_test_activate_surface(
client->test->weston_test,
expect_focus->wl_surface);
-   weston_test_send_key(client->test->weston_test,
-expect_key, expect_state);
+   send_key(client, , expect_key, expect_state);
} else {
break;
}
@@ -71,3 +93,15 @@ TEST(simple_keyboard_test)
client_roundtrip(client);
}
 }
+
+TEST(keyboard_key_event_time)
+{
+   struct client *client = create_client_with_keyboard_focus();
+   struct keyboard *keyboard = client->input->keyboard;
+
+   send_key(client, , 0, WL_KEYBOARD_KEY_STATE_PRESSED);
+   assert(keyboard->key_time == timespec_to_msec());
+
+   send_key(client, , 0, WL_KEYBOARD_KEY_STATE_RELEASED);
+   assert(keyboard->key_time == timespec_to_msec());
+}
diff --git a/tests/weston-test-client-helper.c 
b/tests/weston-test-client-helper.c
index 92def14d..ef58d77a 100644
--- a/tests/weston-test-client-helper.c
+++ b/tests/weston-test-client-helper.c
@@ -280,6 +280,7 @@ keyboard_handle_key(void *data, struct wl_keyboard 
*wl_keyboard,
 
keyboard->key = key;
keyboard->state = state;
+   keyboard->key_time = time;
 
fprintf(stderr, "test-client: got keyboard key %u %u\n", key, state);
 }
diff --git a/tests/weston-test-client-helper.h 
b/tests/weston-test-client-helper.h
index 1b4d83c7..1be727c1 100644
--- a/tests/weston-test-client-helper.h
+++ b/tests/weston-test-client-helper.h
@@ -111,6 +111,7 @@ struct keyboard {
int rate;
int delay;
} repeat_info;
+   uint32_t key_time;
 };
 
 struct touch {

[PATCH weston 6/8] tests: Add test for pointer axis events

[Alexandros Frantzis]

Add test to verify the server correctly emits pointer axis events.  This
requires updating the weston-test protocol with a new request for
pointer axis events.

Signed-off-by: Alexandros Frantzis 
---
 protocol/weston-test.xml  |  7 +++
 tests/pointer-test.c  | 28 
 tests/weston-test-client-helper.c | 13 -
 tests/weston-test-client-helper.h |  4 
 tests/weston-test.c   | 20 
 5 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/protocol/weston-test.xml b/protocol/weston-test.xml
index ae3349ed..a4a7ad4e 100644
--- a/protocol/weston-test.xml
+++ b/protocol/weston-test.xml
@@ -53,6 +53,13 @@
   
   
 
+
+  
+  
+  
+  
+  
+
 
   
 
diff --git a/tests/pointer-test.c b/tests/pointer-test.c
index c241fa1d..b0eb6f6f 100644
--- a/tests/pointer-test.c
+++ b/tests/pointer-test.c
@@ -58,6 +58,18 @@ send_button(struct client *client, const struct timespec 
*time,
client_roundtrip(client);
 }
 
+static void
+send_axis(struct client *client, const struct timespec *time, uint32_t axis,
+ double value)
+{
+   uint32_t tv_sec_hi, tv_sec_lo, tv_nsec;
+
+   timespec_to_proto(time, _sec_hi, _sec_lo, _nsec);
+   weston_test_send_axis(client->test->weston_test, tv_sec_hi, tv_sec_lo,
+ tv_nsec, axis, wl_fixed_from_double(value));
+   client_roundtrip(client);
+}
+
 static void
 check_pointer(struct client *client, int x, int y)
 {
@@ -355,3 +367,19 @@ TEST(pointer_button_events)
assert(pointer->state == WL_POINTER_BUTTON_STATE_RELEASED);
assert(pointer->button_time == timespec_to_msec());
 }
+
+TEST(pointer_axis_events)
+{
+   struct client *client = create_client_with_pointer_focus(100, 100,
+100, 100);
+   struct pointer *pointer = client->input->pointer;
+
+   send_axis(client, , 1, 1.0);
+   assert(pointer->axis == 1);
+   assert(pointer->axis_value == 1.0);
+   assert(pointer->axis_time == timespec_to_msec());
+
+   send_axis(client, , 2, 0.0);
+   assert(pointer->axis == 2);
+   assert(pointer->axis_stop_time == timespec_to_msec());
+}
diff --git a/tests/weston-test-client-helper.c 
b/tests/weston-test-client-helper.c
index 108fecfb..92def14d 100644
--- a/tests/weston-test-client-helper.c
+++ b/tests/weston-test-client-helper.c
@@ -179,6 +179,12 @@ static void
 pointer_handle_axis(void *data, struct wl_pointer *wl_pointer,
uint32_t time, uint32_t axis, wl_fixed_t value)
 {
+   struct pointer *pointer = data;
+
+   pointer->axis = axis;
+   pointer->axis_value = wl_fixed_to_double(value);
+   pointer->axis_time = time;
+
fprintf(stderr, "test-client: got pointer axis %u %f\n",
axis, wl_fixed_to_double(value));
 }
@@ -200,7 +206,12 @@ static void
 pointer_handle_axis_stop(void *data, struct wl_pointer *wl_pointer,
 uint32_t time, uint32_t axis)
 {
-   fprintf(stderr, "test-client: got pointer axis stop\n");
+   struct pointer *pointer = data;
+
+   pointer->axis = axis;
+   pointer->axis_stop_time = time;
+
+   fprintf(stderr, "test-client: got pointer axis stop %u\n", axis);
 }
 
 static void
diff --git a/tests/weston-test-client-helper.h 
b/tests/weston-test-client-helper.h
index 08817242..1b4d83c7 100644
--- a/tests/weston-test-client-helper.h
+++ b/tests/weston-test-client-helper.h
@@ -90,8 +90,12 @@ struct pointer {
int y;
uint32_t button;
uint32_t state;
+   uint32_t axis;
+   double axis_value;
uint32_t motion_time;
uint32_t button_time;
+   double axis_time;
+   double axis_stop_time;
 };
 
 struct keyboard {
diff --git a/tests/weston-test.c b/tests/weston-test.c
index 1799de92..bb1a4cd4 100644
--- a/tests/weston-test.c
+++ b/tests/weston-test.c
@@ -183,6 +183,25 @@ send_button(struct wl_client *client, struct wl_resource 
*resource,
notify_button(seat, , button, state);
 }
 
+static void
+send_axis(struct wl_client *client, struct wl_resource *resource,
+ uint32_t tv_sec_hi, uint32_t tv_sec_lo, uint32_t tv_nsec,
+ uint32_t axis, wl_fixed_t value)
+{
+   struct weston_test *test = wl_resource_get_user_data(resource);
+   struct weston_seat *seat = get_seat(test);
+   struct timespec time;
+   struct weston_pointer_axis_event axis_event;
+
+   timespec_from_proto(, tv_sec_hi, tv_sec_lo, tv_nsec);
+   axis_event.axis = axis;
+   axis_event.value = wl_fixed_to_double(value);
+   axis_event.has_discrete = false;
+   axis_event.discrete = 0;
+
+   notify_axis(seat, , _event);
+}
+
 static void
 activate_surface(struct wl_client *client, struct wl_resource *resource,
 struct wl_resource *surface_resource)
@@ -503,6 

[PATCH weston 3/8] tests: Move wl_pointer tests to their own file

[Alexandros Frantzis]

Move wl_pointer tests from event-test.c to their own pointer-test.c
file. This move makes the test organization clearer and more consistent,
and will make addition of further pointer tests easier.

Signed-off-by: Alexandros Frantzis 
---
 Makefile.am  |   8 +-
 tests/button-test.c  |  61 --
 tests/event-test.c   | 256 -
 tests/pointer-test.c | 318 +++
 4 files changed, 322 insertions(+), 321 deletions(-)
 delete mode 100644 tests/button-test.c
 create mode 100644 tests/pointer-test.c

diff --git a/Makefile.am b/Makefile.am
index e7e6a0ed..47b110df 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1224,7 +1224,7 @@ weston_tests =\
bad_buffer.weston   \
keyboard.weston \
event.weston\
-   button.weston   \
+   pointer.weston  \
text.weston \
presentation.weston \
viewporter.weston   \
@@ -1381,9 +1381,9 @@ event_weston_SOURCES = tests/event-test.c
 event_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
 event_weston_LDADD = libtest-client.la
 
-button_weston_SOURCES = tests/button-test.c
-button_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
-button_weston_LDADD = libtest-client.la
+pointer_weston_SOURCES = tests/pointer-test.c
+pointer_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
+pointer_weston_LDADD = libtest-client.la
 
 devices_weston_SOURCES = tests/devices-test.c
 devices_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
diff --git a/tests/button-test.c b/tests/button-test.c
deleted file mode 100644
index afa6320f..
--- a/tests/button-test.c
+++ /dev/null
@@ -1,61 +0,0 @@
-/*
- * Copyright © 2012 Intel Corporation
- *
- * 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 "weston-test-client-helper.h"
-
-TEST(simple_button_test)
-{
-   struct client *client;
-   struct pointer *pointer;
-
-   client = create_client_and_test_surface(100, 100, 100, 100);
-   assert(client);
-
-   pointer = client->input->pointer;
-
-   assert(pointer->button == 0);
-   assert(pointer->state == 0);
-
-   weston_test_move_pointer(client->test->weston_test, 150, 150);
-   client_roundtrip(client);
-   assert(pointer->x == 50);
-   assert(pointer->y == 50);
-
-   weston_test_send_button(client->test->weston_test, BTN_LEFT,
-   WL_POINTER_BUTTON_STATE_PRESSED);
-   client_roundtrip(client);
-   assert(pointer->button == BTN_LEFT);
-   assert(pointer->state == WL_POINTER_BUTTON_STATE_PRESSED);
-
-   weston_test_send_button(client->test->weston_test, BTN_LEFT,
-   WL_POINTER_BUTTON_STATE_RELEASED);
-   client_roundtrip(client);
-   assert(pointer->button == BTN_LEFT);
-   assert(pointer->state == WL_POINTER_BUTTON_STATE_RELEASED);
-}
diff --git a/tests/event-test.c b/tests/event-test.c
index 64dd7a0c..c1ba3ac1 100644
--- a/tests/event-test.c
+++ b/tests/event-test.c
@@ -28,262 +28,6 @@
 
 #include "weston-test-client-helper.h"
 
-static void
-check_pointer(struct client *client, int x, int y)
-{
-   int sx, sy;
-
-   /* check that the client got the global pointer update */
-   assert(client->test->pointer_x == x);
-   assert(client->test->pointer_y == y);
-
-   /* Does global pointer map onto the surface? */
-   if (surface_contains(client->surface, x, y)) {
-   /* check that the surface has the pointer focus */
-   assert(client->input->pointer->focus == client->surface);
-
-   /*
-* check 

[PATCH weston 2/8] shared: Add helpers to convert between protocol data and timespec

[Alexandros Frantzis]

Add helpers to safely convert between struct timespec values and
tv_sec_hi, tv_sec_lo, tv_nsec triplets used for sending high-resolution
timestamp data over the wayland protocol. Replace existing conversion
code with the helper functions.

Signed-off-by: Alexandros Frantzis 
---
 clients/presentation-shm.c |  9 +--
 libweston/compositor.c |  9 ---
 shared/timespec-util.h | 39 +++
 tests/presentation-test.c  |  9 +--
 tests/timespec-test.c  | 66 ++
 5 files changed, 112 insertions(+), 20 deletions(-)

diff --git a/clients/presentation-shm.c b/clients/presentation-shm.c
index c9fb66cc..d6a939e5 100644
--- a/clients/presentation-shm.c
+++ b/clients/presentation-shm.c
@@ -39,6 +39,7 @@
 #include 
 #include "shared/helpers.h"
 #include "shared/zalloc.h"
+#include "shared/timespec-util.h"
 #include "shared/os-compatibility.h"
 #include "presentation-time-client-protocol.h"
 
@@ -383,14 +384,6 @@ timespec_to_ms(const struct timespec *ts)
return (uint32_t)ts->tv_sec * 1000 + ts->tv_nsec / 100;
 }
 
-static void
-timespec_from_proto(struct timespec *tm, uint32_t tv_sec_hi,
-   uint32_t tv_sec_lo, uint32_t tv_nsec)
-{
-   tm->tv_sec = ((uint64_t)tv_sec_hi << 32) + tv_sec_lo;
-   tm->tv_nsec = tv_nsec;
-}
-
 static int
 timespec_diff_to_usec(const struct timespec *a, const struct timespec *b)
 {
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 7d7a17ed..083664fd 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -341,7 +341,9 @@ weston_presentation_feedback_present(
 {
struct wl_client *client = wl_resource_get_client(feedback->resource);
struct wl_resource *o;
-   uint64_t secs;
+   uint32_t tv_sec_hi;
+   uint32_t tv_sec_lo;
+   uint32_t tv_nsec;
 
wl_resource_for_each(o, >resource_list) {
if (wl_resource_get_client(o) != client)
@@ -350,10 +352,9 @@ weston_presentation_feedback_present(
wp_presentation_feedback_send_sync_output(feedback->resource, 
o);
}
 
-   secs = ts->tv_sec;
+   timespec_to_proto(ts, _sec_hi, _sec_lo, _nsec);
wp_presentation_feedback_send_presented(feedback->resource,
-   secs >> 32, secs & 0x,
-   ts->tv_nsec,
+   tv_sec_hi, tv_sec_lo, tv_nsec,
refresh_nsec,
seq >> 32, seq & 0x,
flags | feedback->psf_flags);
diff --git a/shared/timespec-util.h b/shared/timespec-util.h
index a10edf5b..c734accd 100644
--- a/shared/timespec-util.h
+++ b/shared/timespec-util.h
@@ -175,6 +175,30 @@ timespec_to_usec(const struct timespec *a)
return (int64_t)a->tv_sec * 100 + a->tv_nsec / 1000;
 }
 
+/* Convert timespec to protocol data
+ *
+ * \param a timespec
+ * \param tv_sec_hi[out] the high bytes of the seconds part
+ * \param tv_sec_lo[out] the low bytes of the seconds part
+ * \param tv_nsec[out] the nanoseconds part
+ *
+ * The timespec is normalized before being converted to protocol data.
+ */
+static inline void
+timespec_to_proto(const struct timespec *a, uint32_t *tv_sec_hi,
+  uint32_t *tv_sec_lo, uint32_t *tv_nsec)
+{
+   struct timespec r;
+
+   timespec_normalize(, a);
+
+   /* We check the size of tv_sec, so that we shift only if the size
+* is 64-bits, in order to avoid sign extension on 32-bit systems. */
+   *tv_sec_hi = sizeof(r.tv_sec) == 8 ? (int64_t)r.tv_sec >> 32 : 0;
+   *tv_sec_lo = r.tv_sec;
+   *tv_nsec = r.tv_nsec;
+}
+
 /* Convert nanoseconds to timespec
  *
  * \param a timespec
@@ -209,6 +233,21 @@ timespec_from_msec(struct timespec *a, int64_t b)
timespec_from_nsec(a, b * 100);
 }
 
+/* Convert protocol data to timespec
+ *
+ * \param a[out] timespec
+ * \param tv_sec_hi the high bytes of seconds part
+ * \param tv_sec_lo the low bytes of seconds part
+ * \param tv_nsec the nanoseconds part
+ */
+static inline void
+timespec_from_proto(struct timespec *a, uint32_t tv_sec_hi,
+uint32_t tv_sec_lo, uint32_t tv_nsec)
+{
+   a->tv_sec = ((uint64_t)tv_sec_hi << 32) + tv_sec_lo;
+   a->tv_nsec = tv_nsec;
+}
+
 /* Check if a timespec is zero
  *
  * \param a timespec
diff --git a/tests/presentation-test.c b/tests/presentation-test.c
index f12f8eef..f6ffe480 100644
--- a/tests/presentation-test.c
+++ b/tests/presentation-test.c
@@ -34,6 +34,7 @@
 
 #include "shared/helpers.h"
 #include "shared/xalloc.h"
+#include "shared/timespec-util.h"
 #include "weston-test-client-helper.h"
 #include "presentation-time-client-protocol.h"
 
@@ -85,14 +86,6 @@ struct feedback {
uint32_t flags;
 };
 
-static void

[PATCH weston 5/8] tests: Add checks for pointer motion and button event timestamps

[Alexandros Frantzis]

Enhance the existing pointer motion and button event tests to
additionally verify the event timestamps. This requires updating the
weston-test protocol to support passing motion and button event
timestamps.

Signed-off-by: Alexandros Frantzis 
---
 protocol/weston-test.xml  |  6 ++
 tests/internal-screenshot-test.c  |  2 +-
 tests/pointer-test.c  | 45 ++-
 tests/subsurface-shot-test.c  |  2 +-
 tests/weston-test-client-helper.c |  2 ++
 tests/weston-test-client-helper.h |  2 ++
 tests/weston-test.c   |  6 --
 7 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/protocol/weston-test.xml b/protocol/weston-test.xml
index 74a15214..ae3349ed 100644
--- a/protocol/weston-test.xml
+++ b/protocol/weston-test.xml
@@ -40,10 +40,16 @@
   
 
 
+  
+  
+  
   
   
 
 
+  
+  
+  
   
   
 
diff --git a/tests/internal-screenshot-test.c b/tests/internal-screenshot-test.c
index 3bf9b31b..2a7424b8 100644
--- a/tests/internal-screenshot-test.c
+++ b/tests/internal-screenshot-test.c
@@ -97,7 +97,7 @@ TEST(internal_screenshot)
 */
 
/* Move the pointer away from the screenshot area. */
-   weston_test_move_pointer(client->test->weston_test, 0, 0);
+   weston_test_move_pointer(client->test->weston_test, 0, 1, 0, 0, 0);
 
buf = create_shm_buffer_a8r8g8b8(client, 100, 100);
draw_stuff(buf->image);
diff --git a/tests/pointer-test.c b/tests/pointer-test.c
index e0e700e0..c241fa1d 100644
--- a/tests/pointer-test.c
+++ b/tests/pointer-test.c
@@ -28,8 +28,36 @@
 
 #include 
 
+#include "shared/timespec-util.h"
 #include "weston-test-client-helper.h"
 
+static const struct timespec t0 = { .tv_sec = 0, .tv_nsec = 1 };
+static const struct timespec t1 = { .tv_sec = 1, .tv_nsec = 101 };
+static const struct timespec t2 = { .tv_sec = 2, .tv_nsec = 201 };
+
+static void
+send_motion(struct client *client, const struct timespec *time, int x, int y)
+{
+   uint32_t tv_sec_hi, tv_sec_lo, tv_nsec;
+
+   timespec_to_proto(time, _sec_hi, _sec_lo, _nsec);
+   weston_test_move_pointer(client->test->weston_test, tv_sec_hi, 
tv_sec_lo,
+tv_nsec, x, y);
+   client_roundtrip(client);
+}
+
+static void
+send_button(struct client *client, const struct timespec *time,
+   uint32_t button, uint32_t state)
+{
+   uint32_t tv_sec_hi, tv_sec_lo, tv_nsec;
+
+   timespec_to_proto(time, _sec_hi, _sec_lo, _nsec);
+   weston_test_send_button(client->test->weston_test, tv_sec_hi, tv_sec_lo,
+   tv_nsec, button, state);
+   client_roundtrip(client);
+}
+
 static void
 check_pointer(struct client *client, int x, int y)
 {
@@ -64,8 +92,7 @@ check_pointer(struct client *client, int x, int y)
 static void
 check_pointer_move(struct client *client, int x, int y)
 {
-   weston_test_move_pointer(client->test->weston_test, x, y);
-   client_roundtrip(client);
+   send_motion(client, , x, y);
check_pointer(client, x, y);
 }
 
@@ -303,10 +330,10 @@ TEST(pointer_motion_events)
 100, 100);
struct pointer *pointer = client->input->pointer;
 
-   weston_test_move_pointer(client->test->weston_test, 150, 150);
-   client_roundtrip(client);
+   send_motion(client, , 150, 150);
assert(pointer->x == 50);
assert(pointer->y == 50);
+   assert(pointer->motion_time == timespec_to_msec());
 }
 
 TEST(pointer_button_events)
@@ -318,15 +345,13 @@ TEST(pointer_button_events)
assert(pointer->button == 0);
assert(pointer->state == 0);
 
-   weston_test_send_button(client->test->weston_test, BTN_LEFT,
-   WL_POINTER_BUTTON_STATE_PRESSED);
-   client_roundtrip(client);
+   send_button(client, , BTN_LEFT, WL_POINTER_BUTTON_STATE_PRESSED);
assert(pointer->button == BTN_LEFT);
assert(pointer->state == WL_POINTER_BUTTON_STATE_PRESSED);
+   assert(pointer->button_time == timespec_to_msec());
 
-   weston_test_send_button(client->test->weston_test, BTN_LEFT,
-   WL_POINTER_BUTTON_STATE_RELEASED);
-   client_roundtrip(client);
+   send_button(client, , BTN_LEFT, WL_POINTER_BUTTON_STATE_RELEASED);
assert(pointer->button == BTN_LEFT);
assert(pointer->state == WL_POINTER_BUTTON_STATE_RELEASED);
+   assert(pointer->button_time == timespec_to_msec());
 }
diff --git a/tests/subsurface-shot-test.c b/tests/subsurface-shot-test.c
index 10415ec7..e8bab676 100644
--- a/tests/subsurface-shot-test.c
+++ b/tests/subsurface-shot-test.c
@@ -200,7 +200,7 @@ TEST(subsurface_z_order)
subco = get_subcompositor(client);
 
/* move the pointer clearly away from our screenshooting area */
-   

[PATCH weston 4/8] tests: Use separate test cases for pointer motion and button tests

[Alexandros Frantzis]

Split pointer motion and pointer button tests so that each test case is
more focused and self-contained.

Signed-off-by: Alexandros Frantzis 
---
 tests/pointer-test.c | 36 +---
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/tests/pointer-test.c b/tests/pointer-test.c
index d0b85f5d..e0e700e0 100644
--- a/tests/pointer-test.c
+++ b/tests/pointer-test.c
@@ -69,6 +69,17 @@ check_pointer_move(struct client *client, int x, int y)
check_pointer(client, x, y);
 }
 
+static struct client *
+create_client_with_pointer_focus(int x, int y, int w, int h)
+{
+   struct client *cl = create_client_and_test_surface(x, y, w, h);
+   assert(cl);
+   /* Move the pointer inside the surface to ensure that the surface
+* has the pointer focus. */
+   check_pointer_move(cl, x, y);
+   return cl;
+}
+
 TEST(test_pointer_top_left)
 {
struct client *client;
@@ -286,23 +297,26 @@ TEST(test_pointer_surface_move)
check_pointer(client, 50, 50);
 }
 
-TEST(simple_pointer_button_test)
+TEST(pointer_motion_events)
 {
-   struct client *client;
-   struct pointer *pointer;
-
-   client = create_client_and_test_surface(100, 100, 100, 100);
-   assert(client);
-
-   pointer = client->input->pointer;
-
-   assert(pointer->button == 0);
-   assert(pointer->state == 0);
+   struct client *client = create_client_with_pointer_focus(100, 100,
+100, 100);
+   struct pointer *pointer = client->input->pointer;
 
weston_test_move_pointer(client->test->weston_test, 150, 150);
client_roundtrip(client);
assert(pointer->x == 50);
assert(pointer->y == 50);
+}
+
+TEST(pointer_button_events)
+{
+   struct client *client = create_client_with_pointer_focus(100, 100,
+100, 100);
+   struct pointer *pointer = client->input->pointer;
+
+   assert(pointer->button == 0);
+   assert(pointer->state == 0);
 
weston_test_send_button(client->test->weston_test, BTN_LEFT,
WL_POINTER_BUTTON_STATE_PRESSED);
-- 
2.14.1

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

[PATCH weston 1/8] shared: Add timespec_normalize helper

[Alexandros Frantzis]

Add a helper function to normalize struct timespec values so that the
nanoseconds part is less than 1 second and has the same sign as the
seconds part (if the seconds part is not 0).

Normalization is required to ensure we can safely convert timespec
values to wayland protocol data, i.e, to tv_sec_hi, tv_sec_lo,
tv_sec_nsec triplets, and will be used in upcoming commits.

Signed-off-by: Alexandros Frantzis 
---
 shared/timespec-util.h | 28 ++
 tests/timespec-test.c  | 65 ++
 2 files changed, 93 insertions(+)

diff --git a/shared/timespec-util.h b/shared/timespec-util.h
index f9736c27..a10edf5b 100644
--- a/shared/timespec-util.h
+++ b/shared/timespec-util.h
@@ -33,6 +33,34 @@
 
 #define NSEC_PER_SEC 10
 
+/* Normalize a timespec
+ *
+ * \param r[out] normalized timespec
+ * \param a[in] timespec to normalize
+ *
+ * Normalize a timespec so that tv_nsec is less than 1 second
+ * and has the same sign as tv_sec (if tv_sec is non-zero).
+ *
+ */
+static inline void
+timespec_normalize(struct timespec *r, const struct timespec *a)
+{
+   if (a->tv_nsec >= NSEC_PER_SEC || a->tv_nsec <= -NSEC_PER_SEC) {
+   r->tv_sec = a->tv_sec + a->tv_nsec / NSEC_PER_SEC;
+   r->tv_nsec = a->tv_nsec % NSEC_PER_SEC;
+   } else {
+   *r = *a;
+   }
+
+   if (r->tv_sec > 0 && r->tv_nsec < 0) {
+   r->tv_sec -= 1;
+   r->tv_nsec += NSEC_PER_SEC;
+   } else if (r->tv_sec < 0 && r->tv_nsec > 0) {
+   r->tv_sec += 1;
+   r->tv_nsec -= NSEC_PER_SEC;
+   }
+}
+
 /* Subtract timespecs
  *
  * \param r[out] result: a - b
diff --git a/tests/timespec-test.c b/tests/timespec-test.c
index f127bcee..8c2296d1 100644
--- a/tests/timespec-test.c
+++ b/tests/timespec-test.c
@@ -38,6 +38,71 @@
 #include "shared/helpers.h"
 #include "zunitc/zunitc.h"
 
+ZUC_TEST(timespec_test, timespec_normalize)
+{
+   struct timespec a, r;
+
+   a.tv_sec = 0;
+   a.tv_nsec = 0;
+   timespec_normalize(, );
+   ZUC_ASSERT_EQ(r.tv_sec, 0);
+   ZUC_ASSERT_EQ(r.tv_nsec, 0);
+
+   a.tv_sec = 1;
+   a.tv_nsec = NSEC_PER_SEC - 1;
+   timespec_normalize(, );
+   ZUC_ASSERT_EQ(r.tv_sec, 1);
+   ZUC_ASSERT_EQ(r.tv_nsec, NSEC_PER_SEC - 1);
+
+   a.tv_sec = 1;
+   a.tv_nsec = NSEC_PER_SEC;
+   timespec_normalize(, );
+   ZUC_ASSERT_EQ(r.tv_sec, 2);
+   ZUC_ASSERT_EQ(r.tv_nsec, 0);
+
+   a.tv_sec = 1;
+   a.tv_nsec = NSEC_PER_SEC + 1;
+   timespec_normalize(, );
+   ZUC_ASSERT_EQ(r.tv_sec, 2);
+   ZUC_ASSERT_EQ(r.tv_nsec, 1);
+
+   a.tv_sec = -1;
+   a.tv_nsec = -NSEC_PER_SEC;
+   timespec_normalize(, );
+   ZUC_ASSERT_EQ(r.tv_sec, -2);
+   ZUC_ASSERT_EQ(r.tv_nsec, 0);
+
+   a.tv_sec = -1;
+   a.tv_nsec = -(NSEC_PER_SEC + 1);
+   timespec_normalize(, );
+   ZUC_ASSERT_EQ(r.tv_sec, -2);
+   ZUC_ASSERT_EQ(r.tv_nsec, -1);
+
+   a.tv_sec = -3;
+   a.tv_nsec = NSEC_PER_SEC + 1;
+   timespec_normalize(, );
+   ZUC_ASSERT_EQ(r.tv_sec, -1);
+   ZUC_ASSERT_EQ(r.tv_nsec, -(NSEC_PER_SEC - 1));
+
+   a.tv_sec = 3;
+   a.tv_nsec = -(NSEC_PER_SEC + 1);
+   timespec_normalize(, );
+   ZUC_ASSERT_EQ(r.tv_sec, 1);
+   ZUC_ASSERT_EQ(r.tv_nsec, NSEC_PER_SEC - 1);
+
+   a.tv_sec = -1;
+   a.tv_nsec = 2 * NSEC_PER_SEC + 1;
+   timespec_normalize(, );
+   ZUC_ASSERT_EQ(r.tv_sec, 1);
+   ZUC_ASSERT_EQ(r.tv_nsec, 1);
+
+   a.tv_sec = 1;
+   a.tv_nsec = -(2 * NSEC_PER_SEC + 1);
+   timespec_normalize(, );
+   ZUC_ASSERT_EQ(r.tv_sec, -1);
+   ZUC_ASSERT_EQ(r.tv_nsec, -1);
+}
+
 ZUC_TEST(timespec_test, timespec_sub)
 {
struct timespec a, b, r;
-- 
2.14.1

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

[PATCH weston 0/8] tests: Add input event timestamp tests

[Alexandros Frantzis]

This patchset enhances the test suite with test cases that verify that
the server correctly sets input event timestamps. In the process the
input tests have been reorganized and cleaned up to make it easier to
support the new test cases.

A secondary goal of this patchset is to prepare the test suite for
testing a potential implementation of new high-resolution input
timestamps in the form of a new protocol (discussions on an RFC proposal
are already on-going in the mailing list).

Patches (1) and (2) add timespec helpers to enable safe conversions
between timespec and protocol data triplets.

Patches (3) and (4) reorganize the pointer tests to make it easier to
add new test cases.

Patches (5) to (8) add tests for input events with a focus on verifying
event timestamps.

Alexandros Frantzis (8):
  shared: Add timespec_normalize helper
  shared: Add helpers to convert between protocol data and timespec
  tests: Move wl_pointer tests to their own file
  tests: Use separate test cases for pointer motion and button tests
  tests: Add checks for pointer motion and button event timestamps
  tests: Add test for pointer axis events
  tests: Add test for keyboard key event timestamps
  tests: Add test for touch event timestamps

 Makefile.am   |  15 +-
 clients/presentation-shm.c|   9 +-
 libweston/compositor.c|   9 +-
 protocol/weston-test.xml  |  25 +++
 shared/timespec-util.h|  67 ++
 tests/event-test.c| 256 --
 tests/internal-screenshot-test.c  |   2 +-
 tests/keyboard-test.c |  58 +++--
 tests/pointer-test.c  | 385 ++
 tests/presentation-test.c |   9 +-
 tests/subsurface-shot-test.c  |   2 +-
 tests/timespec-test.c | 131 
 tests/{button-test.c => touch-test.c} |  58 ++---
 tests/weston-test-client-helper.c |  19 +-
 tests/weston-test-client-helper.h |  10 +
 tests/weston-test.c   |  45 +++-
 16 files changed, 777 insertions(+), 323 deletions(-)
 create mode 100644 tests/pointer-test.c
 rename tests/{button-test.c => touch-test.c} (50%)

-- 
2.14.1

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

Re: Can I copy-paste Wayland generated header in my own library header?

[Pekka Paalanen]

On Mon, 04 Dec 2017 05:59:02 -0500
ferreiradaselva  wrote:

> >From: ppaala...@gmail.com
> >To: ferreiradaselva 
> >
> > Hi,
> >
> > first, IANAL, of course.

> > I don't know ZLIB license, or why you picked that instead of MIT, for
> > instance.
> >
> > Since the whole purpose of wayland-protocols is that you take the XML
> > file, run it through wayland-scanner, and include the compiled product
> > of that in your program binaries, there is no intention to change or
> > affect your license. Even totally closed source proprietary programs
> > are just fine using anything generated by wayland-scanner from
> > wayland-protocols.
> >
> > I also see no reason to even attempt to forbid e.g. modifications to
> > the generated code or even the XML itself.  
> 
> 
> Both ZLIB and MIT are very similar, and with similar popularity, the
> difference is that ZLIB is a bit more clear, and more permissive,
> since it doesn't state that "The above copyright notice and this
> permission notice shall be included in all copies or substantial
> portions of the Software." This statement in MIT mean that a user
> making an application must state somewhere that they are using such
> library under MIT. I already state in repository that Wayland is
> used, but the end-user *likely* won't do that (for whatever reason).
> Does the Wayland development community feel like that's a problem?

In my personal opinion, it is not a problem in this case.


Thanks,
pq


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

Re: Can I copy-paste Wayland generated header in my own library header?

[ferreiradaselva]

Hi,


> Original Message 
>From: ppaala...@gmail.com
>To: ferreiradaselva 
>
> Hi,
>
> first, IANAL, of course.
>
> The copyrights header you get in the generated headers and code comes
> from the XML files. It is the license of the XML protocol
> specification. I'm not sure if it applies to the generated code or not,
> I'd like to think it does, but I don't see much difference given how
> they are meant to be used.


To me, it makes sense to consider that the copyrights apply to the headers, too.


> Personally I would have no problem whatsoever for you to do what you
> want. Also the MIT license is very liberal, and you might want to read
> the dissection here:
> https://writing.kemitchell.com/2016/09/21/MIT-License-Line-by-Line.html


I've seen that link. I will talk about MIT bellow.


> I would add annotations in your files to clearly document which parts
> are verbatim/modified copies of the generated stuff, and from where and
> how they were generated - perhaps generating and piecing together the
> files could be part of your project's build from source even if your
> distributables are just more source. I would also keep the (generated)
> license blurbs with notes on which part of the file they apply to.


I like the idea of adding annotations stating where that piece of code come 
from. I will do that.


> I don't know ZLIB license, or why you picked that instead of MIT, for
> instance.
>
> Since the whole purpose of wayland-protocols is that you take the XML
> file, run it through wayland-scanner, and include the compiled product
> of that in your program binaries, there is no intention to change or
> affect your license. Even totally closed source proprietary programs
> are just fine using anything generated by wayland-scanner from
> wayland-protocols.
>
> I also see no reason to even attempt to forbid e.g. modifications to
> the generated code or even the XML itself.


Both ZLIB and MIT are very similar, and with similar popularity, the difference 
is that ZLIB is a bit more clear, and more permissive, since it doesn't state 
that "The above copyright notice and this permission notice shall be included 
in all copies or substantial portions of the Software." This statement in MIT 
mean that a user making an application must state somewhere that they are using 
such library under MIT. I already state in repository that Wayland is used, but 
the end-user *likely* won't do that (for whatever reason). Does the Wayland 
development community feel like that's a problem?


> Mind, that while libwayland-client tries hard to maintain backwards
> compatiblity, it is not guaranteeing forward compatibility. Code
> generated with a newer wayland-scanner may not work with an older
> libwayland-client. This has been necessary to fix bugs.
>
> We encourage all projects to run wayland-scanner as part of their build
> and match the wayland-scanner with the libwayland version they require
> minimum, instead of storing the generated code in VCS. That way
> distributions can choose a more recent libwayland minimum version with
> any bug fixes in the generated code.


Currently, I'm trying to give the user less work as possible. The whole premise 
is "you just need to grab these two files (source and header) into your build 
system and link to the right libraries to have a window framework ready." 
Backwards compatiblity is good enough for me (and I believe it will be for the 
end-user).


> Thanks,
> pq


Thank you. And sorry about this boring subject about licenses, it's that I'm 
trying to offer an open-source project that might be useful for other people, 
too :)
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] protocol: Add deprecation note about wl_shell

[Nils Chr. Brause]

Hi!

On 4 December 2017 at 09:16, Pekka Paalanen  wrote:
> On Sat,  2 Dec 2017 10:57:45 +0800
> Jonas Ådahl  wrote:
>
>> Now that xdg_shell is stable and much better defined than wl_shell we
>> can finally deprecate wl_shell and guide users towards xdg_shell
>> instead.
>>
>> Signed-off-by: Jonas Ådahl 
>> ---
>
> Hi,
>
> Acked-by: Pekka Paalanen 
>
>>
>> Eventually we can consider adding deprecation attributes the
>>  XML nodes that'll turn into compiler warnings, but this is
>> at least a start moving away from wl_shell. I imagine the wl_shell users
>> today (SDL, GLFW at least) to hopefully migrate soon, but keep the
>> wl_shell as a fallback for the time being while compositors add support
>> for xdg_shell. So we probably don't want to add compiler warnings for
>> them just yet.

I really like the idea of a deprecation attribute. The C++ bindings
would use it to add the [[deprecated("reason")]] attribute specifier
sequences to interface classes, event callbacks and request methods.
The value of the XML attribute could be what to use instead, e.g:



This value could be used in the C++ bindings as the "reason" argument.

>
> I would imagine compositors wanting to keep the support for a long
> time, so maybe the warnings would be just in the client header?

Yes, of course, this would be client only.

>
> OTOH, ISTR problems with having deprecation warnings in
> system-installed headers, compilers ignoring them...

I don't know about gcc, but g++ certainly doesn't ignore the
[[deprecated]] attribute on headers in /usr/include. I just tested
this with g++ 7.2.

>
>
> Thanks,
> pq

Cheers,
Nils

>
>
>>  protocol/wayland.xml | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
>> index aabc7ae..77d991e 100644
>> --- a/protocol/wayland.xml
>> +++ b/protocol/wayland.xml
>> @@ -976,6 +976,9 @@
>>
>>It allows clients to associate a wl_shell_surface with
>>a basic surface.
>> +
>> +  Note! This protocol is deprecated and not intended for production use.
>> +  For desktop-style user interfaces, use xdg_shell.
>>  
>>
>>  
>
>
> ___
> 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: Can I copy-paste Wayland generated header in my own library header?

[Pekka Paalanen]

On Sat, 02 Dec 2017 09:39:33 -0500
ferreiradaselva  wrote:

> Good morning.
> 
> I'm writing a cross-platform window framework, like GLFW, but
> containing only two files (a source and a header).
> 
> I would like to copy and paste the content of the xdg-shell.h
> generated with wayland-scanner into my header (swfw.h), between
> #ifdef/#endif, so if the user compiles for Wayland, the user won't
> need to generate the xdg-shell.h.
> 
> My framework is under ZLIB, and Wayland is under MIT. I would like to
> know from the team: does it affect the license of my framework by
> copying and pasting the header to make my lib easier to use? And, if
> does, any recommendations about what to do?
> 
> This is sort of how it would look like:
> 
> #ifndef MY_LIB
> #define MY_LIB
> 
> #ifdef USE_WAYLAND_BACKEND
> /* The whole xdg-shell.h header is pasted here */
> 
> struct window {
> struct wl_surface *surface;
> struct wl_buffer *buffer;
> ...
> };
> 
> struct context {
> struct wl_display *display;
> struct wl_compositor *compositor;
> struct wl_seat *seat;
> struct wl_pointer *pointer;
> struct wl_keyboard *keyboard;
> ...
> };
> #endif /* USE_WAYLAND_BACKEND */
> 
> /* Other backends */
> 
> #endif /* MY_LIB */

Hi,

first, IANAL, of course.

The copyrights header you get in the generated headers and code comes
from the XML files. It is the license of the XML protocol
specification. I'm not sure if it applies to the generated code or not,
I'd like to think it does, but I don't see much difference given how
they are meant to be used.

Personally I would have no problem whatsoever for you to do what you
want. Also the MIT license is very liberal, and you might want to read
the dissection here:
https://writing.kemitchell.com/2016/09/21/MIT-License-Line-by-Line.html

I would add annotations in your files to clearly document which parts
are verbatim/modified copies of the generated stuff, and from where and
how they were generated - perhaps generating and piecing together the
files could be part of your project's build from source even if your
distributables are just more source. I would also keep the (generated)
license blurbs with notes on which part of the file they apply to.

I don't know ZLIB license, or why you picked that instead of MIT, for
instance.

Since the whole purpose of wayland-protocols is that you take the XML
file, run it through wayland-scanner, and include the compiled product
of that in your program binaries, there is no intention to change or
affect *your* license. Even totally closed source proprietary programs
are just fine using anything generated by wayland-scanner from
wayland-protocols.

I also see no reason to even attempt to forbid e.g. modifications to
the generated code or even the XML itself.

---

Mind, that while libwayland-client tries hard to maintain backwards
compatiblity, it is not guaranteeing forward compatibility. Code
generated with a newer wayland-scanner may not work with an older
libwayland-client. This has been necessary to fix bugs.

We encourage all projects to run wayland-scanner as part of their build
and match the wayland-scanner with the libwayland version they require
minimum, instead of storing the generated code in VCS. That way
distributions can choose a more recent libwayland minimum version with
any bug fixes in the generated code.


Thanks,
pq


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

Re: [PATCH wayland] protocol: Add deprecation note about wl_shell

[Pekka Paalanen]

On Sat,  2 Dec 2017 10:57:45 +0800
Jonas Ådahl  wrote:

> Now that xdg_shell is stable and much better defined than wl_shell we
> can finally deprecate wl_shell and guide users towards xdg_shell
> instead.
> 
> Signed-off-by: Jonas Ådahl 
> ---

Hi,

Acked-by: Pekka Paalanen 

> 
> Eventually we can consider adding deprecation attributes the
>  XML nodes that'll turn into compiler warnings, but this is
> at least a start moving away from wl_shell. I imagine the wl_shell users
> today (SDL, GLFW at least) to hopefully migrate soon, but keep the
> wl_shell as a fallback for the time being while compositors add support
> for xdg_shell. So we probably don't want to add compiler warnings for
> them just yet.

I would imagine compositors wanting to keep the support for a long
time, so maybe the warnings would be just in the client header?

OTOH, ISTR problems with having deprecation warnings in
system-installed headers, compilers ignoring them...


Thanks,
pq


>  protocol/wayland.xml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> index aabc7ae..77d991e 100644
> --- a/protocol/wayland.xml
> +++ b/protocol/wayland.xml
> @@ -976,6 +976,9 @@
>  
>It allows clients to associate a wl_shell_surface with
>a basic surface.
> +
> +  Note! This protocol is deprecated and not intended for production use.
> +  For desktop-style user interfaces, use xdg_shell.
>  
>  
>  



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

Re: [PATCH wayland] connection: Print the content of arrays in closures

[Pekka Paalanen]

On Fri, 1 Dec 2017 17:08:15 +
Daniel Stone  wrote:

> Hi,
> 
> On 10 July 2017 at 19:28, Emmanuel Gil Peyrot
>  wrote:
> > The current behaviour when WAYLAND_DEBUG is set is to print “array”,
> > which is quite unhelpful.
> >
> > This patch prints a list of the bytes present in the array.  It doesn’t
> > try to interpret it as uint32_t or anything, leaving that to the reader
> > because this information isn’t present in the protocol description.  
> 
> To be honest, I'm not really wild about this one. All the array users
> I know of (key/button, xdg surface states) are uint32_t. I'd prefer to
> add a pretty-printing hint to the protocol - this could specify how to
> interpret arrays, and also scratch my long-standing itch of printing
> uints with %x rather than %u when it makes sense to ...

Hi,

just a side note, if you start working on that, do also think of
bindings for (strongly) typed languages. ISTR requests to add type
information so that better bindings could be generated, and the
problem was where to stop defining the type system. I'm pretty sure if
printing hints are added, people will use them for bindings, whether we
wanted or not.


Thanks,
pq


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