On Fri, Mar 8, 2019 at 1:34 PM Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>
> >
> > 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@xxxxxxxxxx>
> > ---
> > 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?
Yes, thanks
>
> Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel