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

[Philipp Kerling]

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?

---
 libweston/compositor.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/libweston/compositor.c b/libweston/compositor.c
index 813b6634..747c5ebc 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -3621,11 +3621,16 @@ weston_surface_get_bounding_box(struct weston_surface 
*surface)
  surface->width, surface->height);
 
wl_list_for_each(subsurface, >subsurface_list, parent_link)
-   pixman_region32_union_rect(, ,
-  subsurface->position.x,
-  subsurface->position.y,
-  subsurface->surface->width,
-  subsurface->surface->height);
+   {
+   if (subsurface->surface->buffer_ref.buffer)
+   {
+   pixman_region32_union_rect(, ,
+  subsurface->position.x,
+  subsurface->position.y,
+  subsurface->surface->width,
+  subsurface->surface->height);
+   }
+   }
 
box = pixman_region32_extents();
struct weston_geometry geometry = {
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Running multliple fullscreen applications on each of the the DRM connector

[Prabhu S]

Currently I have a DRM modeset master which have multiple (HDMI) connectors.

I suppose there is a limitation (or security feature) in DRM that only one
user process can be a DRM master and only the master can do drmMode*
operations.

Wondering how to run multiple fullscreen applications (each for a DRM
connector) with wayland? and whether client will have option to choose the
connector?

References:
https://dvdhrm.wordpress.com/2013/05/29/drm-render-and-modeset-nodes/
https://keithp.com/blogs/DRM-lease/
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland 2/3] scanner: Allow adding a prefix to exported symbols

[Emil Velikov]

On 27 July 2017 at 14:01, Pekka Paalanen  wrote:
> On Wed, 26 Jul 2017 16:09:32 +0100
> Emil Velikov  wrote:
>
>> On 25 July 2017 at 10:24, Pekka Paalanen  wrote:
>> > On Tue, 25 Jul 2017 15:25:58 +0800
>> > Jonas Ådahl  wrote:
>> >
>> >> On Mon, Jul 24, 2017 at 02:16:04PM +0300, Pekka Paalanen wrote:
>> >> > On Mon,  3 Jul 2017 17:16:45 +0800
>> >> > Jonas Ådahl  wrote:
>> >> >
>> >> > > Two different protocols may use interfaces with identical names.
>> >> > > Implementing support for both those protocols would result in symbol
>> >> > > clashes, as wayland-scanner generates symbols from the interface 
>> >> > > names.
>> >> > >
>> >> > > Make it possible to avoiding these clashes by adding a way to add a
>> >> > > prefix to the symbols generated by wayland-scanner. Implementations
>> >> > > (servers and clients) can then use these prefix:ed symbols to 
>> >> > > implement
>> >> > > different objects with the same name.
>> >> > >
>> >> > > Signed-off-by: Jonas Ådahl 
>> >> > > ---
>> >> > >
>> >> > > Something like this would be needed if a compositor/client wants to 
>> >> > > implement
>> >> > > xdg-shell unstable v5 alongside xdg-shell stable, unless we want to 
>> >> > > rename all
>> >> > > our xdg-shell interfaces. Implementing xdg-shell unstable v6 alongside
>> >> > > xdg-shell stable does not have this issue.
>> >> > >
>> >> > > See issue raised here:
>> >> > > https://lists.freedesktop.org/archives/wayland-devel/2017-June/034380.html
>> >> > >
>> >> > >
>> >> > > Jonas
>> >> > >
>> >> > >
>> >> > >  src/scanner.c | 94 
>> >> > > ++-
>> >> > >  1 file changed, 73 insertions(+), 21 deletions(-)
>> >> >
>> >> >
>> >> > Hi,
>> >> >
>> >> > while this seems to change the ABI symbol names, it does not change the
>> >> > names in the documentation, and it does not change the names of
>> >> > #defines of enums, or the inline functions. That means that this is not
>> >> > enough to fulfill the purpose: being able to use two similarly named
>> >> > but different protocols by adding a prefix.
>> >>
>> >> The idea I had was rather that one would avoid changing any names on
>> >> non-symbols. It'd still be possible to implement both by doing it in
>> >> separate C files. I can see the point in adding the prefix on everything
>> >> though, so I'll provide a patch for that.
>> >>
>> >
>> > Hi,
>> >
>> > recapping the discussion from IRC, we pretty much agreed that prefixing
>> > is not a nice solution. Jonas found out that we cannot actually prefix
>> > everything, because there usually are references to other protocol
>> > things (like you would never want to prefix wl_surface). So it becomes
>> > very hard to prefix things appropriately.
>> >
>> > The alternative we discussed is solving a different problem: scanner
>> > makes all the public symbols in the generated .c files WL_EXPORT, which
>> > makes them leak into DSO ABI, which is bad. In my opinion, it should
>> > have never happened in the first place. But we missed it, and now it has
>> > spread, so we cannot just fix scanner to stop exporting, the decision
>> > must be with the consumers. So we need a scanner option to stop
>> > exporting.
>> >
>> > Quentin proposed we add a scanner option
>> > --visibility={static|compiler|export}. It would affect all the symbols
>> > exported from the generated .c files as follows:
>> >
>> > - static: the symbols will be static.
>> > - compiler: the symbols will get whatever the default visibility is
>> >   with the compiler, i.e. not explicitly static and not exported
>> > - export: the symbols are exported (this the old behaviour, and will be
>> >   the default)
>> >
>> > Obviously, the only way to actually make use of the 'static' option is
>> > for the consumer to #include the generated .c file. It's ugly, yes, but
>> > it solves the conflicting symbol names issue Jonas was looking into.
>> >
>> > In my opinion, the prefixing approach where we still cannot prefix
>> > everything in a way that one could use conflicting protocols in the
>> > same compilation unit, and where e.g. the static inline functions are
>> > not prefixed, is more ugly than the 'static' option.
>> >
>> > We are going to need an option to stop the exports anyway, and it seems
>> > like we can piggyback on that solution for the problem underlying the
>> > prefixing proposal as well.
>> >
>> It sounds like Quentin proposal is trying to address two distinct
>> things at the same time:
>>  - expose correct symbol visiblity
>>  - avoid symbol crashes due to conflicting naming used for "different" 
>> protocols
>
> Yes, it indeed is.
>
>> Former is addressed by swapping WL_EXPORT with WL_PRIVATE, or alike
>> While the latter can be tackled in a couple of says:
>>  - manually update the sources to use separate distinct name for the protocol
>>  - convince the generator 

Re: [PATCH weston] desktop-shell: Set surface resizing state during interactive resize

[Quentin Glidic]

On 7/28/17 2:11 PM, Philipp Kerling wrote:

xdg_shell requires this information to be shared with the client in
order to conform with the specification.

The code to forward this to the client by way of a configure() event
is already in place and works fine, it was just never being used until
now.

Signed-off-by: Philipp Kerling 


I’ll take that as a bug fix, so let’s land it in beta.

Added a couple of variables to avoid the long lines breaks and:
Reviewed-by: Quentin Glidic 

Then pushed:
e3715527..c5f12416  master -> master

Thanks!


---
  desktop-shell/shell.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 4608cf2f..f0b76352 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -1638,6 +1638,8 @@ resize_grab_button(struct weston_pointer_grab *grab,
  
  	if (pointer->button_count == 0 &&

state == WL_POINTER_BUTTON_STATE_RELEASED) {
+   weston_desktop_surface_set_resizing(
+   resize->base.shsurf->desktop_surface, false);
shell_grab_end(>base);
free(grab);
}
@@ -1648,6 +1650,8 @@ resize_grab_cancel(struct weston_pointer_grab *grab)
  {
struct weston_resize_grab *resize = (struct weston_resize_grab *) grab;
  
+	weston_desktop_surface_set_resizing(resize->base.shsurf->desktop_surface,

+   false);
shell_grab_end(>base);
free(grab);
  }
@@ -1731,6 +1735,7 @@ surface_resize(struct shell_surface *shsurf,
resize->height = geometry.height;
  
  	shsurf->resize_edges = edges;

+   weston_desktop_surface_set_resizing(shsurf->desktop_surface, true);
shell_grab_start(>base, _grab_interface, shsurf,
 pointer, edges);
  
___

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




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 5/5] wayland-scanner.mk: default --object-type to static

[Emil Velikov]

On 27 July 2017 at 14:34, Pekka Paalanen  wrote:
> On Wed, 26 Jul 2017 14:56:21 +0100
> Emil Velikov  wrote:
>
>> From: Emil Velikov 
>>
>> Unlike the core wayland library, it's recommended that one statically
>> embeds the protocol within their binary.
>>
>
> Hi,
>
> that is true, but I'd rather leave this file alone. If people use it,
> they start getting warnings, but nothing else changes for them. If we
> don't hear people complaining for few releases, we might be able to
> remove this file. I would like to see this file go, since none of our
> projects use this AFAIK, so it's very untested, and I believe it cannot
> cope with wayland-protocols' unstable/stable directory conventions.
>
> OTOH, if this patch is applied and someone uses this file, their
> exports will change unexpectedly.
>
>
Very good points - both on the subtle breakage and on the actual
usefulness of the file.

FWIW, Mesa (and libva after I poked them a few times) don't use the file.
I won't decide on the "to be or not be", although I do have slight
inclination about removing it.

I'll purge the file with v2 and if people don't agree, that's fine.

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

Re: [PATCH 4/5] build: set the scanner --object-type option

[Emil Velikov]

On 27 July 2017 at 14:36, Pekka Paalanen  wrote:
> On Wed, 26 Jul 2017 14:56:20 +0100
> Emil Velikov  wrote:
>
>> From: Emil Velikov 
>>
>> Unlike most other scanner users, the core wayland interfaces are
>> public ally available via the libwayland DSO.
>>
>> Signed-off-by: Emil Velikov 
>> ---
>>  Makefile.am | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index d0c8bd3..4055d04 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -97,7 +97,7 @@ nodist_libwayland_client_la_SOURCES =   \
>>  pkgconfig_DATA += src/wayland-client.pc src/wayland-server.pc
>>
>>  protocol/%-protocol.c : $(top_srcdir)/protocol/%.xml
>> - $(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(wayland_scanner) code < $< > $@
>> + $(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(wayland_scanner) 
>> --object-type=shared code < $< > $@
>>
>>  protocol/%-server-protocol.h : $(top_srcdir)/protocol/%.xml
>>   $(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(wayland_scanner) server-header < 
>> $< > $@
>
> Hi,
>
> looks good, but I wonder if the header commands need the type set as
> well to avoid the warning.
>
My goal was to have the option only for "code", but it seems like the
scanner will throw a warning when client/server-header instances are
missing it.

I could omit the warning or simply add have --object-type throughout
the Makefile, for consistence.

I'm fine either splution.
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 3/5] scanner: introduce --object-type option

[Emil Velikov]

On 27 July 2017 at 14:25, Pekka Paalanen  wrote:
> On Wed, 26 Jul 2017 14:56:19 +0100
> Emil Velikov  wrote:
>
>> From: Emil Velikov 
>>
>> The option is used to indicate how the code will be used - would it be a
>> part of shared or static one.
>>
>> In the former case one needs to export the specific symbols, although
>> normally people want to statically build the protocol code into their
>> project.
>>
>> If the option is missing a warning is emitted, to point people and do
>> the right thing.
>>
>> Signed-off-by: Emil Velikov 
>> ---
>>  src/scanner.c | 61 
>> ++-
>>  1 file changed, 56 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/scanner.c b/src/scanner.c
>> index c345ed6..cc45b74 100644
>> --- a/src/scanner.c
>> +++ b/src/scanner.c
>> @@ -57,6 +57,11 @@ enum side {
>>   SERVER,
>>  };
>>
>> +enum object_type {
>> + SHARED,
>> + STATIC,
>> +};
>
> Hi,
>
> I could go with this as well, as long as we have some solution to the
> original problem of conflicting interface names that Jonas is trying to
> solve.
>
i think the original patch from Jonas, addressed that issue nicely.
Let's keep that in the respective thread.

>> +
>>  static int
>>  usage(int ret)
>>  {
>> @@ -70,6 +75,11 @@ usage(int ret)
>>   fprintf(stderr, "-h,  --help  display this help 
>> and exit.\n"
>>   "-v,  --version   print the wayland 
>> library version that\n"
>>   " the scanner was 
>> built against.\n"
>> + "-t,  --object-type=[static,shared]\n"
>> + " How is the resulting 
>> code going to be built/used\n"
>> + " static - standalone 
>> static object, used internally\n"
>> + " shared - shared 
>> library, to be used by multiple projects\n"
>> + " Using static is 
>> highly recommened.\n"
>>   "-c,  --include-core-only include the core 
>> version of the headers,\n"
>>   " that is e.g. 
>> wayland-client-core.h instead\n"
>>   " of 
>> wayland-client.h.\n");
>> @@ -1712,9 +1722,11 @@ emit_messages(struct wl_list *message_list,
>>   printf("};\n\n");
>>  }
>>
>> +
>>  static void
>> -emit_code(struct protocol *protocol)
>> +emit_code(struct protocol *protocol, enum object_type obj_type)
>>  {
>> + const char *symbol_visibility;
>>   struct interface *i, *next;
>>   struct wl_array types;
>>   char **p, *prev;
>> @@ -1728,6 +1740,19 @@ emit_code(struct protocol *protocol)
>>  "#include \n"
>>  "#include \"wayland-util.h\"\n\n");
>>
>> + /* When building a shared library symbols must be exported, otherwise
>> +  * we want to have the symbols hidden. */
>> + if (obj_type == STATIC) {
>> + symbol_visibility = "WL_PRIVATE";
>> + printf("#if defined(__GNUC__) && __GNUC__ >= 4\n"
>> +"#define WL_PRIVATE __attribute__ 
>> ((visibility(\"hidden\")))\n"
>> +"#else\n"
>> +"#define WL_PRIVATE\n"
>> +"#endif\n\n");
>> + } else {
>> + symbol_visibility = "WL_EXPORT";
>> + }
>
> I wonder... would it be any better to just replace 'WL_EXPORT' and
> 'extern' with tokens:
>
> - LOCAL_INTERFACE_DECL for declarations of symbols that will also be
>   defined in the generated code file
>
> - EXTERN_INTERFACE_DECL for declarations of symbols that the generated
>   files will need but the code file will not define, that is,
>   interfaces defined on other XML files.
>
> - LOCAL_INTERFACE_DEF for symbol definitions in the generated code file.
>
> The exported configuration would then be:
> LOCAL_INTERFACE_DECL=extern
> EXTERN_INTERFACE_DECL=extern
> LOCAL_INTERFACE_DEF=WL_EXPORT
>
> That would be far too flexible and no-one would use it right, right?
>
I did not introduce separate tokens, since those are (and should be)
used _only_ in the .c file.
Personally then do not seem necessary, If you prefer we can add them though.

>> +
>>   wl_array_init();
>>   wl_list_for_each(i, >interface_list, link) {
>>   emit_types_forward_declarations(protocol, >request_list, 
>> );
>> @@ -1757,10 +1782,10 @@ emit_code(struct protocol *protocol)
>>   emit_messages(>request_list, i, "requests");
>>   emit_messages(>event_list, i, "events");
>>
>> - printf("WL_EXPORT const struct wl_interface "
>> + printf("%s const struct wl_interface "
>>  "%s_interface = {\n"

[PATCH weston] desktop-shell: Set surface resizing state during interactive resize

[Philipp Kerling]

xdg_shell requires this information to be shared with the client in
order to conform with the specification.

The code to forward this to the client by way of a configure() event
is already in place and works fine, it was just never being used until
now.

Signed-off-by: Philipp Kerling 
---
 desktop-shell/shell.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 4608cf2f..f0b76352 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -1638,6 +1638,8 @@ resize_grab_button(struct weston_pointer_grab *grab,
 
if (pointer->button_count == 0 &&
state == WL_POINTER_BUTTON_STATE_RELEASED) {
+   weston_desktop_surface_set_resizing(
+   resize->base.shsurf->desktop_surface, false);
shell_grab_end(>base);
free(grab);
}
@@ -1648,6 +1650,8 @@ resize_grab_cancel(struct weston_pointer_grab *grab)
 {
struct weston_resize_grab *resize = (struct weston_resize_grab *) grab;
 
+   
weston_desktop_surface_set_resizing(resize->base.shsurf->desktop_surface,
+   false);
shell_grab_end(>base);
free(grab);
 }
@@ -1731,6 +1735,7 @@ surface_resize(struct shell_surface *shsurf,
resize->height = geometry.height;
 
shsurf->resize_edges = edges;
+   weston_desktop_surface_set_resizing(shsurf->desktop_surface, true);
shell_grab_start(>base, _grab_interface, shsurf,
 pointer, edges);
 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland 1/3] Pass input/output files as arguments to wayland-scanner

[Emil Velikov]

On 27 July 2017 at 13:53, Pekka Paalanen  wrote:
> On Wed, 26 Jul 2017 15:57:19 +0100
> Emil Velikov  wrote:
>
>> On 3 July 2017 at 10:16, Jonas Ådahl  wrote:
>> > When input/output files are passed as arguments to wayland-scanner,
>> > instead of using stdin/stdout, warning and error messages will contain
>> > the file name, together with line number, of the warning/error. Doing
>> > this helps IDEs jump to the correct line.
>> >
>> I really like the idea, there is one concern though - some projects do
>> invoke wayland_scanner directly in their build system.
>> Thus this patch will break things for them.
>
> Hi,
>
> how could this patch affect such projects at all?
>
> If they are invoking wayland-scanner directly, nothing changes for them
> as I can see, regardless whether they find wayland-scanner on their own
> or use the $(wayland_scanner) used here.
>
Seemingly I misread things. There should be no issues.
Pardon for the noise.

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

Re: [PATCH weston] libweston-desktop: fix the size of unmaximized xdg surfaces

[Giulio Camuffo]

2017-07-27 15:22 GMT+02:00 Quentin Glidic :
> On 7/27/17 3:01 PM, Giulio Camuffo wrote:
>>
>> When unmaximizing a surface the configure event should send 0,0 as the
>> requested size, so that the client can use the size that it had before
>> maximizing
>
>
> Hi,
>
> I disagree on this patch for a few reasons:
> - the size can be ignored in this case, so there is no harm in keeping it
> non-0

I don't really agree, if the compositor requests a size the client
should try to respect it if it can, and in this case they can.

> - it’s up to the shell/compositor to decide, not a protocol library

This is a fair point... the compositor should be able to decide to ask
for some size, but the default should be to let the client go back to
the size it wants to, imho.

> - I am working on a not-so-tiny rewrite of the state code, based on the
> size_requested patch, that will make it clear that the shell/compositor is
> in charge here

Fair enough.


Cheers,
Giulio

>
> What do you think?
>
> Cheers,
>
>
>
>> ---
>>   libweston-desktop/xdg-shell-v6.c | 31 ++-
>>   1 file changed, 18 insertions(+), 13 deletions(-)
>>
>> diff --git a/libweston-desktop/xdg-shell-v6.c
>> b/libweston-desktop/xdg-shell-v6.c
>> index 1344dda0..3a02815f 100644
>> --- a/libweston-desktop/xdg-shell-v6.c
>> +++ b/libweston-desktop/xdg-shell-v6.c
>> @@ -578,11 +578,29 @@ weston_desktop_xdg_toplevel_send_configure(struct
>> weston_desktop_xdg_toplevel *t
>>   };
>> static void
>> +weston_desktop_xdg_toplevel_set_size(struct weston_desktop_surface
>> *dsurface,
>> +void *user_data,
>> +int32_t width, int32_t height)
>> +{
>> +   struct weston_desktop_xdg_toplevel *toplevel = user_data;
>> +
>> +   toplevel->pending.size.width = width;
>> +   toplevel->pending.size.height = height;
>> +
>> +   weston_desktop_xdg_surface_schedule_configure(>base,
>> false);
>> +}
>> +
>> +static void
>>   weston_desktop_xdg_toplevel_set_maximized(struct weston_desktop_surface
>> *dsurface,
>>   void *user_data, bool maximized)
>>   {
>> struct weston_desktop_xdg_toplevel *toplevel = user_data;
>>   + /* if we're unmaximizing set the size to 0 so that when the
>> configure event is sent
>> +  it will tell the client to use the size it wants */
>> +   if (toplevel->pending.state.maximized && !maximized)
>> +   weston_desktop_xdg_toplevel_set_size(dsurface, toplevel,
>> 0, 0);
>> +
>> toplevel->pending.state.maximized = maximized;
>> weston_desktop_xdg_surface_schedule_configure(>base,
>> false);
>>   }
>> @@ -618,19 +636,6 @@ weston_desktop_xdg_toplevel_set_activated(struct
>> weston_desktop_surface *dsurfac
>>   }
>> static void
>> -weston_desktop_xdg_toplevel_set_size(struct weston_desktop_surface
>> *dsurface,
>> -void *user_data,
>> -int32_t width, int32_t height)
>> -{
>> -   struct weston_desktop_xdg_toplevel *toplevel = user_data;
>> -
>> -   toplevel->pending.size.width = width;
>> -   toplevel->pending.size.height = height;
>> -
>> -   weston_desktop_xdg_surface_schedule_configure(>base,
>> false);
>> -}
>> -
>> -static void
>>   weston_desktop_xdg_toplevel_committed(struct weston_desktop_xdg_toplevel
>> *toplevel,
>>   int32_t sx, int32_t sy)
>>   {
>>
>
>
> --
>
> Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH libinput] lid: disable all types but EV_SYN and EV_SW

[Peter Hutterer]

On Tue, Jul 25, 2017 at 08:12:57AM -0700, Jason Gerecke wrote:
> On Tue, Jul 25, 2017 at 7:34 AM, Peter Hutterer
>  wrote:
> > The lid dispatch interface is a one-trick pony and can only handle SW_LID. 
> > It
> > ignores other switches but crashes on any event type other than EV_SW and
> > EV_SYN. Disable those types so we just ignore the event instead of 
> > asserting.
> >
> > https://bugs.freedesktop.org/show_bug.cgi?id=100057
> 
> Surely you meant https://bugs.freedesktop.org/show_bug.cgi?id=101853
> 
> Aside from that, seems like a reasonable workaround until the pony can
> either be taught a second trick or swapped out for a more capable
> animal.
> 
> Reviewed-by: Jason Gerecke 

just for the record, I swapped out the test device for the real one as
provided by the reporter, but left you review in place. Also fixed the link
to the bug, thanks.

Cheers,
   Peter

> 
> >
> > Signed-off-by: Peter Hutterer 
> > ---
> >  meson.build   |  1 +
> >  src/evdev-lid.c   |  8 
> >  test/litest-device-lid-switch-power.c | 71 
> > +++
> >  test/litest.c |  2 +
> >  test/litest.h |  1 +
> >  test/test-lid.c   | 20 ++
> >  6 files changed, 103 insertions(+)
> >  create mode 100644 test/litest-device-lid-switch-power.c
> >
> > diff --git a/meson.build b/meson.build
> > index cf10b35f..224ff9ce 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -538,6 +538,7 @@ if get_option('tests')
> > 'test/litest-device-keyboard-all-codes.c',
> > 'test/litest-device-keyboard-razer-blackwidow.c',
> > 'test/litest-device-lid-switch.c',
> > +   'test/litest-device-lid-switch-power.c',
> > 'test/litest-device-lid-switch-surface3.c',
> > 'test/litest-device-logitech-trackball.c',
> > 'test/litest-device-nexus4-touch-screen.c',
> > diff --git a/src/evdev-lid.c b/src/evdev-lid.c
> > index b3df37c2..6c438e00 100644
> > --- a/src/evdev-lid.c
> > +++ b/src/evdev-lid.c
> > @@ -298,6 +298,7 @@ struct evdev_dispatch *
> >  evdev_lid_switch_dispatch_create(struct evdev_device *lid_device)
> >  {
> > struct lid_switch_dispatch *dispatch;
> > +   int type;
> >
> > dispatch = zalloc(sizeof *dispatch);
> > dispatch->base.dispatch_type = DISPATCH_LID_SWITCH;
> > @@ -309,5 +310,12 @@ evdev_lid_switch_dispatch_create(struct evdev_device 
> > *lid_device)
> >
> > dispatch->lid_is_closed = false;
> >
> > +   for (type = EV_KEY; type < EV_CNT; type++) {
> > +   if (type == EV_SW)
> > +   continue;
> > +
> > +   libevdev_disable_event_type(lid_device->evdev, type);
> > +   }
> > +
> > return >base;
> >  }
> > diff --git a/test/litest-device-lid-switch-power.c 
> > b/test/litest-device-lid-switch-power.c
> > new file mode 100644
> > index ..c7a3d557
> > --- /dev/null
> > +++ b/test/litest-device-lid-switch-power.c
> > @@ -0,0 +1,71 @@
> > +/*
> > + * Copyright © 2017 James Ye 
> > + *
> > + * 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 "litest.h"
> > +#include "litest-int.h"
> > +
> > +static void
> > +litest_lid_switch_setup(void)
> > +{
> > +   struct litest_device *d = litest_create_device(LITEST_LID_SWITCH);
> > +   litest_set_current_device(d);
> > +}
> > +
> > +static struct input_id input_id = {
> > +   .bustype = 0x19,
> > +   .vendor = 0x0,
> > +   .product = 0x5,
> > +};
> > +
> > +static int events[] = {
> > +   EV_SW, SW_LID,
> > +   EV_KEY,