<div dir="ltr">Ok, yeah, it's just a problem with my breakpoints. I added a debug message on WM_MOVE and I see it when using Win+Shift+Arrows. My bad.<div><br></div><div>The only issue I see is finding out if we are currently in fullscreen mode. There is no reference to the vout_window_cfg_t object in WinVoutEventProc (where we could just look at is_fullscreen). Can we get that object from there? Or will we have to store our own is_fullscreen variable and toggle it in SetFullscreen() and UnsetFullscreen()?</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 24, 2019 at 9:40 AM Gabriel Luci <<a href="mailto:github@luci.ca">github@luci.ca</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Well I just set a breakpoint at the start of WinVoutEventProc and it wasn't hit, but maybe it's not being hit for other reasons. It seems inconsistent. Spy++ does show WM_MOVE and WM_WINDOWPOSCHANGED. I'll add a case to the switch block for WM_MOVE and see what happens.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 24, 2019 at 9:11 AM Steve Lhomme <<a href="mailto:robux4@ycbcr.xyz" target="_blank">robux4@ycbcr.xyz</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 2019-10-24 15:01, Gabriel Luci wrote:<br>
> I think this depends on what value is going to be in whatever setting we <br>
> end up deciding to use. If we store the device name (like <br>
> "\\.\DISPLAY2"), then you have to enumerate the displays to figure out <br>
> which one has that name, since there isn't any GetMonitorByName <br>
> function. But once you find it, you already have the MONITORINFOEX <br>
> struct that you need to go fullscreen. That's why I did what I did.<br>
> <br>
> If you want to use the SetFullscreenOnMonitor, then the enumeration <br>
> would have to pass back just the handle to the monitor <br>
> and SetFullscreenOnMonitor would have to use the handle to go find the <br>
> monitor info again.<br>
<br>
I guess you could pass the MONITORINFOEX to SetFullscreenOnMonitor then. <br>
We should read it in the current code as well.<br>
<br>
> To Alexandre's point about switching monitors: This is a bug that exists <br>
> in VLC 3 too. I'm just testing this now in the current v4 code, and I <br>
> don't see any message at all sent to WinVoutEventProc when using <br>
> Win+Shift+Arrow. I would have expected to see WM_MOVE and figured we <br>
> could just check if we're in fullscreen mode and resize to the new <br>
> monitor. But as it is, I don't see VLC being notified at all that the <br>
> window moved. Any idea why?<br>
<br>
This is weird and unexpected. Did you log all the messages received at <br>
the beginning of WinVoutEventProc() ? You may also check with Spy++ (or <br>
Spy32++?)<br>
<br>
> If you move from a larger resolution monitor to a smaller one, you see <br>
> WM_SIZE because Windows resizes the window to the new display, but minus <br>
> the taskbar. But you see no notifications at all if you move from a <br>
> smaller monitor to a larger one.<br>
> <br>
> On Thu, Oct 24, 2019 at 7:32 AM Steve Lhomme <<a href="mailto:robux4@ycbcr.xyz" target="_blank">robux4@ycbcr.xyz</a> <br>
> <mailto:<a href="mailto:robux4@ycbcr.xyz" target="_blank">robux4@ycbcr.xyz</a>>> wrote:<br>
> <br>
> Here's a patch that allows going fullscreen on a given HMONITOR. Thats'<br>
> what this patch should use in the end and the logo+shift+arrow patch.<br>
> <br>
> On 2019-10-24 13:17, Steve Lhomme wrote:<br>
> > On 2019-10-24 13:14, Steve Lhomme wrote:<br>
> >> On 2019-10-24 12:21, Alexandre Janniaux wrote:<br>
> >>> When in fullscreen, the window can be moved to another<br>
> >>> screen by using logo+shift+arrow key. When doing so on<br>
> >>> screens which are not the same size, the window is not<br>
> >>> resized and keep the size from the former screen. It should<br>
> >>> probably keep the resize state somewhere and resize<br>
> >>> adequatly while using vout_window_ReportSize() so that<br>
> >>> video outputs can be resized too.<br>
> >>><br>
> >>> I don't know if you want to add this to your patch too, as<br>
> >>> the behaviour is already present without yours, but maybe<br>
> >>> Windows dev here can provide advice for this?<br>
> >><br>
> >> I should be able to fix it. We probably need to handle the WM_MOVE<br>
> >> and, when in fullscreen, adapt the size of the window if the<br>
> monitor<br>
> >> changed.<br>
> >><br>
> >> It's probably better to apply a fix after this change as it would<br>
> >> impact how we apply the fullscreen based on the selected<br>
> monitor. In<br>
> >> this case it wouldn't come the core anymore but from the local<br>
> >> detection of the monitor. That means the monitor from the core<br>
> should<br>
> >> return just a<br>
> ><br>
> > the monitor *selection* (based on the screen from the core)<br>
> ><br>
> >> HMONITOR, which is always what we'll use when we use the one<br>
> forced by<br>
> >> the OS.<br>
> >><br>
> >>> PS: just for giving context, I used your patch in a VR demo<br>
> >>> so as to force a VR sidebyside output to the HMD screen and<br>
> >>> a classic 360° pano output to the main screen.<br>
> >>><br>
> >>> On Sat, Oct 12, 2019 at 11:58:31PM -0400, Gabriel Luci wrote:<br>
> >>>> This reads the id parameter as the name of the monitor that<br>
> should be<br>
> >>>> used. If anything goes wrong, it will fall back to the monitor the<br>
> >>>> window is already on (which is what it always did before).<br>
> >>>><br>
> >>>> ---<br>
> >>>> modules/video_output/win32/window.c | 72<br>
> >>>> +++++++++++++++++++++++++++++++++----<br>
> >>>> 1 file changed, 65 insertions(+), 7 deletions(-)<br>
> >>>><br>
> >>>> diff --git a/modules/video_output/win32/window.c<br>
> >>>> b/modules/video_output/win32/window.c<br>
> >>>> index 50e172b9b4..eefaed256e 100644<br>
> >>>> --- a/modules/video_output/win32/window.c<br>
> >>>> +++ b/modules/video_output/win32/window.c<br>
> >>>> @@ -37,6 +37,7 @@<br>
> >>>> #include <vlc_actions.h><br>
> >>>><br>
> >>>> #include<br>
> <shellapi.h> /*<br>
> >>>> ExtractIcon */<br>
> >>>> +#include <vlc_charset.h> /*<br>
> >>>> FromWide */<br>
> >>>><br>
> >>>> #define RECTWidth(r) (LONG)((r).right - (r).left)<br>
> >>>> #define RECTHeight(r) (LONG)((r).bottom - (r).top)<br>
> >>>> @@ -153,9 +154,37 @@ static void SetTitle(vout_window_t *wnd,<br>
> const<br>
> >>>> char *title)<br>
> >>>> PostMessage( sys->hwnd, WM_VLC_CHANGE_TEXT, 0, 0 );<br>
> >>>> }<br>
> >>>><br>
> >>>> +typedef struct enum_monitors_callback_params_t {<br>
> >>>> + bool is_monitor_found;<br>
> >>>> + const char *psz_name_to_find;<br>
> >>>> + MONITORINFOEX *found_monitor_info;<br>
> >>>> +} enum_monitors_callback_params_t;<br>
> >>>> +<br>
> >>>> +static BOOL CALLBACK EnumDisplayMonitorsCallback(HMONITOR<br>
> hMonitor,<br>
> >>>> HDC hdcMonitor, LPRECT lprcMonitor, LPARAM dwData)<br>
> >>>> +{<br>
> >>>> + /* this is called for each active monitor */<br>
> >>>> +<br>
> >>>> + VLC_UNUSED(hdcMonitor);<br>
> >>>> + VLC_UNUSED(lprcMonitor);<br>
> >>>> +<br>
> >>>> + MONITORINFOEX mi;<br>
> >>>> + mi.cbSize = sizeof(MONITORINFOEX);<br>
> >>>> + if (!GetMonitorInfo(hMonitor, (LPMONITORINFO) &mi))<br>
> return true;<br>
> >>>> +<br>
> >>>> + enum_monitors_callback_params_t *params =<br>
> >>>> (enum_monitors_callback_params_t*) dwData;<br>
> >>>> + char *psz_monitor_name = FromWide(mi.szDevice);<br>
> >>>> + if (!strcmp(psz_monitor_name, params->psz_name_to_find))<br>
> >>>> + {<br>
> >>>> + /* the monitor name matches what we're looking for */<br>
> >>>> + params->is_monitor_found = true;<br>
> >>>> + params->found_monitor_info = &mi;<br>
> >>>> + }<br>
> >>>> + free(psz_monitor_name);<br>
> >>>> + return !params->is_monitor_found;<br>
> >>>> +}<br>
> >>>> +<br>
> >>>> static void SetFullscreen(vout_window_t *wnd, const char *id)<br>
> >>>> {<br>
> >>>> - VLC_UNUSED(id);<br>
> >>>> msg_Dbg(wnd, "entering fullscreen mode");<br>
> >>>> vout_window_sys_t *sys = wnd->sys;<br>
> >>>><br>
> >>>> @@ -167,12 +196,41 @@ static void SetFullscreen(vout_window_t<br>
> *wnd,<br>
> >>>> const char *id)<br>
> >>>> /* Change window style, no borders and no title bar */<br>
> >>>> SetWindowLong(sys->hwnd, GWL_STYLE, WS_CLIPCHILDREN |<br>
> >>>> WS_VISIBLE);<br>
> >>>><br>
> >>>> - /* Retrieve current window position so fullscreen will happen<br>
> >>>> - * on the right screen */<br>
> >>>> - HMONITOR hmon = MonitorFromWindow(sys->hwnd,<br>
> >>>> MONITOR_DEFAULTTONEAREST);<br>
> >>>> - MONITORINFO mi;<br>
> >>>> - mi.cbSize = sizeof(MONITORINFO);<br>
> >>>> - if (GetMonitorInfo(hmon, &mi))<br>
> >>>> + /* Figure out which monitor we should be using */<br>
> >>>> + MONITORINFOEX mi;<br>
> >>>> + bool is_monitor_found = false;<br>
> >>>> +<br>
> >>>> + if (id)<br>
> >>>> + {<br>
> >>>> + /* We're being told to use a specific monitor, so see<br>
> if we<br>
> >>>> can find an<br>
> >>>> + * active monitor with the same name */<br>
> >>>> + enum_monitors_callback_params_t params = {<br>
> >>>> + .is_monitor_found = false,<br>
> >>>> + .psz_name_to_find = id<br>
> >>>> + };<br>
> >>>> + EnumDisplayMonitors(NULL, NULL,<br>
> >>>> &EnumDisplayMonitorsCallback, (LPARAM) ¶ms);<br>
> >>>> +<br>
> >>>> + if (params.is_monitor_found)<br>
> >>>> + {<br>
> >>>> + is_monitor_found = true;<br>
> >>>> + mi = *(params.found_monitor_info);<br>
> >>>> + }<br>
> >>>> + else<br>
> >>>> + {<br>
> >>>> + msg_Dbg(wnd, "display device with name \"%s\" was<br>
> >>>> requested, but no such device exists. Falling back to<br>
> primary", id);<br>
> >>>> + }<br>
> >>>> + }<br>
> >>>> +<br>
> >>>> + if (!is_monitor_found)<br>
> >>>> + {<br>
> >>>> + /* fall back to using the screen the window is currently<br>
> >>>> mostly on */<br>
> >>>> + HMONITOR hmon;<br>
> >>>> + hmon = MonitorFromWindow(sys->hwnd,<br>
> MONITOR_DEFAULTTONEAREST);<br>
> >>>> + mi.cbSize = sizeof(MONITORINFOEX);<br>
> >>>> + is_monitor_found = GetMonitorInfo(hmon,<br>
> (LPMONITORINFO) &mi);<br>
> >>>> + }<br>
> >>>> +<br>
> >>>> + if (is_monitor_found)<br>
> >>>> SetWindowPos(sys->hwnd, 0,<br>
> >>>> mi.rcMonitor.left,<br>
> >>>> mi.rcMonitor.top,<br>
> >>>> --<br>
> >>>> 2.11.0<br>
> >>>><br>
> >>>> _______________________________________________<br>
> >>>> vlc-devel mailing list<br>
> >>>> To unsubscribe or modify your subscription options:<br>
> >>>> <a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a><br>
> >>> _______________________________________________<br>
> >>> vlc-devel mailing list<br>
> >>> To unsubscribe or modify your subscription options:<br>
> >>> <a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a><br>
> >>><br>
> >> _______________________________________________<br>
> >> vlc-devel mailing list<br>
> >> To unsubscribe or modify your subscription options:<br>
> >> <a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a><br>
> > _______________________________________________<br>
> > vlc-devel mailing list<br>
> > To unsubscribe or modify your subscription options:<br>
> > <a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a><br>
> _______________________________________________<br>
> vlc-devel mailing list<br>
> To unsubscribe or modify your subscription options:<br>
> <a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a><br>
> <br>
> <br>
> _______________________________________________<br>
> vlc-devel mailing list<br>
> To unsubscribe or modify your subscription options:<br>
> <a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a><br>
> <br>
_______________________________________________<br>
vlc-devel mailing list<br>
To unsubscribe or modify your subscription options:<br>
<a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a></blockquote></div>
</blockquote></div>