[Spice-devel] [PATCH] Add support for resolution 800x600

[Christophe de Dinechin]

> On 19 Feb 2018, at 14:35, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
>> 
>> https://bugzilla.redhat.com/show_bug.cgi?id=1477492
>> https://docs.microsoft.com/en-us/windows-hardware/design/minimum/minimum-hardware-requirements-overview
>> requires 800x600 to be supported.
>> 
>> Signed-off-by: Yuri Benditovich <yuri.benditovich at daynix.com>
>> ---
>> qxldod/QxlDod.cpp | 28 ++++++++++++++--------------
>> qxldod/QxlDod.h   |  6 ++++--
>> 2 files changed, 18 insertions(+), 16 deletions(-)
>> 
>> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
>> index 4f508bd..ee97b09 100755
>> --- a/qxldod/QxlDod.cpp
>> +++ b/qxldod/QxlDod.cpp
>> @@ -2572,15 +2572,15 @@ NTSTATUS
>> VgaDevice::GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo)
>> 
>>         DbgPrint(TRACE_LEVEL_INFORMATION, ("ModeTemp = 0x%X %dx%d@%d\n",
>>         ModeTemp, tmpModeInfo.XResolution, tmpModeInfo.YResolution,
>>         tmpModeInfo.BitsPerPixel));
>> 
>> -        if (tmpModeInfo.XResolution >= MIN_WIDTH_SIZE &&
>> -            tmpModeInfo.YResolution >= MIN_HEIGHT_SIZE &&
>> +        if (tmpModeInfo.XResolution >= MINIMAL_WIDTH &&
>> +            tmpModeInfo.YResolution >= MINIMAL_HEIGHT &&
>>             tmpModeInfo.BitsPerPixel == BitsPerPixel &&
>>             tmpModeInfo.PhysBasePtr != 0)
>>         {
>>             m_ModeNumbers[SuitableModeCount] = ModeTemp;
>>             SetVideoModeInfo(SuitableModeCount, &tmpModeInfo);
>> -            if (tmpModeInfo.XResolution == MIN_WIDTH_SIZE &&
>> -                tmpModeInfo.YResolution == MIN_HEIGHT_SIZE)
>> +            if (tmpModeInfo.XResolution == INITIAL_WIDTH &&
>> +                tmpModeInfo.YResolution == INITIAL_HEIGHT)
>>             {
>>                 m_CurrentMode = (USHORT)SuitableModeCount;
>>             }
>> @@ -3186,8 +3186,8 @@ NTSTATUS
>> QxlDevice::GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo)
>>     UINT BitsPerPixel = BPPFromPixelFormat(pDispInfo->ColorFormat);
>>     if (Width == 0 || Height == 0 || BitsPerPixel != QXL_BPP)
>>     {
>> -        Width = MIN_WIDTH_SIZE;
>> -        Height = MIN_HEIGHT_SIZE;
>> +        Width = MINIMAL_WIDTH;
>> +        Height = MINIMAL_HEIGHT;
>>         BitsPerPixel = QXL_BPP;
>>     }
>> 
> 
> Why not setting to the initial values here?
> 
>> @@ -3200,14 +3200,14 @@ NTSTATUS
>> QxlDevice::GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo)
>> 
>>         DbgPrint(TRACE_LEVEL_INFORMATION, ("%s: modes[%d] x_res = %d, y_res
>>         = %d, bits = %d BitsPerPixel = %d\n", __FUNCTION__, CurrentMode,
>>         tmpModeInfo->x_res, tmpModeInfo->y_res, tmpModeInfo->bits,
>>         BitsPerPixel));
>> 
>> -        if (tmpModeInfo->x_res >= MIN_WIDTH_SIZE &&
>> -            tmpModeInfo->y_res >= MIN_HEIGHT_SIZE &&
>> +        if (tmpModeInfo->x_res >= MINIMAL_WIDTH &&
>> +            tmpModeInfo->y_res >= MINIMAL_HEIGHT &&
>>             tmpModeInfo->bits == QXL_BPP)
>>         {
>>             m_ModeNumbers[SuitableModeCount] = SuitableModeCount;
>>             SetVideoModeInfo(SuitableModeCount, tmpModeInfo);
>> -            if (tmpModeInfo->x_res == MIN_WIDTH_SIZE &&
>> -                tmpModeInfo->y_res == MIN_HEIGHT_SIZE)
>> +            if (tmpModeInfo->x_res == INITIAL_WIDTH &&
>> +                tmpModeInfo->y_res == INITIAL_HEIGHT)
>>             {
>>                 m_CurrentMode = SuitableModeCount;
>>             }
>> @@ -4883,9 +4883,9 @@ NTSTATUS
>> QxlDevice::SetCustomDisplay(QXLEscapeSetCustomDisplay* custom_display)
>>     UINT yres = custom_display->yres;
>>     UINT bpp = QXL_BPP;
>>     DbgPrint(TRACE_LEVEL_WARNING, ("%s - %d (%dx%d#%d)\n", __FUNCTION__,
>>     m_Id, xres, yres, bpp));
>> -    if (xres < MIN_WIDTH_SIZE || yres < MIN_HEIGHT_SIZE) {
>> +    if (xres < MINIMAL_WIDTH || yres < MINIMAL_HEIGHT) {
>>         DbgPrint(TRACE_LEVEL_VERBOSE, ("%s: (%dx%d#%d) less than (%dx%d)\n",
>>         __FUNCTION__,
>> -            xres, yres, bpp, MIN_WIDTH_SIZE, MIN_HEIGHT_SIZE));
>> +            xres, yres, bpp, MINIMAL_WIDTH, MINIMAL_HEIGHT));
>>     }
>>     m_CustomMode =(USHORT) ((m_CustomMode == m_ModeCount-1)?  m_ModeCount -
>>     2 : m_ModeCount - 1);
>> 
>> @@ -5147,8 +5147,8 @@ NTSTATUS
>> HwDeviceInterface::AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInf
>>     if (DispInfo.Width == 0)
>>     {
>>         DispInfo.ColorFormat = D3DDDIFMT_A8R8G8B8;
>> -        DispInfo.Width = MIN_WIDTH_SIZE;
>> -        DispInfo.Height = MIN_HEIGHT_SIZE;
>> +        DispInfo.Width = INITIAL_WIDTH;
>> +        DispInfo.Height = INITIAL_HEIGHT;
>>         DispInfo.Pitch = DispInfo.Width *
>>         BPPFromPixelFormat(DispInfo.ColorFormat) / BITS_PER_BYTE;
>>         DispInfo.TargetId = 0;
>>     }
>> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
>> index 695b83a..ba7e6a7 100755
>> --- a/qxldod/QxlDod.h
>> +++ b/qxldod/QxlDod.h
>> @@ -19,8 +19,10 @@
>> #define BITS_PER_BYTE              8
>> 
>> #define POINTER_SIZE               64
>> -#define MIN_WIDTH_SIZE             1024
>> -#define MIN_HEIGHT_SIZE            768
>> +#define MINIMAL_WIDTH              800
>> +#define MINIMAL_HEIGHT             600
>> +#define INITIAL_WIDTH              1024
>> +#define INITIAL_HEIGHT             768
>> #define QXL_BPP                    32
>> #define VGA_BPP                    24
>> 
> 
> Patch is good but the renames MIN_WIDTH_SIZE -> MINIMAL_WIDTH and MIN_HEIGHT_SIZE -> MINIMAL_HEIGHT
> are not really necessary and I don't think the new names are more clear than the old ones.

Well, clearer I don’t know, but closer to regular English syntax, they are. “Min width size” means the size of the width, which does not make much sense. So unless you strongly object, I’d keep that rename.

> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel