include/sysemu/sysemu.h | 1 - ui/gtk.c | 8 +++++++- ui/sdl2.c | 28 ++++++++++++++++++++-------- vl.c | 6 ++++-- qapi/ui.json | 3 +++ qemu-deprecated.texi | 5 +++++ ui/cocoa.m | 4 ++++ 7 files changed, 43 insertions(+), 12 deletions(-)
Gerd Hoffmann (6): ui: add show-cursor option ui/gtk: implement show-cursor option ui/sdl: implement show-cursor option ui/cocoa: implement show-cursor option ui: wire up legacy -show-cursor option ui: deprecate legacy -show-cursor option include/sysemu/sysemu.h | 1 - ui/gtk.c | 8 +++++++- ui/sdl2.c | 28 ++++++++++++++++++++-------- vl.c | 6 ++++-- qapi/ui.json | 3 +++ qemu-deprecated.texi | 5 +++++ ui/cocoa.m | 4 ++++ 7 files changed, 43 insertions(+), 12 deletions(-) -- 2.18.1
On Thu, 6 Feb 2020 at 11:29, Gerd Hoffmann <kraxel@redhat.com> wrote: > > This cover letter is missing a description of what the patchset does... The closest thing the patchset seems to get to documentation is the oneliner in ui.json: +# @show-cursor: Force showing the mouse cursor (default: off). but looking at the ui/cocoa.m implementation that isn't what it actually does -- it just seems to mean "default to shown on startup", because the logic that unconditionally hides the host cursor on mousegrab and unhides it on ungrab remains unchanged. This doesn't on the face of it sound like very useful behaviour, since the option will only have an effect for the short period of time between QEMU startup and the first mouse-grab, but without documentation of what the option is intended to do and in particular how it's intended to interact with grab/ungrab I don't know what your intention for the behaviour was. thanks -- PMM
On Thu, Feb 06, 2020 at 11:52:05AM +0000, Peter Maydell wrote:
> On Thu, 6 Feb 2020 at 11:29, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> >
>
> This cover letter is missing a description of what the patchset does...
>
> The closest thing the patchset seems to get to documentation is the
> oneliner in ui.json:
> +# @show-cursor: Force showing the mouse cursor (default: off).
>
> but looking at the ui/cocoa.m implementation that isn't what it
> actually does -- it just seems to mean "default to shown on
> startup", because the logic that unconditionally hides the host
> cursor on mousegrab and unhides it on ungrab remains
> unchanged. This doesn't on the face of it sound like very
> useful behaviour, since the option will only have an effect for
> the short period of time between QEMU startup and the first
> mouse-grab, but without documentation of what the option
> is intended to do and in particular how it's intended to
> interact with grab/ungrab I don't know what your intention
> for the behaviour was.
Well, it doesn't change actual behavior for SDL and cocoa. It only adds
"-display {sdl,cocoa},show-cursor=on" as replacement for the global
"-show-cursor" option. Guess I should reorder the patches (move 5/6
before the individual UI patches) and reword the commit messages.
If you think cocoa behavior isn't useful we can revert commit
13aefd303cf996c2d183e94082413885bf1d15bf instead, or drop the
cursor_hide check in hideCursor() + unhideCursor(). Your call.
It also adds gtk support (based on a patch by jpewhacker@gmail.com).
cheers,
Gerd
On Thu, 6 Feb 2020 at 13:20, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Thu, Feb 06, 2020 at 11:52:05AM +0000, Peter Maydell wrote:
> > On Thu, 6 Feb 2020 at 11:29, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > This cover letter is missing a description of what the patchset does...
> >
> > The closest thing the patchset seems to get to documentation is the
> > oneliner in ui.json:
> > +# @show-cursor: Force showing the mouse cursor (default: off).
> >
> > but looking at the ui/cocoa.m implementation that isn't what it
> > actually does -- it just seems to mean "default to shown on
> > startup", because the logic that unconditionally hides the host
> > cursor on mousegrab and unhides it on ungrab remains
> > unchanged. This doesn't on the face of it sound like very
> > useful behaviour, since the option will only have an effect for
> > the short period of time between QEMU startup and the first
> > mouse-grab, but without documentation of what the option
> > is intended to do and in particular how it's intended to
> > interact with grab/ungrab I don't know what your intention
> > for the behaviour was.
>
> Well, it doesn't change actual behavior for SDL and cocoa. It only adds
> "-display {sdl,cocoa},show-cursor=on" as replacement for the global
> "-show-cursor" option. Guess I should reorder the patches (move 5/6
> before the individual UI patches) and reword the commit messages.
>
> If you think cocoa behavior isn't useful we can revert commit
> 13aefd303cf996c2d183e94082413885bf1d15bf instead, or drop the
> cursor_hide check in hideCursor() + unhideCursor(). Your call.
I think we should start by documenting what the behaviour we
expect of the UI with this option set and unset is. Once we know
what we're trying to achieve, we can look at what we need to do
to the code to get there from here...
Not having the requirements/expectations on the UI frontend
clearly documented makes it a lot harder to keep the behaviour
of our UIs in sync -- we end up making changes to one UI
or another that make sense in isolation but result in random
unintended inconsistencies between them.
thanks
-- PMM
On Thu, Feb 06, 2020 at 02:20:02PM +0100, Gerd Hoffmann wrote:
> On Thu, Feb 06, 2020 at 11:52:05AM +0000, Peter Maydell wrote:
> > On Thu, 6 Feb 2020 at 11:29, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >
> > >
> >
> > This cover letter is missing a description of what the patchset does...
> >
> > The closest thing the patchset seems to get to documentation is the
> > oneliner in ui.json:
> > +# @show-cursor: Force showing the mouse cursor (default: off).
> >
> > but looking at the ui/cocoa.m implementation that isn't what it
> > actually does -- it just seems to mean "default to shown on
> > startup", because the logic that unconditionally hides the host
> > cursor on mousegrab and unhides it on ungrab remains
> > unchanged. This doesn't on the face of it sound like very
> > useful behaviour, since the option will only have an effect for
> > the short period of time between QEMU startup and the first
> > mouse-grab, but without documentation of what the option
> > is intended to do and in particular how it's intended to
> > interact with grab/ungrab I don't know what your intention
> > for the behaviour was.
>
> Well, it doesn't change actual behavior for SDL and cocoa. It only adds
> "-display {sdl,cocoa},show-cursor=on" as replacement for the global
> "-show-cursor" option. Guess I should reorder the patches (move 5/6
> before the individual UI patches) and reword the commit messages.
I suppose this patch set didn't intend to workaround the missing cursor over
VNC with NVIDIA vGPUs, right? IIRC the default is that the local cursor is
sent over to the destination whereas we need to force the remote cursor to be
sent back, do I remember that correctly the setting for which have to advertize
vncviewer?
Erik
© 2016 - 2024 Red Hat, Inc.