Re: [PATCH libinput] touchpad: short-circuit the edge scroll handling when it's not enabled

[Peter Hutterer]

On Tue, May 31, 2016 at 10:18:58AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 31-05-16 02:16, Peter Hutterer wrote:
> > No need to handle events properly in the edge scroll state machine when it's
> > not enabled. Just set any beginning touch to state AREA and move on. The 
> > rest
> > of the code guarantees neutral state when edge scrolling is enabled or
> > disabled.
> > 
> > This reduces the debug output produced by libinput-debug-events when edge
> > scrolling is disabled, preventing users from seemingly identifying
> > bugs where there are none.
> > 
> > Signed-off-by: Peter Hutterer 
> > ---
> >  src/evdev-mt-touchpad-edge-scroll.c | 12 +---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/evdev-mt-touchpad-edge-scroll.c 
> > b/src/evdev-mt-touchpad-edge-scroll.c
> > index fcc0512..8222bba 100644
> > --- a/src/evdev-mt-touchpad-edge-scroll.c
> > +++ b/src/evdev-mt-touchpad-edge-scroll.c
> > @@ -318,6 +318,15 @@ tp_edge_scroll_handle_state(struct tp_dispatch *tp, 
> > uint64_t time)
> >  {
> > struct tp_touch *t;
> > 
> > +   if (tp->scroll.method != LIBINPUT_CONFIG_SCROLL_EDGE) {
> > +   tp_for_each_touch(tp, t) {
> > +   if (t->state == TOUCH_BEGIN)
> > +   t->scroll.edge_state =
> > +   EDGE_SCROLL_TOUCH_STATE_AREA;
> > +   }
> > +   return;
> > +   }
> > +
> > tp_for_each_touch(tp, t) {
> > if (!t->dirty)
> > continue;
> 
> IMHO it would be cleaner to replace this hunk with:
> 
> @@ -141,7 +141,8 @@ tp_edge_scroll_handle_none(struct tp_dispatch *tp,
> 
> switch (event) {
> case SCROLL_EVENT_TOUCH:
> -   if (tp_touch_get_edge(tp, t)) {
> + if (tp->scroll.method == LIBINPUT_CONFIG_SCROLL_EDGE &&
> + tp_touch_get_edge(tp, t)) {
> tp_edge_scroll_set_state(tp, t,
> EDGE_SCROLL_TOUCH_STATE_EDGE_NEW);
> } else {
> 
> This is the cleanest solution IMHO, but then we still get one log_debug for 
> new touches;
> alternatively we could do:

unfortunately in both of your cases we still get the verbose output. this
hunk has no real effect because if edge scrolling is disabled,
tp_touch_get_edge() is always false.
> 
> @@ -327,7 +327,13 @@ tp_edge_scroll_handle_state(struct tp_dispatch *tp, 
> uint64_t time)
> case TOUCH_HOVERING:
> break;
> case TOUCH_BEGIN:
> -   tp_edge_scroll_handle_event(tp, t, SCROLL_EVENT_TOUCH);
> + if (tp->scroll.method == LIBINPUT_CONFIG_SCROLL_EDGE) {
> + tp_edge_scroll_handle_event(tp, t,
> + SCROLL_EVENT_TOUCH);
> + } else {
> + tp_edge_scroll_set_state(tp, t,
> + EDGE_SCROLL_TOUCH_STATE_AREA);
> + }
> break;
> case TOUCH_UPDATE:
> tp_edge_scroll_handle_event(tp, t, 
> SCROLL_EVENT_MOTION);

and in this case we're still calling into tp_edge_scroll_handle_event() from
TOUCH_UPDATE/END and timeouts. and that is where the log messages come from.

> That at least avoids the double tp_for_each_touch(tp, t) {}
> 
> I've a slight preference for the first solution, and just living with the
> one debug line for a new touch, after all this is for debugging only, and
> code-wise it is by far the cleanest.
> 
> Anyways all 3 get the job done in the end, so this patch is:
> 
> Reviewed-by: Hans de Goede 

thanks, I think I'll stick with the originally proposed patch. it's not as
nice but it keeps things in one place and does the job.

Cheers,
   Peter

> 
> Feel free to pick any of the outlined solutions (including your original one).
> 
> > @@ -350,9 +359,6 @@ tp_edge_scroll_post_events(struct tp_dispatch *tp, 
> > uint64_t time)
> > const struct normalized_coords zero = { 0.0, 0.0 };
> > const struct discrete_coords zero_discrete = { 0.0, 0.0 };
> > 
> > -   if (tp->scroll.method != LIBINPUT_CONFIG_SCROLL_EDGE)
> > -   return 0;
> > -
> > tp_for_each_touch(tp, t) {
> > if (!t->dirty)
> > continue;
> > 
> 
> Regards,
> 
> Hans
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[ANNOUNCE] wayland 1.11.0

[Bryce Harrington]

This is the official release of Wayland 1.11.  Here's a brief synopsis
of some of the key enhancements provided by this update.

Proxy wrappers were introduced, which help avoid race conditions in
multi-threaded clients.  A new "proxy wrapper" API is added for the
client to use when sending requests, that doesn't proxy events.  This
helps avoid one thread causing events to be dropped before they can be
handled by other threads.  Tests have been added to verify
functionality, and the new functionality is now used in fixing a race
condition present with wl_display_roundtrip_queue().

Wayland's shared memory (shm) received several improvements.  When
undergoing a resize operation while external users are holding
references to it, the resize is deferred to prevent leading to a crash;
Wayland now counts external and internal users differently to permit
tracking this.  Other invalid situations during resizes now emit better
warnings to the log and properly clean up their memory allocations.

As part of Wayland's ongoing work in improving enum for language
bindings, support for cross-interface enum attributes is added.  This
allows the documentation to reference enums defined in other interfaces
using a dot notation.

Documentation now includes HTML generation of doxygen comments in the
source code.  This provides client-side and server-side API
documentation, allowing developers easier reference to Wayland's
functionality from the web.  Each protocol gets a separate doxygen
@page, and each interface its own @subpage; in the case of Wayland there
is just the one core protocol, but for the wayland-protocols package
this will better organize the various content since each will have a
fixed HTML file name so that it is directly linkable/bookmark-able.
A few configuration settings for doxygen were tweaked to enable better
scanning C code.

Other changes this release include:

* Add --version arg for wayland-scanner.

* Add summaries to protocol event parameters.

* Make scanner's stack non-executable, for security purposes.

* Fix configuration with --disable-dtd-validation due to incorrect
  autoconf variable name.
  (Fixes https://bugs.gentoo.org/show_bug.cgi?id=575212)

* Fix a crash when errors are sent involving objects that have already
  been destroyed.  Log messages are changed to report [unknown
  interface] and [unknown id] in this case.  A test case is added to
  cover this as well.

* Add an --enable-fatal-warnings config option for making build warnings
  terminate compilation.  This is not set for build distcheck because
  some distros are overly-pedantic and would terminate building
  unnecessarily.

* Various grammar, spelling, and punctuation cleanup throughout protocol
  documentation.

* Various fixes and enhancements to test cases.

* Various code cleanups related to header includes


Our next major release will be version 1.12, which will be scheduled
tentatively as follows:

  √ Development opens immediately

  - 1.12-alpha in mid-August

  - 1.12-beta in early September

  - 1.12-rc1 in mid-September, with rc2 only if necessary

  - 1.12.0 by the end of September


Changes since RC:
-

Bryce Harrington (1):
  configure.ac: bump to version 1.11.0 for the official release


Full list of changes since 1.10.0:
--

Armin Krezović (1):
  scanner: Add version argument to wayland-scanner

Auke Booij (1):
  protocol: add support for cross-interface enum attributes

Bill Spitzak (1):
  doc: Use enum argument type to make links in protocol documentation

Bryce Harrington (6):
  configure.ac: bump to version 1.10.90 for open development
  doc: Note strong recommendation to use S-o-b in contributions
  configure.ac: bump to version 1.10.91 for the alpha release
  configure.ac: bump to version 1.10.92 for the beta release
  configure.ac: bump to version 1.10.93 for the RC1 release
  configure.ac: bump to version 1.11.0 for the official release

Derek Foreman (9):
  resource-test: Use wl_seat instead of wl_display for testing
  server: validate resource versions at creation time
  build: Add an --enable-fatal-warnings configure option
  build: build distcheck with --enable-fatal-warnings
  Revert "build: build distcheck with --enable-fatal-warnings"
  Revert "server: validate resource versions at creation time"
  shm: Split pool reference counting into external and internal references
  shm: Defer wl_shm_pool_resize if a pool has external references
  shm: Log a warning if a shm buffer address is requested when it may be 
invalid

Emil Velikov (3):
  scanner: move include directives before extern "C" wrapper
  server: move include directives before extern "C" wrapper
  utils: move include directives before extern "C" wrapper

Eric Engestrom (7):
  protocol: fix spelling mistake
  wayland-client: fix spelling mistake
  client: 

[PATCH libinput] touchpad: warn if we have invalid touchpad ranges

[Peter Hutterer]

Quite a few bugs are caused by touchpad ranges being out of whack. If we get
input events significantly outside the expected range (5% width/height as
error margin) print a warning to the log.

And add a new doc page to explain what is happening and how to fix it.

Signed-off-by: Peter Hutterer 
---
 doc/Makefile.am|   1 +
 doc/absolute-coordinate-ranges.dox | 120 +
 doc/page-hierarchy.dox |   1 +
 src/evdev-mt-touchpad.c|  59 ++
 src/evdev-mt-touchpad.h|   5 ++
 5 files changed, 186 insertions(+)
 create mode 100644 doc/absolute-coordinate-ranges.dox

diff --git a/doc/Makefile.am b/doc/Makefile.am
index f2a47cb..1e501a8 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -11,6 +11,7 @@ header_files = \
$(top_srcdir)/src/libinput.h \
$(top_srcdir)/README.txt \
$(srcdir)/absolute-axes.dox \
+   $(srcdir)/absolute-coordinate-ranges.dox \
$(srcdir)/clickpad-softbuttons.dox \
$(srcdir)/device-configuration-via-udev.dox \
$(srcdir)/faqs.dox \
diff --git a/doc/absolute-coordinate-ranges.dox 
b/doc/absolute-coordinate-ranges.dox
new file mode 100644
index 000..f9d9e98
--- /dev/null
+++ b/doc/absolute-coordinate-ranges.dox
@@ -0,0 +1,120 @@
+/**
+@page absolute_coordinate_ranges Coordinate ranges for absolute axes
+
+libinput requires that all touchpads provide a correct axis range and
+resolution. These are used to enable or disable certain features or adapt
+the interaction with the touchpad. For example, the software button area is
+narrower on small touchpads to avoid reducing the interactive surface too
+much. Likewise, palm detection works differently on small touchpads as palm
+interference is less likely to happen.
+
+Touchpads with incorrect axis ranges generate error messages
+in the form:
+
+Axis 0x35 value 4000 is outside expected range [0, 3000]
+
+
+This error message indicates that the ABS_MT_POSITION_X axis (i.e. the x
+axis) generated an event outside the expected range of 0-3000. In this case
+the value was 4000.
+This discrepancy between the coordinate range the kernels advertises vs.
+what the touchpad sends can be the source of a number of perceived
+bugs in libinput.
+
+@section absolute_coordinate_ranges_fix Measuring and fixing touchpad ranges
+
+To fix the touchpad you need to:
+-# measure the physical size of your touchpad in mm
+-# run touchpad-edge-detector
+-# trim the udev match rule to something sensible
+-# replace the resolution with the calculated resolution based on physical
+  settings
+-# test locally
+-# send a patch to the systemd project
+
+Detailed explanations are below.
+
+[libevdev](http://freedesktop.org/wiki/Software/libevdev/) provides a tool
+called **touchpad-edge-detector** that allows measuring the touchpad's input
+ranges. Run the tool as root against the device node of your touchpad device
+and repeatedly move a finger around the whole outside area of the
+touchpad. Then control+c the process and note the output.
+An example output is below:
+
+@code
+$> sudo touchpad-edge-detector /dev/input/event4
+Touchpad SynPS/2 Synaptics TouchPad on /dev/input/event4
+Move one finger around the touchpad to detect the actual edges
+Kernel says:   x [1024..3112], y [2024..4832]
+Touchpad sends:x [2445..4252], y [3464..4071]
+
+Touchpad size as listed by the kernel: 49x66mm
+Calculate resolution as:
+   x axis: 2088/
+   y axis: 2808/
+
+Suggested udev rule:
+# 
+evdev:name:SynPS/2 Synaptics 
TouchPad:dmi:bvnLENOVO:bvrGJET72WW(2.22):bd02/21/2014:svnLENOVO:pn20ARS25701:pvrThinkPadT440s:rvnLENOVO:rn20ARS25701:rvrSDK0E50512STD:cvnLENOVO:ct10:cvrNotAvailable:*
+ EVDEV_ABS_00=2445:4252:
+ EVDEV_ABS_01=3464:4071:
+ EVDEV_ABS_35=2445:4252:
+ EVDEV_ABS_36=3464:4071:
+
+@endcode
+
+Note the discrepancy between the coordinate range the kernels advertises vs.
+what the touchpad sends.
+To fix the advertised ranges, the udev rule should be taken and trimmed
+before being sent to the [systemd project](https://github.com/systemd/systemd).
+An example commit can be found
+[here](https://github.com/systemd/systemd/commit/26f667eac1c5e89b689aa0a1daef6a80f473e045).
+
+In most cases the match can and should be trimmed to the system vendor (svn)
+and the product version (pvr), with everything else replaced by a wildcard
+(*). In this case, a Lenovo T440s, a suitable match string would be: @code
+evdev:name:SynPS/2 Synaptics TouchPad:dmi:*svnLENOVO:*pvrThinkPadT440s*
+@endcode
+
+@note hwdb match strings only allow for alphanumeric ascii characters. Use a
+whildcard (* or ?, whichever appropriate) for special characters.
+
+The actual axis overrides are in the form:
+@code
+# axis number=min:max:resolution
+ EVDEV_ABS_00=2445:4252:42
+@endcode
+or, if the range is correct but the resolution is wrong
+@code
+# axis number=::resolution
+ EVDEV_ABS_00=::42
+@endcode
+
+Note the leading single 

Re: [PATCH] xdg-shell: add draw states for xdg_surface

[Benoit Gschwind]

Hello Derek,

Maybe I fail to explain my point of view:

I propose to have only one state array, I also propose that client
provide hints about which optional state he support. This is somehow
similar to EWMH - _NET_WM_STATE and EWMH - _NET_WM_ALLOWED_ACTIONS
respectively.

my arguments are:
 - state flags and draw flags are the same things, even if they look
different.
 - separating optionals and mandatory state has no benefit, the client
and the compositor can ignore them, even if they are mandatory.
consequently that make them /de facto/ optional. I do not mean it's a
good things, I mean separating them does not improve the situation.
Keeping them together just avoid long discussion on which side they must
go state or drawing state?

On 31/05/2016 23:04, Derek Foreman wrote:
> On 31/05/16 02:31 PM, Benoit Gschwind wrote:
>> Hello mike,
>>
>> Thank for your clarification, I try to summarize the issue/need:
>>
>> A compositor may want to draw a client without the shadow. In particular
>> if the window is integrated to a GUI component, I can imagine tilling.
> 
> Tiling seems only tangentially related to this proposal, since it's just
> one reason why a compositor might want to turn off drop shadows.
> 

Just tried to get a tangible definition of the issue/need to not discuss
hypothetical cases.

>> Taking your issue, the drawing state look very similar to the state
>> flags. You wrote: "[the draw state is] specifically related to drawing
>> only". Actually, removing the shadow may have side effect related to
>> behavior, in particular for client that use shadow as resize handlers,
> 
> The spec mentions that removing the drop shadow may interfere with
> interactive resize - from my reading it seems the intent is just to tell
> the client not to render drop shadows (not to draw something else in
> their place).  Perhaps that could be clarified in the next revision.
> 

My intension wasn't to get this clarified ;)

>> those client may replace shadow by plain borders. In that case the
>> difference is very thin. In the oposite way, the most expected changes
>> of fullscreen or maximized state is the client layout.
>>
>> You wrote: "[the draw state has] negotiation", I agree, but I will use
>> the wording "hint" because look more appropriate. Why do not add state
>> hint ? Some will argue that state are mandatory, to that I reply: ok,
>> how do you enforce it? You have no way to check that a client do not
>> draw a shadow while maximized or in fullscreen state, or basically
>> ignore the state. Thus just consider that those mandatory hints are
>> always included. States hints are similar to EWMH [1] or the WM_PROTOCOL
>> of ICCCM.
> 
> I'm not sure I understand your point, but I think you just successfully
> shot it down yourself?
> 

I don't know where I shot myself, I re-explained my point of view above.

> Moving right along then... ;)
> 
> Indeed states are mandatory and the compositor has to assume they've
> been obeyed - any app that doesn't is buggy.  Adding optional behaviour
> to that mechanism seems like a really bad idea.
> 

Please add arguments why it is a bad idea, I don't see why. The proposal
was basically to add optional drawing states. I do not see why optional
drawing states is better idea than optional states, moreover, I do not
see the *real* difference between states and drawing states.

> The negotiated method presented here seems less bug prone, and allowing
> toolkits not to have to implement all states seems less onerous.
> 

This match my proposal, but it look like I failed to explain it
properly. Or maybe I misunderstood the proposal.

> PS: When we're done re-inventing EWMH for wayland, can we have xatoms
> too?  Hmm, maybe not everything from the old days needs a direct mapping
> to wayland. :)

I agree, and the opposite apply, not all old stuff are broken. Analyzing
old stuff and picking what working save time. In that particular case I
do not say please add vertical_maximized state, I just says the old
stuff did the job and I propose a similar things, thus it's not insane,
thus please consider it as an (better) alternative.

Best regards,
Benoit Gschwind

> Thanks,
> Derek
> 
>> About the state enum, you are correct, the current spec. has reserved
>> ranges. My opinion still the same, this ranges are basically anti-standard.
>>
>> Best regards.
>>
>> [1]
>> https://specifications.freedesktop.org/wm-spec/wm-spec-latest.html#idm140200472593792
>>
>> On 31/05/2016 17:40, Mike Blumenkrantz wrote:
>>> After a long weekend of not reading mails, I've read some mails; I'll
>>> try to make comments to everything in this reply.
>>>
>>> On Tue, May 31, 2016 at 7:38 AM Olivier Fourdan >> > wrote:
>>>
>>> Hey
>>>
>>> - Original Message -
>>> > On Tue, May 31, 2016 at 05:24:35AM -0400, Olivier Fourdan wrote:
>>> > > [...]
>>> > > If we considered and dismissed them, fine.
>>> >
>>> > They are not 

Re: [PATCH] xdg-shell: add draw states for xdg_surface

[Derek Foreman]

On 31/05/16 02:31 PM, Benoit Gschwind wrote:
> Hello mike,
> 
> Thank for your clarification, I try to summarize the issue/need:
> 
> A compositor may want to draw a client without the shadow. In particular
> if the window is integrated to a GUI component, I can imagine tilling.

Tiling seems only tangentially related to this proposal, since it's just
one reason why a compositor might want to turn off drop shadows.

> Taking your issue, the drawing state look very similar to the state
> flags. You wrote: "[the draw state is] specifically related to drawing
> only". Actually, removing the shadow may have side effect related to
> behavior, in particular for client that use shadow as resize handlers,

The spec mentions that removing the drop shadow may interfere with
interactive resize - from my reading it seems the intent is just to tell
the client not to render drop shadows (not to draw something else in
their place).  Perhaps that could be clarified in the next revision.

> those client may replace shadow by plain borders. In that case the
> difference is very thin. In the oposite way, the most expected changes
> of fullscreen or maximized state is the client layout.
> 
> You wrote: "[the draw state has] negotiation", I agree, but I will use
> the wording "hint" because look more appropriate. Why do not add state
> hint ? Some will argue that state are mandatory, to that I reply: ok,
> how do you enforce it? You have no way to check that a client do not
> draw a shadow while maximized or in fullscreen state, or basically
> ignore the state. Thus just consider that those mandatory hints are
> always included. States hints are similar to EWMH [1] or the WM_PROTOCOL
> of ICCCM.

I'm not sure I understand your point, but I think you just successfully
shot it down yourself?

Moving right along then... ;)

Indeed states are mandatory and the compositor has to assume they've
been obeyed - any app that doesn't is buggy.  Adding optional behaviour
to that mechanism seems like a really bad idea.

The negotiated method presented here seems less bug prone, and allowing
toolkits not to have to implement all states seems less onerous.

PS: When we're done re-inventing EWMH for wayland, can we have xatoms
too?  Hmm, maybe not everything from the old days needs a direct mapping
to wayland. :)

Thanks,
Derek

> About the state enum, you are correct, the current spec. has reserved
> ranges. My opinion still the same, this ranges are basically anti-standard.
> 
> Best regards.
> 
> [1]
> https://specifications.freedesktop.org/wm-spec/wm-spec-latest.html#idm140200472593792
> 
> On 31/05/2016 17:40, Mike Blumenkrantz wrote:
>> After a long weekend of not reading mails, I've read some mails; I'll
>> try to make comments to everything in this reply.
>>
>> On Tue, May 31, 2016 at 7:38 AM Olivier Fourdan > > wrote:
>>
>> Hey
>>
>> - Original Message -
>> > On Tue, May 31, 2016 at 05:24:35AM -0400, Olivier Fourdan wrote:
>> > > [...]
>> > > If we considered and dismissed them, fine.
>> >
>> > They are not "actively" considered in this patch, but as I mentioned
>> > earlier, I think tiling states can later be added to the window state
>> > enum.
>>
>> Just a quick note before my main power source goes down here, my
>> comment are not about tiling, as stated before, I just tool tiling
>> as an example of where we eanted to have a value per edge.
>>
>> FWIW, I don't think tiling is a "draw state", I reckon it's a plain
>> state just like "maximized" or "active". But tiling is worth its own
>> thread :)
>>
>> Cheers,
>> Olviier
>> ___
>> wayland-devel mailing list
>> wayland-devel@lists.freedesktop.org
>> 
>> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>
>>
>> * Range allocations were added only because it was created in a similar
>> way to the configure state enum, which has such allocations. I don't
>> care much about them and I had no plans to use them at this time.
>>
>> * This patch is solely about implementing the basic idea of "draw
>> states"--a method for negotiating and controlling the manner in which
>> the client draws its windows. I don't see a need to "define the issue"
>> further than "this is a new display server protocol, let's ensure
>> there's a standard way to negotiate/control drawing between the server
>> and clients". This is clearly different from window states due to 1)
>> negotiation, and 2) specifically related to drawing only, not behavior.
>> ** The initial value of "no shadow" exists because it's a useful thing
>> to have in some cases (e.g., flat ui theme in compositor+toolkit,
>> compositor wants to draw special shadow effects, ...) and is not
>> controversial--moreover, it's trivial to implement compared to other
>> potential states.
>>
>> The concept of 

Re: [PATCH] xdg-shell: add draw states for xdg_surface

[Benoit Gschwind]

Hello mike,

Thank for your clarification, I try to summarize the issue/need:

A compositor may want to draw a client without the shadow. In particular
if the window is integrated to a GUI component, I can imagine tilling.

Taking your issue, the drawing state look very similar to the state
flags. You wrote: "[the draw state is] specifically related to drawing
only". Actually, removing the shadow may have side effect related to
behavior, in particular for client that use shadow as resize handlers,
those client may replace shadow by plain borders. In that case the
difference is very thin. In the oposite way, the most expected changes
of fullscreen or maximized state is the client layout.

You wrote: "[the draw state has] negotiation", I agree, but I will use
the wording "hint" because look more appropriate. Why do not add state
hint ? Some will argue that state are mandatory, to that I reply: ok,
how do you enforce it? You have no way to check that a client do not
draw a shadow while maximized or in fullscreen state, or basically
ignore the state. Thus just consider that those mandatory hints are
always included. States hints are similar to EWMH [1] or the WM_PROTOCOL
of ICCCM.

About the state enum, you are correct, the current spec. has reserved
ranges. My opinion still the same, this ranges are basically anti-standard.

Best regards.

[1]
https://specifications.freedesktop.org/wm-spec/wm-spec-latest.html#idm140200472593792

On 31/05/2016 17:40, Mike Blumenkrantz wrote:
> After a long weekend of not reading mails, I've read some mails; I'll
> try to make comments to everything in this reply.
> 
> On Tue, May 31, 2016 at 7:38 AM Olivier Fourdan  > wrote:
> 
> Hey
> 
> - Original Message -
> > On Tue, May 31, 2016 at 05:24:35AM -0400, Olivier Fourdan wrote:
> > > [...]
> > > If we considered and dismissed them, fine.
> >
> > They are not "actively" considered in this patch, but as I mentioned
> > earlier, I think tiling states can later be added to the window state
> > enum.
> 
> Just a quick note before my main power source goes down here, my
> comment are not about tiling, as stated before, I just tool tiling
> as an example of where we eanted to have a value per edge.
> 
> FWIW, I don't think tiling is a "draw state", I reckon it's a plain
> state just like "maximized" or "active". But tiling is worth its own
> thread :)
> 
> Cheers,
> Olviier
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> 
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 
> 
> * Range allocations were added only because it was created in a similar
> way to the configure state enum, which has such allocations. I don't
> care much about them and I had no plans to use them at this time.
> 
> * This patch is solely about implementing the basic idea of "draw
> states"--a method for negotiating and controlling the manner in which
> the client draws its windows. I don't see a need to "define the issue"
> further than "this is a new display server protocol, let's ensure
> there's a standard way to negotiate/control drawing between the server
> and clients". This is clearly different from window states due to 1)
> negotiation, and 2) specifically related to drawing only, not behavior.
> ** The initial value of "no shadow" exists because it's a useful thing
> to have in some cases (e.g., flat ui theme in compositor+toolkit,
> compositor wants to draw special shadow effects, ...) and is not
> controversial--moreover, it's trivial to implement compared to other
> potential states.
> 
> The concept of draw states is easy to envision extensions for when
> provided with this base example, and these extensions can be discussed
> at a later time and in a separate thread if everyone can agree on the
> premise. A tiled state is certainly something worthwhile to consider at
> that time and place.
> 
> 
> 
> 
>  
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] xdg-shell: add draw states for xdg_surface

[Mike Blumenkrantz]

After a long weekend of not reading mails, I've read some mails; I'll try
to make comments to everything in this reply.

On Tue, May 31, 2016 at 7:38 AM Olivier Fourdan  wrote:

> Hey
>
> - Original Message -
> > On Tue, May 31, 2016 at 05:24:35AM -0400, Olivier Fourdan wrote:
> > > [...]
> > > If we considered and dismissed them, fine.
> >
> > They are not "actively" considered in this patch, but as I mentioned
> > earlier, I think tiling states can later be added to the window state
> > enum.
>
> Just a quick note before my main power source goes down here, my comment
> are not about tiling, as stated before, I just tool tiling as an example of
> where we eanted to have a value per edge.
>
> FWIW, I don't think tiling is a "draw state", I reckon it's a plain state
> just like "maximized" or "active". But tiling is worth its own thread :)
>
> Cheers,
> Olviier
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel


* Range allocations were added only because it was created in a similar way
to the configure state enum, which has such allocations. I don't care much
about them and I had no plans to use them at this time.

* This patch is solely about implementing the basic idea of "draw
states"--a method for negotiating and controlling the manner in which the
client draws its windows. I don't see a need to "define the issue" further
than "this is a new display server protocol, let's ensure there's a
standard way to negotiate/control drawing between the server and clients".
This is clearly different from window states due to 1) negotiation, and 2)
specifically related to drawing only, not behavior.
** The initial value of "no shadow" exists because it's a useful thing to
have in some cases (e.g., flat ui theme in compositor+toolkit, compositor
wants to draw special shadow effects, ...) and is not
controversial--moreover, it's trivial to implement compared to other
potential states.

The concept of draw states is easy to envision extensions for when provided
with this base example, and these extensions can be discussed at a later
time and in a separate thread if everyone can agree on the premise. A tiled
state is certainly something worthwhile to consider at that time and place.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland-protocols v7] text: Create second version of text input protocol

[Yong Bakos]

On May 30, 2016, at 4:41 AM, Jan Arne Petersen  wrote:
> 
> There were some shortcomings in the first version of the protocol which
> makes it not really useful in real world applications. It is not really
> possible to fix them in a compatible way so introduce a new v2 of the
> protocol.
> 
> Fixes some shortcomings of the first version:
> 
> * Use only one wp_text_input per wl_seat (client side should be
>  handled by client toolkit)
> * Allow focus tracking without wl_keyboard present
> * Improve update state handling and better define state handling
> ---
> Changes to v6:
> * Fix some typos

Hi Jan,
There are some minor nits I'll address w/ a follow up patch, but this is
Reviewed-by: Yong Bakos 

(Note: careful of what you manually put below the ---, this breaks the
format of the patch.)

yong


> Makefile.am|   1 +
> unstable/text-input/text-input-unstable-v2.xml | 480 +
> 2 files changed, 481 insertions(+)
> create mode 100644 unstable/text-input/text-input-unstable-v2.xml
> 
> diff --git a/Makefile.am b/Makefile.am
> index 71d2632..cc8d531 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -3,6 +3,7 @@ unstable_protocols =  
> \
>   unstable/fullscreen-shell/fullscreen-shell-unstable-v1.xml  
> \
>   unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml  
> \
>   unstable/text-input/text-input-unstable-v1.xml  
> \
> + unstable/text-input/text-input-unstable-v2.xml  
> \
>   unstable/input-method/input-method-unstable-v1.xml  
> \
>   unstable/xdg-shell/xdg-shell-unstable-v5.xml
> \
>   unstable/relative-pointer/relative-pointer-unstable-v1.xml  
> \
> diff --git a/unstable/text-input/text-input-unstable-v2.xml 
> b/unstable/text-input/text-input-unstable-v2.xml
> new file mode 100644
> index 000..ea35d9b
> --- /dev/null
> +++ b/unstable/text-input/text-input-unstable-v2.xml
> @@ -0,0 +1,480 @@
> +
> +
> +
> +  
> +Copyright © 2012, 2013 Intel Corporation
> +Copyright © 2015, 2016 Jan Arne Petersen
> +
> +Permission to use, copy, modify, distribute, and sell this
> +software and its documentation for any purpose is hereby granted
> +without fee, provided that the above copyright notice appear in
> +all copies and that both that copyright notice and this permission
> +notice appear in supporting documentation, and that the name of
> +the copyright holders not be used in advertising or publicity
> +pertaining to distribution of the software without specific,
> +written prior permission.  The copyright holders make no
> +representations about the suitability of this software for any
> +purpose.  It is provided "as is" without express or implied
> +warranty.
> +
> +THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> +SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> +FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> +SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> +WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
> +AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> +ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
> +THIS SOFTWARE.
> +  
> +
> +  
> +
> +  The zwp_text_input_v2 interface represents text input and input methods
> +  associated with a seat. It provides enter/leave events to follow the
> +  text input focus for a seat.
> +
> +  Requests are used to enable/disable the text-input object and set
> +  state information like surrounding and selected text or the content 
> type.
> +  The information about the entered text is sent to the text-input object
> +  via the pre-edit and commit events. Using this interface removes the 
> need
> +  for applications to directly process hardware key events and compose 
> text
> +  out of them.
> +
> +  Text is valid UTF-8 encoded, indices and lengths are in bytes. Indices
> +  have to always point to the first byte of an UTF-8 encoded code point.
> +  Lengths are not allowed to contain just a part of an UTF-8 encoded code
> +  point.
> +
> +  State is sent by the state requests (set_surrounding_text,
> +  set_content_type, set_cursor_rectangle and set_preferred_language) and
> +  an update_state request. After an enter or an input_method_change event
> +  all state information is invalidated and needs to be resent from the
> +  client. A reset or entering a new widget on client side also
> +  invalidates all current state information.
> +
> +
> +
> +  
> + Destroy the wp_text_input object. Also disables all surfaces enabled
> + through this 

Re: Protocol for window previews/thumbnails

[Jonas Ådahl]

On Tue, May 31, 2016 at 02:49:38PM +0100, adlo wrote:
> 
> > On 20 May 2016, at 08:50, Pekka Paalanen  wrote:
> > 
> > You would design a new protocol extension private to Weston, with which
> > you deliver to your client the handles for top-level windows as they
> > come and go.
> > 
> 
> This should probably be a separate protocol from the preview protocol. Is it 
> possible to integrate the handle and preview protocol with another protocol, 
> such as xdg-shell, so that the handles are delivered as xdg_surfaces and the 
> preview protocol accepts xdg_surfaces as input?

You won't be able to create an xdg_surface from not-xdg_shell because of
the single-factory principle, but with v6 it would theoretically be
possible to create an xdg_surface and assign it a role not defined in
xdg_shell.

> 
> Is xdg-shell designed to be used by third-party clients for controlling 
> windows in a similar way to libwnck? Does xdg-shell expose the inner workings 
> of the compositor thus making it unsuitable?

No. xdg_shell will not work as a desktop environment component protocol.
It's targeted at toolkits and clients that act as regular clients.


Jonas

> 
> Regards
> 
> adlo
> ___
> 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: Protocol for window previews/thumbnails

[adlo]

> On 20 May 2016, at 08:50, Pekka Paalanen  wrote:
> 
> You would design a new protocol extension private to Weston, with which
> you deliver to your client the handles for top-level windows as they
> come and go.
> 

This should probably be a separate protocol from the preview protocol. Is it 
possible to integrate the handle and preview protocol with another protocol, 
such as xdg-shell, so that the handles are delivered as xdg_surfaces and the 
preview protocol accepts xdg_surfaces as input?

Is xdg-shell designed to be used by third-party clients for controlling windows 
in a similar way to libwnck? Does xdg-shell expose the inner workings of the 
compositor thus making it unsuitable?

Regards

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

Re: [PATCH weston v2] ivi-layout: Initialize surface source rectange to 1x1

[Pekka Paalanen]

On Tue, 31 May 2016 09:13:13 +
"Potrola, MateuszX"  wrote:

> Hi,
> 
> > Even more so, because as far as I can see, to actually see the badly sized 
> > surface
> > you have to do this in ivi-layout API:
> > 1. add the surface to a layer
> > 2. set visibility true
> > 3. commit_changes
> > 4. set source rectangle
> > 5. commit_changes
> > 
> > At step 3, the surface will be shown. It looks to me that you are very 
> > specifically
> > asking the surface to be shown with an unset source rectangle. Is there any
> > other way it can happen?  
> 
> That is correct, we are specifically asking surface to be shown
> before setting source rectangle - we are just testing different test
> case of how ivi shell can be used, that is one of them.
> 
> > > This is because initial setting of destination rectangle to 1x1
> > > is not causing surface resize and because source rectangle is 0x0
> > > appropiate transformation matrix is not calculated  
> > 
> > Ok, but why is that wrong?
> > 
> > If you do not set source and destination rectangles, what do you
> > expect to get?  
> 
> I expect to not see anything, but in that case I can see that surface
> is being displayed in original size. 

Hi,

is there some layer manager specification somewhere or something you
could refer to, that we would want to mirror?

I agree it is surprising, but one might also say it is intentional to
catch unexpected use of the API.

I think defaulting to show the whole window scaled down to 1x1 is the
worst choice of a default behaviour. The destination 1x1 is a hack
anyway, waiting to be removed when other code gets fixed to handle
destination 0x0 properly.

Maybe fixing the other code and dropping the 1x1 hack would actually
get you what you want, as the destination size would default to 0x0.
I'd still argue that source size should default to the whole surface,
rather than point-sampling the color from some pixel and painting
destination rectangle with that.

In no case would the default source of 1x1 be correct. The same can be
said about destination.

> > I thought they were not optional to set, but the documentation does
> > not really say if, or what is expected.  
> 
> If that is wrong use case then it will be nice to get some info in
> documentation about what steps are mandatory to do before eg. setting
> visibility to true.

The problem is, we at Weston upstream do not know how it should behave.
All we have is the current implementation.

In any case, whatever we find is the correct behaviour, it should be
implemented properly. Are you sure your patch does not cause a
transparent window to show up and possibly get input?


Thanks,
pq


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

Re: Protocol for window previews/thumbnails

[Pekka Paalanen]

On Thu, 26 May 2016 13:52:58 +0100
adlo  wrote:

> > On 24 May 2016, at 16:06, Pekka Paalanen  wrote:
> > 
> > The "private to weston" also means it should be a privileged interface:
> > arbitrary clients must not be able to use it, as it's none of their
> > concern and could be a security concern.
> >   
> 
> What about Wayland Security Modules (libwsm)? In what ways does this
> not solve the security issues of privileged interfaces?

I am only saying you have to pay attention to security there too. What
you will use to implement it is up to each compositor on its own.


Thanks,
pq


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

Re: Protocol for window previews/thumbnails

[adlo]

> On 11 May 2016, at 09:07, Pekka Paalanen  wrote:
> 
> If some sort of protocol would be needed, then you have to figure out
> how to not make it a gaping security breach
> 

What about Wayland Security Modules (libwsm)? In what ways does this fail to 
address the security issues with privileged interfaces?

Regards

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

Re: [PATCH] xdg-shell: add draw states for xdg_surface

[Olivier Fourdan]

Hey

- Original Message -
> On Tue, May 31, 2016 at 05:24:35AM -0400, Olivier Fourdan wrote:
> > [...]
> > If we considered and dismissed them, fine.
> 
> They are not "actively" considered in this patch, but as I mentioned
> earlier, I think tiling states can later be added to the window state
> enum.

Just a quick note before my main power source goes down here, my comment are 
not about tiling, as stated before, I just tool tiling as an example of where 
we eanted to have a value per edge.

FWIW, I don't think tiling is a "draw state", I reckon it's a plain state just 
like "maximized" or "active". But tiling is worth its own thread :)

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

Re: [PATCH] xdg-shell: add draw states for xdg_surface

[Jonas Ådahl]

On Tue, May 31, 2016 at 05:24:35AM -0400, Olivier Fourdan wrote:
> Hey
> 
> Just to make it clear, as a foreword to my comments inline below, I am not 
> against the patch, far from it, I just wanted to make sure we considered edge 
> values instead of only "no shadow", and we also considered other draw states.
> 
> If we considered and dismissed them, fine.

They are not "actively" considered in this patch, but as I mentioned
earlier, I think tiling states can later be added to the window state
enum.

> 
> For reference, this reminds me of the windows' semantics in EWMH. Before 
> EWMH, apps would use Motif MWM hints to specify if they wanted to have 
> borders, title, buttons, etc. With EWMH, apps would rather advertise a type 
> of window (dialog, menu, etc.), and the WM would decide how to decorate them, 
> so it would be a lot more consistent between apps. I still prefer the EWMH 
> approach to the MWM hints ^_~
> 
> Back to the topic...
> 
> - Original Message -
> > On Tue, May 31, 2016 at 10:31:28AM +0200, Benoit Gschwind wrote:
> > > Hello Jonas, Mike and Olivier,
> > > 
> > > [...]
> > > > I see nothing wrong with it in a separate (optional) protocol for
> > > > experimenting, but as soon as we have clients and compositors using
> > > > private values out in the field, it might become harder to put things
> > > > back into the agreed standard set.
> > 
> > I don't see why. We either have two integer values meaning the same
> > thing (the "upstreamed" number, and the private), or we have an
> > upstreamed integer part of the xdg_ and a private integer part of a
> > separate protocol.
> > 
> > The "upstreaming" procedure would be identical anyhow: writing a patch
> > adding the new entry to the enum.
> 
> No, for private ranges, no patch is needed as the values are private and do 
> not need to be documented in the standard. 
> 
> I think it's the sore point, having undocumented, private values in a 
> standard.

As said in the other mail, personally I'd be fine with dropping range
allocation and requiring toolkits etc to add their own enums and
configure events.

> 
> > Either way, semantically, the result will be identical. Personally I
> > don't care much whether per DE state enum entry allocations are to be
> > the current way or or in an extensions really with its own configure
> > event. Removing alloctions it from xdg_ would only an inconvenience to
> > do the exact same thing.
> > 
> > Just to repeat, the purpose of range allocations is to DE:s to make use
> > of the already present configure/ack_configure and now set_available_...
> > parts, but with their own private (not part of xdg_) integers, without
> > the issues of introducing any incompatibilities with other toolkits and
> > compositors. None of these private entries would ever be added or
> > referenced in xdg_shell.
> 
> Precisely, the point is that it defeats the purpose of a "standard" which is, 
> by definition, aimed to be shared between different implementations.
> 
> > > At the moment, You proposal just address the issue of the shadow, thus a
> > > simple set_no_shadow event should be enough ?
> > 
> > The reason for having a "draw state" besides "window state" (the state
> > we already have) is so that we have one state that is properly
> > negotiated. The window state is not negotiated; a compositor will just
> > add states it thinks the client should know about, but doesn't care much
> > whether the client changed its drawing in any way.
> > 
> > For draw states, the purpose of adding an entry is for when the
> > compositor actually cares. Shadows is one such thing, because as far as
> > I know for example KDE intends to draw its own server side shadows, and
> > could use a "no shadow" draw state to figure out whether it is possible.
> 
> OMG SSD! :) In this case you'd need "no border" and "no title bar" as well.

I imagine such entries could be added to the draw state enum indeed. I
also will assume such entries will stay unused by for example the
default weston desktop shell and mutter, but thats another story.


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

Re: [PATCH] xdg-shell: add draw states for xdg_surface

[Jonas Ådahl]

On Tue, May 31, 2016 at 11:16:51AM +0200, Benoit Gschwind wrote:
> 
> 
> On 31/05/2016 10:53, Jonas Ådahl wrote:
> > On Tue, May 31, 2016 at 10:31:28AM +0200, Benoit Gschwind wrote:
> >> Hello Jonas, Mike and Olivier,
> >>
> >> On 30/05/2016 15:09, Olivier Fourdan wrote:
> >>> Hi Jonas,
> >>>
> >>> - Original Message -
>  On Mon, May 30, 2016 at 05:01:58AM -0400, Olivier Fourdan wrote:
> >
> > Do we really want reserved ranges here?
> >
> > Some people reckon having (undocumented) reserved ranges was a bad idea 
> > in
> > "states", I wonder if we should redo this here again.
> >
> > Undocumented states from the reserved range are unlikely to be adopted 
> > by
> > other desktops, and that might lead to duplication of similar mechanisms
> > with different values.
> 
>  The purpose of a private range is not to have its values adopted by
>  other desktop environments, but rather a place to put experimental
>  things or things that might not suite a proper documented state. The
>  ranges is intended so that different DE:s don't conflict.
> 
>  I don't see what is wrong with that.
> >>>
> >>> I see nothing wrong with it in a separate (optional) protocol for 
> >>> experimenting, but as soon as we have clients and compositors using 
> >>> private values out in the field, it might become harder to put things 
> >>> back into the agreed standard set. 
> > 
> > I don't see why. We either have two integer values meaning the same
> > thing (the "upstreamed" number, and the private), or we have an
> > upstreamed integer part of the xdg_ and a private integer part of a
> > separate protocol.
> > 
> > The "upstreaming" procedure would be identical anyhow: writing a patch
> > adding the new entry to the enum.
> > 
> >>>
> >>
> >> Here I agree Olivier for this point, my argue is that xdg intend to
> >> provide a standard, and anyone expect that every DE follow this
> >> standard, thus each flags must be well defined and acknowledged. Any
> >> custom flags must fall in extension, that are not under the umbrella of 
> >> xdg.
> > 
> > Either way, semantically, the result will be identical. Personally I
> > don't care much whether per DE state enum entry allocations are to be
> > the current way or or in an extensions really with its own configure
> > event. Removing alloctions it from xdg_ would only an inconvenience to
> > do the exact same thing.
> 
> This is intended here, make difficult the wrong things, in our case
> custom states, make easier the proper things, in our case, following
> standard cases.
> 
> Thus the convenience argument here work against the standard we trying
> to bring to the community.

I see your point. I'll ack a patch that drops range allocation. It'd
probably need an ack from Mike as well, since I think they
(Enlightenment) have been one of the user of such a range.

> 
> > 
> > Just to repeat, the purpose of range allocations is to DE:s to make use
> > of the already present configure/ack_configure and now set_available_...
> > parts, but with their own private (not part of xdg_) integers, without
> > the issues of introducing any incompatibilities with other toolkits and
> > compositors. None of these private entries would ever be added or
> > referenced in xdg_shell.
> > 
> >>
> >> At the moment, You proposal just address the issue of the shadow, thus a
> >> simple set_no_shadow event should be enough ?
> > 
> > The reason for having a "draw state" besides "window state" (the state
> > we already have) is so that we have one state that is properly
> > negotiated. The window state is not negotiated; a compositor will just
> > add states it thinks the client should know about, but doesn't care much
> > whether the client changed its drawing in any way.
> > 
> > For draw states, the purpose of adding an entry is for when the
> > compositor actually cares. Shadows is one such thing, because as far as
> > I know for example KDE intends to draw its own server side shadows, and
> > could use a "no shadow" draw state to figure out whether it is possible.
> > 
> >>
>  The alternative is to have a separate configure event in an extension.
>  It would work the same way, is a bit more code to write, but it'd
>  effectively result in the same problem.
> >
> >> +  
> >> +
> >> +  The "no_shadow" draw state implies that the client must not
> >> draw
> >> +  drop shadow around the surface. This may have side effects
> >> +  on usability, e.g., the inability to activate 
> >> client-initiated
> >> +  interactive resize.
> >> +
> >> +  
> >> +
> >
> > Is a single "no shadow" state enough, isn't that too restrictive? If we 
> > put
> > this in perspective with a "tiled" state where we consider having
> > tiled_left/right/top/bottom, similarly, we could have "no_shadow_left",
> > 

Re: [PATCH] xdg-shell: add draw states for xdg_surface

[Benoit Gschwind]

Hello,

Just a reply to EWMH/MOTIF comments,

EWMH semantic is the path already started within xdg, as far as remember
v6 have toplevel, popup and tooltips surfaces, and will probably have
many more in the future.

That why we need a description of the issue we trying to solve. I can
assume that the issue is about toplevel window or not, thus should be a
flags list per surface role ?

Thinking about the proposal, and guessing the underlying issue, I
figured out that drawing request is the proper choice, even if it
introduce the trashed motif hint [1], and it is justified, because
compositor may want draw toplevel window in different manner. But at the
moment I see only one case that is for draw_without_shadow, but we can
easily drift to draw_tiled_mode, draw_glued_left, draw_glued_top,
draw_glued_right, draw_moving, draw_while_wobbling. In the case of drop
down menu: draw_span_left, draw_span_top, draw_without_shadow ? Once
again please define the issue.

I'm not against this proposal, but in current shape it does not comply
my requirement to include it as "a standard that any DE will have to
implement".

My opinion does not weight a lot anyway.

Best regards

[1] Side note: funnily, still implemented by all X11 WM, because EWMH
does not allow clients to declare that they draw their own borders.

On 31/05/2016 11:24, Olivier Fourdan wrote:
> Hey
> 
> Just to make it clear, as a foreword to my comments inline below, I am not 
> against the patch, far from it, I just wanted to make sure we considered edge 
> values instead of only "no shadow", and we also considered other draw states.
> 
> If we considered and dismissed them, fine.
> 
> For reference, this reminds me of the windows' semantics in EWMH. Before 
> EWMH, apps would use Motif MWM hints to specify if they wanted to have 
> borders, title, buttons, etc. With EWMH, apps would rather advertise a type 
> of window (dialog, menu, etc.), and the WM would decide how to decorate them, 
> so it would be a lot more consistent between apps. I still prefer the EWMH 
> approach to the MWM hints ^_~
> 
> Back to the topic...
> 
> - Original Message -
>> On Tue, May 31, 2016 at 10:31:28AM +0200, Benoit Gschwind wrote:
>>> Hello Jonas, Mike and Olivier,
>>>
>>> [...]
 I see nothing wrong with it in a separate (optional) protocol for
 experimenting, but as soon as we have clients and compositors using
 private values out in the field, it might become harder to put things
 back into the agreed standard set.
>>
>> I don't see why. We either have two integer values meaning the same
>> thing (the "upstreamed" number, and the private), or we have an
>> upstreamed integer part of the xdg_ and a private integer part of a
>> separate protocol.
>>
>> The "upstreaming" procedure would be identical anyhow: writing a patch
>> adding the new entry to the enum.
> 
> No, for private ranges, no patch is needed as the values are private and do 
> not need to be documented in the standard. 
> 
> I think it's the sore point, having undocumented, private values in a 
> standard.
> 
>> Either way, semantically, the result will be identical. Personally I
>> don't care much whether per DE state enum entry allocations are to be
>> the current way or or in an extensions really with its own configure
>> event. Removing alloctions it from xdg_ would only an inconvenience to
>> do the exact same thing.
>>
>> Just to repeat, the purpose of range allocations is to DE:s to make use
>> of the already present configure/ack_configure and now set_available_...
>> parts, but with their own private (not part of xdg_) integers, without
>> the issues of introducing any incompatibilities with other toolkits and
>> compositors. None of these private entries would ever be added or
>> referenced in xdg_shell.
> 
> Precisely, the point is that it defeats the purpose of a "standard" which is, 
> by definition, aimed to be shared between different implementations.
> 
>>> At the moment, You proposal just address the issue of the shadow, thus a
>>> simple set_no_shadow event should be enough ?
>>
>> The reason for having a "draw state" besides "window state" (the state
>> we already have) is so that we have one state that is properly
>> negotiated. The window state is not negotiated; a compositor will just
>> add states it thinks the client should know about, but doesn't care much
>> whether the client changed its drawing in any way.
>>
>> For draw states, the purpose of adding an entry is for when the
>> compositor actually cares. Shadows is one such thing, because as far as
>> I know for example KDE intends to draw its own server side shadows, and
>> could use a "no shadow" draw state to figure out whether it is possible.
> 
> OMG SSD! :) In this case you'd need "no border" and "no title bar" as well.
> 
>>> This last comments look to overlap with some already existing feature
>>> within the protocol, I think about the input shape.
>>
>> Not sure how this has anything to do with input 

Re: [PATCH] xdg-shell: add draw states for xdg_surface

[Olivier Fourdan]

Hey

Just to make it clear, as a foreword to my comments inline below, I am not 
against the patch, far from it, I just wanted to make sure we considered edge 
values instead of only "no shadow", and we also considered other draw states.

If we considered and dismissed them, fine.

For reference, this reminds me of the windows' semantics in EWMH. Before EWMH, 
apps would use Motif MWM hints to specify if they wanted to have borders, 
title, buttons, etc. With EWMH, apps would rather advertise a type of window 
(dialog, menu, etc.), and the WM would decide how to decorate them, so it would 
be a lot more consistent between apps. I still prefer the EWMH approach to the 
MWM hints ^_~

Back to the topic...

- Original Message -
> On Tue, May 31, 2016 at 10:31:28AM +0200, Benoit Gschwind wrote:
> > Hello Jonas, Mike and Olivier,
> > 
> > [...]
> > > I see nothing wrong with it in a separate (optional) protocol for
> > > experimenting, but as soon as we have clients and compositors using
> > > private values out in the field, it might become harder to put things
> > > back into the agreed standard set.
> 
> I don't see why. We either have two integer values meaning the same
> thing (the "upstreamed" number, and the private), or we have an
> upstreamed integer part of the xdg_ and a private integer part of a
> separate protocol.
> 
> The "upstreaming" procedure would be identical anyhow: writing a patch
> adding the new entry to the enum.

No, for private ranges, no patch is needed as the values are private and do not 
need to be documented in the standard. 

I think it's the sore point, having undocumented, private values in a standard.

> Either way, semantically, the result will be identical. Personally I
> don't care much whether per DE state enum entry allocations are to be
> the current way or or in an extensions really with its own configure
> event. Removing alloctions it from xdg_ would only an inconvenience to
> do the exact same thing.
> 
> Just to repeat, the purpose of range allocations is to DE:s to make use
> of the already present configure/ack_configure and now set_available_...
> parts, but with their own private (not part of xdg_) integers, without
> the issues of introducing any incompatibilities with other toolkits and
> compositors. None of these private entries would ever be added or
> referenced in xdg_shell.

Precisely, the point is that it defeats the purpose of a "standard" which is, 
by definition, aimed to be shared between different implementations.

> > At the moment, You proposal just address the issue of the shadow, thus a
> > simple set_no_shadow event should be enough ?
> 
> The reason for having a "draw state" besides "window state" (the state
> we already have) is so that we have one state that is properly
> negotiated. The window state is not negotiated; a compositor will just
> add states it thinks the client should know about, but doesn't care much
> whether the client changed its drawing in any way.
> 
> For draw states, the purpose of adding an entry is for when the
> compositor actually cares. Shadows is one such thing, because as far as
> I know for example KDE intends to draw its own server side shadows, and
> could use a "no shadow" draw state to figure out whether it is possible.

OMG SSD! :) In this case you'd need "no border" and "no title bar" as well.

> > This last comments look to overlap with some already existing feature
> > within the protocol, I think about the input shape.
> 
> Not sure how this has anything to do with input shapes.

True, I really meant fat borders, as in weston-terminal for example, not input 
shapes.

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

Re: [PATCH] xdg-shell: add draw states for xdg_surface

[Benoit Gschwind]

On 31/05/2016 10:53, Jonas Ådahl wrote:
> On Tue, May 31, 2016 at 10:31:28AM +0200, Benoit Gschwind wrote:
>> Hello Jonas, Mike and Olivier,
>>
>> On 30/05/2016 15:09, Olivier Fourdan wrote:
>>> Hi Jonas,
>>>
>>> - Original Message -
 On Mon, May 30, 2016 at 05:01:58AM -0400, Olivier Fourdan wrote:
>
> Do we really want reserved ranges here?
>
> Some people reckon having (undocumented) reserved ranges was a bad idea in
> "states", I wonder if we should redo this here again.
>
> Undocumented states from the reserved range are unlikely to be adopted by
> other desktops, and that might lead to duplication of similar mechanisms
> with different values.

 The purpose of a private range is not to have its values adopted by
 other desktop environments, but rather a place to put experimental
 things or things that might not suite a proper documented state. The
 ranges is intended so that different DE:s don't conflict.

 I don't see what is wrong with that.
>>>
>>> I see nothing wrong with it in a separate (optional) protocol for 
>>> experimenting, but as soon as we have clients and compositors using private 
>>> values out in the field, it might become harder to put things back into the 
>>> agreed standard set. 
> 
> I don't see why. We either have two integer values meaning the same
> thing (the "upstreamed" number, and the private), or we have an
> upstreamed integer part of the xdg_ and a private integer part of a
> separate protocol.
> 
> The "upstreaming" procedure would be identical anyhow: writing a patch
> adding the new entry to the enum.
> 
>>>
>>
>> Here I agree Olivier for this point, my argue is that xdg intend to
>> provide a standard, and anyone expect that every DE follow this
>> standard, thus each flags must be well defined and acknowledged. Any
>> custom flags must fall in extension, that are not under the umbrella of xdg.
> 
> Either way, semantically, the result will be identical. Personally I
> don't care much whether per DE state enum entry allocations are to be
> the current way or or in an extensions really with its own configure
> event. Removing alloctions it from xdg_ would only an inconvenience to
> do the exact same thing.

This is intended here, make difficult the wrong things, in our case
custom states, make easier the proper things, in our case, following
standard cases.

Thus the convenience argument here work against the standard we trying
to bring to the community.

> 
> Just to repeat, the purpose of range allocations is to DE:s to make use
> of the already present configure/ack_configure and now set_available_...
> parts, but with their own private (not part of xdg_) integers, without
> the issues of introducing any incompatibilities with other toolkits and
> compositors. None of these private entries would ever be added or
> referenced in xdg_shell.
> 
>>
>> At the moment, You proposal just address the issue of the shadow, thus a
>> simple set_no_shadow event should be enough ?
> 
> The reason for having a "draw state" besides "window state" (the state
> we already have) is so that we have one state that is properly
> negotiated. The window state is not negotiated; a compositor will just
> add states it thinks the client should know about, but doesn't care much
> whether the client changed its drawing in any way.
> 
> For draw states, the purpose of adding an entry is for when the
> compositor actually cares. Shadows is one such thing, because as far as
> I know for example KDE intends to draw its own server side shadows, and
> could use a "no shadow" draw state to figure out whether it is possible.
> 
>>
 The alternative is to have a separate configure event in an extension.
 It would work the same way, is a bit more code to write, but it'd
 effectively result in the same problem.
>
>> +  
>> +
>> +  The "no_shadow" draw state implies that the client must not
>> draw
>> +  drop shadow around the surface. This may have side effects
>> +  on usability, e.g., the inability to activate client-initiated
>> +  interactive resize.
>> +
>> +  
>> +
>
> Is a single "no shadow" state enough, isn't that too restrictive? If we 
> put
> this in perspective with a "tiled" state where we consider having
> tiled_left/right/top/bottom, similarly, we could have "no_shadow_left",
> "no_shadow_right", "no_shadow_top" and ""no_shadow_bottom", that would
> give a finer granularity.

 I think we should just add such tiling states to the window state enum,
 rather than the draw state. The only point with putting things in the
 draw state enum is to get the negotiation, while tiled vs not tiled is
 closer to maximized vs not maximized, i.e. a compositor wouldn't change
 its behaviour if the client couldn't not draw itself "tiled" or not.
>>>
>>> Sorry, I 

RE: [PATCH weston v2] ivi-layout: Initialize surface source rectange to 1x1

[Potrola, MateuszX]

Hi,

> Even more so, because as far as I can see, to actually see the badly sized 
> surface
> you have to do this in ivi-layout API:
> 1. add the surface to a layer
> 2. set visibility true
> 3. commit_changes
> 4. set source rectangle
> 5. commit_changes
> 
> At step 3, the surface will be shown. It looks to me that you are very 
> specifically
> asking the surface to be shown with an unset source rectangle. Is there any
> other way it can happen?

That is correct, we are specifically asking surface to be shown before setting 
source rectangle - we are just testing different test case of how ivi shell can 
be used, that is one of them.

> > This is because initial setting of destination rectangle to 1x1 is not
> > causing surface resize and because source rectangle is 0x0 appropiate
> > transformation matrix is not calculated
> 
> Ok, but why is that wrong?
> 
> If you do not set source and destination rectangles, what do you expect to 
> get?

I expect to not see anything, but in that case I can see that surface is being 
displayed in original size.
 
> I thought they were not optional to set, but the documentation does not really
> say if, or what is expected.

If that is wrong use case then it will be nice to get some info in 
documentation about what steps are mandatory to do before eg. setting 
visibility to true.

Regards,
Mateusz
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

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

Re: [PATCH] xdg-shell: add draw states for xdg_surface

[Jonas Ådahl]

On Tue, May 31, 2016 at 10:31:28AM +0200, Benoit Gschwind wrote:
> Hello Jonas, Mike and Olivier,
> 
> On 30/05/2016 15:09, Olivier Fourdan wrote:
> > Hi Jonas,
> > 
> > - Original Message -
> >> On Mon, May 30, 2016 at 05:01:58AM -0400, Olivier Fourdan wrote:
> >>>
> >>> Do we really want reserved ranges here?
> >>>
> >>> Some people reckon having (undocumented) reserved ranges was a bad idea in
> >>> "states", I wonder if we should redo this here again.
> >>>
> >>> Undocumented states from the reserved range are unlikely to be adopted by
> >>> other desktops, and that might lead to duplication of similar mechanisms
> >>> with different values.
> >>
> >> The purpose of a private range is not to have its values adopted by
> >> other desktop environments, but rather a place to put experimental
> >> things or things that might not suite a proper documented state. The
> >> ranges is intended so that different DE:s don't conflict.
> >>
> >> I don't see what is wrong with that.
> > 
> > I see nothing wrong with it in a separate (optional) protocol for 
> > experimenting, but as soon as we have clients and compositors using private 
> > values out in the field, it might become harder to put things back into the 
> > agreed standard set. 

I don't see why. We either have two integer values meaning the same
thing (the "upstreamed" number, and the private), or we have an
upstreamed integer part of the xdg_ and a private integer part of a
separate protocol.

The "upstreaming" procedure would be identical anyhow: writing a patch
adding the new entry to the enum.

> > 
> 
> Here I agree Olivier for this point, my argue is that xdg intend to
> provide a standard, and anyone expect that every DE follow this
> standard, thus each flags must be well defined and acknowledged. Any
> custom flags must fall in extension, that are not under the umbrella of xdg.

Either way, semantically, the result will be identical. Personally I
don't care much whether per DE state enum entry allocations are to be
the current way or or in an extensions really with its own configure
event. Removing alloctions it from xdg_ would only an inconvenience to
do the exact same thing.

Just to repeat, the purpose of range allocations is to DE:s to make use
of the already present configure/ack_configure and now set_available_...
parts, but with their own private (not part of xdg_) integers, without
the issues of introducing any incompatibilities with other toolkits and
compositors. None of these private entries would ever be added or
referenced in xdg_shell.

> 
> At the moment, You proposal just address the issue of the shadow, thus a
> simple set_no_shadow event should be enough ?

The reason for having a "draw state" besides "window state" (the state
we already have) is so that we have one state that is properly
negotiated. The window state is not negotiated; a compositor will just
add states it thinks the client should know about, but doesn't care much
whether the client changed its drawing in any way.

For draw states, the purpose of adding an entry is for when the
compositor actually cares. Shadows is one such thing, because as far as
I know for example KDE intends to draw its own server side shadows, and
could use a "no shadow" draw state to figure out whether it is possible.

> 
> >> The alternative is to have a separate configure event in an extension.
> >> It would work the same way, is a bit more code to write, but it'd
> >> effectively result in the same problem.
> >>>
>  +  
>  +
>  +  The "no_shadow" draw state implies that the client must not
>  draw
>  +  drop shadow around the surface. This may have side effects
>  +  on usability, e.g., the inability to activate client-initiated
>  +  interactive resize.
>  +
>  +  
>  +
> >>>
> >>> Is a single "no shadow" state enough, isn't that too restrictive? If we 
> >>> put
> >>> this in perspective with a "tiled" state where we consider having
> >>> tiled_left/right/top/bottom, similarly, we could have "no_shadow_left",
> >>> "no_shadow_right", "no_shadow_top" and ""no_shadow_bottom", that would
> >>> give a finer granularity.
> >>
> >> I think we should just add such tiling states to the window state enum,
> >> rather than the draw state. The only point with putting things in the
> >> draw state enum is to get the negotiation, while tiled vs not tiled is
> >> closer to maximized vs not maximized, i.e. a compositor wouldn't change
> >> its behaviour if the client couldn't not draw itself "tiled" or not.
> > 
> > Sorry, I did not make myself clear, I was not asking for tiling to a be a 
> > draw state.
> > 
> >> As discussed in the bug you linked, there might be more to tiling than
> >> shadow as well; it might effect rounded-ness of corners and maybe other
> >> things as well, and adding "no_shadow_right" wouldn't address that.
> > 
> > No I was taking tiling as an example where we'd 

Re: [PATCH] xdg-shell: add draw states for xdg_surface

[Benoit Gschwind]

Hello Jonas, Mike and Olivier,

On 30/05/2016 15:09, Olivier Fourdan wrote:
> Hi Jonas,
> 
> - Original Message -
>> On Mon, May 30, 2016 at 05:01:58AM -0400, Olivier Fourdan wrote:
>>>
>>> Do we really want reserved ranges here?
>>>
>>> Some people reckon having (undocumented) reserved ranges was a bad idea in
>>> "states", I wonder if we should redo this here again.
>>>
>>> Undocumented states from the reserved range are unlikely to be adopted by
>>> other desktops, and that might lead to duplication of similar mechanisms
>>> with different values.
>>
>> The purpose of a private range is not to have its values adopted by
>> other desktop environments, but rather a place to put experimental
>> things or things that might not suite a proper documented state. The
>> ranges is intended so that different DE:s don't conflict.
>>
>> I don't see what is wrong with that.
> 
> I see nothing wrong with it in a separate (optional) protocol for 
> experimenting, but as soon as we have clients and compositors using private 
> values out in the field, it might become harder to put things back into the 
> agreed standard set. 
> 

Here I agree Olivier for this point, my argue is that xdg intend to
provide a standard, and anyone expect that every DE follow this
standard, thus each flags must be well defined and acknowledged. Any
custom flags must fall in extension, that are not under the umbrella of xdg.

At the moment, You proposal just address the issue of the shadow, thus a
simple set_no_shadow event should be enough ?

>> The alternative is to have a separate configure event in an extension.
>> It would work the same way, is a bit more code to write, but it'd
>> effectively result in the same problem.
>>>
 +  
 +
 +  The "no_shadow" draw state implies that the client must not
 draw
 +  drop shadow around the surface. This may have side effects
 +  on usability, e.g., the inability to activate client-initiated
 +  interactive resize.
 +
 +  
 +
>>>
>>> Is a single "no shadow" state enough, isn't that too restrictive? If we put
>>> this in perspective with a "tiled" state where we consider having
>>> tiled_left/right/top/bottom, similarly, we could have "no_shadow_left",
>>> "no_shadow_right", "no_shadow_top" and ""no_shadow_bottom", that would
>>> give a finer granularity.
>>
>> I think we should just add such tiling states to the window state enum,
>> rather than the draw state. The only point with putting things in the
>> draw state enum is to get the negotiation, while tiled vs not tiled is
>> closer to maximized vs not maximized, i.e. a compositor wouldn't change
>> its behaviour if the client couldn't not draw itself "tiled" or not.
> 
> Sorry, I did not make myself clear, I was not asking for tiling to a be a 
> draw state.
> 
>> As discussed in the bug you linked, there might be more to tiling than
>> shadow as well; it might effect rounded-ness of corners and maybe other
>> things as well, and adding "no_shadow_right" wouldn't address that.
> 
> No I was taking tiling as an example where we'd want the edge, so we might 
> consider having a "no shadow" value per edge as well, but not related to 
> tiling though.
> 
> This might come useful for surfaces placed next to the edge of a monitor in a 
> multi-monitor setup for example (so we don't have shadows crossing monitors 
> for example) - Maybe it's just overkill, dunno.
> 
> We could also consider "no border", again, per edge as well. Or not.
>

This last comments look to overlap with some already existing feature
within the protocol, I think about the input shape.

Also, because the issue is not well described the discussion overflow to
severals other issues.

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

Best regards
--
Benoit Gschwind

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

Re: [PATCH libinput] touchpad: short-circuit the edge scroll handling when it's not enabled

[Hans de Goede]

Hi,

On 31-05-16 02:16, Peter Hutterer wrote:

No need to handle events properly in the edge scroll state machine when it's
not enabled. Just set any beginning touch to state AREA and move on. The rest
of the code guarantees neutral state when edge scrolling is enabled or
disabled.

This reduces the debug output produced by libinput-debug-events when edge
scrolling is disabled, preventing users from seemingly identifying
bugs where there are none.

Signed-off-by: Peter Hutterer 
---
 src/evdev-mt-touchpad-edge-scroll.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/evdev-mt-touchpad-edge-scroll.c 
b/src/evdev-mt-touchpad-edge-scroll.c
index fcc0512..8222bba 100644
--- a/src/evdev-mt-touchpad-edge-scroll.c
+++ b/src/evdev-mt-touchpad-edge-scroll.c
@@ -318,6 +318,15 @@ tp_edge_scroll_handle_state(struct tp_dispatch *tp, 
uint64_t time)
 {
struct tp_touch *t;

+   if (tp->scroll.method != LIBINPUT_CONFIG_SCROLL_EDGE) {
+   tp_for_each_touch(tp, t) {
+   if (t->state == TOUCH_BEGIN)
+   t->scroll.edge_state =
+   EDGE_SCROLL_TOUCH_STATE_AREA;
+   }
+   return;
+   }
+
tp_for_each_touch(tp, t) {
if (!t->dirty)
continue;


IMHO it would be cleaner to replace this hunk with:

@@ -141,7 +141,8 @@ tp_edge_scroll_handle_none(struct tp_dispatch *tp,

switch (event) {
case SCROLL_EVENT_TOUCH:
-   if (tp_touch_get_edge(tp, t)) {
+ if (tp->scroll.method == LIBINPUT_CONFIG_SCROLL_EDGE &&
+ tp_touch_get_edge(tp, t)) {
tp_edge_scroll_set_state(tp, t,
EDGE_SCROLL_TOUCH_STATE_EDGE_NEW);
} else {

This is the cleanest solution IMHO, but then we still get one log_debug for new 
touches;
alternatively we could do:

@@ -327,7 +327,13 @@ tp_edge_scroll_handle_state(struct tp_dispatch *tp, 
uint64_t time)
case TOUCH_HOVERING:
break;
case TOUCH_BEGIN:
-   tp_edge_scroll_handle_event(tp, t, SCROLL_EVENT_TOUCH);
+ if (tp->scroll.method == LIBINPUT_CONFIG_SCROLL_EDGE) {
+ tp_edge_scroll_handle_event(tp, t,
+ SCROLL_EVENT_TOUCH);
+ } else {
+ tp_edge_scroll_set_state(tp, t,
+ EDGE_SCROLL_TOUCH_STATE_AREA);
+ }
break;
case TOUCH_UPDATE:
tp_edge_scroll_handle_event(tp, t, SCROLL_EVENT_MOTION);

That at least avoids the double tp_for_each_touch(tp, t) {}

I've a slight preference for the first solution, and just living with the
one debug line for a new touch, after all this is for debugging only, and
code-wise it is by far the cleanest.

Anyways all 3 get the job done in the end, so this patch is:

Reviewed-by: Hans de Goede 

Feel free to pick any of the outlined solutions (including your original one).


@@ -350,9 +359,6 @@ tp_edge_scroll_post_events(struct tp_dispatch *tp, uint64_t 
time)
const struct normalized_coords zero = { 0.0, 0.0 };
const struct discrete_coords zero_discrete = { 0.0, 0.0 };

-   if (tp->scroll.method != LIBINPUT_CONFIG_SCROLL_EDGE)
-   return 0;
-
tp_for_each_touch(tp, t) {
if (!t->dirty)
continue;



Regards,

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

Re: [RFC wayland] Add wl_proxy destruction callbacks

[Pekka Paalanen]

On Mon, 30 May 2016 13:10:42 +0200
Miguel Angel Vico  wrote:

> Hi all,
> 
> A few days ago, I had a little chat over IRC with Pekka about addition
> of proxy objects destruction callbacks to the wayland client protocol.
> 
> 
> Summing up, we recently ran into some applications where native objects
> (wl_surface, wl_egl_window, wl_display) used by EGL are destroyed/made
> invalid before destroying the corresponding EGL objects. This sometimes
> causes crashes of the EGL driver, which is not nice. We have seen this
> with the NVIDIA EGL implementation, but I assume the Mesa EGL
> implementation is similarly exposed.
> 
> I agree this is in fact an application bug, but the EGL spec states that
> functions such as makeCurrent or swapBuffers should return error (not
> crash) if the native objects become invalid. I also agree the spec
> should have been clearer and probably allowed "undefined behavior", but
> it is not the case.
> 
> Having an objects destruction notification mechanism such as destruction
> callbacks would allow us to satisfy the spec.
> 
> Also, such functionality would also make life way easier under certain
> circumstances. I'm basically thinking about multi-threaded applications,
> where several threads make use of the same native objects, and for some
> reason one of the threads has to destroy one or more of them due to some
> sort of error happening.
> 
> Of course, this can still be considered an application bug, and the
> application could still make sure none of the threads is going to use
> the native objects before destroying them, but again, specs allow users
> to do many non-recommended things.
> 
> I think we should try to do our best to gracefully handle those
> non-desirable API usages, and avoid crashes whenever is possible.
> 
> 
> Pekka did not see this as something crazy to have, but wanted to hear
> from some of the toolkits guys before making the decision of whether
> changing the wl_proxy ABI is a good idea.

Hi,

indeed. I think we could do such an addition to the wl_proxy ABI
safely, if we want to. I just don't know if we want to.

I'm not so sure about the point of helping multithreaded applications.
Applications use toolkits, and toolkits' job is to support whatever
multi-threading ways they want, because they probably have to
explicitly support threading anyway. The issue is use cases where there
is no toolkit in between: EGL.

Another point that diminishes the multi-threaded use case is that one
cannot have multiple listeners on a wl_proxy anyway. If you need to
process events for the same object in more than one thread, you must
have the support code outside of libwayland-client. Also, the thread
assignment with listeners (wl_event_queue) must be done on the object
creation. It cannot be done afterwards without potential races. Again
EGL is somewhat special, since it does not need to listen on externally
created objects.

Then there is the question of what the locking should look like.

The EGL specification language is troublesome. I guess it was written
with X11 in mind, where anyone even outside of the current process can
go and destroy the native window at any time, without any coordination.
Obviously it is desireable that it does not lead to a crash. However,
that is not possible on Wayland. No external actor can go and destroy
the wl_display or wl_surface from behind your back, only the
application itself can do that. Therefore, I think if the application
deliberately shoots its own foot, a crash is ok, just like if you pass
a bad pointer through EGL API in the first place. I am very tempted to
say that this should be raised as a EGL spec bug, to allow undefined
behaviour or such. EGL platform specifications could then make their
own additional requirements, like on X11 you must not crash if Window
disappears.

Those were the cons, then the pros not yet mentioned:

Currently there is a problem with identifying Wayland protocol objects
referenced in incoming events, in case there are multiple components
creating them. See Section "Multiple input handlers" in
http://ppaalanen.blogspot.fi/2013/11/sub-surfaces-now.html
for an example of a problematic case. It is not the only one.

If wl_proxy had destroy listeners like wl_resource does, one could use
a wl_signal_get()/wl_resource_get_destroy_listener() kind of an
approach to fetch a component-specific struct, rather than assuming who
happens to own the singular "user data" pointer and risk a crash.

> 
> As an alternative, destruction callbacks could be hung off of
> wl_egl_window. In a similar way we support wl_egl_window_resize
> callbacks, we could support wl_egl_window_destroy callbacks.

At the moment I would slightly favour the wl_egl_window approach if it
allows to circumvent the EGL wording and the wording cannot be fixed.

> However, this isn't as foolproof as adding wl_proxy destruction
> callbacks, since destruction of wl_surface or wl_display objects before
> wl_egl_window