[PATCH libinput] touchpad: fix tapping that happens after a moving thumb

2018-05-04 Thread Friedrich Schöller
When finger movement exceeded the motion threshold before the finger was
recognized as a thumb, it would never be regarded as a thumb by the tap system.
This prohibited tapping until the thumb was lifted.

This is fixed by moving the check for the thumb state up such that it
happens before the motion threshold check.
---
 src/evdev-mt-touchpad-tap.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index 5946dc8f..e3e051ca 100644
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -1039,6 +1039,9 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time)
tp_tap_handle_event(tp, t, TAP_EVENT_RELEASE, 
time);
}
t->tap.state = TAP_TOUCH_STATE_IDLE;
+   } else if (tp->tap.state != TAP_STATE_IDLE &&
+  t->thumb.state == THUMB_STATE_YES) {
+   tp_tap_handle_event(tp, t, TAP_EVENT_THUMB, time);
} else if (tp->tap.state != TAP_STATE_IDLE &&
   tp_tap_exceeds_motion_threshold(tp, t)) {
struct tp_touch *tmp;
@@ -1051,10 +1054,6 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t 
time)
}
 
tp_tap_handle_event(tp, t, TAP_EVENT_MOTION, time);
-   } else if (tp->tap.state != TAP_STATE_IDLE &&
-  t->thumb.state == THUMB_STATE_YES &&
-  !t->tap.is_thumb) {
-   tp_tap_handle_event(tp, t, TAP_EVENT_THUMB, time);
}
}
 
-- 
2.14.3

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


Re: [PATCH libinput] touchpad: fix tapping that happens after a moving thumb

2018-05-04 Thread Friedrich Schöller
I have attached an evemu recording that illustrates the issue. In the recording 
a thumb touches the touchpad, gets state THUMB_STATE_MAYBE, then 
THUMB_STATE_YES. After that, a tap is performed, but it is ignored without the 
patch.


Best,
Friedrich

On 05.05.2018 03:49, Friedrich Schöller wrote:

When finger movement exceeded the motion threshold before the finger was
recognized as a thumb, it would never be regarded as a thumb by the tap system.
This prohibited tapping until the thumb was lifted.

This is fixed by moving the check for the thumb state up such that it
happens before the motion threshold check.
---
  src/evdev-mt-touchpad-tap.c | 7 +++
  1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index 5946dc8f..e3e051ca 100644
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -1039,6 +1039,9 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time)
tp_tap_handle_event(tp, t, TAP_EVENT_RELEASE, 
time);
}
t->tap.state = TAP_TOUCH_STATE_IDLE;
+   } else if (tp->tap.state != TAP_STATE_IDLE &&
+  t->thumb.state == THUMB_STATE_YES) {
+   tp_tap_handle_event(tp, t, TAP_EVENT_THUMB, time);
} else if (tp->tap.state != TAP_STATE_IDLE &&
   tp_tap_exceeds_motion_threshold(tp, t)) {
struct tp_touch *tmp;
@@ -1051,10 +1054,6 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t 
time)
}
  
  			tp_tap_handle_event(tp, t, TAP_EVENT_MOTION, time);

-   } else if (tp->tap.state != TAP_STATE_IDLE &&
-  t->thumb.state == THUMB_STATE_YES &&
-  !t->tap.is_thumb) {
-   tp_tap_handle_event(tp, t, TAP_EVENT_THUMB, time);
}
}
  

# EVEMU 1.3
# Kernel: 4.17.0-rc3-custom
# DMI: 
dmi:bvnLENOVO:bvrN1CET66W(1.34):bd03/22/2018:svnLENOVO:pn20F9CTO1WW:pvrThinkPadT460s:rvnLENOVO:rn20F9CTO1WW:rvrSDK0J40700WIN:cvnLENOVO:ct10:cvrNone:
# Input device name: "Synaptics TM3145-003"
# Input device ID: bus 0x1d vendor 0x6cb product  version 
# Size in mm: 97x53
# Supported events:
#   Event type 0 (EV_SYN)
# Event code 0 (SYN_REPORT)
# Event code 1 (SYN_CONFIG)
# Event code 2 (SYN_MT_REPORT)
# Event code 3 (SYN_DROPPED)
# Event code 4 ((null))
# Event code 5 ((null))
# Event code 6 ((null))
# Event code 7 ((null))
# Event code 8 ((null))
# Event code 9 ((null))
# Event code 10 ((null))
# Event code 11 ((null))
# Event code 12 ((null))
# Event code 13 ((null))
# Event code 14 ((null))
# Event code 15 (SYN_MAX)
#   Event type 1 (EV_KEY)
# Event code 272 (BTN_LEFT)
# Event code 325 (BTN_TOOL_FINGER)
# Event code 328 (BTN_TOOL_QUINTTAP)
# Event code 330 (BTN_TOUCH)
# Event code 333 (BTN_TOOL_DOUBLETAP)
# Event code 334 (BTN_TOOL_TRIPLETAP)
# Event code 335 (BTN_TOOL_QUADTAP)
#   Event type 3 (EV_ABS)
# Event code 0 (ABS_X)
#   Value  988
#   Min  0
#   Max   1940
#   Fuzz 0
#   Flat 0
#   Resolution  20
# Event code 1 (ABS_Y)
#   Value  970
#   Min  0
#   Max   1074
#   Fuzz 0
#   Flat 0
#   Resolution  20
# Event code 24 (ABS_PRESSURE)
#   Value0
#   Min  0
#   Max255
#   Fuzz 0
#   Flat 0
#   Resolution   0
# Event code 47 (ABS_MT_SLOT)
#   Value0
#   Min  0
#   Max  4
#   Fuzz 0
#   Flat 0
#   Resolution   0
# Event code 48 (ABS_MT_TOUCH_MAJOR)
#   Value0
#   Min  0
#   Max 15
#   Fuzz 0
#   Flat 0
#   Resolution   0
# Event code 49 (ABS_MT_TOUCH_MINOR)
#   Value0
#   Min  0
#   Max 15
#   Fuzz 0
#   Flat 0
#   Resolution   0
# Event code 52 (ABS_MT_ORIENTATION)
#   Value0
#   Min  0
#   Max  1
#   Fuzz 0
#   Flat 0
#   Resolution   0
# Event code 53 (ABS_MT_POSITION_X)
#   Value0
#   Min  0
#   Max   1940
#   Fuzz 0
#   Flat 0
#   Resolution  20
# Event code 54 (ABS_MT_POSITION_Y)
#   Value0
#   Min  0
#   Max   1074
#   Fuzz 0
#   Flat 0
#   Resolution  20
# Event code 55 (ABS_MT_TOOL_TYPE)
#   Value0
#   Min  0
#   Max  2
#   Fuzz 0
#   Flat 0
#   Resolution   0
# Event code 57 (ABS_MT_TRACKING_ID)
#   Value0
#   Min  0
#   Max  65535
#   

Re: [PATCHv4 wayland-protocols] text-input: Add v3 of the text-input protocol

2018-05-04 Thread Silvan Jegen
On Thu, May 03, 2018 at 10:46:47PM +0200, Dorota Czaplejewicz wrote:
> On Thu, 3 May 2018 21:55:40 +0200
> Silvan Jegen  wrote:
> 
> > On Thu, May 03, 2018 at 09:22:46PM +0200, Dorota Czaplejewicz wrote:
> > > On Thu, 3 May 2018 20:47:27 +0200
> > > Silvan Jegen  wrote:
> > >   
> > > > Hi Dorota
> > > > 
> > > > Some comments and typo fixes below.
> > > > 
> > > > On Thu, May 03, 2018 at 05:41:21PM +0200, Dorota Czaplejewicz wrote:  
> > > > > +  Text is valid UTF-8 encoded, indices and lengths are in code 
> > > > > points. If a
> > > > > +  grapheme is made up of multiple code points, an index pointing 
> > > > > to any of
> > > > > +  them should be interpreted as pointing to the first one.
> > > > 
> > > > That way we make sure we don't put the cursor/anchor between bytes that
> > > > belong to the same UTF-8 encoded Unicode code point which is nice. It
> > > > also means that the client has to parse all the UTF-8 encoded strings
> > > > into Unicode code points up to the desired cursor/anchor position
> > > > on each "preedit_string" event. For each "delete_surrounding_text" event
> > > > the client has to parse the UTF-8 sequences before and after the cursor
> > > > position up to the requested Unicode code point.
> > > > 
> > > > I feel like we are processing the UTF-8 string already in the
> > > > input-method. So I am not sure that we should parse it again on the
> > > > client side. Parsing it again would also mean that the client would need
> > > > to know about UTF-8 which would be nice to avoid.
> > > > 
> > > > Thoughts?  
> > > 
> > > The client needs to know about Unicode, but not necessarily about
> > > UTF-8. Specifying code points is actually an advantage here, because
> > > byte offsets are inherently expressed relative to UTF-8. By counting
> > > with code points, client's internal representation can be UTF-16 or
> > > maybe even something else.  
> > 
> > Maybe I am misunderstanding something but the protocol specifies that
> > the strings are valid UTF-8 encoded and the cursor/anchor offsets into
> > the strings are specified in Unicode points. To me that indicates that
> > the application *has to parse* the UTF-8 string into Unicode points
> > when receiving the event otherwise it doesn't know after which Unicode
> > point to draw the cursor. Of course the application can then decide to
> > convert the UTF-8 string into another encoding like UTF-16 for internal
> > processing (for whatever reason) but that doesn't change the fact that
> > it still would have to parse the incoming UTF-8 (and thus know about
> > UTF-8).
> > 
> Can you see any way to avoid parsing UTF-8 in order to draw the
> cursor? I tried to come up with a way to do that, but even with
> specifying byte strings, I believe that calculating the position of
> the cursor - either in pixels or in glyphs - requires full parsing of
> the input string.

Yes, I don't think it's avoidable either. You just don't have to do
it twice if your text rendering library consumes UTF-8 strings with
byte-offsets though. See my response below.


> > > There's no avoiding the parsing either. What the application cares
> > > about is that the cursor falls between glyphs. The application cannot
> > > know that in all cases. Unicode allows the same sequence to be
> > > displayed in multiple ways (fallback):
> > > 
> > > https://unicode.org/emoji/charts/emoji-zwj-sequences.html
> > > 
> > > One could make an argument that byte offsets should never be close
> > > to ZWJ characters, but I think this decision is better left to the
> > > application, which knows what exactly it is presenting to the user.  
> > 
> > The idea of the previous version of the protocol (from my understanding)
> > was to make sure that only valid UTF-8 and valid byte-offsets (== not
> > falling between bytes of a Unicode code point) into the string will be
> > sent to the client. If you just get a byte-offset into a UTF-8 encoded
> > string you trust the sender to honor the protocol and thus you can just
> > pass the UTF-8 encoded string unprocessed to your text rendering library
> > (provided that the library supports UTF-8 strings which is what I am
> > assuming) without having to parse the UTF-8 string into Unicode code
> > points.
> > 
> > Of course the Unicode code points will have to be parsed at some point
> > if you want to render them. Using byte-offsets just lets you do that at
> > a later stage if your libraries support UTF-8.
> > 
> > 
> Doesn't that chiefly depend on what kind of the text rendering library
> though? As far as I understand, passing text to rendering is necessary
> to calculate the cursor position. At the same time, it doesn't matter
> much for the calculations whether the cursor offset is in bytes or
> code points - the library does the parsing in the last step anyway.
> 
> I think you mean that if the rendering library accepts byte offsets
> as the only format, the application would have to parse the UTF-8
> unnecessarily. 

Re: libweston program segfaults on weston_compositor_load_backend

2018-05-04 Thread adlo
On Fri, 2018-05-04 at 10:13 +0300, Pekka Paalanen wrote:
> 
> Is that perhaps the very first call to weston_log()?
> 
> You need to initialize the logging mechanism before anything can call
> weston_log(). Yeah, it's awkward, could probably be improved, but
> OTOH
> one would always want all log messages going to the same place, so
> you
> really do want to initialize the logging before any logging happens
> to
> avoid losing messages.
> 
> The crucial call to make is weston_log_set_handler() with non-NULL
> arguments before any other libweston calls.
> 

That's fixed it, thank you!

Regards

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


What is wesron_plane?

2018-05-04 Thread Sichem Zhou
Hi all,

Many data structure in libweston is straightforward, but I don't really
know what does weston_plane do in the compositor. As it seems I don't need
to deal with it alot. It seems that it's a abstract place to stack
`weston_output` and `weston_views`. Like a virtual plane to draw all the
outputs on it. What is the relationship between `weston_plane`,
`weston_view` and `weston_output`?

Thanks alot

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


Re: libweston program segfaults on weston_compositor_load_backend

2018-05-04 Thread Matt Hoosier
On Fri, May 4, 2018 at 8:46 AM, Pekka Paalanen  wrote:

> On Fri, 4 May 2018 07:55:10 -0500
> Matt Hoosier  wrote:
>
> > On Fri, May 4, 2018 at 2:13 AM, Pekka Paalanen 
> wrote:
> >
> > > The crucial call to make is weston_log_set_handler() with non-NULL
> > > arguments before any other libweston calls.
> > >
> >
> > I wonder if this could be made clearer by adding an assert in
> weston_log()
> > if no handler has been installed yet.
>
> Definitely. :-)
> A shout to stderr and abort() would be fine. Or even have the handlers
> initilized to ones that shout and abort().
>
>
> Thanks,
> pq
>


Done and published on the list for comments.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH] log: improve handling of use-before-init

2018-05-04 Thread Matt Hoosier
Rather than segfaulting by attempting to traverse an initially
null log handler pointer, explicitly print a message and abort.

Signed-off-by: Matt Hoosier 
---
 libweston/log.c | 22 --
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/libweston/log.c b/libweston/log.c
index 7d99a95d..c21f25e9 100644
--- a/libweston/log.c
+++ b/libweston/log.c
@@ -36,8 +36,26 @@
 
 #include "compositor.h"
 
-static log_func_t log_handler = 0;
-static log_func_t log_continue_handler = 0;
+static int default_log_handler(const char *fmt, va_list ap);
+
+static log_func_t log_handler = default_log_handler;
+static log_func_t log_continue_handler = default_log_handler;
+
+/** Sentinel log message handler
+ *
+ * This function is used as the default handler for log messages. It
+ * exists only to issue a noisy reminder to the user that a real handler
+ * must be installed prior to issuing logging calls. The process is
+ * immediately aborted after the reminder is printed.
+ *
+ * \param fmt The format string. Ignored.
+ * \param va The variadic argument list. Ignored.
+ */
+static int default_log_handler(const char *fmt, va_list ap)
+{
+fprintf(stderr, "weston_log_set_handler() must be called before using 
of weston_log().\n");
+abort();
+}
 
 /** Install the log handler
  *
-- 
2.13.6

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


Re: libweston program segfaults on weston_compositor_load_backend

2018-05-04 Thread Pekka Paalanen
On Fri, 4 May 2018 07:55:10 -0500
Matt Hoosier  wrote:

> On Fri, May 4, 2018 at 2:13 AM, Pekka Paalanen  wrote:
> 
> > The crucial call to make is weston_log_set_handler() with non-NULL
> > arguments before any other libweston calls.
> >  
> 
> I wonder if this could be made clearer by adding an assert in weston_log()
> if no handler has been installed yet.

Definitely. :-)
A shout to stderr and abort() would be fine. Or even have the handlers
initilized to ones that shout and abort().


Thanks,
pq


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


Re: [PATCH weston] libweston: don't accumulate damage from transparent views

2018-05-04 Thread Pekka Paalanen
On Fri, 4 May 2018 12:09:55 +
"Ucan, Emre (ADITG/ESB)"  wrote:

> Hi Pekka,
> 
> Sorry for late response.

Hi Emre,

no worries!

> > -Original Message-
> > From: Pekka Paalanen [mailto:ppaala...@gmail.com]
> > Sent: Donnerstag, 19. April 2018 12:36
> > To: Ucan, Emre (ADITG/ESB)
> > Cc: wayland-devel@lists.freedesktop.org
> > Subject: Re: [PATCH weston] libweston: don't accumulate damage from
> > transparent views
> > 
> > On Thu, 19 Apr 2018 09:26:59 +
> > "Ucan, Emre (ADITG/ESB)"  wrote:
> >   
> > > Hi Pekka,
> > >
> > > If we remove the view from scenegraph, application will be blocked.
> > > Because it is not getting any surface frame events. It is not OK to
> > > block unexpected applications. Especially if the application is
> > > sending output of a camera or digital TV, weston should always get
> > > latest buffer from the application.  
> > 
> > Hi Emre,
> > 
> > sounds like the application is broken if it gets literally blocked
> > by that.
> > 
> > Applications very much have to be able to deal with frame callbacks
> > not coming back in a timely fashion. This is what compositors use to
> > throttle down well-behaving clients. It's not blocking the app,
> > unless the app is badly written.
> > 
> > An invisible surface is a very good reason to try and throttle down
> > the client.
> >   
> > > You might say that camera application can use wl_display_sync
> > > instead wl_surface_frame.  
> > 
> > Not necessarily. It would certainly make the app greedy instead of
> > well-behaved in my opinion.
> > 
> > But, this would definitely make the latest buffer from the
> > application always available to the compositor. You shouldn't need
> > wl_display.sync even, the app can simply keep on committing new
> > buffers whenever it wants.  
> 
> You are right that this issue can be work around for an EGL
> application when we set swapInterval to 0, and send a new buffer
> every time internal state of the application requires to change its
> contents. But this does not fix the greediness problem. Because
> application state can change more often than display refresh rate.

The key with EGL is to never allow eglSwapBuffers() block. The app can
still ask for frame callbacks itself and throttle to those while
remaining otherwise responsive.

eglSwapBuffers() has been designed to be blocking in nature, i.e. it
has been designed to waste time. Any application that has more than one
window or needs to react to more than just Wayland input events or
needs to run a simulation (games!) would better set swapInterval to
zero. If they don't, and they also don't manually throttle to frame
callbacks, they are bound to waste time in eglSwapBuffers(), and this
is not only a Wayland issue. Wayland just allows us to make this bad
client design painfully obvious.

> > Or is the camera framerate an exact integer fraction of the display
> > refresh rate?  
> Ideally yes. Let's think that camera is sending buffer with 60 Hz and
> display has also the same refresh rate. But camera and display could
> be still out of sync, so that camera sends its buffer 8ms after
> Vsync. Therefore, it will miss the repaint window of weston.
> 
> It is also possible that camera buffers sometimes hit, sometimes miss
> the repaint window. This would cause visible video stutter. Because
> delay of displaying a buffer would swing between 0,5 Vsync to 1,5
> Vsync ( Repaint window is 0.5 Vsync). Therefore, it is not ok to send
> buffer every time when it is ready. We should send it directly after
> repaint, so that (hopefully) we won't miss the repaint window.

I don't think it changes much with respect to timings though. If the
client was posting frames as soon as it got it from the camera, the
compositor would be picking which ones make it to the screen. If the
client is throttled to frame callbacks, then yes, any frame the client
chooses will hit the screen, but now it is the client making the same
choice as the compositor in the former case. Except, the client needs
to make that decision earlier than the compositor.

Can the client here be smarter than the compositor to make a difference?
Given that we don't know about the deadline yet, discussed further
below.

> > I know there is a problem that when the surface becomes visible
> > again, it would take a frame cycle to have the client send an
> > updated buffer. This could be worked around on either side: the
> > compositor could be sending frame callbacks at a slow rate even if
> > the surface is not visible, or the client could be updating the
> > surface content at a slow rate even if it doesn't get frame
> > callbacks.  
> 
> But your proposal is also a hack.

Right, but what else could you do, when you do not know in advance when
the window will become visible and yet you want to minimize the number of
frames drawn and discarded?

The smartest option might be to postpone the actual show in the
compositor until the client has updated, but that's a trade-off between
latency fro

Re: libweston program segfaults on weston_compositor_load_backend

2018-05-04 Thread Matt Hoosier
On Fri, May 4, 2018 at 2:13 AM, Pekka Paalanen  wrote:

> On Thu, 03 May 2018 23:38:55 +0100
> adlo  wrote:
>
> > On Thu, 2018-05-03 at 14:12 +0300, Pekka Paalanen wrote:
> > >
> > > what's the backtrace from gdb for that?
> > >
> >
> > Is this any help?
> >
> > #0  0x in  ()
> > #1  0x7799c88c in weston_log (fmt=fmt@entry=0x779b6a7e
> > "Loading module '%s'\n") at libweston/log.c:75
> > #2  0x779a603c in weston_load_module (name=name@entry=0x779
> > b6b24 "wayland-backend.so", entrypoint=entrypoint@entry=0x779b6aae
> > "weston_backend_init") at libweston/compositor.c:6375
> > #3  0x779a6264 in weston_compositor_load_backend
> > (compositor=0x611090, backend=backend@entry=
> > WESTON_BACKEND_WAYLAND, config_base=config_base@entry=0x7fffdc5
> > 0)
> > at libweston/compositor.c:6485
> > #4  0x00400704 in main (argc=, argv= > out>)
> > at ../main.c:42
>
> Is that perhaps the very first call to weston_log()?
>
> You need to initialize the logging mechanism before anything can call
> weston_log(). Yeah, it's awkward, could probably be improved, but OTOH
> one would always want all log messages going to the same place, so you
> really do want to initialize the logging before any logging happens to
> avoid losing messages.
>
> The crucial call to make is weston_log_set_handler() with non-NULL
> arguments before any other libweston calls.
>

I wonder if this could be made clearer by adding an assert in weston_log()
if no handler has been installed yet.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


RE: [PATCH weston] libweston: don't accumulate damage from transparent views

2018-05-04 Thread Ucan, Emre (ADITG/ESB)
Hi Pekka,

Sorry for late response.

> -Original Message-
> From: Pekka Paalanen [mailto:ppaala...@gmail.com]
> Sent: Donnerstag, 19. April 2018 12:36
> To: Ucan, Emre (ADITG/ESB)
> Cc: wayland-devel@lists.freedesktop.org
> Subject: Re: [PATCH weston] libweston: don't accumulate damage from
> transparent views
> 
> On Thu, 19 Apr 2018 09:26:59 +
> "Ucan, Emre (ADITG/ESB)"  wrote:
> 
> > Hi Pekka,
> >
> > If we remove the view from scenegraph, application will be blocked.
> > Because it is not getting any surface frame events. It is not OK to
> > block unexpected applications. Especially if the application is
> > sending output of a camera or digital TV, weston should always get
> > latest buffer from the application.
> 
> Hi Emre,
> 
> sounds like the application is broken if it gets literally blocked by
> that.
> 
> Applications very much have to be able to deal with frame callbacks
> not coming back in a timely fashion. This is what compositors use to
> throttle down well-behaving clients. It's not blocking the app, unless
> the app is badly written.
> 
> An invisible surface is a very good reason to try and throttle down the
> client.
> 
> > You might say that camera application can use wl_display_sync instead
> > wl_surface_frame.
> 
> Not necessarily. It would certainly make the app greedy instead of
> well-behaved in my opinion.
> 
> But, this would definitely make the latest buffer from the application
> always available to the compositor. You shouldn't need wl_display.sync
> even, the app can simply keep on committing new buffers whenever it
> wants.

You are right that this issue can be work around for an EGL application when we 
set
swapInterval to 0, and send a new buffer every time internal state of the 
application requires to
change its contents. But this does not fix the greediness problem. Because 
application state can change
more often than display refresh rate.

> 
> If the application is a video player that just cannot avoid decoding or
> receiving frames anyway, then the savings of not posting those frames
> may be insignificant. The major saving the throttling aims for is with
> apps that paint on demand, where painting is a heavy task that can
> easily be skipped.
> 
> > This would cause stutter and frame drop issues,
> > because camera stream and display are not in synch.
> 
> Are your camera and display both using the exact same refresh rate,
> just not synchronized by hardware?
> 
> I fail to see how the frame callbacks would synchronize anything any
> better than just always committing new buffers whenever the app has
> one. It's either the compositor or the client that eventually makes the
> decision to drop or repeat frames to account for differing refresh
> rates. But if it is the app doing it, it could interpolate video frames
> to match the display refresh cycle.
> 
> Or is the camera framerate an exact integer fraction of the display
> refresh rate?
Ideally yes. Let's think that camera is sending buffer with 60 Hz and display 
has also the same refresh rate.
But camera and display could be still out of sync, so that camera sends its 
buffer 8ms after Vsync. Therefore,
it will miss the repaint window of weston.

It is also possible that camera buffers sometimes hit, sometimes miss the 
repaint window. This would cause visible video stutter.
Because delay of displaying a buffer would swing between 0,5 Vsync to 1,5 Vsync 
( Repaint window is 0.5 Vsync).
Therefore, it is not ok to send buffer every time when it is ready. We should 
send it directly after repaint, so that (hopefully) we won't miss the repaint 
window.
> 
> > I know that I have to modify weston_compositor_pick_view(). I will
> > send a patch if this one is accepted.
> 
> It still seems like a hack to me.
> 
> If the view is not visible (does not matter how it is not visible), it
> should not be getting frame callbacks, because there is no point for
> the client to update surface as it is not visible. Weston is missing
> some implementation bits to postpone frame callbacks for e.g. surfaces
> that are completely occluded. I suppose being completely transparent
> could be another case.
You are right that it is a kind of hack. But I don't know how it should work 
otherwise especially for camera/video use-cases.

> 
> I know there is a problem that when the surface becomes visible again,
> it would take a frame cycle to have the client send an updated buffer.
> This could be worked around on either side: the compositor could be
> sending frame callbacks at a slow rate even if the surface is not
> visible, or the client could be updating the surface content at a slow
> rate even if it doesn't get frame callbacks.

But your proposal is also a hack. Maybe better solution would be that we 
introduce a repaint_window event to wl_output interface, 
so that camera/video applications can synchronize themselves with this event 
instead of surface frame events. 
Compositor can send a timestamp and duration of repai

[PATCH weston 0/2] xwm: Dump properties.

2018-05-04 Thread Fabien Lahoudere
This series adds code to help debug by dumping properties of type CARDINAL and
WINDOW.

Pekka Paalanen (2):
  xwm: dump properties of type CARDINAL
  xwm: dump properties of type WINDOW

 xwayland/window-manager.c | 70 ++-
 1 file changed, 69 insertions(+), 1 deletion(-)

-- 
1.8.3.1

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


[PATCH weston 1/2] xwm: dump properties of type CARDINAL

2018-05-04 Thread Fabien Lahoudere
From: Pekka Paalanen 

Add code to dump CARDINAL arrays from properties. Useful for debugging.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 xwayland/window-manager.c | 66 ++-
 1 file changed, 65 insertions(+), 1 deletion(-)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 06370b7..061ce17 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -27,6 +27,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -377,6 +378,67 @@ xcb_cursor_library_load_cursor(struct weston_wm *wm, const 
char *file)
return cursor;
 }
 
+static unsigned
+dump_cardinal_array_elem(FILE *fp, unsigned format,
+void *arr, unsigned len, unsigned ind)
+{
+   const char *comma;
+
+   /* If more than 16 elements, print 0-14, ..., last */
+   if (ind > 14 && ind < len - 1) {
+   fprintf(fp, ", ...");
+   return len - 1;
+   }
+
+   comma = ind ? ", " : "";
+
+   switch (format) {
+   case 32:
+   fprintf(fp, "%s%" PRIu32, comma, ((uint32_t *)arr)[ind]);
+   break;
+   case 16:
+   fprintf(fp, "%s%" PRIu16, comma, ((uint16_t *)arr)[ind]);
+   break;
+   case 8:
+   fprintf(fp, "%s%" PRIu8, comma, ((uint8_t *)arr)[ind]);
+   break;
+   default:
+   fprintf(fp, "%s???", comma);
+   }
+
+   return ind + 1;
+}
+
+static void
+dump_cardinal_array(xcb_get_property_reply_t *reply)
+{
+   unsigned i = 0;
+   FILE *fp;
+   void *arr;
+   char *str = NULL;
+   size_t size = 0;
+
+   assert(reply->type == XCB_ATOM_CARDINAL);
+
+   fp = open_memstream(&str, &size);
+   if (!fp)
+   return;
+
+   arr = xcb_get_property_value(reply);
+
+   fprintf(fp, "[");
+   while (i < reply->value_len)
+   i = dump_cardinal_array_elem(fp, reply->format,
+arr, reply->value_len, i);
+   fprintf(fp, "]");
+
+   if (fclose(fp) != 0)
+   return;
+
+   wm_log_continue("%s\n", str);
+   free(str);
+}
+
 void
 dump_property(struct weston_wm *wm,
  xcb_atom_t property, xcb_get_property_reply_t *reply)
@@ -403,7 +465,7 @@ dump_property(struct weston_wm *wm,
incr_value = xcb_get_property_value(reply);
wm_log_continue("%d\n", *incr_value);
} else if (reply->type == wm->atom.utf8_string ||
- reply->type == wm->atom.string) {
+  reply->type == wm->atom.string) {
text_value = xcb_get_property_value(reply);
if (reply->value_len > 40)
len = 40;
@@ -424,6 +486,8 @@ dump_property(struct weston_wm *wm,
width +=  wm_log_continue("%s", name);
}
wm_log_continue("\n");
+   } else if (reply->type == XCB_ATOM_CARDINAL) {
+   dump_cardinal_array(reply);
} else {
wm_log_continue("huh?\n");
}
-- 
1.8.3.1

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


[PATCH weston 2/2] xwm: dump properties of type WINDOW

2018-05-04 Thread Fabien Lahoudere
From: Pekka Paalanen 

Very useful for TRANSIENT_FOR property debugging.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 xwayland/window-manager.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 061ce17..2b3defb 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -446,6 +446,7 @@ dump_property(struct weston_wm *wm,
int32_t *incr_value;
const char *text_value, *name;
xcb_atom_t *atom_value;
+   xcb_window_t *window_value;
int width, len;
uint32_t i;
 
@@ -488,6 +489,9 @@ dump_property(struct weston_wm *wm,
wm_log_continue("\n");
} else if (reply->type == XCB_ATOM_CARDINAL) {
dump_cardinal_array(reply);
+   } else if (reply->type == XCB_ATOM_WINDOW && reply->format == 32) {
+   window_value = xcb_get_property_value(reply);
+   wm_log_continue("win %u\n", *window_value);
} else {
wm_log_continue("huh?\n");
}
-- 
1.8.3.1

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


[PATCH weston 0/1] Improve override-redirect debug messages

2018-05-04 Thread Fabien Lahoudere
This patch is an addition to https://patchwork.freedesktop.org/patch/211161/.

Pekka Paalanen (1):
  xwm: improve override-redirect debug messages

 xwayland/window-manager.c | 29 -
 1 file changed, 20 insertions(+), 9 deletions(-)

-- 
1.8.3.1

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


[PATCH weston 1/1] xwm: improve override-redirect debug messages

2018-05-04 Thread Fabien Lahoudere
From: Pekka Paalanen 

Explicitly say if O-R is yes or no, to not have to remember which
messages possibly printed it.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 

Conflicts:
xwayland/window-manager.c
---
 xwayland/window-manager.c | 29 -
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 32a756c..0ac8639 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -747,14 +747,25 @@ weston_wm_handle_configure_notify(struct weston_wm *wm, 
xcb_generic_event_t *eve
const struct weston_desktop_xwayland_interface *xwayland_api =
wm->server->compositor->xwayland_interface;
struct weston_wm_window *window;
+   const char *or_str = " unk";
 
-   wm_log("XCB_CONFIGURE_NOTIFY (window %d) %d,%d @ %dx%d%s\n",
+   if (wm_lookup_window(wm, configure_notify->window, &window)) {
+   if (!!configure_notify->override_redirect == 
!!window->override_redirect)
+   or_str = ".";
+   else
+   or_str = " BUG!";
+   } else {
+   window = NULL;
+   }
+
+   wm_log("XCB_CONFIGURE_NOTIFY (window %d) %d,%d @ %dx%d, O-R: %s%s\n",
   configure_notify->window,
   configure_notify->x, configure_notify->y,
   configure_notify->width, configure_notify->height,
-  configure_notify->override_redirect ? ", override" : "");
+  configure_notify->override_redirect ? "yes" : "no",
+  or_str);
 
-   if (!wm_lookup_window(wm, configure_notify->window, &window))
+   if (!window)
return;
 
window->x = configure_notify->x;
@@ -1136,8 +1147,8 @@ weston_wm_handle_map_notify(struct weston_wm *wm, 
xcb_generic_event_t *event)
return;
}
 
-   wm_log("XCB_MAP_NOTIFY (window %d%s)\n", map_notify->window,
-  map_notify->override_redirect ? ", override" : "");
+   wm_log("XCB_MAP_NOTIFY (window %d) O-R : %s\n", map_notify->window,
+  map_notify->override_redirect ? "yes" : "no");
 
if (!wm_lookup_window(wm, map_notify->window, &window))
return;
@@ -1449,11 +1460,11 @@ weston_wm_handle_create_notify(struct weston_wm *wm, 
xcb_generic_event_t *event)
xcb_create_notify_event_t *create_notify =
(xcb_create_notify_event_t *) event;
 
-   wm_log("XCB_CREATE_NOTIFY (window %d, at (%d, %d), width %d, height 
%d%s%s)\n",
+   wm_log("XCB_CREATE_NOTIFY (window %d, at (%d, %d), width %d, height %d, 
O-R: %s%s)\n",
   create_notify->window,
   create_notify->x, create_notify->y,
   create_notify->width, create_notify->height,
-  create_notify->override_redirect ? ", override" : "",
+  create_notify->override_redirect ? "yes" : "no",
   our_resource(wm, create_notify->window) ? ", ours" : "");
 
if (our_resource(wm, create_notify->window))
@@ -1493,11 +1504,11 @@ weston_wm_handle_reparent_notify(struct weston_wm *wm, 
xcb_generic_event_t *even
(xcb_reparent_notify_event_t *) event;
struct weston_wm_window *window;
 
-   wm_log("XCB_REPARENT_NOTIFY (window %d, parent %d, event %d%s)\n",
+   wm_log("XCB_REPARENT_NOTIFY (window %d, parent %d, event %d, O-R: 
%s)\n",
   reparent_notify->window,
   reparent_notify->parent,
   reparent_notify->event,
-  reparent_notify->override_redirect ? ", override" : "");
+  reparent_notify->override_redirect ? "yes" : "no");
 
if (reparent_notify->parent == wm->screen->root) {
weston_wm_window_create(wm, reparent_notify->window, 10, 10,
-- 
1.8.3.1

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


Re: libweston program segfaults on weston_compositor_load_backend

2018-05-04 Thread Pekka Paalanen
On Thu, 03 May 2018 23:38:55 +0100
adlo  wrote:

> On Thu, 2018-05-03 at 14:12 +0300, Pekka Paalanen wrote:
> > 
> > what's the backtrace from gdb for that?
> >   
> 
> Is this any help?
> 
> #0  0x in  ()
> #1  0x7799c88c in weston_log (fmt=fmt@entry=0x779b6a7e
> "Loading module '%s'\n") at libweston/log.c:75
> #2  0x779a603c in weston_load_module (name=name@entry=0x779
> b6b24 "wayland-backend.so", entrypoint=entrypoint@entry=0x779b6aae
> "weston_backend_init") at libweston/compositor.c:6375
> #3  0x779a6264 in weston_compositor_load_backend
> (compositor=0x611090, backend=backend@entry=
> WESTON_BACKEND_WAYLAND, config_base=config_base@entry=0x7fffdc5
> 0)
> at libweston/compositor.c:6485
> #4  0x00400704 in main (argc=, argv= out>)  
> at ../main.c:42

Is that perhaps the very first call to weston_log()?

You need to initialize the logging mechanism before anything can call
weston_log(). Yeah, it's awkward, could probably be improved, but OTOH
one would always want all log messages going to the same place, so you
really do want to initialize the logging before any logging happens to
avoid losing messages.

The crucial call to make is weston_log_set_handler() with non-NULL
arguments before any other libweston calls.


Thanks,
pq


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