<div dir="ltr">Ah ok. So I add a new variable to vout_window_sys_t and save the state there. I follow.<div><br></div><div>On the issue of forcing fullscreen display to specific monitor, the implementation here depends on how the setting is stored. And that discussion in the other thread wasn't really resolved. I'll follow up in the other thread.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 24, 2019 at 10:47 AM Alexandre Janniaux <<a href="mailto:ajanni@videolabs.io">ajanni@videolabs.io</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">Hi, you shouldn't have to care about the vout_window_cfg_t<br>
object, just mirror the current state in the sys of the<br>
vout_window instance.<br>
<br>
At the beginning of WinVoutEventProc, you have:<br>
<br>
```<br>
LONG_PTR p_user_data = GetWindowLongPtr( hwnd, GWLP_USERDATA );<br>
if( p_user_data == 0 )<br>
return DefWindowProc(hwnd, message, wParam, lParam);<br>
vout_window_t *wnd = (vout_window_t *)p_user_data;<br>
```<br>
<br>
Then you can get the sys from wnd, which is the module<br>
private data implementation.<br>
<br>
If you mirror the fullscreen state here, you can also<br>
handle fullscreen state change from<br>
`vout_window_SetFullscreen` and from other WM events, like<br>
you wrote.<br>
<br>
Regards,<br>
--<br>
Alexandre Janniaux<br>
Videolabs<br>
<br>
<br>
On Thu, Oct 24, 2019 at 10:38:10AM -0400, Gabriel Luci wrote:<br>
> Ok, yeah, it's just a problem with my breakpoints. I added a debug message<br>
> on WM_MOVE and I see it when using Win+Shift+Arrows. My bad.<br>
><br>
> The only issue I see is finding out if we are currently in fullscreen mode.<br>
> There is no reference to the vout_window_cfg_t object<br>
> in WinVoutEventProc (where we could just look at is_fullscreen). Can we get<br>
> that object from there? Or will we have to store our own is_fullscreen<br>
> variable and toggle it in SetFullscreen() and UnsetFullscreen()?<br>
><br>
> On Thu, Oct 24, 2019 at 9:40 AM Gabriel Luci <<a href="mailto:github@luci.ca" target="_blank">github@luci.ca</a>> wrote:<br>
><br>
> > Well I just set a breakpoint at the start of WinVoutEventProc and it<br>
> > wasn't hit, but maybe it's not being hit for other reasons. It seems<br>
> > inconsistent. Spy++ does show WM_MOVE and WM_WINDOWPOSCHANGED. I'll add a<br>
> > case to the switch block for WM_MOVE and see what happens.<br>
> ><br>
> > 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>
> ><br>
> >> On 2019-10-24 15:01, Gabriel Luci wrote:<br>
> >> > I think this depends on what value is going to be in whatever setting<br>
> >> 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<br>
> >> 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<br>
> >> 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.<br>
> >> Thats'<br>
> >> > what this patch should use in the end and the logo+shift+arrow<br>
> >> 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<br>
> >> 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<br>
> >> 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<br>
> >> 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<br>
> >> <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<br>
> >> *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<br>
> >> 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<br>
> >> 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><br>
> ><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>
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>