[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 15:44:08 CET 2016
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.
>
> +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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160211/ee97b737/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/ee97b737/attachment-0008.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/ee97b737/attachment-0009.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/ee97b737/attachment-0010.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/ee97b737/attachment-0011.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/ee97b737/attachment-0012.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/ee97b737/attachment-0013.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/ee97b737/attachment-0014.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/ee97b737/attachment-0015.png>
More information about the Spice-devel
mailing list