[Spice-devel] [vd-agent-win32 2/2] Use CCD API to update the WDDM display modes.
Javier Celaya
javier.celaya at flexvdi.com
Thu Feb 11 16:50:00 CET 2016
El 11/02/16 a las 15:44, Javier Celaya escribió:
>
>
> El 11/02/16 a las 12:04, Frediano Ziglio escribió:
>>
>>
>> Hi, I answer inline
>>
>> El 10/02/16 a las 16:33, Frediano Ziglio escribió:
>>
>> When a new custom display mode is added, the current WDDM driver notifies
>> a disconnection and reconnection of the virtual monitor to force Windows
>> to update the display modes. This produces an ugly effect, keeping the
>> screen black for up to some seconds and usually not repainting it
>> afterwards.
>>
>> This patch uses the CCD API to update the display modes, and produces
>> just a quick flash followed by a whole screen repaint. For best results,
>> it should be used with a driver that does not update the display modes
>> by itself, but it is still compatible with the current implementation.
>> ---
>> common/vdcommon.h | 40 +++++++++++++++++++++++++++
>> vdagent/desktop_layout.cpp | 68
>> +++++++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 107 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/vdcommon.h b/common/vdcommon.h
>> index f40e68e..29437c3 100644
>> --- a/common/vdcommon.h
>> +++ b/common/vdcommon.h
>> @@ -102,6 +102,46 @@ private:
>> PFND3DKMT_OPENADAPTERFROMHDC _pD3DKMTOpenAdapterFromHdc;
>> };
>>
>> +// CCD API
>> +#define SDC_APPLY 0x00000080
>> +#define SDC_USE_SUPPLIED_DISPLAY_CONFIG 0x00000020
>> +#define SDC_FORCE_MODE_ENUMERATION 0x00001000
>> +#define QDC_ALL_PATHS 1
>> +struct DISPLAYCONFIG_PATH_INFO;
>> +struct DISPLAYCONFIG_MODE_INFO;
>> +struct DISPLAYCONFIG_TOPOLOGY_ID;
>> +typedef LONG (*PFNGETDISPLAYCONFIGBUFFERSIZES)(UINT32, UINT32*, UINT32*);
>> +typedef LONG (*PFNQUERYDISPLAYCONFIG)(UINT32, UINT32*,
>> DISPLAYCONFIG_PATH_INFO*, UINT32*,
>> + DISPLAYCONFIG_MODE_INFO*,
>> DISPLAYCONFIG_TOPOLOGY_ID*);
>> +typedef LONG (*PFNSETDISPLAYCONFIG)(UINT32, DISPLAYCONFIG_PATH_INFO*,
>> UINT32,
>> + DISPLAYCONFIG_MODE_INFO*, UINT32);
>>
>>
>> So your system headers does not have definitions for these!
>> My MingW have these definition in wingdi.h but you have to define
>> WINVER >= 0x0601 before including the file.
>> If your environment does not have such defines I would suggest to put
>> these into a compat_ccd.h (for instance) file so
>> is easy to remove in a future.
> I don't know how I missed that... Even the CCD API functions are
> there! So there is no need for the CCDLibrary class either, unless we
> want compatibility with quite old MinGW versions, but I don't see the
> point.
Well, not so old, actually. The type definitions and function
declarations were added to the headers on December 2013, but the
functions are undefined in libuser32.a until April 2015. My MinGW build
(from Fedora 22) issues undefined reference errors for all three
functions, so I'm going to consider your suggestion of a
ccd_compat.{h,c} file.
>>
>> +class CCDLibrary {
>> +public:
>> + ~CCDLibrary();
>> + static CCDLibrary& singleton() {
>> + static CCDLibrary instance;
>> + return instance;
>> + }
>> + bool found() {
>> + return _hUser32 && _pGetDisplayConfigBufferSizes &&
>> + _pQuieryDisplayConfig && _pSetDisplayConfig;
>> + }
>>
>> Why don't you return a CCDLibrary* from singleton returning NULL if
>> you couldn't find the required APIs?
>> This would avoid to expose a not initialized object and would remove
>> the need for a found method.
>>
>> + LONG get_display_config_buffer_sizes(UINT32* num_paths, UINT32*
>> num_modes);
>> + LONG query_display_config(UINT32* num_paths, DISPLAYCONFIG_PATH_INFO*
>> paths,
>> + UINT32* num_modes, DISPLAYCONFIG_MODE_INFO*
>> modes);
>> + LONG set_display_config(UINT32 num_paths, DISPLAYCONFIG_PATH_INFO*
>> paths,
>> + UINT32 num_modes, DISPLAYCONFIG_MODE_INFO*
>> modes,
>> + UINT32 flags);
>> +
>> +private:
>> + CCDLibrary();
>>
>> probably here you want also the copy constructor and assignment operator.
>>
>> +
>> + HINSTANCE _hUser32;
>> + PFNGETDISPLAYCONFIGBUFFERSIZES _pGetDisplayConfigBufferSizes;
>> + PFNQUERYDISPLAYCONFIG _pQuieryDisplayConfig;
>> + PFNSETDISPLAYCONFIG _pSetDisplayConfig;
>> +};
>> +
>> #if defined __GNUC__
>> #define ALIGN_GCC __attribute__ ((packed))
>> #define ALIGN_VC
>> diff --git a/vdagent/desktop_layout.cpp b/vdagent/desktop_layout.cpp
>> index b5e8cfa..92493f3 100644
>> --- a/vdagent/desktop_layout.cpp
>> +++ b/vdagent/desktop_layout.cpp
>> @@ -312,6 +312,70 @@ NTSTATUS D3DKMTLibrary::close_adapter(D3DKMT_HANDLE
>> adapter)
>> return _pD3DKMTCloseAdapter(&func_params);
>> }
>>
>> +CCDLibrary::CCDLibrary()
>> + : _pGetDisplayConfigBufferSizes(NULL)
>> + , _pQuieryDisplayConfig(NULL)
>> + , _pSetDisplayConfig(NULL)
>> +{
>> + _hUser32 = LoadLibrary(L"User32.dll");
>>
>> probably you want a GetModuleHandle, user32 have to be already available.
>>
>> Ok, I'll send another patch
>>
>> + if (_hUser32) {
>> + _pGetDisplayConfigBufferSizes =
>> (PFNGETDISPLAYCONFIGBUFFERSIZES)GetProcAddress(_hUser32,
>> "GetDisplayConfigBufferSizes");
>> + _pQuieryDisplayConfig =
>> (PFNQUERYDISPLAYCONFIG)GetProcAddress(_hUser32, "QueryDisplayConfig");
>> + _pSetDisplayConfig = (PFNSETDISPLAYCONFIG)GetProcAddress(_hUser32,
>> "SetDisplayConfig");
>> + }
>> +}
>> +
>> +
>> +CCDLibrary::~CCDLibrary()
>> +{
>> + if (_hUser32)
>> + FreeLibrary(_hUser32);
>>
>> If you use GetModuleHandle this is not necessary
>>
>> +}
>> +
>> +LONG CCDLibrary::get_display_config_buffer_sizes(UINT32* num_paths, UINT32*
>> num_modes)
>> +{
>> + return _pGetDisplayConfigBufferSizes(QDC_ALL_PATHS, num_paths,
>> num_modes);
>> +}
>> +
>> +LONG CCDLibrary::query_display_config(UINT32* num_paths,
>> DISPLAYCONFIG_PATH_INFO* paths,
>> + UINT32* num_modes,
>> DISPLAYCONFIG_MODE_INFO* modes)
>> +{
>> + return _pQuieryDisplayConfig(QDC_ALL_PATHS, num_paths, paths, num_modes,
>> modes, NULL);
>> +}
>> +
>> +LONG CCDLibrary::set_display_config(UINT32 num_paths,
>> DISPLAYCONFIG_PATH_INFO* paths,
>> + UINT32 num_modes,
>> DISPLAYCONFIG_MODE_INFO* modes,
>> + UINT32 flags)
>> +{
>> + return _pSetDisplayConfig(num_paths, paths, num_modes, modes, flags);
>> +}
>> +
>> +static void update_display_modes()
>> +{
>> + // The trick here is to call SetDisplayConfig with the current config
>> and
>> + // the SDC_FORCE_MODE_ENUMERATION flag
>> + CCDLibrary& ccd = CCDLibrary::singleton();
>> + if (!ccd.found()) return;
>> + UINT32 num_paths = 0, num_modes = 0;
>> + DISPLAYCONFIG_PATH_INFO * paths;
>> + DISPLAYCONFIG_MODE_INFO * modes;
>> + LONG status = ccd.get_display_config_buffer_sizes(&num_paths,
>> &num_modes);
>> + if (NT_SUCCESS(status)) {
>> + // Being conservative, struct sizes are under 100 bytes
>> + paths = reinterpret_cast<DISPLAYCONFIG_PATH_INFO *>(new
>> char[num_paths * 100]);
>> + modes = reinterpret_cast<DISPLAYCONFIG_MODE_INFO *>(new
>> char[num_modes * 100]);
>>
>> why not
>>
>> paths = new DISPLAYCONFIG_PATH_INFO[num_paths];
>> modes = new DISPLAYCONFIG_MODE_INFO[num_modes];
>>
>>
>> once defined the types you could also use
>>
>> std::vector<DISPLAYCONFIG_PATH_INFO> paths(num_paths);
>> std::vector<DISPLAYCONFIG_MODE_INFO> modes(num_modes);
>>
>> This is to avoid having to fully define both
>> DISPLAYCONFIG_PATH_INFO and DISPLAYCONFIG_MODE_INFO structs. The
>> definition is quite long, and I am not using any of their
>> members. Just filling two buffers with query_display_config and
>> passing them to set_display_config. But if you think it is
>> cleaner to include the full definition, I can do so in the next
>> version of the patch.
>>
>> + status = ccd.query_display_config(&num_paths, paths, &num_modes,
>> modes);
>>
>>
>> Here you would use paths.front()
>>
>> + if (NT_SUCCESS(status)) {
>> + ccd.set_display_config(num_paths, paths, num_modes, modes,
>> + SDC_APPLY |
>> + SDC_USE_SUPPLIED_DISPLAY_CONFIG |
>> + SDC_FORCE_MODE_ENUMERATION);
>> + }
>> + delete[] reinterpret_cast<char *>(paths);
>> + delete[] reinterpret_cast<char *>(modes);
>>
>> and here
>>
>> delete[] paths;
>> delete[] modes;
>>
>>
>> And here nothing!
>>
>> Frediano
>>
>> Same here
>>
>> Frediano
>>
>> + }
>> +}
>> +
>> bool DesktopLayout::init_dev_mode(LPCTSTR dev_name, DEVMODE* dev_mode,
>> DisplayMode* mode,
>> LONG normal_x, LONG normal_y, bool
>> set_pos)
>> {
>> @@ -384,7 +448,9 @@ bool DesktopLayout::init_dev_mode(LPCTSTR dev_name,
>> DEVMODE* dev_mode, DisplayMo
>> vd_printf("escape xres = %d, yres = %d, bpp = %d", custom.xres,
>> custom.yres, custom.bpp);
>> NTSTATUS status = d3dkmt.escape_driver_private(adapter, &custom,
>> sizeof(QXLEscapeSetCustomDisplay));
>> - if (!NT_SUCCESS(status)) {
>> + if (NT_SUCCESS(status)) {
>> + update_display_modes();
>> + } else {
>> vd_printf("escape failed with error 0x%08X", status);
>> }
>> }
>>
>>
>> --
>>
>>
>>
>> <http://flexvdi.com><http://flexvdi.com>
>>
>> Javier Celaya
>>
>> Software Engineer
>>
>>
>>
>> j <mailto:javier.celaya at flexvdi.com>avier.celaya at flexvdi.com
>>
>> +34 876 60 00 73
>>
>> @j_celaya
>>
>>
>
> --
>
>
>
> <http://flexvdi.com><http://flexvdi.com>
>
> Javier Celaya
>
> Software Engineer
>
>
>
> j <mailto:javier.celaya at flexvdi.com>avier.celaya at flexvdi.com
>
> +34 876 60 00 73
>
> @j_celaya
>
--
<http://flexvdi.com><http://flexvdi.com>
Javier Celaya
Software Engineer
j <mailto:javier.celaya at flexvdi.com>avier.celaya at flexvdi.com
+34 876 60 00 73
@j_celaya
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/9f2f6509/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 17075 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/9f2f6509/attachment-0012.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 9350 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/9f2f6509/attachment-0013.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 15099 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/9f2f6509/attachment-0014.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 14444 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/9f2f6509/attachment-0015.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 17075 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/9f2f6509/attachment-0016.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 9350 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/9f2f6509/attachment-0017.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 15099 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/9f2f6509/attachment-0018.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 14444 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/9f2f6509/attachment-0019.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: logo.flexvdi.png
Type: image/png
Size: 17075 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/9f2f6509/attachment-0020.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mail.png
Type: image/png
Size: 9350 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/9f2f6509/attachment-0021.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: phone.png
Type: image/png
Size: 15099 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/9f2f6509/attachment-0022.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: skype.png
Type: image/png
Size: 14444 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/9f2f6509/attachment-0023.png>
More information about the Spice-devel
mailing list