[weston,2/2] input: Update to-be-restored focus when unfocused

Submitted by Quentin Glidic on Aug. 2, 2018, 8:29 a.m.

Details

Message ID 20180802082953.11780-2-sardemff7+wayland@sardemff7.net
State New
Series "libinput: Restore keyboard focus after VT switch"
Headers show

Commit Message

Quentin Glidic Aug. 2, 2018, 8:29 a.m.
From: Quentin Glidic <sardemff7+git@sardemff7.net>

If we start a special (grabbing) client when Weston is unfocused, it
would lose focus when coming back to Weston.

A first attempt to fix this was 85d55540cb64bf97a08b40f79dc66843f8295d3b
but it messed with VT switching.

This fix just updates the saved focus, so when Weston gets focused back,
it will focus the correct client.

Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net>
---

Sorry for the delay, I hoped I could make a Gitlab MR but sadly it
didn’t happen yet. :-)

I think this patch won’t conflict with VT switching, and it does fix the
issue I had initially.

Cheers,

 libweston/input.c | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

Patch hide | download patch | download mbox

diff --git a/libweston/input.c b/libweston/input.c
index f1017dc1b..6a7f584fd 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -1507,6 +1507,17 @@  weston_pointer_set_focus(struct weston_pointer *pointer,
 	wl_signal_emit(&pointer->focus_signal, pointer);
 }
 
+static void
+destroy_device_saved_kbd_focus(struct wl_listener *listener, void *data)
+{
+	struct weston_seat *ws;
+
+	ws = container_of(listener, struct weston_seat,
+			  saved_kbd_focus_listener);
+
+	ws->saved_kbd_focus = NULL;
+}
+
 static void
 send_enter_to_resource_list(struct wl_list *list,
 			    struct weston_keyboard *keyboard,
@@ -1528,7 +1539,8 @@  weston_keyboard_set_focus(struct weston_keyboard *keyboard,
 			  struct weston_surface *surface)
 {
 	struct wl_resource *resource;
-	struct wl_display *display = keyboard->seat->compositor->wl_display;
+	struct weston_seat *seat = keyboard->seat;
+	struct wl_display *display = seat->compositor->wl_display;
 	uint32_t serial;
 	struct wl_list *focus_resource_list;
 
@@ -1540,6 +1552,17 @@  weston_keyboard_set_focus(struct weston_keyboard *keyboard,
 	if (surface && !surface->resource)
 		surface = NULL;
 
+	/* If we have a saved focus, this means Weston itself is unfocused.
+	 * In this case, we just want to update our to-be-restored focus.
+	 */
+	if (seat->saved_kbd_focus != NULL && surface != NULL) {
+		wl_list_remove(&seat->saved_kbd_focus_listener.link);
+		seat->saved_kbd_focus = surface;
+		wl_signal_add(&surface->destroy_signal,
+			      &seat->saved_kbd_focus_listener);
+		return;
+	}
+
 	focus_resource_list = &keyboard->focus_resource_list;
 
 	if (!wl_list_empty(focus_resource_list) && keyboard->focus != surface) {
@@ -2229,17 +2252,6 @@  notify_pointer_focus(struct weston_seat *seat, struct weston_output *output,
 	}
 }
 
-static void
-destroy_device_saved_kbd_focus(struct wl_listener *listener, void *data)
-{
-	struct weston_seat *ws;
-
-	ws = container_of(listener, struct weston_seat,
-			  saved_kbd_focus_listener);
-
-	ws->saved_kbd_focus = NULL;
-}
-
 WL_EXPORT void
 notify_keyboard_focus_in(struct weston_seat *seat, struct wl_array *keys,
 			 enum weston_key_state_update update_state)
@@ -2262,8 +2274,8 @@  notify_keyboard_focus_in(struct weston_seat *seat, struct wl_array *keys,
 
 	if (surface) {
 		wl_list_remove(&seat->saved_kbd_focus_listener.link);
-		weston_keyboard_set_focus(keyboard, surface);
 		seat->saved_kbd_focus = NULL;
+		weston_keyboard_set_focus(keyboard, surface);
 	}
 }
 

Comments

Quentin Glidic Aug. 2, 2018, 8:32 a.m.
On 8/2/18 10:29 AM, Quentin Glidic wrote:
> From: Quentin Glidic <sardemff7+git@sardemff7.net>
> 
> If we start a special (grabbing) client when Weston is unfocused, it
> would lose focus when coming back to Weston.
> 
> A first attempt to fix this was 85d55540cb64bf97a08b40f79dc66843f8295d3b
> but it messed with VT switching.
> 
> This fix just updates the saved focus, so when Weston gets focused back,
> it will focus the correct client.
> 
> Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net>
> ---
> 
> Sorry for the delay, I hoped I could make a Gitlab MR but sadly it
> didn’t happen yet. :-)
> 
> I think this patch won’t conflict with VT switching, and it does fix the
> issue I had initially.
> 
> Cheers,

Forgot to CC everyone.

>   libweston/input.c | 38 +++++++++++++++++++++++++-------------
>   1 file changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/libweston/input.c b/libweston/input.c
> index f1017dc1b..6a7f584fd 100644
> --- a/libweston/input.c
> +++ b/libweston/input.c
> @@ -1507,6 +1507,17 @@ weston_pointer_set_focus(struct weston_pointer *pointer,
>   	wl_signal_emit(&pointer->focus_signal, pointer);
>   }
>   
> +static void
> +destroy_device_saved_kbd_focus(struct wl_listener *listener, void *data)
> +{
> +	struct weston_seat *ws;
> +
> +	ws = container_of(listener, struct weston_seat,
> +			  saved_kbd_focus_listener);
> +
> +	ws->saved_kbd_focus = NULL;
> +}
> +
>   static void
>   send_enter_to_resource_list(struct wl_list *list,
>   			    struct weston_keyboard *keyboard,
> @@ -1528,7 +1539,8 @@ weston_keyboard_set_focus(struct weston_keyboard *keyboard,
>   			  struct weston_surface *surface)
>   {
>   	struct wl_resource *resource;
> -	struct wl_display *display = keyboard->seat->compositor->wl_display;
> +	struct weston_seat *seat = keyboard->seat;
> +	struct wl_display *display = seat->compositor->wl_display;
>   	uint32_t serial;
>   	struct wl_list *focus_resource_list;
>   
> @@ -1540,6 +1552,17 @@ weston_keyboard_set_focus(struct weston_keyboard *keyboard,
>   	if (surface && !surface->resource)
>   		surface = NULL;
>   
> +	/* If we have a saved focus, this means Weston itself is unfocused.
> +	 * In this case, we just want to update our to-be-restored focus.
> +	 */
> +	if (seat->saved_kbd_focus != NULL && surface != NULL) {
> +		wl_list_remove(&seat->saved_kbd_focus_listener.link);
> +		seat->saved_kbd_focus = surface;
> +		wl_signal_add(&surface->destroy_signal,
> +			      &seat->saved_kbd_focus_listener);
> +		return;
> +	}
> +
>   	focus_resource_list = &keyboard->focus_resource_list;
>   
>   	if (!wl_list_empty(focus_resource_list) && keyboard->focus != surface) {
> @@ -2229,17 +2252,6 @@ notify_pointer_focus(struct weston_seat *seat, struct weston_output *output,
>   	}
>   }
>   
> -static void
> -destroy_device_saved_kbd_focus(struct wl_listener *listener, void *data)
> -{
> -	struct weston_seat *ws;
> -
> -	ws = container_of(listener, struct weston_seat,
> -			  saved_kbd_focus_listener);
> -
> -	ws->saved_kbd_focus = NULL;
> -}
> -
>   WL_EXPORT void
>   notify_keyboard_focus_in(struct weston_seat *seat, struct wl_array *keys,
>   			 enum weston_key_state_update update_state)
> @@ -2262,8 +2274,8 @@ notify_keyboard_focus_in(struct weston_seat *seat, struct wl_array *keys,
>   
>   	if (surface) {
>   		wl_list_remove(&seat->saved_kbd_focus_listener.link);
> -		weston_keyboard_set_focus(keyboard, surface);
>   		seat->saved_kbd_focus = NULL;
> +		weston_keyboard_set_focus(keyboard, surface);
>   	}
>   }
>   
>
Peter Hutterer Aug. 3, 2018, 4:15 a.m.
On Thu, Aug 02, 2018 at 10:29:53AM +0200, Quentin Glidic wrote:
> From: Quentin Glidic <sardemff7+git@sardemff7.net>
> 
> If we start a special (grabbing) client when Weston is unfocused, it
> would lose focus when coming back to Weston.
> 
> A first attempt to fix this was 85d55540cb64bf97a08b40f79dc66843f8295d3b
> but it messed with VT switching.
> 
> This fix just updates the saved focus, so when Weston gets focused back,
> it will focus the correct client.
> 
> Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net>
> ---
> 
> Sorry for the delay, I hoped I could make a Gitlab MR but sadly it
> didn’t happen yet. :-)
> 
> I think this patch won’t conflict with VT switching, and it does fix the
> issue I had initially.

this seems like it should do the thing, thanks.

Acked-by: Peter Hutterer <peter.hutterer@who-t.net>

Cheers,
   Peter

>  libweston/input.c | 38 +++++++++++++++++++++++++-------------
>  1 file changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/libweston/input.c b/libweston/input.c
> index f1017dc1b..6a7f584fd 100644
> --- a/libweston/input.c
> +++ b/libweston/input.c
> @@ -1507,6 +1507,17 @@ weston_pointer_set_focus(struct weston_pointer *pointer,
>  	wl_signal_emit(&pointer->focus_signal, pointer);
>  }
>  
> +static void
> +destroy_device_saved_kbd_focus(struct wl_listener *listener, void *data)
> +{
> +	struct weston_seat *ws;
> +
> +	ws = container_of(listener, struct weston_seat,
> +			  saved_kbd_focus_listener);
> +
> +	ws->saved_kbd_focus = NULL;
> +}
> +
>  static void
>  send_enter_to_resource_list(struct wl_list *list,
>  			    struct weston_keyboard *keyboard,
> @@ -1528,7 +1539,8 @@ weston_keyboard_set_focus(struct weston_keyboard *keyboard,
>  			  struct weston_surface *surface)
>  {
>  	struct wl_resource *resource;
> -	struct wl_display *display = keyboard->seat->compositor->wl_display;
> +	struct weston_seat *seat = keyboard->seat;
> +	struct wl_display *display = seat->compositor->wl_display;
>  	uint32_t serial;
>  	struct wl_list *focus_resource_list;
>  
> @@ -1540,6 +1552,17 @@ weston_keyboard_set_focus(struct weston_keyboard *keyboard,
>  	if (surface && !surface->resource)
>  		surface = NULL;
>  
> +	/* If we have a saved focus, this means Weston itself is unfocused.
> +	 * In this case, we just want to update our to-be-restored focus.
> +	 */
> +	if (seat->saved_kbd_focus != NULL && surface != NULL) {
> +		wl_list_remove(&seat->saved_kbd_focus_listener.link);
> +		seat->saved_kbd_focus = surface;
> +		wl_signal_add(&surface->destroy_signal,
> +			      &seat->saved_kbd_focus_listener);
> +		return;
> +	}
> +
>  	focus_resource_list = &keyboard->focus_resource_list;
>  
>  	if (!wl_list_empty(focus_resource_list) && keyboard->focus != surface) {
> @@ -2229,17 +2252,6 @@ notify_pointer_focus(struct weston_seat *seat, struct weston_output *output,
>  	}
>  }
>  
> -static void
> -destroy_device_saved_kbd_focus(struct wl_listener *listener, void *data)
> -{
> -	struct weston_seat *ws;
> -
> -	ws = container_of(listener, struct weston_seat,
> -			  saved_kbd_focus_listener);
> -
> -	ws->saved_kbd_focus = NULL;
> -}
> -
>  WL_EXPORT void
>  notify_keyboard_focus_in(struct weston_seat *seat, struct wl_array *keys,
>  			 enum weston_key_state_update update_state)
> @@ -2262,8 +2274,8 @@ notify_keyboard_focus_in(struct weston_seat *seat, struct wl_array *keys,
>  
>  	if (surface) {
>  		wl_list_remove(&seat->saved_kbd_focus_listener.link);
> -		weston_keyboard_set_focus(keyboard, surface);
>  		seat->saved_kbd_focus = NULL;
> +		weston_keyboard_set_focus(keyboard, surface);
>  	}
>  }
>  
> -- 
> 2.18.0
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
Derek Foreman Aug. 10, 2018, 5:55 p.m.
On 2018-08-02 03:32 AM, Quentin Glidic wrote:
> On 8/2/18 10:29 AM, Quentin Glidic wrote:
>> From: Quentin Glidic <sardemff7+git@sardemff7.net>
>>
>> If we start a special (grabbing) client when Weston is unfocused, it
>> would lose focus when coming back to Weston.
>>
>> A first attempt to fix this was 85d55540cb64bf97a08b40f79dc66843f8295d3b
>> but it messed with VT switching.
>>
>> This fix just updates the saved focus, so when Weston gets focused back,
>> it will focus the correct client.
>>
>> Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net>
>> ---
>>
>> Sorry for the delay, I hoped I could make a Gitlab MR but sadly it
>> didn’t happen yet. :-)
>>
>> I think this patch won’t conflict with VT switching, and it does fix the
>> issue I had initially.

I'm a bit confused as to where we're at with this.

How did the reverted patch "mess with" or "conflict with" VT switching?

Is it intended that these two patches be applied, and then Jamey's patch
(marked as "superseded" in patchwork) be applied on top to resolve the
loss of focus on VT switch away/back?

Thought this might be important to land before the release, but it's not
terribly clear what it actually fixes.  I'd assumed it was the VT switch
thing, but that remains unresolved.

Help? :)

Thanks,
Derek

>> Cheers,
> 
> Forgot to CC everyone.
> 
>>   libweston/input.c | 38 +++++++++++++++++++++++++-------------
>>   1 file changed, 25 insertions(+), 13 deletions(-)
>>
>> diff --git a/libweston/input.c b/libweston/input.c
>> index f1017dc1b..6a7f584fd 100644
>> --- a/libweston/input.c
>> +++ b/libweston/input.c
>> @@ -1507,6 +1507,17 @@ weston_pointer_set_focus(struct weston_pointer
>> *pointer,
>>       wl_signal_emit(&pointer->focus_signal, pointer);
>>   }
>>   +static void
>> +destroy_device_saved_kbd_focus(struct wl_listener *listener, void *data)
>> +{
>> +    struct weston_seat *ws;
>> +
>> +    ws = container_of(listener, struct weston_seat,
>> +              saved_kbd_focus_listener);
>> +
>> +    ws->saved_kbd_focus = NULL;
>> +}
>> +
>>   static void
>>   send_enter_to_resource_list(struct wl_list *list,
>>                   struct weston_keyboard *keyboard,
>> @@ -1528,7 +1539,8 @@ weston_keyboard_set_focus(struct weston_keyboard
>> *keyboard,
>>                 struct weston_surface *surface)
>>   {
>>       struct wl_resource *resource;
>> -    struct wl_display *display = keyboard->seat->compositor->wl_display;
>> +    struct weston_seat *seat = keyboard->seat;
>> +    struct wl_display *display = seat->compositor->wl_display;
>>       uint32_t serial;
>>       struct wl_list *focus_resource_list;
>>   @@ -1540,6 +1552,17 @@ weston_keyboard_set_focus(struct
>> weston_keyboard *keyboard,
>>       if (surface && !surface->resource)
>>           surface = NULL;
>>   +    /* If we have a saved focus, this means Weston itself is
>> unfocused.
>> +     * In this case, we just want to update our to-be-restored focus.
>> +     */
>> +    if (seat->saved_kbd_focus != NULL && surface != NULL) {
>> +        wl_list_remove(&seat->saved_kbd_focus_listener.link);
>> +        seat->saved_kbd_focus = surface;
>> +        wl_signal_add(&surface->destroy_signal,
>> +                  &seat->saved_kbd_focus_listener);
>> +        return;
>> +    }
>> +
>>       focus_resource_list = &keyboard->focus_resource_list;
>>         if (!wl_list_empty(focus_resource_list) && keyboard->focus !=
>> surface) {
>> @@ -2229,17 +2252,6 @@ notify_pointer_focus(struct weston_seat *seat,
>> struct weston_output *output,
>>       }
>>   }
>>   -static void
>> -destroy_device_saved_kbd_focus(struct wl_listener *listener, void *data)
>> -{
>> -    struct weston_seat *ws;
>> -
>> -    ws = container_of(listener, struct weston_seat,
>> -              saved_kbd_focus_listener);
>> -
>> -    ws->saved_kbd_focus = NULL;
>> -}
>> -
>>   WL_EXPORT void
>>   notify_keyboard_focus_in(struct weston_seat *seat, struct wl_array
>> *keys,
>>                enum weston_key_state_update update_state)
>> @@ -2262,8 +2274,8 @@ notify_keyboard_focus_in(struct weston_seat
>> *seat, struct wl_array *keys,
>>         if (surface) {
>>           wl_list_remove(&seat->saved_kbd_focus_listener.link);
>> -        weston_keyboard_set_focus(keyboard, surface);
>>           seat->saved_kbd_focus = NULL;
>> +        weston_keyboard_set_focus(keyboard, surface);
>>       }
>>   }
>>  
> 
>
Peter Hutterer Aug. 16, 2018, 3:24 a.m.
On Fri, Aug 10, 2018 at 12:55:42PM -0500, Derek Foreman wrote:
> On 2018-08-02 03:32 AM, Quentin Glidic wrote:
> > On 8/2/18 10:29 AM, Quentin Glidic wrote:
> >> From: Quentin Glidic <sardemff7+git@sardemff7.net>
> >>
> >> If we start a special (grabbing) client when Weston is unfocused, it
> >> would lose focus when coming back to Weston.
> >>
> >> A first attempt to fix this was 85d55540cb64bf97a08b40f79dc66843f8295d3b
> >> but it messed with VT switching.
> >>
> >> This fix just updates the saved focus, so when Weston gets focused back,
> >> it will focus the correct client.
> >>
> >> Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net>
> >> ---
> >>
> >> Sorry for the delay, I hoped I could make a Gitlab MR but sadly it
> >> didn’t happen yet. :-)
> >>
> >> I think this patch won’t conflict with VT switching, and it does fix the
> >> issue I had initially.
> 
> I'm a bit confused as to where we're at with this.
> 
> How did the reverted patch "mess with" or "conflict with" VT switching?

it ended up always setting the keyboard focus to NULL on VT switch (due to
how libinput devices are handled), so on vt switch back you had no focus.
 
> Is it intended that these two patches be applied, and then Jamey's patch
> (marked as "superseded" in patchwork) be applied on top to resolve the
> loss of focus on VT switch away/back?

AIUI, these two need supersede Jamey's patchl but I'm not 100% sure on that,
sorry.

Cheers,
   Peter

> 
> Thought this might be important to land before the release, but it's not
> terribly clear what it actually fixes.  I'd assumed it was the VT switch
> thing, but that remains unresolved.
> 
> Help? :)
> 
> Thanks,
> Derek
> 
> >> Cheers,
> > 
> > Forgot to CC everyone.
> > 
> >>   libweston/input.c | 38 +++++++++++++++++++++++++-------------
> >>   1 file changed, 25 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/libweston/input.c b/libweston/input.c
> >> index f1017dc1b..6a7f584fd 100644
> >> --- a/libweston/input.c
> >> +++ b/libweston/input.c
> >> @@ -1507,6 +1507,17 @@ weston_pointer_set_focus(struct weston_pointer
> >> *pointer,
> >>       wl_signal_emit(&pointer->focus_signal, pointer);
> >>   }
> >>   +static void
> >> +destroy_device_saved_kbd_focus(struct wl_listener *listener, void *data)
> >> +{
> >> +    struct weston_seat *ws;
> >> +
> >> +    ws = container_of(listener, struct weston_seat,
> >> +              saved_kbd_focus_listener);
> >> +
> >> +    ws->saved_kbd_focus = NULL;
> >> +}
> >> +
> >>   static void
> >>   send_enter_to_resource_list(struct wl_list *list,
> >>                   struct weston_keyboard *keyboard,
> >> @@ -1528,7 +1539,8 @@ weston_keyboard_set_focus(struct weston_keyboard
> >> *keyboard,
> >>                 struct weston_surface *surface)
> >>   {
> >>       struct wl_resource *resource;
> >> -    struct wl_display *display = keyboard->seat->compositor->wl_display;
> >> +    struct weston_seat *seat = keyboard->seat;
> >> +    struct wl_display *display = seat->compositor->wl_display;
> >>       uint32_t serial;
> >>       struct wl_list *focus_resource_list;
> >>   @@ -1540,6 +1552,17 @@ weston_keyboard_set_focus(struct
> >> weston_keyboard *keyboard,
> >>       if (surface && !surface->resource)
> >>           surface = NULL;
> >>   +    /* If we have a saved focus, this means Weston itself is
> >> unfocused.
> >> +     * In this case, we just want to update our to-be-restored focus.
> >> +     */
> >> +    if (seat->saved_kbd_focus != NULL && surface != NULL) {
> >> +        wl_list_remove(&seat->saved_kbd_focus_listener.link);
> >> +        seat->saved_kbd_focus = surface;
> >> +        wl_signal_add(&surface->destroy_signal,
> >> +                  &seat->saved_kbd_focus_listener);
> >> +        return;
> >> +    }
> >> +
> >>       focus_resource_list = &keyboard->focus_resource_list;
> >>         if (!wl_list_empty(focus_resource_list) && keyboard->focus !=
> >> surface) {
> >> @@ -2229,17 +2252,6 @@ notify_pointer_focus(struct weston_seat *seat,
> >> struct weston_output *output,
> >>       }
> >>   }
> >>   -static void
> >> -destroy_device_saved_kbd_focus(struct wl_listener *listener, void *data)
> >> -{
> >> -    struct weston_seat *ws;
> >> -
> >> -    ws = container_of(listener, struct weston_seat,
> >> -              saved_kbd_focus_listener);
> >> -
> >> -    ws->saved_kbd_focus = NULL;
> >> -}
> >> -
> >>   WL_EXPORT void
> >>   notify_keyboard_focus_in(struct weston_seat *seat, struct wl_array
> >> *keys,
> >>                enum weston_key_state_update update_state)
> >> @@ -2262,8 +2274,8 @@ notify_keyboard_focus_in(struct weston_seat
> >> *seat, struct wl_array *keys,
> >>         if (surface) {
> >>           wl_list_remove(&seat->saved_kbd_focus_listener.link);
> >> -        weston_keyboard_set_focus(keyboard, surface);
> >>           seat->saved_kbd_focus = NULL;
> >> +        weston_keyboard_set_focus(keyboard, surface);
> >>       }
> >>   }
> >>  
> > 
> > 
>
Quentin Glidic Aug. 16, 2018, 7:33 a.m.
On 8/16/18 5:24 AM, Peter Hutterer wrote:
> On Fri, Aug 10, 2018 at 12:55:42PM -0500, Derek Foreman wrote:
>> On 2018-08-02 03:32 AM, Quentin Glidic wrote:
>>> On 8/2/18 10:29 AM, Quentin Glidic wrote:
>>>> From: Quentin Glidic <sardemff7+git@sardemff7.net>
>>>>
>>>> If we start a special (grabbing) client when Weston is unfocused, it
>>>> would lose focus when coming back to Weston.
>>>>
>>>> A first attempt to fix this was 85d55540cb64bf97a08b40f79dc66843f8295d3b
>>>> but it messed with VT switching.
>>>>
>>>> This fix just updates the saved focus, so when Weston gets focused back,
>>>> it will focus the correct client.
>>>>
>>>> Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net>
>>>> ---
>>>>
>>>> Sorry for the delay, I hoped I could make a Gitlab MR but sadly it
>>>> didn’t happen yet. :-)
>>>>
>>>> I think this patch won’t conflict with VT switching, and it does fix the
>>>> issue I had initially.
>>
>> I'm a bit confused as to where we're at with this.
>>
>> How did the reverted patch "mess with" or "conflict with" VT switching?
> 
> it ended up always setting the keyboard focus to NULL on VT switch (due to
> how libinput devices are handled), so on vt switch back you had no focus.
>   
>> Is it intended that these two patches be applied, and then Jamey's patch
>> (marked as "superseded" in patchwork) be applied on top to resolve the
>> loss of focus on VT switch away/back?
> 
> AIUI, these two need supersede Jamey's patchl but I'm not 100% sure on that,
> sorry.
> 
> Cheers,
>     Peter
> 
>>
>> Thought this might be important to land before the release, but it's not
>> terribly clear what it actually fixes.  I'd assumed it was the VT switch
>> thing, but that remains unresolved.
>>
>> Help? :)
Sorry for the confusion. This (second) patch is a cleaner fix of the 
issue that was “fixed” by the reverted commit. Then on top of it, you’ll 
have to apply Jamey’s patch, which is an independent issue+fix (which 
the old fix conflicted with). I’m not sure why it was marked 
superseeded, maybe Patchwork detecting my patch as a reply?
Derek Foreman Aug. 24, 2018, 2:11 p.m.
On 2018-08-16 02:33 AM, Quentin Glidic wrote:
> On 8/16/18 5:24 AM, Peter Hutterer wrote:
>> On Fri, Aug 10, 2018 at 12:55:42PM -0500, Derek Foreman wrote:
>>> On 2018-08-02 03:32 AM, Quentin Glidic wrote:
>>>> On 8/2/18 10:29 AM, Quentin Glidic wrote:
>>>>> From: Quentin Glidic <sardemff7+git@sardemff7.net>
>>>>>
>>>>> If we start a special (grabbing) client when Weston is unfocused, it
>>>>> would lose focus when coming back to Weston.
>>>>>
>>>>> A first attempt to fix this was
>>>>> 85d55540cb64bf97a08b40f79dc66843f8295d3b
>>>>> but it messed with VT switching.
>>>>>
>>>>> This fix just updates the saved focus, so when Weston gets focused
>>>>> back,
>>>>> it will focus the correct client.
>>>>>
>>>>> Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net>
>>>>> ---
>>>>>
>>>>> Sorry for the delay, I hoped I could make a Gitlab MR but sadly it
>>>>> didn’t happen yet. :-)
>>>>>
>>>>> I think this patch won’t conflict with VT switching, and it does
>>>>> fix the
>>>>> issue I had initially.
>>>
>>> I'm a bit confused as to where we're at with this.
>>>
>>> How did the reverted patch "mess with" or "conflict with" VT switching?
>>
>> it ended up always setting the keyboard focus to NULL on VT switch
>> (due to
>> how libinput devices are handled), so on vt switch back you had no focus.
>>  
>>> Is it intended that these two patches be applied, and then Jamey's patch
>>> (marked as "superseded" in patchwork) be applied on top to resolve the
>>> loss of focus on VT switch away/back?
>>
>> AIUI, these two need supersede Jamey's patchl but I'm not 100% sure on
>> that,
>> sorry.
>>
>> Cheers,
>>     Peter
>>
>>>
>>> Thought this might be important to land before the release, but it's not
>>> terribly clear what it actually fixes.  I'd assumed it was the VT switch
>>> thing, but that remains unresolved.
>>>
>>> Help? :)
> Sorry for the confusion. This (second) patch is a cleaner fix of the
> issue that was “fixed” by the reverted commit. Then on top of it, you’ll
> have to apply Jamey’s patch, which is an independent issue+fix (which
> the old fix conflicted with). I’m not sure why it was marked
> superseeded, maybe Patchwork detecting my patch as a reply?
> 

Thanks guys.  Due to hilariously misconfigured inbox filters I didn't
catch these replies until today.  Sorry.

I think the VT switch problem has been around for at least 1 release
now, possibly a few more, so I think it's ok to release with the long
standing (mostly cosmetic) bug, and deal with these fixes shortly after.

Thanks again,
Derek
Jamey Sharp Aug. 24, 2018, 5:23 p.m.
For what it's worth, I'm happy to use backported patches. I just hope this
gets addressed upstream eventually.

It's a little more than just cosmetic if you have a graphical application
that can be driven purely by keyboard, and sometimes you don't have a
working pointer input device so you can't get focus back after a VT switch.
I grant that's a somewhat niche use case, but it's the one I'm dealing
with... :-)

I was confused about the state of these patches too, because I didn't see
the original mails. Hopefully next week I can test the combination of
Quentin's revert+fix pair with my patch and make sure it passes the tests I
set up.

On that note, I would offer my test framework upstream, except I set up an
entire qemu image using NixOS to test this, and that seems a little
heavyweight. I can't think of an easier way to test drm-backend stuff
though...

Jamey

On Fri, Aug 24, 2018 at 7:11 AM, Derek Foreman <
derek.foreman.samsung@gmail.com> wrote:

> On 2018-08-16 02:33 AM, Quentin Glidic wrote:
> > On 8/16/18 5:24 AM, Peter Hutterer wrote:
> >> On Fri, Aug 10, 2018 at 12:55:42PM -0500, Derek Foreman wrote:
> >>> On 2018-08-02 03:32 AM, Quentin Glidic wrote:
> >>>> On 8/2/18 10:29 AM, Quentin Glidic wrote:
> >>>>> From: Quentin Glidic <sardemff7+git@sardemff7.net>
> >>>>>
> >>>>> If we start a special (grabbing) client when Weston is unfocused, it
> >>>>> would lose focus when coming back to Weston.
> >>>>>
> >>>>> A first attempt to fix this was
> >>>>> 85d55540cb64bf97a08b40f79dc66843f8295d3b
> >>>>> but it messed with VT switching.
> >>>>>
> >>>>> This fix just updates the saved focus, so when Weston gets focused
> >>>>> back,
> >>>>> it will focus the correct client.
> >>>>>
> >>>>> Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net>
> >>>>> ---
> >>>>>
> >>>>> Sorry for the delay, I hoped I could make a Gitlab MR but sadly it
> >>>>> didn’t happen yet. :-)
> >>>>>
> >>>>> I think this patch won’t conflict with VT switching, and it does
> >>>>> fix the
> >>>>> issue I had initially.
> >>>
> >>> I'm a bit confused as to where we're at with this.
> >>>
> >>> How did the reverted patch "mess with" or "conflict with" VT switching?
> >>
> >> it ended up always setting the keyboard focus to NULL on VT switch
> >> (due to
> >> how libinput devices are handled), so on vt switch back you had no
> focus.
> >>
> >>> Is it intended that these two patches be applied, and then Jamey's
> patch
> >>> (marked as "superseded" in patchwork) be applied on top to resolve the
> >>> loss of focus on VT switch away/back?
> >>
> >> AIUI, these two need supersede Jamey's patchl but I'm not 100% sure on
> >> that,
> >> sorry.
> >>
> >> Cheers,
> >>     Peter
> >>
> >>>
> >>> Thought this might be important to land before the release, but it's
> not
> >>> terribly clear what it actually fixes.  I'd assumed it was the VT
> switch
> >>> thing, but that remains unresolved.
> >>>
> >>> Help? :)
> > Sorry for the confusion. This (second) patch is a cleaner fix of the
> > issue that was “fixed” by the reverted commit. Then on top of it, you’ll
> > have to apply Jamey’s patch, which is an independent issue+fix (which
> > the old fix conflicted with). I’m not sure why it was marked
> > superseeded, maybe Patchwork detecting my patch as a reply?
> >
>
> Thanks guys.  Due to hilariously misconfigured inbox filters I didn't
> catch these replies until today.  Sorry.
>
> I think the VT switch problem has been around for at least 1 release
> now, possibly a few more, so I think it's ok to release with the long
> standing (mostly cosmetic) bug, and deal with these fixes shortly after.
>
> Thanks again,
> Derek
>
Derek Foreman Aug. 24, 2018, 5:52 p.m.
On 2018-08-24 12:23 PM, Jamey Sharp wrote:
> For what it's worth, I'm happy to use backported patches. I just hope
> this gets addressed upstream eventually.
> 
> It's a little more than just cosmetic if you have a graphical
> application that can be driven purely by keyboard, and sometimes you
> don't have a working pointer input device so you can't get focus back
> after a VT switch. I grant that's a somewhat niche use case, but it's
> the one I'm dealing with... :-)

Sorry if it seemed I was dismissing this work entirely.  This bug has
been annoying me for quite some time and I hadn't looked into it at all
myself. For my use case, I can use the keyboard focus switch shortcut to
focus something, so it hasn't been a showstopper.

I think we're going to do a weston 5.0.1 in a month or so, and this will
be among the fixes it contains. :)

> I was confused about the state of these patches too, because I didn't
> see the original mails. Hopefully next week I can test the combination
> of Quentin's revert+fix pair with my patch and make sure it passes the
> tests I set up.

That would be great!

> On that note, I would offer my test framework upstream, except I set up
> an entire qemu image using NixOS to test this, and that seems a little
> heavyweight. I can't think of an easier way to test drm-backend stuff
> though...

Would still be interesting to take a look at, I think.

Thanks,
Derek

> Jamey
> 
> On Fri, Aug 24, 2018 at 7:11 AM, Derek Foreman
> <derek.foreman.samsung@gmail.com
> <mailto:derek.foreman.samsung@gmail.com>> wrote:
> 
>     On 2018-08-16 02:33 AM, Quentin Glidic wrote:
>     > On 8/16/18 5:24 AM, Peter Hutterer wrote:
>     >> On Fri, Aug 10, 2018 at 12:55:42PM -0500, Derek Foreman wrote:
>     >>> On 2018-08-02 03:32 AM, Quentin Glidic wrote:
>     >>>> On 8/2/18 10:29 AM, Quentin Glidic wrote:
>     >>>>> From: Quentin Glidic <sardemff7+git@sardemff7.net
>     <mailto:sardemff7%2Bgit@sardemff7.net>>
>     >>>>>
>     >>>>> If we start a special (grabbing) client when Weston is
>     unfocused, it
>     >>>>> would lose focus when coming back to Weston.
>     >>>>>
>     >>>>> A first attempt to fix this was
>     >>>>> 85d55540cb64bf97a08b40f79dc66843f8295d3b
>     >>>>> but it messed with VT switching.
>     >>>>>
>     >>>>> This fix just updates the saved focus, so when Weston gets focused
>     >>>>> back,
>     >>>>> it will focus the correct client.
>     >>>>>
>     >>>>> Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net
>     <mailto:sardemff7%2Bgit@sardemff7.net>>
>     >>>>> ---
>     >>>>>
>     >>>>> Sorry for the delay, I hoped I could make a Gitlab MR but sadly it
>     >>>>> didn’t happen yet. :-)
>     >>>>>
>     >>>>> I think this patch won’t conflict with VT switching, and it does
>     >>>>> fix the
>     >>>>> issue I had initially.
>     >>>
>     >>> I'm a bit confused as to where we're at with this.
>     >>>
>     >>> How did the reverted patch "mess with" or "conflict with" VT
>     switching?
>     >>
>     >> it ended up always setting the keyboard focus to NULL on VT switch
>     >> (due to
>     >> how libinput devices are handled), so on vt switch back you had
>     no focus.
>     >>  
>     >>> Is it intended that these two patches be applied, and then
>     Jamey's patch
>     >>> (marked as "superseded" in patchwork) be applied on top to
>     resolve the
>     >>> loss of focus on VT switch away/back?
>     >>
>     >> AIUI, these two need supersede Jamey's patchl but I'm not 100%
>     sure on
>     >> that,
>     >> sorry.
>     >>
>     >> Cheers,
>     >>     Peter
>     >>
>     >>>
>     >>> Thought this might be important to land before the release, but
>     it's not
>     >>> terribly clear what it actually fixes.  I'd assumed it was the
>     VT switch
>     >>> thing, but that remains unresolved.
>     >>>
>     >>> Help? :)
>     > Sorry for the confusion. This (second) patch is a cleaner fix of the
>     > issue that was “fixed” by the reverted commit. Then on top of it,
>     you’ll
>     > have to apply Jamey’s patch, which is an independent issue+fix (which
>     > the old fix conflicted with). I’m not sure why it was marked
>     > superseeded, maybe Patchwork detecting my patch as a reply?
>     >
> 
>     Thanks guys.  Due to hilariously misconfigured inbox filters I didn't
>     catch these replies until today.  Sorry.
> 
>     I think the VT switch problem has been around for at least 1 release
>     now, possibly a few more, so I think it's ok to release with the long
>     standing (mostly cosmetic) bug, and deal with these fixes shortly after.
> 
>     Thanks again,
>     Derek
> 
>
Jamey Sharp Sept. 18, 2018, 12:04 a.m.
It took me longer than I hoped to get back to this, but I have now tested
Quentin Glidic's two patches (the revert followed by the revised fix) plus
my "Restore keyboard focus after VT switch", applied to Weston 4. With all
three patches applied, Weston passes my tests. Hooray!

And just to be sure, I verified that applying Quentin's patches without
mine do _not_ pass my tests. So Quentin's patches don't interfere with mine
(good!) but also don't supersede mine, despite how Patchwork apparently got
set.

Thanks everyone for keeping this going and working out the right details!
Jamey

On Fri, Aug 24, 2018 at 10:52 AM Derek Foreman <
derek.foreman.samsung@gmail.com> wrote:

> On 2018-08-24 12:23 PM, Jamey Sharp wrote:
> > For what it's worth, I'm happy to use backported patches. I just hope
> > this gets addressed upstream eventually.
> >
> > It's a little more than just cosmetic if you have a graphical
> > application that can be driven purely by keyboard, and sometimes you
> > don't have a working pointer input device so you can't get focus back
> > after a VT switch. I grant that's a somewhat niche use case, but it's
> > the one I'm dealing with... :-)
>
> Sorry if it seemed I was dismissing this work entirely.  This bug has
> been annoying me for quite some time and I hadn't looked into it at all
> myself. For my use case, I can use the keyboard focus switch shortcut to
> focus something, so it hasn't been a showstopper.
>
> I think we're going to do a weston 5.0.1 in a month or so, and this will
> be among the fixes it contains. :)
>
> > I was confused about the state of these patches too, because I didn't
> > see the original mails. Hopefully next week I can test the combination
> > of Quentin's revert+fix pair with my patch and make sure it passes the
> > tests I set up.
>
> That would be great!
>
> > On that note, I would offer my test framework upstream, except I set up
> > an entire qemu image using NixOS to test this, and that seems a little
> > heavyweight. I can't think of an easier way to test drm-backend stuff
> > though...
>
> Would still be interesting to take a look at, I think.
>
> Thanks,
> Derek
>
> > Jamey
> >
> > On Fri, Aug 24, 2018 at 7:11 AM, Derek Foreman
> > <derek.foreman.samsung@gmail.com
> > <mailto:derek.foreman.samsung@gmail.com>> wrote:
> >
> >     On 2018-08-16 02:33 AM, Quentin Glidic wrote:
> >     > On 8/16/18 5:24 AM, Peter Hutterer wrote:
> >     >> On Fri, Aug 10, 2018 at 12:55:42PM -0500, Derek Foreman wrote:
> >     >>> On 2018-08-02 03:32 AM, Quentin Glidic wrote:
> >     >>>> On 8/2/18 10:29 AM, Quentin Glidic wrote:
> >     >>>>> From: Quentin Glidic <sardemff7+git@sardemff7.net
> >     <mailto:sardemff7%2Bgit@sardemff7.net>>
> >     >>>>>
> >     >>>>> If we start a special (grabbing) client when Weston is
> >     unfocused, it
> >     >>>>> would lose focus when coming back to Weston.
> >     >>>>>
> >     >>>>> A first attempt to fix this was
> >     >>>>> 85d55540cb64bf97a08b40f79dc66843f8295d3b
> >     >>>>> but it messed with VT switching.
> >     >>>>>
> >     >>>>> This fix just updates the saved focus, so when Weston gets
> focused
> >     >>>>> back,
> >     >>>>> it will focus the correct client.
> >     >>>>>
> >     >>>>> Signed-off-by: Quentin Glidic <sardemff7+git@sardemff7.net
> >     <mailto:sardemff7%2Bgit@sardemff7.net>>
> >     >>>>> ---
> >     >>>>>
> >     >>>>> Sorry for the delay, I hoped I could make a Gitlab MR but
> sadly it
> >     >>>>> didn’t happen yet. :-)
> >     >>>>>
> >     >>>>> I think this patch won’t conflict with VT switching, and it
> does
> >     >>>>> fix the
> >     >>>>> issue I had initially.
> >     >>>
> >     >>> I'm a bit confused as to where we're at with this.
> >     >>>
> >     >>> How did the reverted patch "mess with" or "conflict with" VT
> >     switching?
> >     >>
> >     >> it ended up always setting the keyboard focus to NULL on VT switch
> >     >> (due to
> >     >> how libinput devices are handled), so on vt switch back you had
> >     no focus.
> >     >>
> >     >>> Is it intended that these two patches be applied, and then
> >     Jamey's patch
> >     >>> (marked as "superseded" in patchwork) be applied on top to
> >     resolve the
> >     >>> loss of focus on VT switch away/back?
> >     >>
> >     >> AIUI, these two need supersede Jamey's patchl but I'm not 100%
> >     sure on
> >     >> that,
> >     >> sorry.
> >     >>
> >     >> Cheers,
> >     >>     Peter
> >     >>
> >     >>>
> >     >>> Thought this might be important to land before the release, but
> >     it's not
> >     >>> terribly clear what it actually fixes.  I'd assumed it was the
> >     VT switch
> >     >>> thing, but that remains unresolved.
> >     >>>
> >     >>> Help? :)
> >     > Sorry for the confusion. This (second) patch is a cleaner fix of
> the
> >     > issue that was “fixed” by the reverted commit. Then on top of it,
> >     you’ll
> >     > have to apply Jamey’s patch, which is an independent issue+fix
> (which
> >     > the old fix conflicted with). I’m not sure why it was marked
> >     > superseeded, maybe Patchwork detecting my patch as a reply?
> >     >
> >
> >     Thanks guys.  Due to hilariously misconfigured inbox filters I didn't
> >     catch these replies until today.  Sorry.
> >
> >     I think the VT switch problem has been around for at least 1 release
> >     now, possibly a few more, so I think it's ok to release with the long
> >     standing (mostly cosmetic) bug, and deal with these fixes shortly
> after.
> >
> >     Thanks again,
> >     Derek
> >
> >
>
>