[vlc-devel] [PATCH 4/4] win32: Send fullscreen to specific monitor

[Alexandre Janniaux]

Hi,

On Fri, Oct 25, 2019 at 07:18:49PM +0300, Rémi Denis-Courmont wrote:
> Le sunnuntaina 13. lokakuuta 2019, 6.58.31 EEST Gabriel Luci a écrit :
> > This reads the id parameter as the name of the monitor that should be
> > used. If anything goes wrong, it will fall back to the monitor the
> > window is already on (which is what it always did before).
> >
> > ---
> >  modules/video_output/win32/window.c | 72
> > +++++++++++++++++++++++++++++++++---- 1 file changed, 65 insertions(+), 7
> > deletions(-)
> >
> > diff --git a/modules/video_output/win32/window.c
> > b/modules/video_output/win32/window.c index 50e172b9b4..eefaed256e 100644
> > --- a/modules/video_output/win32/window.c
> > +++ b/modules/video_output/win32/window.c
> > @@ -37,6 +37,7 @@
> >  #include <vlc_actions.h>
> >
> >  #include <shellapi.h>                                         /*
> > ExtractIcon */ +#include <vlc_charset.h>
> >   /* FromWide */
> >
> >  #define RECTWidth(r)   (LONG)((r).right - (r).left)
> >  #define RECTHeight(r)  (LONG)((r).bottom - (r).top)
> > @@ -153,9 +154,37 @@ static void SetTitle(vout_window_t *wnd, const char
> > *title) PostMessage( sys->hwnd, WM_VLC_CHANGE_TEXT, 0, 0 );
> >  }
> >
> > +typedef struct enum_monitors_callback_params_t {
> > +    bool is_monitor_found;
> > +    const char *psz_name_to_find;
> > +    MONITORINFOEX *found_monitor_info;
> > +} enum_monitors_callback_params_t;
> > +
> > +static BOOL CALLBACK EnumDisplayMonitorsCallback(HMONITOR hMonitor, HDC
> > hdcMonitor, LPRECT lprcMonitor, LPARAM dwData) +{
> > +    /* this is called for each active monitor */
> > +
> > +    VLC_UNUSED(hdcMonitor);
> > +    VLC_UNUSED(lprcMonitor);
> > +
> > +    MONITORINFOEX mi;
> > +    mi.cbSize = sizeof(MONITORINFOEX);
> > +    if (!GetMonitorInfo(hMonitor, (LPMONITORINFO) &mi)) return true;
> > +
> > +    enum_monitors_callback_params_t *params =
> > (enum_monitors_callback_params_t*) dwData;
> > +    char *psz_monitor_name =
> > FromWide(mi.szDevice);
>
> Enumeration is supposed to be done when either the window provider is
> initialized with the respective callback, or when the setting choices are
> requested by the CLI or GUI.
>
> So then it would seem saner to keep the list of monitors internally, than to
> enumerate them again.
>

I agree with you, but it means that you have to handle
device plug and unplug, and adds unecessary noise to this
patch. May I suggest to do the caching logic in an
additional patch so as to follow a good incremental change
policy and be sure we have a working version without
caching in the git history, while describing why caching is
needed?

I have questions about the enumeration with CLI/GUI that
you suggest, but I'll move it into a dedicated ML thread
in the following days as it is unrelated.

> > +    if (!strcmp(psz_monitor_name, params->psz_name_to_find))
> > +    {
> > +        /* the monitor name matches what we're looking for */
> > +        params->is_monitor_found = true;
> > +        params->found_monitor_info = &mi;
> > +    }
> > +    free(psz_monitor_name);
> > +    return !params->is_monitor_found;
> > +}
> > +
> >  static void SetFullscreen(vout_window_t *wnd, const char *id)
> >  {
> > -    VLC_UNUSED(id);
> >      msg_Dbg(wnd, "entering fullscreen mode");
> >      vout_window_sys_t *sys = wnd->sys;
> >
> > @@ -167,12 +196,41 @@ static void SetFullscreen(vout_window_t *wnd, const
> > char *id) /* Change window style, no borders and no title bar */
> >      SetWindowLong(sys->hwnd, GWL_STYLE, WS_CLIPCHILDREN | WS_VISIBLE);
> >
> > -    /* Retrieve current window position so fullscreen will happen
> > -     * on the right screen */
> > -    HMONITOR hmon = MonitorFromWindow(sys->hwnd, MONITOR_DEFAULTTONEAREST);
> > -    MONITORINFO mi;
> > -    mi.cbSize = sizeof(MONITORINFO);
> > -    if (GetMonitorInfo(hmon, &mi))
> > +    /* Figure out which monitor we should be using */
> > +    MONITORINFOEX mi;
> > +    bool is_monitor_found = false;
> > +
> > +    if (id)
> > +    {
> > +        /* We're being told to use a specific monitor, so see if we can
> > find an +         * active monitor with the same name */
> > +        enum_monitors_callback_params_t params = {
> > +            .is_monitor_found = false,
> > +            .psz_name_to_find = id
> > +        };
> > +        EnumDisplayMonitors(NULL, NULL, &EnumDisplayMonitorsCallback,
> > (LPARAM) &params); +
> > +        if (params.is_monitor_found)
> > +        {
> > +            is_monitor_found = true;
> > +            mi = *(params.found_monitor_info);
> > +        }
> > +        else
> > +        {
> > +            msg_Dbg(wnd, "display device with name \"%s\" was requested,
> > but no such device exists. Falling back to primary", id); +        }
> > +    }
> > +
> > +    if (!is_monitor_found)
> > +    {
> > +        /* fall back to using the screen the window is currently mostly on
> > */ +        HMONITOR hmon;
> > +        hmon = MonitorFromWindow(sys->hwnd, MONITOR_DEFAULTTONEAREST);
> > +        mi.cbSize = sizeof(MONITORINFOEX);
> > +        is_monitor_found = GetMonitorInfo(hmon, (LPMONITORINFO) &mi);
> > +    }
> > +
> > +    if (is_monitor_found)
> >          SetWindowPos(sys->hwnd, 0,
> >                       mi.rcMonitor.left,
> >                       mi.rcMonitor.top,
>
>
> --
> レミ・デニ-クールモン
> http://www.remlab.net/
>
>
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel