[Spice-devel] [PATCH win-vdagent v6 2/3] Encapsulating XPDM implementation
Frediano Ziglio
fziglio at redhat.com
Mon Aug 15 08:17:13 UTC 2016
> On Thu, Aug 11, 2016 at 1:23 PM, Frediano Ziglio < fziglio at redhat.com >
> wrote:
> > >
>
> > > The Direct3D 9 API operates on either the Windows XP display driver
>
> > > model (XPDM) or the Windows Vista display driver model (WDDM), depending
>
> > > on the operating system installed.
>
> > >
>
> > > This patch encapsulates the current XPDM interface implementation into
>
> > > XPDMInterface class which inherits DisplayConfig class. This patch
>
> > > makes it easier to introduce WDDM interface to Vdagent in future
>
> > > patches.
>
> > >
>
> > > Based on a patch by Sandy Stutsman < sstutsma at redhat.com >
>
> > >
>
> > > Signed-off-by: Dmitry Fleytman < dfleytma at redhat.com >
>
> > > Signed-off-by: Sameeh Jubran < sameeh at daynix.com >
>
> > > ---
>
> > > vdagent/desktop_layout.cpp | 148 +++++++-------------------------
>
> > > vdagent/desktop_layout.h | 9 +-
>
> > > vdagent/display_configuration.cpp | 174
>
> > > ++++++++++++++++++++++++++++++++++++++
>
> > > vdagent/display_configuration.h | 36 ++++++++
>
> > > 4 files changed, 245 insertions(+), 122 deletions(-)
>
> > >
>
> > > diff --git a/vdagent/desktop_layout.cpp b/vdagent/desktop_layout.cpp
>
> > > index a7666ca..9c3e873 100644
>
> > > --- a/vdagent/desktop_layout.cpp
>
> > > +++ b/vdagent/desktop_layout.cpp
>
> > > @@ -18,6 +18,7 @@
>
> > > #include <spice/qxl_windows.h>
>
> > > #include <spice/qxl_dev.h>
>
> > > #include "desktop_layout.h"
>
> > > +#include "display_configuration.h"
>
> > > #include "vdlog.h"
>
> > >
>
> > > #ifdef __MINGW32__
>
> > > @@ -35,15 +36,17 @@ void DisplayMode::set_res(DWORD width, DWORD height,
>
> > > DWORD depth)
>
> > > DesktopLayout::DesktopLayout()
>
> > > : _total_width (0)
>
> > > , _total_height (0)
>
> > > - , _send_monitors_position(false)
>
> > > + , _display_config (NULL)
>
> > > {
>
> > > MUTEX_INIT(_mutex);
>
> > > + _display_config = DisplayConfig::create_config();
>
> > > get_displays();
>
> > > }
>
> > >
>
> > > DesktopLayout::~DesktopLayout()
>
> > > {
>
> > > clean_displays();
>
> > > + delete _display_config;
>
> > > }
>
> > >
>
> > > void DesktopLayout::get_displays()
>
> > > @@ -59,6 +62,7 @@ void DesktopLayout::get_displays()
>
> > > unlock();
>
> > > return;
>
> > > }
>
> > > + _display_config->update_config_path();
>
> > > clean_displays();
>
> > > ZeroMemory(&dev_info, sizeof(dev_info));
>
> > > dev_info.cb = sizeof(dev_info);
>
> > > @@ -82,12 +86,13 @@ void DesktopLayout::get_displays()
>
> > > _displays[i] = NULL;
>
> > > }
>
> > > }
>
> > > - attached = !!(dev_info.StateFlags &
>
> > > DISPLAY_DEVICE_ATTACHED_TO_DESKTOP);
>
> > > + attached = _display_config->is_attached(&dev_info);
>
> > > +
>
> > > EnumDisplaySettings(dev_info.DeviceName, ENUM_CURRENT_SETTINGS,
>
> > > &mode);
>
> > > _displays[display_id] = new DisplayMode(mode.dmPosition.x,
>
> > > mode.dmPosition.y,
>
> > > mode.dmPelsWidth,
>
> > > mode.dmPelsHeight,
>
> > > mode.dmBitsPerPel,
>
> > > attached);
>
> > > - update_monitor_config(dev_info.DeviceName, _displays[display_id]);
>
> > > + _display_config->update_monitor_config(dev_info.DeviceName,
>
> > > _displays[display_id], &mode);
>
> > > }
>
> > > normalize_displays_pos();
>
> > > unlock();
>
> > > @@ -121,6 +126,7 @@ void DesktopLayout::set_displays()
>
> > > unlock();
>
> > > return;
>
> > > }
>
> > > + _display_config->update_config_path();
>
> > > ZeroMemory(&dev_info, sizeof(dev_info));
>
> > > dev_info.cb = sizeof(dev_info);
>
> > > ZeroMemory(&dev_mode, sizeof(dev_mode));
>
> > > @@ -146,28 +152,32 @@ void DesktopLayout::set_displays()
>
> > > break;
>
> > > }
>
> > > DisplayMode * mode(_ displays.at (display_id));
>
> > > - if (!init_dev_mode(dev_info.DeviceName, &dev_mode, mode, normal_x,
>
> > > normal_y, true)) {
>
> > > + if (!init_dev_mode(dev_info.DeviceName, &dev_mode, mode)) {
>
> > > vd_printf("No suitable mode found for display %S",
>
> > > dev_info.DeviceName);
>
> > > break;
>
> > > }
>
> > > vd_printf("Set display mode %lux%lu", dev_mode.dmPelsWidth,
>
> > > dev_mode.dmPelsHeight);
>
> > > - LONG ret = ChangeDisplaySettingsEx(dev_info.DeviceName, &dev_mode,
>
> > > NULL,
>
> > > - CDS_UPDATEREGISTRY | CDS_NORESET,
>
> > > NULL);
>
> > > - if (ret == DISP_CHANGE_SUCCESSFUL) {
>
> > > + if (_display_config->update_dev_mode_position(dev_info.DeviceName,
>
> > > &dev_mode,
>
> > > + mode->_pos_x -
>
> > > normal_x,
>
> > > + mode->_pos_y -
>
> > > normal_y)) {
>
> > > dev_sets++;
>
> > > - update_monitor_config(dev_info.DeviceName, mode);
>
> > > + _display_config->update_monitor_config(dev_info.DeviceName,
>
> > > mode, &dev_mode);
>
> > > }
>
> > > if (!is_qxl) {
>
> > > display_id++;
>
> > > }
>
> > > }
>
> > > if (dev_sets) {
>
> > > - ChangeDisplaySettingsEx(NULL, NULL, NULL, 0, NULL);
>
> > > + _display_config->update_display_settings();
>
> > > normalize_displays_pos();
>
> > > }
>
> > > unlock();
>
> > > }
>
> > >
>
> > > +void DesktopLayout::set_position_configurable(bool flag) {
>
> > > + _display_config->set_monitors_config(flag);
>
> > > +}
>
> > > +
>
> > > // Normalize all display positions to non-negative coordinates and update
>
> > > total width and height of
>
> > > // the virtual desktop. Caller is responsible to lock() & unlock().
>
> > > void DesktopLayout::normalize_displays_pos()
>
> > > @@ -265,125 +275,29 @@ bool DesktopLayout::get_qxl_device_id(WCHAR*
>
> > > device_key, DWORD* device_id)
>
> > > return key_found;
>
> > > }
>
> > >
>
> > > -bool DesktopLayout::init_dev_mode(LPCTSTR dev_name, DEVMODE* dev_mode,
>
> > > DisplayMode* mode,
>
> > > - LONG normal_x, LONG normal_y, bool
>
> > > set_pos)
>
> > > +bool DesktopLayout::init_dev_mode(LPCTSTR dev_name, DEVMODE* dev_mode,
>
> > > DisplayMode* mode)
>
> > > {
>
> > > - DWORD closest_diff = -1;
>
> > > - DWORD best = -1;
>
> > > - QXLEscapeSetCustomDisplay custom;
>
> > > - HDC hdc = NULL;
>
> > > - LONG ret;
>
> > > -
>
> > > ZeroMemory(dev_mode, sizeof(DEVMODE));
>
> > > dev_mode->dmSize = sizeof(DEVMODE);
>
> > > - if (!mode || !mode->_attached) {
>
> > > - //Detach monitor
>
> > > - dev_mode->dmFields = DM_PELSWIDTH | DM_PELSHEIGHT | DM_POSITION;
>
> > > - return true;
>
> > > - }
>
> > > -
>
> > > - hdc = CreateDC(dev_name, NULL, NULL, NULL);
>
> > > - if (!hdc) {
>
> > > - // for some reason, windows want those 3 flags to enable monitor
>
> > > - dev_mode->dmFields = DM_PELSWIDTH | DM_PELSHEIGHT | DM_POSITION;
>
> > > - dev_mode->dmPelsWidth = mode->_width;
>
> > > - dev_mode->dmPelsHeight = mode->_height;
>
> > > - ret = ChangeDisplaySettingsEx(dev_name, dev_mode, NULL,
>
> > > CDS_UPDATEREGISTRY, NULL);
>
> > > - if (ret == DISP_CHANGE_BADMODE) {
>
> > > - // custom resolution might not be set yet, use known resolution
>
> > > - // FIXME: this causes client temporary resize... a
>
> > > - // solution would involve passing custom resolution before
>
> > > - // driver initialization, perhaps through registry
>
> > > - dev_mode->dmPelsWidth = 640;
>
> > > - dev_mode->dmPelsHeight = 480;
>
> > > - ret = ChangeDisplaySettingsEx(dev_name, dev_mode, NULL,
>
> > > CDS_UPDATEREGISTRY, NULL);
>
> > > - }
>
> > >
>
> > > - vd_printf("attach %ld", ret);
>
> > > - hdc = CreateDC(dev_name, NULL, NULL, NULL);
>
> > > + //Update monitor state
>
> > > + MONITOR_STATE monitor_state = (!mode || !mode->_attached)?
>
> > > MONITOR_DETACHED : MONITOR_ATTACHED;
>
> > > + _display_config->set_monitor_state(dev_name, dev_mode, monitor_state);
>
> > This looks like an extra call to ChangeDisplaySettingsEx if monitor is
> > detached.
>
> > This has nothing to do with encapsulation.
>
> It's instead of the one that was deleted which attaches the monitor.
So there was a previous patch that deleted some code causing regression?
In this case would be better to revert the offending patch instead of putting
a fix into an encapsulation patch.
> > > + if (monitor_state == MONITOR_DETACHED) {
>
> > > + return true;
>
> > > }
>
> > >
>
> > > - if (!hdc) {
>
> > > - vd_printf("failed to create DC");
>
> > > + // Update custom resolution
>
> > > + dev_mode->dmFields = DM_PELSWIDTH | DM_PELSHEIGHT | DM_POSITION;
>
> > > + dev_mode->dmPelsWidth = mode->_width;
>
> > > + dev_mode->dmPelsHeight = mode->_height;
>
> > > + dev_mode->dmBitsPerPel = mode->_depth;
>
> > > + if (!_display_config->custom_display_escape(dev_name, dev_mode))
>
> > > return false;
>
> > > - } else {
>
> > > - // Update custom resolution
>
> > > - custom.xres = mode->_width;
>
> > > - custom.yres = mode->_height;
>
> > > - custom.bpp = mode->_depth;
>
> > > -
>
> > > - int err = ExtEscape(hdc, QXL_ESCAPE_SET_CUSTOM_DISPLAY,
>
> > > - sizeof(QXLEscapeSetCustomDisplay),
>
> > > (LPCSTR)&custom, 0, NULL);
>
> > > - if (err <= 0) {
>
> > > - vd_printf("can't set custom display, perhaps an old driver");
>
> > > - }
>
> > > - DeleteDC(hdc);
>
> > > - }
>
> > > -
>
> > > - // force refresh mode table
>
> > > - DEVMODE tempDevMode;
>
> > > - ZeroMemory(&tempDevMode, sizeof (tempDevMode));
>
> > > - tempDevMode.dmSize = sizeof(DEVMODE);
>
> > > - EnumDisplaySettings(dev_name, 0xffffff, &tempDevMode);
>
> > > -
>
> > > - //Find the closest size which will fit within the monitor
>
> > > - for (DWORD i = 0; EnumDisplaySettings(dev_name, i, dev_mode); i++) {
>
> > > - if (dev_mode->dmPelsWidth > mode->_width ||
>
> > > - dev_mode->dmPelsHeight > mode->_height ||
>
> > > - dev_mode->dmBitsPerPel != mode->_depth) {
>
> > > - continue;
>
> > > - }
>
> > > - DWORD wdiff = mode->_width - dev_mode->dmPelsWidth;
>
> > > - DWORD hdiff = mode->_height - dev_mode->dmPelsHeight;
>
> > > - DWORD diff = wdiff * wdiff + hdiff * hdiff;
>
> > > - if (diff < closest_diff) {
>
> > > - closest_diff = diff;
>
> > > - best = i;
>
> > > - }
>
> > > - }
>
> > > - if (best == (DWORD)-1 || !EnumDisplaySettings(dev_name, best,
> > > dev_mode))
>
> > > {
>
> > > - return false;
>
> > > - }
>
> > > - dev_mode->dmFields = DM_BITSPERPEL | DM_PELSWIDTH | DM_PELSHEIGHT;
>
> > > - if (set_pos) {
>
> > > - //Convert the position so that the primary is always at (0,0)
>
> > > - dev_mode->dmPosition.x = mode->_pos_x - normal_x;
>
> > > - dev_mode->dmPosition.y = mode->_pos_y - normal_y;
>
> > > - dev_mode->dmFields |= DM_POSITION;
>
> > > - }
>
> > >
>
> > > // update current DisplayMode (so mouse scaling works properly)
>
> > > mode->_width = dev_mode->dmPelsWidth;
>
> > > mode->_height = dev_mode->dmPelsHeight;
>
> > > -
>
> > > return true;
>
> > > -}
>
> > > -
>
> > > -bool DesktopLayout::update_monitor_config(LPCTSTR dev_name, DisplayMode*
>
> > > mode)
>
> > > -{
>
> > > - QXLHead monitor_config;
>
> > > -
>
> > > - if (!mode || !mode->get_attached())
>
> > > - return false;
>
> > > -
>
> > > - //Don't configure monitors unless the client supports it
>
> > > - if(!_send_monitors_position) return FALSE;
>
> > > -
>
> > > - HDC hdc = CreateDC(dev_name, NULL, NULL, NULL);
>
> > > -
>
> > > - memset(&monitor_config, 0, sizeof(monitor_config));
>
> > > - monitor_config.x = mode->_pos_x;
>
> > > - monitor_config.y = mode->_pos_y;
>
> > > - monitor_config.width = mode->_width;
>
> > > - monitor_config.height = mode->_height;
>
> > > -
>
> > > - int err = ExtEscape(hdc, QXL_ESCAPE_MONITOR_CONFIG,
>
> > > - sizeof(QXLHead), (LPCSTR) &monitor_config, 0, NULL);
>
> > > -
>
> > > - if (err < 0){
>
> > > - vd_printf("can't update monitor config, may have an older driver");
>
> > > - }
>
> > >
>
> > > - DeleteDC(hdc);
>
> > > - return (err >= 0);
>
> > > }
>
> > > diff --git a/vdagent/desktop_layout.h b/vdagent/desktop_layout.h
>
> > > index ece3946..fd6af76 100644
>
> > > --- a/vdagent/desktop_layout.h
>
> > > +++ b/vdagent/desktop_layout.h
>
> > > @@ -60,6 +60,7 @@ private:
>
> > > };
>
> > >
>
> > > typedef std::vector<DisplayMode*> Displays;
>
> > > +class DisplayConfig;
>
> > >
>
> > > class DesktopLayout {
>
> > > public:
>
> > > @@ -73,23 +74,21 @@ public:
>
> > > size_t get_display_count() { return _displays.size();}
>
> > > DWORD get_total_width() { return _total_width;}
>
> > > DWORD get_total_height() { return _total_height;}
>
> > > - void set_position_configurable(bool flag) { _send_monitors_position =
>
> > > flag; }
>
> > > + void set_position_configurable(bool flag);
>
> > > private:
>
> > > void clean_displays();
>
> > > void normalize_displays_pos();
>
> > > DisplayMode * get_primary_display();
>
> > > - bool update_monitor_config(LPCTSTR dev_name, DisplayMode* mode);
>
> > > + bool init_dev_mode(LPCTSTR dev_name, DEVMODE* dev_mode, DisplayMode*
>
> > > mode);
>
> > > static bool consistent_displays();
>
> > > static bool is_attached(LPCTSTR dev_name);
>
> > > static bool get_qxl_device_id(WCHAR* device_key, DWORD* device_id);
>
> > > - static bool init_dev_mode(LPCTSTR dev_name, DEVMODE* dev_mode,
>
> > > DisplayMode* mode,
>
> > > - LONG normal_x, LONG normal_y, bool set_pos);
>
> > > private:
>
> > > mutex_t _mutex;
>
> > > Displays _displays;
>
> > > DWORD _total_width;
>
> > > DWORD _total_height;
>
> > > - bool _send_monitors_position;
>
> > > + DisplayConfig* _display_config;
>
> > > };
>
> > >
>
> > > #endif
>
> > > diff --git a/vdagent/display_configuration.cpp
>
> > > b/vdagent/display_configuration.cpp
>
> > > index a7aa0d7..5e40d05 100755
>
> > > --- a/vdagent/display_configuration.cpp
>
> > > +++ b/vdagent/display_configuration.cpp
>
> > > @@ -152,6 +152,180 @@ struct DISPLAYCONFIG_PATH_INFO {
>
> > > UINT32 flags;
>
> > > };
>
> > >
>
> > > +struct QXLMonitorEscape {
>
> > > + QXLMonitorEscape(DEVMODE* dev_mode)
>
> > > + {
>
> > > + ZeroMemory(&_head, sizeof(_head));
>
> > > + _head.x = dev_mode->dmPosition.x;
>
> > > + _head.y = dev_mode->dmPosition.y;
>
> > > + _head.width = dev_mode->dmPelsWidth;
>
> > > + _head.height = dev_mode->dmPelsHeight;
>
> > > + }
>
> > > + QXLHead _head;
>
> > > +};
>
> > > +
>
> > > +struct QxlCustomEscapeObj : public QXLEscapeSetCustomDisplay {
>
> > > + QxlCustomEscapeObj(uint32_t bitsPerPel, uint32_t width, uint32_t
> > > height)
>
> > > + {
>
> > > + xres = width;
>
> > > + yres = height;
>
> > > + bpp = bitsPerPel;
>
> > > + }
>
> > > + QxlCustomEscapeObj() {};
>
> > > +};
>
> > > +
>
> > > +DisplayConfig* DisplayConfig::create_config()
>
> > > +{
>
> > > + DisplayConfig* new_interface;
>
> > > + new_interface = new XPDMInterface();
>
> > > + return new_interface;
>
> > > +}
>
> > > +
>
> > > +DisplayConfig::DisplayConfig()
>
> > > + : _send_monitors_config(false)
>
> > > +{}
>
> > > +
>
> > > +bool XPDMInterface::is_attached(DISPLAY_DEVICE* dev_info)
>
> > > +{
>
> > > + return !!(dev_info->StateFlags & DISPLAY_DEVICE_ATTACHED_TO_DESKTOP);
>
> > > +}
>
> > > +
>
> > > +bool XPDMInterface::set_monitor_state(LPCTSTR device_name, DEVMODE*
>
> > > dev_mode, MONITOR_STATE state)
>
> > > +{
>
> > > + dev_mode->dmFields = DM_PELSWIDTH | DM_PELSHEIGHT | DM_POSITION;
>
> > > + if (state == MONITOR_ATTACHED) {
>
> > > + return true;
>
> > > + }
>
> > > +
>
> > > + LONG status = ChangeDisplaySettingsEx(device_name, dev_mode, NULL,
>
> > > CDS_UPDATEREGISTRY, NULL);
>
> > > + return (status == DISP_CHANGE_SUCCESSFUL);
>
> > > +}
>
> > > +
>
> > > +LONG XPDMInterface::update_display_settings()
>
> > > +{
>
> > > + return ChangeDisplaySettingsEx(NULL, NULL, NULL, 0, NULL);
>
> > > +}
>
> > > +
>
> > > +bool XPDMInterface::update_dev_mode_position(LPCTSTR device_name,
>
> > > + DEVMODE* dev_mode, LONG x, LONG
>
> > > y)
>
> > > +{
>
> > > + //Convert the position so that the primary is always at (0,0)
>
> > > + dev_mode->dmPosition.x = x;
>
> > > + dev_mode->dmPosition.y = y;
>
> > > + dev_mode->dmFields |= DM_POSITION;
>
> > > + vd_printf("%s: setting %S at (%lu, %lu)", __FUNCTION__, device_name,
>
> > > dev_mode->dmPosition.x,
>
> > > + dev_mode->dmPosition.y);
>
> > > +
>
> > > + LONG status = ChangeDisplaySettingsEx(device_name, dev_mode, NULL,
>
> > > + CDS_UPDATEREGISTRY | CDS_NORESET,
>
> > > NULL);
>
> > > + return (status == DISP_CHANGE_SUCCESSFUL);
>
> > > +}
>
> > > +
>
> > > +bool XPDMInterface::custom_display_escape(LPCTSTR device_name, DEVMODE*
>
> > > dev_mode)
>
> > I found this name quite misleading.
>
> > What about set_custom_display or set_device_mode ?
>
> > > +{
>
> > > + LONG ret;
>
> > > + NTSTATUS Status (ERROR_SUCCESS);
>
> > > + HDC hdc = CreateDC(device_name, NULL, NULL, NULL);
>
> > > +
>
> > > + if (!hdc) {
>
> > > + ret = ChangeDisplaySettingsEx(device_name, dev_mode, NULL,
>
> > > CDS_UPDATEREGISTRY, NULL);
>
> > > + if (ret == DISP_CHANGE_BADMODE) {
>
> > > + // custom resolution might not be set yet, use known resolution
>
> > > + // FIXME: this causes client temporary resize... a
>
> > > + // solution would involve passing custom resolution before
>
> > > + // driver initialization, perhaps through registry
>
> > > + dev_mode->dmPelsWidth = 640;
>
> > > + dev_mode->dmPelsHeight = 480;
>
> > > + ret = ChangeDisplaySettingsEx(device_name, dev_mode, NULL,
>
> > > CDS_UPDATEREGISTRY, NULL);
>
> > > + }
>
> > > +
>
> > > + vd_printf("attach %ld", ret);
>
> > > + if (!(hdc = CreateDC(device_name, NULL, NULL, NULL))) {
>
> > > + vd_printf("%s: failed to create DC", __FUNCTION__);
>
> > > + return false;
>
> > > + }
>
> > > + }
>
> > > +
>
> > > + QxlCustomEscapeObj custom_escape(dev_mode->dmBitsPerPel,
>
> > > + dev_mode->dmPelsWidth,
>
> > > dev_mode->dmPelsHeight);
>
> > > +
>
> > > + int err = ExtEscape(hdc, QXL_ESCAPE_SET_CUSTOM_DISPLAY,
>
> > > + sizeof(QXLEscapeSetCustomDisplay), (LPCSTR) &custom_escape, 0,
>
> > > NULL);
>
> > > + if (err <= 0) {
>
> > > + vd_printf("%s: Can't set custom display, perhaps running with an
>
> > > older driver?",
>
> > > + __FUNCTION__);
>
> > > + }
>
> > > +
>
> > > + if (!find_best_mode(device_name, dev_mode)) {
>
> > > + Status = E_FAIL;
>
> > > + }
>
> > > +
>
> > > + DeleteDC(hdc);
>
> > > + return NT_SUCCESS(Status);
>
> > > +}
>
> > > +
>
> > > +bool XPDMInterface::update_monitor_config(LPCTSTR device_name,
> > > DisplayMode*
>
> > > mode,
>
> > > + DEVMODE* dev_mode)
>
> > Here you are declaring you can change dev_mode and mode as they are
>
> > not constant. Are you expecting to change them?
>
> > I would use a const DEVMODE &dev_mode instead.
>
> In the next patch which adds WDDMinterface, mode changes but DEVMODE doesn't.
> > > +{
>
> > > + if (!mode || !mode->get_attached()) {
>
> > > + return false;
>
> > > + }
>
> > > +
>
> > > + QXLMonitorEscape monitor_config(dev_mode);
>
> > > + HDC hdc(CreateDC(device_name, NULL, NULL, NULL));
>
> > > + int err(0);
>
> > > +
>
> > > + if (!hdc || !_send_monitors_config) {
>
> > > + return false;
>
> > > + }
>
> > > +
>
> > > + err = ExtEscape(hdc, QXL_ESCAPE_MONITOR_CONFIG, sizeof(QXLHead),
>
> > > + (LPCSTR) &monitor_config, 0, NULL);
>
> > here it's better to use (LPCSTR) &monitor_config._head to make sure
>
> > you are sending head. Just adding a virtual function would break this code.
>
> > > + if (err < 0) {
>
> > > + vd_printf("%s: %S can't update monitor config, may have old, old
>
> > > driver",
>
> > > + __FUNCTION__, device_name);
>
> > > + }
>
> > > + DeleteDC(hdc);
>
> > > + return (err >= 0);
>
> > > +}
>
> > > +
>
> > > +bool XPDMInterface::find_best_mode(LPCTSTR Device, DEVMODE* dev_mode)
>
> > > +{
>
> > > + DWORD closest_diff = -1;
>
> > > + DWORD best = -1;
>
> > > +
>
> > > + // force refresh mode table
>
> > > + DEVMODE test_dev_mode;
>
> > > + ZeroMemory(&test_dev_mode, sizeof(test_dev_mode));
>
> > > + test_dev_mode.dmSize = sizeof(DEVMODE);
>
> > > + EnumDisplaySettings(Device, 0xffffff, &test_dev_mode);
>
> > > +
>
> > > + //Find the closest size which will fit within the monitor
>
> > > + for (DWORD i = 0; EnumDisplaySettings(Device, i, &test_dev_mode); i++)
> > > {
>
> > > + if (dev_mode->dmPelsWidth > test_dev_mode.dmPelsWidth ||
>
> > > + dev_mode->dmPelsHeight > test_dev_mode.dmPelsHeight ||
>
> > > + dev_mode->dmBitsPerPel != test_dev_mode.dmBitsPerPel) {
>
> > > + continue;
>
> > > + }
>
> > > + DWORD wdiff = dev_mode->dmPelsWidth - test_dev_mode.dmPelsWidth;
>
> > > + DWORD hdiff = dev_mode->dmPelsHeight - test_dev_mode.dmPelsHeight;
>
> > > + DWORD diff = wdiff * wdiff + hdiff * hdiff;
>
> > > + if (diff < closest_diff) {
>
> > > + closest_diff = diff;
>
> > > + best = i;
>
> > > + }
>
> > > + }
>
> > > + vd_printf("%s: closest_diff at %lu best %lu", __FUNCTION__,
>
> > > closest_diff, best);
>
> > > + if (best == (DWORD) -1 || !EnumDisplaySettings(Device, best, dev_mode))
>
> > > {
>
> > > + return false;
>
> > > + }
>
> > > +
>
> > > + //Change to the best fit
>
> > > + LONG status = ChangeDisplaySettingsEx(Device, dev_mode, NULL,
>
> > > + CDS_UPDATEREGISTRY | CDS_NORESET,
>
> > > NULL);
>
> > This change the behavior. Previously there was no such calls from
> > init_dev_mode.
>
> > Also the name is not suggesting that you set the mode but just that you
> > find
> > it.
>
> > Where these lines are now?
>
> > dev_mode->dmFields = DM_BITSPERPEL | DM_PELSWIDTH | DM_PELSHEIGHT;
>
> > if (set_pos) {
>
> > //Convert the position so that the primary is always at (0,0)
>
> > dev_mode->dmPosition.x = mode->_pos_x - normal_x;
>
> > dev_mode->dmPosition.y = mode->_pos_y - normal_y;
>
> > dev_mode->dmFields |= DM_POSITION;
>
> > }
>
> This part is now in a separate function "update_dev_mode_position".
> Setting the flags ( dev_mode->dmFields = DM...) is still done in
> "init_dev_mode" However the flags should be
> "DM_PELSWIDTH | DM_PELSHEIGHT | DM_BITSPERPEL"
> and not
> "DM_PELSWIDTH | DM_PELSHEIGHT | DM_POSITION"
> as they are in this patch.
> Thanks for notifying me of that.
> > > + return NT_SUCCESS(status);
>
> > > +}
>
> > > +
>
> > > CCD::CCD()
>
> > > :_numPathElements(0)
>
> > > ,_numModeElements(0)
>
> > > diff --git a/vdagent/display_configuration.h
>
> > > b/vdagent/display_configuration.h
>
> > > index 94e52e4..05b35f4 100755
>
> > > --- a/vdagent/display_configuration.h
>
> > > +++ b/vdagent/display_configuration.h
>
> > > @@ -89,4 +89,40 @@ private:
>
> > > PATH_STATE _path_state;
>
> > > };
>
> > >
>
> > > +class DisplayMode;
>
> > > +
>
> > > +//Class provides interface to get/set display configurations
>
> > > +class DisplayConfig {
>
> > > +public:
>
> > > + static DisplayConfig* create_config();
>
> > > + DisplayConfig();
>
> > > + virtual ~DisplayConfig() {};
>
> > > + virtual bool is_attached(DISPLAY_DEVICE* dev_info) = 0;
>
> > > + virtual bool custom_display_escape(LPCTSTR device, DEVMODE* mode) = 0;
>
> > > + virtual bool update_monitor_config(LPCTSTR device, DisplayMode* mode,
>
> > > DEVMODE* dev_mode) = 0;
>
> > > + virtual bool set_monitor_state(LPCTSTR device_name, DEVMODE* dev_mode,
>
> > > MONITOR_STATE state) = 0;
>
> > > + virtual LONG update_display_settings() = 0;
>
> > > + virtual bool update_dev_mode_position(LPCTSTR dev_name, DEVMODE*
>
> > > dev_mode, LONG x, LONG y) = 0;
>
> > > + void set_monitors_config(bool flag) { _send_monitors_config = flag; }
>
> > > + virtual void update_config_path() {};
>
> > > +
>
> > > +protected:
>
> > > + bool _send_monitors_config;
>
> > > +};
>
> > > +
>
> > > +//DisplayConfig implementation for guest with XPDM graphics drivers
>
> > > +class XPDMInterface : public DisplayConfig {
>
> > > +public:
>
> > > + XPDMInterface() :DisplayConfig() {};
>
> > > + bool is_attached(DISPLAY_DEVICE* dev_info);
>
> > > + bool custom_display_escape(LPCTSTR device_name, DEVMODE* dev_mode);
>
> > > + bool update_monitor_config(LPCTSTR device_name, DisplayMode* mode,
>
> > > DEVMODE* dev_mode);
>
> > > + bool set_monitor_state(LPCTSTR device_name, DEVMODE* dev_mode,
>
> > > MONITOR_STATE state);
>
> > > + LONG update_display_settings();
>
> > > + bool update_dev_mode_position(LPCTSTR device_name, DEVMODE * dev_mode,
>
> > > LONG x, LONG y);
>
> > > +
>
> > > +private:
>
> > > + bool find_best_mode(LPCTSTR Device, DEVMODE* dev_mode);
>
> > > +};
>
> > > +
>
> > > #endif
>
> > > \ No newline at end of file
>
Frediano
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160815/3f97bfed/attachment-0001.html>
More information about the Spice-devel
mailing list