[vlc-devel] [PATCH 10/11] display: always set VOUT_DISPLAY_HIDE_MOUSE if needs_hide_mouse is set

[Rémi Denis-Courmont]

Le keskiviikkona 9. elokuuta 2017, 12.43.55 EEST Steve Lhomme a écrit :
> On Tue, Aug 8, 2017 at 5:42 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:
> > Le tiistaina 8. elokuuta 2017, 15.22.30 EEST Steve Lhomme a écrit :
> >> That means the vout will handle the mouse hiding on top of the GUI.
> >> 
> >> Fix #17819
> >> ---
> >> 
> >>  src/video_output/display.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/src/video_output/display.c b/src/video_output/display.c
> >> index 1925e2691d..d85079cf39 100644
> >> --- a/src/video_output/display.c
> >> +++ b/src/video_output/display.c
> >> @@ -756,8 +756,8 @@ bool vout_ManageDisplay(vout_display_t *vd, bool
> >> allow_reset_pictures)
> >> 
> >>      if (hide_mouse_changed) {
> >>      
> >>          msg_Dbg(vd, "auto %sing mouse cursor", hide_mouse?"hid":"show");
> >> 
> >> -        if (vout_HideWindowMouse(osys->vout, hide_mouse) != VLC_SUCCESS
> >> -         && vd->info.needs_hide_mouse)
> >> +        vout_HideWindowMouse(osys->vout, hide_mouse);
> >> +        if (vd->info.needs_hide_mouse)
> >> 
> >>              vout_display_Control(vd, VOUT_DISPLAY_HIDE_MOUSE,
> >>              hide_mouse);
> >>      
> >>      }
> > 
> > This does not make sense. You can´t have both the display and the window
> > controlling the cursor. Normally, the window does all mouse handling. But
> > even

> Except when it doesn't.

That´s abnormal. At this point, that either means the video output does not 
use windows, or does not implement them correctly.

> It seems there are plenty of vout that handle the mouse by themselves.

Then there´s plenty of buggy vouts, I guess.

> At least for Windows there's a reason for that,

Yes. Nobody cared to fix it. In fact, since Laurent vanished, nobody worked on 
the HWND support.

> I can't tell for the others. But there's no way for the GUI to
> show the mouse over another HWND, even a child one.

That is a rather absurd claim. Obviously lots of apps need to do exactly that, 
not just the LibVLC embedding API. Indeed, 3 minutes browsing MSDN confirm 
that it is very much possible with forwarded events.

Also, the EGL display requires the window to handle the input events.

> And therefore if the core wants the mouse hidden/shown it needs to tell the
> vout.

And so it does... The keyboard-events and mouse-events flags have been there 
for that purpose for many years.

> Or we assume they are unrelated and the vout deals with the mouse hiding
> when it wants to. Duplicating the timeout handling in each vout that
> has a cursor handling on top of the GUI.

There would not be such code duplication if the common input events handling 
was done by the window provider. That was the original motivation for moving 
it there on X11, even before it became necessary for EGL and Wayland.

> That's especially true if we're going to remove VOUT_DISPLAY_HIDE_MOUSE.
> Then the cursor shown over the vout Windows will be the sole responsibility
> of the vout.

I doubt that running a timer is that much more code than the HIDE_MOUSE 
boilerplate.

> Is that what we want ?

Removing HIDE_MOUSE, yes.

Handling input handling event in display, I don´t think so.

> > if a windowing system decides to do it the other way, i.e. by the display,
> > it can´t be done by both.
> 
> Given the GUI has no way to tell which vout will be used,

That would be a violation of the encapsulation principles. The window is not 
supposed to know what´s inside it. Already now, it could be an OpenGL 
visualization rather than a video output.

However, it does know which windowing system is used. And thus, it does know 
what are the conventions for embedding between display, window and LibVLC for 
that windowing system.

> there no reason for it to return an error even if what it does has zero
> effect.

> If anything, the core knows the vout will or will not handle the mouse
> and so should decide which one to tell and never both.

It does not know that. The core has no notions of different windowing systems, 
other than each of them is a member of an enumeration.

> That's if we keep VOUT_DISPLAY_HIDE_MOUSE.

> And in that case VOUT_DISPLAY_HIDE_MOUSE
> needs to tell if we want to show or hide the cursor.

I just don´t see the causality there.

An explicit SHOW_MOUSE would be truly only needed if mouse movements were 
handled in a separate component than cursor. At this point, I don´t see any  
reasons to do that.

-- 
雷米‧德尼-库尔蒙
https://www.remlab.net/