[Spice-devel] [win32/vd_agent v2 2/3] introduce turn_monitor_off method of WDDM interface

[Frediano Ziglio]

> 
> Adding method 'turn_monitor_off' to WDDM interface.
> It sends QXL_ESCAPE_MONITOR_CONFIG escape with zeroed
> display size to qxl-wddm-dod driver.
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich at daynix.com>
> ---
>  vdagent/display_configuration.cpp | 18 ++++++++++++++++++
>  vdagent/display_configuration.h   |  2 +-
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/vdagent/display_configuration.cpp
> b/vdagent/display_configuration.cpp
> index bb2fb80..420836b 100644
> --- a/vdagent/display_configuration.cpp
> +++ b/vdagent/display_configuration.cpp
> @@ -488,6 +488,24 @@ bool WDDMInterface::update_monitor_config(LPCTSTR
> device_name, DisplayMode* disp
>  
>  }
>  
> +bool WDDMInterface::turn_monitor_off(LPCTSTR device_name)
> +{
> +    vd_printf("for %S", device_name);

<ot>
Off topic: It seems that the code is assuming that the strings
are always unicode. I agree they should, maybe we should start
using LPCWSTR instead of LPCTSTR to make sure nobody will try
to compile without unicode? In this case the code would use
device_name as multi-byte but vd_printf would assume the string
is wide causing possible corruptions. Or an easy safety against
it could be a check like

#if !defined(UNICODE) || !defined(_UNICODE)
#error This code is designed to use Unicode
#endif

in some main header file (like common/vdlog.h).

Also if somebody is using some internationalization I think
using single-byte for the output file would cause some conversions
leading to replace some no-ANSI character to be replaced by
question marks ('?'). It would make sense to move the log file
to Unicode too to avoid that.
</ot>

> +    if (!_send_monitors_config)
> +    {

Minor style: in most of occurrences the open bracket is on the
same line ...

> +        vd_printf("do nothing as _send_monitors_config is off");
> +        return false;
> +    }
> +
> +    WDDMMonitorConfigEscape wddm_escape(NULL);
> +    if (escape(device_name, &wddm_escape, sizeof(wddm_escape))) {

... like this

> +        return true;
> +    }
> +
> +    vd_printf("%S failed", device_name);
> +    return false;
> +}
> +
>  LONG WDDMInterface::update_display_settings()
>  {
>      LONG error(0);
> diff --git a/vdagent/display_configuration.h
> b/vdagent/display_configuration.h
> index 7b5578e..8f3b93a 100644
> --- a/vdagent/display_configuration.h
> +++ b/vdagent/display_configuration.h
> @@ -150,7 +150,6 @@ public:
>      bool update_monitor_config(LPCTSTR device_name, DisplayMode* mode,
>      DEVMODE* dev_mode);
>      bool update_dev_mode_position(LPCTSTR device_name, DEVMODE * dev_mode,
>      LONG x, LONG y);
>      void update_config_path();
> -

Minor: spurious change

>  private:
>      bool init_d3d_api();
>      D3D_HANDLE adapter_handle(LPCTSTR device_name);
> @@ -160,6 +159,7 @@ private:
>  
>      void close_adapter(D3D_HANDLE handle);
>      bool escape(LPCTSTR device_name, void* data, UINT sizeData);
> +    bool turn_monitor_off(LPCTSTR device_name);
>  
>      //GDI Function pointers
>      PFND3DKMT_OPENADAPTERFROMHDC _pfnOpen_adapter_hdc;

Otherwise the patch is fine. Do you want me to fix these minor styles
and merge?

Frediano