All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: Erik Rull <erik.rull@rdsoftware.de>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] git bisect results
Date: Mon, 30 Jan 2012 15:48:27 +0100	[thread overview]
Message-ID: <4F26ADBB.3060403@siemens.com> (raw)
In-Reply-To: <628842726.26312.1327933056168.JavaMail.open-xchange@email.1und1.de>

On 2012-01-30 15:17, Erik Rull wrote:
> 
> 
> 
> On January 30, 2012 at 2:48 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
> 
>> On 2012-01-30 14:17, Erik Rull wrote:
>>>
>>>
>>>
>>> On January 30, 2012 at 12:52 PM Jan Kiszka <jan.kiszka@siemens.com>
> wrote:
>>>
>>>> On 2012-01-30 12:34, Erik Rull wrote:
>>>>> Hi Jan,
>>>>>
>>>>> I'm sorry, but this does not solve my issue. I applied the patch and
>>>>> crosschecked that the resulting file looks fine.
>>>>>
>>>>> The final function looks like:
>>>>>
>>>>> static void sdl_grab_start(void)
>>>>> {
>>>>> /*
>>>>> * If the application is not active, do not try to enter grab state.
>>> This
>>>>> * prevents 'SDL_WM_GrabInput(SDL_GRAB_ON)' from blocking all the
>>>>> * application (SDL bug).
>>>>> */
>>>>> if (!(SDL_GetAppState() & SDL_APPINPUTFOCUS)) {
>>>>> return;
>>>>> }
>>>>> if (guest_cursor) {
>>>>> SDL_SetCursor(guest_sprite);
>>>>> if (!kbd_mouse_is_absolute() && !absolute_enabled)
>>>>> SDL_WarpMouse(guest_x, guest_y);
>>>>> } else
>>>>> sdl_hide_cursor();
>>>>> SDL_WM_GrabInput(SDL_GRAB_ON);
>>>>> gui_grab = 1;
>>>>> sdl_update_caption();
>>>>> }
>>>>
>>>> That makes no sense as gui_grab must be 1 now. Please retry your
>>>> previous instrumentation.
>>>>
>>>> Thanks,
>>>> Jan
>>>>
>>>
>>> You're right. So I added the instrumentation again.
>>>
>>> Still looks strange.
>>>
>>> So I added into the sdl_grab_start() a printf.
>>> Wow - a lot of output!
>>> This pointed me to all other sdl_grab_start() calls (and in additon to
> that
>>> all sdl_grab_end() calls).
>>>
>>> And here are the results of the qemu voting :-)
>>>
>>> I already assigned a usable name to the printf output that is directly
> one
>>> line above the corresponding sdl_grab_*() call, so you should be able
> to
>>> find this easily in your code as well.
>>>
>>> The huge number of recurring printf's are:
>>>
>>> sdl_grab_start() called from absolute_mouse_grab()
>>> sdl_grab_end() called from handle_activation()
>>> sdl_grab_start() called from absolute_mouse_grab()
>>> sdl_grab_end() called from handle_activation()
>>> sdl_grab_start() called from absolute_mouse_grab()
>>> sdl_grab_end() called from handle_activation()
>>> sdl_grab_start() called from absolute_mouse_grab()
>>> sdl_grab_end() called from handle_activation()
>>> sdl_grab_start() called from absolute_mouse_grab()
>>> sdl_grab_end() called from handle_activation()
>>> sdl_grab_start() called from absolute_mouse_grab()
>>> sdl_grab_end() called from handle_activation()
>>> sdl_grab_start() called from absolute_mouse_grab()
>>> sdl_grab_end() called from handle_activation()
>>>
>>> Any idea how to proceed?
>>>
>>> Maybe the first two if-statements in handle_activation() cause the
> problem?
>>> Because there the two given functions are called in sequence if both
>>> if-clauses are valid one after the other. Maybe the first one sets the
>>> state so that the second if is valid, too. Maybe a simple else if
> solves
>>> the issue?
>>
>> ev->active.gain makes both clauses mutually exclusive - unless someone
>> messes with the memory of the event object.
>>
>>> I'm not familiar with the variables that are checked here, so
>>> it's just a guess.
>>
>> So handle_activation() is called directly after absolute_mouse_grab(),
>> and the reported event contains
>>
>> state = SDL_APPINPUTFOCUS
>> gain = 0
>> (please validate!)
>>
>> That would mean we are constantly losing the input focus again after
>> trying to gain it via SDL_WM_GrabInput. Weird.
>>
>> What's the call chain for absolute_mouse_grab()?
>>
>> Jan
>>
>> --
>> Siemens AG, Corporate Technology, CT T DE IT 1
>> Corporate Competence Center Embedded Linux
> 
> 
> Here the results:
> 
> Handle Activation 0: at function start (before the first if)
> Handle Activation 1: between the first and the second if
> Handle Activation 2: after the second if
> 
> The output is formatted like:
> printf("Handle Activation 0: %d %d %d %d\n",gui_grab,(ev->active.state ==
> SDL_APPINPUTFOCUS),ev->active.gain,gui_fullscreen);
> 
> Handle Activation 0: 0 1 1 0
> Handle Activation 1: 0 1 1 0
> absolute_mouse_grab() called from handle_activation()
> sdl_grab_start() called from absolute_mouse_grab()
> Handle Activation 2: 1 1 1 0
> Handle Activation 0: 1 1 0 0
> sdl_grab_end() called from handle_activation()
> Handle Activation 1: 0 1 0 0
> Handle Activation 2: 0 1 0 0
> Handle Activation 0: 0 1 1 0
> Handle Activation 1: 0 1 1 0
> absolute_mouse_grab() called from handle_activation()
> sdl_grab_start() called from absolute_mouse_grab()
> Handle Activation 2: 1 1 1 0
> Handle Activation 0: 1 1 0 0
> sdl_grab_end() called from handle_activation()
> Handle Activation 1: 0 1 0 0
> Handle Activation 2: 0 1 0 0
> Handle Activation 0: 0 1 1 0
> Handle Activation 1: 0 1 1 0
> absolute_mouse_grab() called from handle_activation()
> sdl_grab_start() called from absolute_mouse_grab()
> Handle Activation 2: 1 1 1 0
> 
> The gain seems to toggle between the successive calls of
> handle_activation...
> Next steps?

Try this:

diff --git a/ui/sdl.c b/ui/sdl.c
index 73e5839..c3fe80d 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -828,10 +828,6 @@ static void handle_mousebutton(DisplayState *ds, SDL_Event *ev)
 
 static void handle_activation(DisplayState *ds, SDL_Event *ev)
 {
-    if (gui_grab && ev->active.state == SDL_APPINPUTFOCUS &&
-        !ev->active.gain && !gui_fullscreen) {
-        sdl_grab_end();
-    }
     if (!gui_grab && ev->active.gain && is_graphic_console() &&
         (kbd_mouse_is_absolute() || absolute_enabled)) {
         absolute_mouse_grab();

I'm wondering what this code is branch should do anyway. If we grabbed
the input we can't unwillingly lose it. But that's just a theory for
now.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

  reply	other threads:[~2012-01-30 14:48 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-30 11:34 [Qemu-devel] git bisect results Erik Rull
2012-01-30 11:52 ` Jan Kiszka
2012-01-30 13:17   ` Erik Rull
2012-01-30 13:48     ` Jan Kiszka
2012-01-30 14:17       ` Erik Rull
2012-01-30 14:48         ` Jan Kiszka [this message]
2012-01-31  8:31           ` Erik Rull
  -- strict thread matches above, loose matches on Subject: below --
2012-01-26 13:10 Erik Rull
2012-01-26 16:24 ` Jan Kiszka
2012-01-27 22:52 ` Jan Kiszka
2012-01-28 11:55   ` Jan Kiszka
2012-01-28 12:39     ` Erik Rull
2012-01-28 12:43       ` Jan Kiszka
2012-01-28 13:01         ` Erik Rull
2012-01-28 14:52           ` Jan Kiszka
2012-01-25 11:48 erik.rull
2012-01-25 14:19 ` Jan Kiszka
2012-01-25 21:13   ` Erik Rull
2012-01-23  8:57 [Qemu-devel] bad USB tablet update rate on qemu-1.0 erik.rull
2012-01-24 17:24 ` [Qemu-devel] git bisect results (was: Re: bad USB tablet update rate on qemu-1.0) Erik Rull
2012-01-24 18:19   ` [Qemu-devel] git bisect results Jan Kiszka
2012-01-24 18:55     ` Erik Rull
2012-01-24 20:15       ` Jan Kiszka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F26ADBB.3060403@siemens.com \
    --to=jan.kiszka@siemens.com \
    --cc=erik.rull@rdsoftware.de \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.