Re: ddraw: Implement ForceRefreshRate registry entry for overriding DirectX refresh rate

[Denver Gingerich]

On Mon, Mar 10, 2008 at 12:44 PM, Denver Gingerich <denver(a)ossguy.com> wrote:
> The DirectX refresh rate override feature that this patch implements is described in main.c below.
>
> In order for the override to take effect only on SetDisplayMode and not on RestoreDisplayMode it was necessary to move the functionality of SetDisplayMode into a helper function (IDirectDrawImpl_SetDisplayModeNoOverride) and let IDirectDrawImpl_SetDisplayMode and IDirectDrawImpl_RestoreDisplayMode call the helper function.
>
> Denver Gingerich
>
> diff --git a/dlls/ddraw/ddraw.c b/dlls/ddraw/ddraw.c
>  index 4cac55d..0f7eeaf 100644
>  --- a/dlls/ddraw/ddraw.c
>  +++ b/dlls/ddraw/ddraw.c
>  @@ -3,6 +3,7 @@
>   * Copyright 1998-2000 Lionel Ulmer
>   * Copyright 2000-2001 TransGaming Technologies Inc.
>   * Copyright 2006 Stefan Dösinger
>  + * Copyright 2008 Denver Gingerich
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
>  @@ -60,6 +61,9 @@ static const DDDEVICEIDENTIFIER2 deviceidentifier =
>      0
>   };
>
> +/* in main.c; stores value of ForceRefreshRate */
>  +extern DWORD force_refresh_rate;
>  +
>   /*****************************************************************************
>   * IUnknown Methods
>   *****************************************************************************/
>  @@ -534,34 +538,24 @@ IDirectDrawImpl_SetCooperativeLevel(IDirectDraw7 *iface,
>   }
>
> /*****************************************************************************
>  - * IDirectDraw7::SetDisplayMode
>   *
>  - * Sets the display screen resolution, color depth and refresh frequency
>  - * when in fullscreen mode (in theory).
>  - * Possible return values listed in the SDK suggest that this method fails
>  - * when not in fullscreen mode, but this is wrong. Windows 2000 happily sets
>  - * the display mode in DDSCL_NORMAL mode without an hwnd specified.
>  - * It seems to be valid to pass 0 for With and Height, this has to be tested
>  - * It could mean that the current video mode should be left as-is. (But why
>  - * call it then?)
>  + * Helper function for SetDisplayMode and RestoreDisplayMode
>   *
>  - * Params:
>  - *  Height, Width: Screen dimension
>  - *  BPP: Color depth in Bits per pixel
>  - *  Refreshrate: Screen refresh rate
>  - *  Flags: Other stuff
>  - *
>  - * Returns
>  - *  DD_OK on success
>  + * Implements DirectDraw's SetDisplayMode, but ignores the value of
>  + * ForceRefreshRate, since it is already handled by
>  + * IDirectDrawImpl_SetDisplayMode.  RestoreDisplayMode can use this function
>  + * without worrying that ForceRefreshRate will override the refresh rate.  For
>  + * argument and return value documentation, see
>  + * IDirectDrawImpl_SetDisplayMode.
>   *
>   *****************************************************************************/
>  -static HRESULT WINAPI
>  -IDirectDrawImpl_SetDisplayMode(IDirectDraw7 *iface,
>  -                               DWORD Width,
>  -                               DWORD Height,
>  -                               DWORD BPP,
>  -                               DWORD RefreshRate,
>  -                               DWORD Flags)
>  +static HRESULT
>  +IDirectDrawImpl_SetDisplayModeNoOverride(IDirectDraw7 *iface,
>  +                                         DWORD Width,
>  +                                         DWORD Height,
>  +                                         DWORD BPP,
>  +                                         DWORD RefreshRate,
>  +                                         DWORD Flags)
>   {
>      ICOM_THIS_FROM(IDirectDrawImpl, IDirectDraw7, iface);
>      WINED3DDISPLAYMODE Mode;
>  @@ -616,6 +610,46 @@ IDirectDrawImpl_SetDisplayMode(IDirectDraw7 *iface,
>   }
>
> /*****************************************************************************
>  + * IDirectDraw7::SetDisplayMode
>  + *
>  + * Sets the display screen resolution, color depth and refresh frequency
>  + * when in fullscreen mode (in theory).
>  + * Possible return values listed in the SDK suggest that this method fails
>  + * when not in fullscreen mode, but this is wrong. Windows 2000 happily sets
>  + * the display mode in DDSCL_NORMAL mode without an hwnd specified.
>  + * It seems to be valid to pass 0 for With and Height, this has to be tested
>  + * It could mean that the current video mode should be left as-is. (But why
>  + * call it then?)
>  + *
>  + * Params:
>  + *  Height, Width: Screen dimension
>  + *  BPP: Color depth in Bits per pixel
>  + *  Refreshrate: Screen refresh rate
>  + *  Flags: Other stuff
>  + *
>  + * Returns
>  + *  DD_OK on success
>  + *
>  + *****************************************************************************/
>  +static HRESULT WINAPI
>  +IDirectDrawImpl_SetDisplayMode(IDirectDraw7 *iface,
>  +                               DWORD Width,
>  +                               DWORD Height,
>  +                               DWORD BPP,
>  +                               DWORD RefreshRate,
>  +                               DWORD Flags)
>  +{
>  +    if (force_refresh_rate != 0)
>  +    {
>  +        TRACE("ForceRefreshRate overriding passed-in refresh rate (%d Hz) to %d Hz\n", RefreshRate, force_refresh_rate);
>  +        RefreshRate = force_refresh_rate;
>  +    }
>  +
>  +    return IDirectDrawImpl_SetDisplayModeNoOverride(iface, Width, Height, BPP,
>  +                                                    RefreshRate, Flags);
>  +}
>  +
>  +/*****************************************************************************
>   * IDirectDraw7::RestoreDisplayMode
>   *
>   * Restores the display mode to what it was at creation time. Basically.
>  @@ -642,12 +676,12 @@ IDirectDrawImpl_RestoreDisplayMode(IDirectDraw7 *iface)
>      ICOM_THIS_FROM(IDirectDrawImpl, IDirectDraw7, iface);
>      TRACE("(%p)\n", This);
>
> -    return IDirectDraw7_SetDisplayMode(ICOM_INTERFACE(This, IDirectDraw7),
>  -                                       This->orig_width,
>  -                                       This->orig_height,
>  -                                       This->orig_bpp,
>  -                                       0,
>  -                                       0);
>  +    return IDirectDrawImpl_SetDisplayModeNoOverride(ICOM_INTERFACE(This, IDirectDraw7),
>  +                                                    This->orig_width,
>  +                                                    This->orig_height,
>  +                                                    This->orig_bpp,
>  +                                                    0,
>  +                                                    0);
>   }
>
> /*****************************************************************************
>  diff --git a/dlls/ddraw/main.c b/dlls/ddraw/main.c
>  index 4e77d4f..a33cb34 100644
>  --- a/dlls/ddraw/main.c
>  +++ b/dlls/ddraw/main.c
>  @@ -4,6 +4,7 @@
>   * Copyright 1998 Lionel Ulmer
>   * Copyright 2000-2001 TransGaming Technologies Inc.
>   * Copyright 2006 Stefan Dösinger
>  + * Copyright 2008 Denver Gingerich
>   *
>   * This file contains the (internal) driver registration functions,
>   * driver enumeration APIs and DirectDraw creation functions.
>  @@ -68,6 +69,9 @@ static CRITICAL_SECTION_DEBUG ddraw_cs_debug =
>   };
>   CRITICAL_SECTION ddraw_cs = { &ddraw_cs_debug, -1, 0, 0, 0, 0 };
>
> +/* value of ForceRefreshRate */
>  +DWORD force_refresh_rate = 0;
>  +
>   /***********************************************************************
>   *
>   * Helper function for DirectDrawCreate and friends
>  @@ -886,6 +890,38 @@ DllMain(HINSTANCE hInstDLL,
>              }
>          }
>
> +        /* On Windows one can force the refresh rate that DirectDraw uses by
>  +         * setting an override value in dxdiag.  This is documented in KB315614
>  +         * (main article), KB230002, and KB217348.  By comparing registry dumps
>  +         * before and after setting the override, we see that the override value
>  +         * is stored in HKLM\Software\Microsoft\DirectDraw\ForceRefreshRate as a
>  +         * DWORD that represents the refresh rate to force.  We use this
>  +         * registry entry to modify the behavior of SetDisplayMode so that Wine
>  +         * users can override the refresh rate in a Windows-compatible way.
>  +         *
>  +         * dxdiag will not accept a refresh rate lower than 40 or higher than
>  +         * 120 so this value should be within that range.  It is, of course,
>  +         * possible for a user to set the registry entry value directly so that
>  +         * assumption might not hold.
>  +         *
>  +         * There is no current mechanism for setting this value through the Wine
>  +         * GUI.  It would be most appropriate to set this value through a dxdiag
>  +         * clone, but it may be sufficient to use winecfg.
>  +         *
>  +         * TODO: Create a mechanism for setting this value through the Wine GUI.
>  +         */
>  +        if ( !RegOpenKeyA( HKEY_LOCAL_MACHINE, "Software\\Microsoft\\DirectDraw", &hkey ) )
>  +        {
>  +            DWORD type, data;
>  +            size = sizeof(data);
>  +            if (!RegQueryValueExA( hkey, "ForceRefreshRate", NULL, &type, (LPBYTE)&data, &size ) && type == REG_DWORD)
>  +            {
>  +                TRACE("ForceRefreshRate set; overriding refresh rate to %d Hz\n", data);
>  +                force_refresh_rate = data;
>  +            }
>  +            RegCloseKey( hkey );
>  +        }
>  +
>          DisableThreadLibraryCalls(hInstDLL);
>      }
>      else if (Reason == DLL_PROCESS_DETACH)

This patch differs from my previous patch [1] in the following ways:
a) DDRAW_SetDisplayMode renamed to IDirectDrawImpl_SetDisplayModeNoOverride
b) sentence in main.c description changed from

"To ensure compatibility with Windows, we use this registry entry to
modify the behavior of SetDisplayMode."

to

"We use this registry entry to modify the behavior of SetDisplayMode
so that Wine users can override the refresh rate in a
Windows-compatible way."


Hopefully these changes are sufficient for the patch to be accepted.
Please let me know if there are further concerns with it.  Thanks.

Denver


1. http://www.winehq.org/pipermail/wine-patches/2008-March/051213.html