[Spice-devel] [PATCH win-vdagent v6 3/3] Implementing WDDM interface to support multiple monitors and arbitrary resolution

[Frediano Ziglio]

> On Mon, Aug 15, 2016 at 10:52 AM, 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 implements the WDDM interface while using the CCD API to do
> 
> > > so. Moreover it introduces multiple monitors support and arbitrary
> 
> > > resolution for Windows 10 while preserving backward compatiblity with
> 
> > > previous
> 
> > > versions of Windows.
> 
> > >
> 
> > > 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/display_configuration.cpp | 355
> 
> > > +++++++++++++++++++++++++++++++++++++-
> 
> > > vdagent/display_configuration.h | 47 +++++
> 
> > > 2 files changed, 401 insertions(+), 1 deletion(-)
> 
> > >
> 
> > > diff --git a/vdagent/display_configuration.cpp
> 
> > > b/vdagent/display_configuration.cpp
> 
> > > index 5e40d05..4db093f 100755
> 
> > > --- a/vdagent/display_configuration.cpp
> 
> > > +++ b/vdagent/display_configuration.cpp
> 
> > > @@ -152,6 +152,54 @@ struct DISPLAYCONFIG_PATH_INFO {
> 
> > > UINT32 flags;
> 
> > > };
> 
> > >
> 
> > > +
> 
> > > +enum D3DKMT_ESCAPETYPE {
> 
> > > + D3DKMT_ESCAPE_DRIVERPRIVATE = 0
> 
> > > +};
> 
> > > +
> 
> > > +struct D3DDDI_ESCAPEFLAGS {
> 
> > > + union {
> 
> > > + struct {
> 
> > > + UINT Reserved : 31;
> 
> > > + };
> 
> > > + UINT Value;
> 
> > > + };
> 
> > > +};
> 
> > > +
> 
> > > +struct D3DKMT_ESCAPE {
> 
> > > + D3D_HANDLE hAdapter;
> 
> > > + D3D_HANDLE hDevice;
> 
> > > + D3DKMT_ESCAPETYPE Type;
> 
> > > + D3DDDI_ESCAPEFLAGS Flags;
> 
> > > + VOID* pPrivateDriverData;
> 
> > > + UINT PrivateDriverDataSize;
> 
> > > + D3D_HANDLE hContext;
> 
> > > +};
> 
> > > +
> 
> > > +struct D3DKMT_OPENADAPTERFROMHDC {
> 
> > > + HDC hDc;
> 
> > > + D3D_HANDLE hAdapter;
> 
> > > + LUID AdapterLuid;
> 
> > > + UINT VidPnSourceId;
> 
> > > +};
> 
> > > +
> 
> > > +struct D3DKMT_CLOSEADAPTER {
> 
> > > + D3D_HANDLE hAdapter;
> 
> > > +};
> 
> > > +
> 
> > > +struct D3DKMT_OPENADAPTERFROMDEVICENAME {
> 
> > > + const WCHAR *pDeviceName;
> 
> > > + D3D_HANDLE hAdapter;
> 
> > > + LUID AdapterLuid;
> 
> > > +};
> 
> > > +
> 
> > > +struct D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME {
> 
> > > + WCHAR DeviceName[32];
> 
> > > + D3D_HANDLE hAdapter;
> 
> > > + LUID AdapterLuid;
> 
> > > + UINT VidPnSourceId;
> 
> > > +};
> 
> > > +
> 

> > Where these definition came from ?
> 

> We have already discussed this in previous versions of the patches:
> https://mail.google.com/mail/u/0/#search/in%3Asent+from+scratch/15650fbeba9cfd30
> We have written from scratch the ones that we didn't find here:
> https://github.com/notr1ch/DWMCapture/blob/master/DWMCaptureSource.cpp

Just put a comment. 
And the "written from scratch" does not make sense, it's an API and you cannot get these 
information from nothing, you need to know the protocol. 

> > > struct QXLMonitorEscape {
> 
> > > QXLMonitorEscape(DEVMODE* dev_mode)
> 
> > > {
> 
> > > @@ -174,10 +222,43 @@ struct QxlCustomEscapeObj : public
> 
> > > QXLEscapeSetCustomDisplay {
> 
> > > QxlCustomEscapeObj() {};
> 
> > > };
> 
> > >
> 
> > > +struct WDDMCustomDisplayEscape {
> 
> > > + WDDMCustomDisplayEscape(DEVMODE* dev_mode)
> 
> > > + {
> 
> > > + _ioctl = QXL_ESCAPE_SET_CUSTOM_DISPLAY;
> 
> > > + _custom.bpp = dev_mode->dmBitsPerPel;
> 
> > > + _custom.xres = dev_mode->dmPelsWidth;
> 
> > > + _custom.yres = dev_mode->dmPelsHeight;
> 
> > > + }
> 
> > > + int _ioctl;
> 
> > > + QXLEscapeSetCustomDisplay _custom;
> 
> > > +};
> 
> > > +
> 
> > > +struct WDDMMonitorConfigEscape {
> 
> > > + WDDMMonitorConfigEscape(DisplayMode* mode)
> 
> > > + {
> 
> > > + _ioctl = QXL_ESCAPE_MONITOR_CONFIG;
> 
> > > + _ head.id = _head.surface_id = 0;
> 
> > > + _head.x = mode->get_pos_x();
> 
> > > + _head.y = mode->get_pos_y();
> 
> > > + _head.width = mode->get_width();
> 
> > > + _head.height = mode->get_height();
> 
> > > + }
> 
> > > + int _ioctl;
> 
> > > + QXLHead _head;
> 
> > > +};
> 
> > > +
> 
> > > DisplayConfig* DisplayConfig::create_config()
> 
> > > {
> 
> > > DisplayConfig* new_interface;
> 
> > > - new_interface = new XPDMInterface();
> 
> > > + /* Try to open a WDDM adapter.
> 
> > > + If that failed, assume we have an XPDM driver */
> 
> > > + try {
> 
> > > + new_interface = new WDDMInterface();
> 
> > > + }
> 
> > > + catch (std::exception& e) {
> 
> > > + new_interface = new XPDMInterface();
> 
> > > + }
> 
> > > return new_interface;
> 
> > > }
> 
> > >
> 
> > > @@ -326,6 +407,278 @@ bool XPDMInterface::find_best_mode(LPCTSTR Device,
> 
> > > DEVMODE* dev_mode)
> 
> > > return NT_SUCCESS(status);
> 
> > > }
> 
> > >
> 
> > > +WDDMInterface::WDDMInterface()
> 
> > > + : _pfnOpen_adapter_hdc(NULL)
> 
> > > + , _pfnClose_adapter(NULL)
> 
> > > + , _pfnEscape(NULL)
> 
> > > + , _pfnOpen_adapter_device_name(NULL)
> 
> > > + , _pfnOpen_adapter_gdi_name(NULL)
> 
> > > +{
> 
> > > + LONG error(0);
> 
> > > + //Can we find the D3D calls we need?
> 
> > > + if (!init_d3d_api()) {
> 
> > > + throw std::exception();
> 
> > > + }
> 
> > > +
> 
> > > + //Initialize CCD path stuff
> 
> > > + if (!_ccd.query_display_config()) {
> 
> > > + throw std::exception();
> 
> > > + }
> 
> > > +
> 
> > > + if (!_ccd.set_display_config(error)) {
> 
> > > + throw std::exception();
> 
> > > + }
> 
> > > +}
> 
> > > +
> 
> > > +bool WDDMInterface::is_attached(DISPLAY_DEVICE* dev_info)
> 
> > > +{
> 
> > > + return _ccd.is_attached(dev_info->DeviceName);
> 
> > > +}
> 
> > > +
> 
> > > +bool WDDMInterface::set_monitor_state(LPCTSTR device_name, DEVMODE*
> 
> > > dev_mode, MONITOR_STATE state)
> 
> > > +{
> 
> > > + return _ccd.set_path_state(device_name, state);
> 
> > > +}
> 
> > > +
> 
> > > +bool WDDMInterface::custom_display_escape(LPCTSTR device_name, DEVMODE*
> 
> > > dev_mode)
> 
> > > +{
> 
> > > + DISPLAYCONFIG_MODE_INFO* mode = _ccd.get_active_mode(device_name,
> 
> > > false);
> 
> > > + if (!mode) {
> 
> > > + return false;
> 
> > > + }
> 
> > > +
> 
> > > + //Don't bother if we are already set to the new resolution
> 
> > > + if (mode->sourceMode.width == dev_mode->dmPelsWidth &&
> 
> > > + mode->sourceMode.height == dev_mode->dmPelsHeight) {
> 
> > > + return true;
> 
> > > + }
> 
> > > +
> 
> > > + vd_printf("%s: updating %S resolution\n", __FUNCTION__, device_name);
> 
> > > +
> 
> > > + WDDMCustomDisplayEscape wddm_escape(dev_mode);
> 
> > > + if (escape(device_name, &wddm_escape, sizeof(wddm_escape))) {
> 

> > This is not portable, not even between 32 and 64 bit.
> 

> Can you please further explain why?

You are sending to the driver a structure that you defined here. 
Usually there is an header that both include which is used to make sure you are 
using the same structure layout. Also you have to define the structure in order (on a 
system that can have 32/64 bit mixed) avoid bit size problems. 

Now you send two structure which are 

struct Xxx { 
int ioctl; 
shared_defined_structure ioctl_data; 
}; 

depending on shared_defined_structure alignment the structure layout between 32 and 64 
can potentially change. You are lucky and the 2 structure have the same alignment 
on both sizes (QXLHead and QXLEscapeSetCustomDisplay are packed so 
aligned to 4 bytes) and the alignment is compatible with int. 
So now works but it's not hard to break this adding a new escape ioctl that does not 
follow these rules. So what's the rule? A packed structure after an int? 
Potentially this will cause the protocol to have unaligned accesses if a field should 
be 8-byte aligned (for instance a pointer/handle). Intel architecture have just some speed 
penalty but if you define the protocol differently you can avoid this penalty. 
If you look at ioctl structures rarely they are packet for this reason. 

> > > + return _ccd.update_mode_size(device_name, dev_mode);
> 
> > > + }
> 
> > > +
> 
> > > + vd_printf("%s: (%dx%d)", __FUNCTION__, mode->sourceMode.width,
> 
> > > mode->sourceMode.height);
> 
> > > + return false;
> 
> > > +}
> 
> > > +
> 
> > > +bool WDDMInterface::update_monitor_config(LPCTSTR device_name,
> > > DisplayMode*
> 
> > > display_mode,
> 
> > > + DEVMODE* dev_mode)
> 
> > > +{
> 
> > > + if (!display_mode || !display_mode->get_attached()) {
> 
> > > + return false;
> 
> > > + }
> 
> > > + DISPLAYCONFIG_MODE_INFO* mode = _ccd.get_active_mode(device_name,
> 
> > > false);
> 
> > > + if (!mode || !_send_monitors_config)
> 
> > > + return false;
> 
> > > +
> 
> > > + WDDMMonitorConfigEscape wddm_escape(display_mode);
> 
> > > + if (escape(device_name, &wddm_escape, sizeof(wddm_escape))) {
> 

> > Same as above. It's not clear who is receiving these escape commands...
> 
> > the driver?
> 

> Yes the driver is receiving those escape commands.

> > > + //Update the path position
> 
> > > + return _ccd.update_mode_position(device_name, dev_mode);
> 
> > > + }
> 
> > > +
> 
> > > + vd_printf("%s: %S failed", __FUNCTION__, device_name);
> 
> > > + return false;
> 
> > > +
> 
> > > +}
> 
> > > +
> 
> > > +LONG WDDMInterface::update_display_settings()
> 
> > > +{
> 
> > > + LONG error(0);
> 
> > > + //If we removed the primary monitor since the last call, we need to
> 
> > > + //reorder the other monitors, making the leftmost one the primary
> 
> > > + _ccd.verify_primary_position();
> 
> > > + _ccd.set_display_config(error);
> 
> > > + return error;
> 
> > > +}
> 
> > > +
> 
> > > +void WDDMInterface::update_config_path()
> 
> > > +{
> 
> > > + _ccd.query_display_config();
> 
> > > +}
> 
> > > +
> 
> > > +bool WDDMInterface::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)
> 

> > This comment does not make any sense here.
> 

> > > + dev_mode->dmPosition.x = x;
> 
> > > + dev_mode->dmPosition.y = y;
> 
> > > + return _ccd.update_mode_position(device_name, dev_mode);
> 
> > > +}
> 
> > > +
> 
> > > +bool WDDMInterface::init_d3d_api()
> 
> > > +{
> 
> > > + HMODULE hModule = LoadLibrary(L"gdi32.dll");
> 
> > > +
> 
> > > + //Look for the gdi32 functions we need to perform driver escapes
> 
> > > + if (!hModule) {
> 
> > > + vd_printf("%s something wildly wrong as we can't open gdi32.dll",
> 
> > > __FUNCTION__);
> 
> > > + return false;
> 
> > > + }
> 
> > > +
> 
> > > + do {
> 
> > > + _pfnClose_adapter = (PFND3DKMT_CLOSEADAPTER)
> 
> > > + GetProcAddress(hModule, "D3DKMTCloseAdapter");
> 
> > > + if (!_pfnClose_adapter) {
> 
> > > + break;
> 
> > > + }
> 
> > > +
> 
> > > + _pfnEscape = (PFND3DKMT_ESCAPE) GetProcAddress(hModule,
> 
> > > "D3DKMTEscape");
> 
> > > + if (!_pfnEscape) {
> 
> > > + break;
> 
> > > + }
> 
> > > +
> 
> > > + _pfnOpen_adapter_hdc = (PFND3DKMT_OPENADAPTERFROMHDC)
> 
> > > + GetProcAddress(hModule, "D3DKMTOpenAdapterFromHdc");
> 
> > > + if (!_pfnOpen_adapter_hdc) {
> 
> > > + break;
> 
> > > + }
> 
> > > +
> 
> > > + _pfnOpen_adapter_device_name = (PFND3DKMT_OPENADAPTERFROMDEVICENAME)
> 
> > > + GetProcAddress(hModule, "D3DKMTOpenAdapterFromDeviceName");
> 
> > > + if (!_pfnOpen_adapter_device_name) {
> 
> > > + break;
> 
> > > + }
> 
> > > +
> 
> > > + _pfnOpen_adapter_gdi_name =
> 
> > > (PFND3DKMT_OPENADAPTERFROMGDIDISPLAYNAME)
> 
> > > + GetProcAddress(hModule, "D3DKMTOpenAdapterFromGdiDisplayName");
> 
> > > + if (!_pfnOpen_adapter_gdi_name) {
> 
> > > + break;
> 
> > > + }
> 
> > > +
> 
> > > + }
> 
> > > + while(0);
> 
> > > +
> 
> > > + FreeLibrary(hModule);
> 
> > > +
> 

> > If the library was loaded the program is going to crash as the function
> 
> > will disappear from memory.
> 

> I'll use GetModuleHandle instead.

> > > + //Did we get them ?
> 
> > > + if (!_pfnClose_adapter || !_pfnOpen_adapter_hdc || !_pfnEscape) {
> 
> > > + return false;
> 
> > > + }
> 
> > > + return true;
> 
> > > +}
> 
> > > +
> 
> > > +D3D_HANDLE WDDMInterface::adapter_handle(LPCTSTR device_name)
> 
> > > +{
> 
> > > + D3D_HANDLE hAdapter(0);
> 
> > > +
> 
> > > + //For some reason, unknown to me, this call will occasionally fail.
> 

> > Who is the "me" here? Would be better to make it explicit.
> 

> > > + if ((hAdapter = handle_from_DC(device_name))) {
> 
> > > + return hAdapter;
> 
> > > + }
> 
> > > + //So try other available methods.
> 
> > > + if (_pfnOpen_adapter_device_name && (hAdapter =
> 
> > > handle_from_device_name(device_name))) {
> 
> > > + return hAdapter;
> 
> > > + }
> 
> > > + //One last chance to open this guy
> 
> > > + if (_pfnOpen_adapter_gdi_name) {
> 
> > > + hAdapter = handle_from_GDI_name(device_name);
> 
> > > + }
> 
> > > +
> 
> > > + if (!hAdapter) {
> 
> > > + vd_printf("%s: failed to open adapter %S", __FUNCTION__,
> 
> > > device_name);
> 
> > > + }
> 
> > > +
> 
> > > + return hAdapter;
> 
> > > +}
> 
> > > +
> 
> > > +D3D_HANDLE WDDMInterface::handle_from_DC(LPCTSTR adapter_name)
> 
> > > +{
> 
> > > + NTSTATUS status;
> 
> > > + D3DKMT_OPENADAPTERFROMHDC open_data;
> 
> > > + HDC hDc(CreateDC(adapter_name, NULL, NULL, NULL));
> 
> > > +
> 
> > > + if (!hDc) {
> 
> > > + vd_printf("%s: %S CreateDC failed with %lu", __FUNCTION__,
> 
> > > adapter_name, GetLastError());
> 
> > > + return 0;
> 
> > > + }
> 
> > > +
> 
> > > + ZeroMemory(&open_data, sizeof(D3DKMT_OPENADAPTERFROMHDC));
> 
> > > + open_data.hDc = hDc;
> 
> > > +
> 
> > > + if (!NT_SUCCESS(status = _pfnOpen_adapter_hdc(&open_data))) {
> 
> > > + vd_printf("%s: %S open adapter from hdc failed with %lu",
> 
> > > __FUNCTION__, adapter_name,
> 
> > > + status);
> 
> > > + open_data.hAdapter = 0;
> 
> > > + }
> 
> > > +
> 
> > > + DeleteDC(hDc);
> 
> > > + return open_data.hAdapter;
> 
> > > +}
> 
> > > +
> 
> > > +D3D_HANDLE WDDMInterface::handle_from_device_name(LPCTSTR adapter_name)
> 
> > > +{
> 
> > > + D3DKMT_OPENADAPTERFROMDEVICENAME display_name_data;
> 
> > > + NTSTATUS status;
> 
> > > +
> 
> > > + ZeroMemory(&display_name_data, sizeof(display_name_data));
> 
> > > + display_name_data.pDeviceName = adapter_name;
> 
> > > +
> 
> > > + if (NT_SUCCESS(status =
> 
> > > _pfnOpen_adapter_device_name(&display_name_data))) {
> 
> > > + return display_name_data.hAdapter;
> 
> > > + }
> 
> > > +
> 
> > > + vd_printf("%s %S failed with 0x%lx", __FUNCTION__, adapter_name,
> 
> > > status);
> 
> > > + return 0;
> 
> > > +}
> 
> > > +
> 
> > > +D3D_HANDLE WDDMInterface::handle_from_GDI_name(LPCTSTR adapter_name)
> 
> > > +{
> 
> > > + D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME gdi_display_name;
> 
> > > + NTSTATUS status;
> 
> > > +
> 
> > > + ZeroMemory(&gdi_display_name, sizeof(gdi_display_name));
> 
> > > + memcpy((void *) gdi_display_name.DeviceName, adapter_name,
> 
> > > sizeof(TCHAR)* CCHDEVICENAME);
> 
> > > +
> 

> > This assume adapter_name is into a buffer of CCHDEVICENAME characters so
> 
> > can in theory overflow. As the destination buffer is already 0 filled
> 
> > would be better a strncpy style function. The void * conversion is useless
> 
> > and misleading.
> 
> > > + if (NT_SUCCESS(status = _pfnOpen_adapter_gdi_name(&gdi_display_name)))
> > > {
> 
> > > + return gdi_display_name.hAdapter;
> 
> > > + }
> 
> > > +
> 
> > > + vd_printf("%s: %S aurrrgghh nothing works..error is 0x%lx",
> 
> > > __FUNCTION__, adapter_name,
> 
> > > + status);
> 
> > > + return 0;
> 
> > > +}
> 
> > > +
> 
> > > +void WDDMInterface::close_adapter(D3D_HANDLE handle)
> 
> > > +{
> 
> > > + D3DKMT_CLOSEADAPTER closeData;
> 
> > > + if (handle) {
> 
> > > + closeData.hAdapter = handle;
> 
> > > + _pfnClose_adapter(&closeData);
> 
> > > + }
> 
> > > +}
> 
> > > +
> 
> > > +bool WDDMInterface::escape(LPCTSTR device_name, void* data, UINT
> > > size_data)
> 
> > > +{
> 
> > > + D3DKMT_ESCAPE escapeData;
> 
> > > + NTSTATUS status;
> 
> > > + D3D_HANDLE hAdapter(0);
> 
> > > +
> 
> > > + if (!(hAdapter = adapter_handle(device_name)))
> 
> > > + return false;
> 
> > > +
> 
> > > + escapeData.hAdapter = hAdapter;
> 
> > > + escapeData.hDevice = 0;
> 
> > > + escapeData.hContext = 0;
> 
> > > + escapeData.Type = D3DKMT_ESCAPE_DRIVERPRIVATE;
> 
> > > + escapeData.Flags.Value = 0;
> 
> > > + escapeData.pPrivateDriverData = data;
> 
> > > + escapeData.PrivateDriverDataSize = size_data;
> 
> > > +
> 
> > > + status = _pfnEscape(&escapeData);
> 
> > > +
> 
> > > + if (!NT_SUCCESS(status)) {
> 
> > > + vd_printf("%s: this should never happen. Status is 0x%lx",
> 
> > > __FUNCTION__, status);
> 
> > > + }
> 
> > > +
> 
> > > + //Close the handle to this device
> 
> > > + close_adapter(hAdapter);
> 
> > > + return NT_SUCCESS(status);
> 
> > > +}
> 
> > > +
> 
> > > CCD::CCD()
> 
> > > :_numPathElements(0)
> 
> > > ,_numModeElements(0)
> 
> > > diff --git a/vdagent/display_configuration.h
> 
> > > b/vdagent/display_configuration.h
> 
> > > index 05b35f4..e5ee90d 100755
> 
> > > --- a/vdagent/display_configuration.h
> 
> > > +++ b/vdagent/display_configuration.h
> 
> > > @@ -125,4 +125,51 @@ private:
> 
> > > bool find_best_mode(LPCTSTR Device, DEVMODE* dev_mode);
> 
> > > };
> 
> > >
> 
> > > +//DisplayConfig implementation for guest with WDDM graphics drivers
> 
> > > +typedef UINT D3D_HANDLE;
> 
> > > +
> 
> > > +struct D3DKMT_ESCAPE;
> 
> > > +struct D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME;
> 
> > > +struct D3DKMT_OPENADAPTERFROMDEVICENAME;
> 
> > > +struct D3DKMT_CLOSEADAPTER;
> 
> > > +struct D3DKMT_OPENADAPTERFROMHDC;
> 
> > > +
> 
> > > +typedef NTSTATUS(APIENTRY* PFND3DKMT_ESCAPE)(CONST D3DKMT_ESCAPE*);
> 
> > > +typedef NTSTATUS(APIENTRY*
> 
> > > PFND3DKMT_OPENADAPTERFROMGDIDISPLAYNAME)(D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME*);
> 
> > > +typedef NTSTATUS(APIENTRY*
> 
> > > PFND3DKMT_OPENADAPTERFROMDEVICENAME)(D3DKMT_OPENADAPTERFROMDEVICENAME*);
> 
> > > +typedef NTSTATUS(APIENTRY*
> > > PFND3DKMT_CLOSEADAPTER)(D3DKMT_CLOSEADAPTER*);
> 
> > > +typedef NTSTATUS(APIENTRY*
> 
> > > PFND3DKMT_OPENADAPTERFROMHDC)(D3DKMT_OPENADAPTERFROMHDC*);
> 
> > > +
> 
> > > +class WDDMInterface : public DisplayConfig {
> 
> > > +public:
> 
> > > + WDDMInterface();
> 
> > > + bool is_attached(DISPLAY_DEVICE* dev_info);
> 
> > > + bool set_monitor_state(LPCTSTR device_name, DEVMODE* dev_mode,
> 
> > > MONITOR_STATE state);
> 
> > > + LONG update_display_settings();
> 
> > > + bool custom_display_escape(LPCTSTR device_name, DEVMODE* dev_mode);
> 
> > > + 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();
> 
> > > +
> 
> > > +private:
> 
> > > + bool init_d3d_api();
> 
> > > + D3D_HANDLE adapter_handle(LPCTSTR device_name);
> 
> > > + D3D_HANDLE handle_from_DC(LPCTSTR adapter_name);
> 
> > > + D3D_HANDLE handle_from_device_name(LPCTSTR adapter_name);
> 
> > > + D3D_HANDLE handle_from_GDI_name(LPCTSTR adapter_name);
> 
> > > +
> 
> > > + void close_adapter(D3D_HANDLE handle);
> 
> > > + bool escape(LPCTSTR device_name, void* data, UINT sizeData);
> 
> > > +
> 
> > > + //GDI Function pointers
> 
> > > + PFND3DKMT_OPENADAPTERFROMHDC _pfnOpen_adapter_hdc;
> 
> > > + PFND3DKMT_CLOSEADAPTER _pfnClose_adapter;
> 
> > > + PFND3DKMT_ESCAPE _pfnEscape;
> 
> > > + PFND3DKMT_OPENADAPTERFROMDEVICENAME _pfnOpen_adapter_device_name;
> 
> > > + PFND3DKMT_OPENADAPTERFROMGDIDISPLAYNAME _pfnOpen_adapter_gdi_name;
> 
> > > +
> 
> > > + //object handles the CCD API
> 
> > > + CCD _ccd;
> 
> > > +};
> 
> > > +
> 
> > > #endif
> 
> > > \ No newline at end of file
> 

> > The update_ prefix is confusing. I would personally use
> > set/get/query/read/write.
> 
> > The reason is that is not clear if they are updating a class internal state
> > (so
> 
> > reading from the device) or updating the device state or a mix of the two.
> 

Frediano 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160815/0e287f43/attachment-0001.html>