[PATCH v2 3/4] winemac.drv: Fake success for changing non-primary adapter settings.

[Zhiyi Zhang]

Changing non-primary adapter settings is currently unsupported. Return
success for non-primary adapter settings changes so that the primary
adapter settings don't get changed unintentionally.

Signed-off-by: Zhiyi Zhang 
---
 dlls/winemac.drv/display.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c
index 968b9277bba..402f54982a5 100644
--- a/dlls/winemac.drv/display.c
+++ b/dlls/winemac.drv/display.c
@@ -805,6 +805,12 @@ LONG CDECL macdrv_ChangeDisplaySettingsEx(LPCWSTR devname, LPDEVMODEW devmode,
         devmode = &default_mode;
     }

+    if (lstrcmpiW(primary_adapter, devname))
+    {
+        FIXME("Changing non-primary adapter settings is currently unsupported.\n");
+        return DISP_CHANGE_SUCCESSFUL;
+    }
+
     if (macdrv_get_displays(&displays, &num_displays))
         return DISP_CHANGE_FAILED;

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=72250

Your paranoid android.

=== debiant (64 bit WoW report) ===

user32:
clipboard.c:760: Test failed: 2: gle 5
clipboard.c:765: Test failed: 2.0: got 0000 instead of 000d
clipboard.c:805: Test failed: 2: gle 1418
clipboard.c:815: Test failed: 2: count 4
clipboard.c:818: Test failed: 2: gle 1418
clipboard.c:833: Test failed: 2: gle 5
clipboard.c:838: Test failed: 2.0: got 0000 instead of 000d
clipboard.c:868: Test failed: 2: gle 1418

[Ken Thomases]

Most of the infrastructure is there to implement this properly.  You just have to figure out which element of the "displays" array corresponds to the devname and use that instead of displays[0].

-Ken
> On May 26, 2020, at 3:35 AM, Zhiyi Zhang  wrote:
>
> Changing non-primary adapter settings is currently unsupported. Return
> success for non-primary adapter settings changes so that the primary
> adapter settings don't get changed unintentionally.
> Signed-off-by: Zhiyi Zhang
>
> dlls/winemac.drv/display.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c
> index 968b9277bba..402f54982a5 100644
> --- a/dlls/winemac.drv/display.c
> +++ b/dlls/winemac.drv/display.c
> @@ -805,6 +805,12 @@ LONG CDECL macdrv_ChangeDisplaySettingsEx(LPCWSTR devname, LPDEVMODEW devmode,
>         devmode = &default_mode;
>     }
> - if (lstrcmpiW(primary_adapter, devname))
> - {
> -
>    FIXME("Changing non-primary adapter settings is currently unsupported.\n");
> -
>    return DISP_CHANGE_SUCCESSFUL;
> - }
> - if (macdrv_get_displays(&displays, &num_displays))
>       return DISP_CHANGE_FAILED;
>
> -- 
> 2.25.1

[Zhiyi Zhang]

Hi Ken,

Yes. There is enough to implement this properly. I am trying to introduce changes gradually
instead of one big patch. If passing non-primary adapters to ChangeDisplaySettingsExW() changes
the primary adapter's settings, it would cause test failures in between patches. I would like to
add the implementation to read/write multiple adapter settings to registry first, then to support
actually changing current display mode.

Thanks,
Zhiyi

On 5/27/20 4:16 AM, Ken Thomases wrote:
> Most of the infrastructure is there to implement this properly.  You just have to figure out which element of the "displays" array corresponds to the devname and use that instead of displays[0].
>
> -Ken
> > On May 26, 2020, at 3:35 AM, Zhiyi Zhang  wrote:
> >
> > Changing non-primary adapter settings is currently unsupported. Return
> > success for non-primary adapter settings changes so that the primary
> > adapter settings don't get changed unintentionally.
> > Signed-off-by: Zhiyi Zhang
> >
> > dlls/winemac.drv/display.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c
> > index 968b9277bba..402f54982a5 100644
> > --- a/dlls/winemac.drv/display.c
> > +++ b/dlls/winemac.drv/display.c
> > @@ -805,6 +805,12 @@ LONG CDECL macdrv_ChangeDisplaySettingsEx(LPCWSTR devname, LPDEVMODEW devmode,
> >         devmode = &default_mode;
> >     }
> > - if (lstrcmpiW(primary_adapter, devname))
> > - {
> > -
> >    FIXME("Changing non-primary adapter settings is currently unsupported.\n");
> > -
> >    return DISP_CHANGE_SUCCESSFUL;
> > - }
> > - if (macdrv_get_displays(&displays, &num_displays))
> >       return DISP_CHANGE_FAILED;
> >
> > -- 
> > 2.25.1

[Ken Thomases]

Hi Zhiyi,

Why do you need to fake success rather than return failure?  Is it so that ChangeDisplaySettingsExW() actually goes through with setting the registry settings?

-Ken
> On May 26, 2020, at 7:51 PM, Zhiyi Zhang  wrote:
>
> Hi Ken,
>
> Yes. There is enough to implement this properly. I am trying to introduce changes gradually
> instead of one big patch. If passing non-primary adapters to ChangeDisplaySettingsExW() changes
> the primary adapter's settings, it would cause test failures in between patches. I would like to
> add the implementation to read/write multiple adapter settings to registry first, then to support
> actually changing current display mode.
>
> Thanks,
> Zhiyi
>
> On 5/27/20 4:16 AM, Ken Thomases wrote:
> > Most of the infrastructure is there to implement this properly.  You just have to figure out which element of the "displays" array corresponds to the devname and use that instead of displays[0].
> >
> > -Ken
> > > On May 26, 2020, at 3:35 AM, Zhiyi Zhang  wrote:
> > >
> > > Changing non-primary adapter settings is currently unsupported. Return
> > > success for non-primary adapter settings changes so that the primary
> > > adapter settings don't get changed unintentionally.
> > > Signed-off-by: Zhiyi Zhang
> > >
> > > dlls/winemac.drv/display.c | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c
> > > index 968b9277bba..402f54982a5 100644
> > > --- a/dlls/winemac.drv/display.c
> > > +++ b/dlls/winemac.drv/display.c
> > > @@ -805,6 +805,12 @@ LONG CDECL macdrv_ChangeDisplaySettingsEx(LPCWSTR devname, LPDEVMODEW devmode,
> > >        devmode = &default_mode;
> > >    }
> > > - if (lstrcmpiW(primary_adapter, devname))
> > > - {
> > > -
> > >    FIXME("Changing non-primary adapter settings is currently unsupported.\n");
> > > -
> > >    return DISP_CHANGE_SUCCESSFUL;
> > > - }
> > > - if (macdrv_get_displays(&displays, &num_displays))
> > >      return DISP_CHANGE_FAILED;
> > >
> > > -- 
> > > 2.25.1

[Zhiyi Zhang]

Hi Ken,

Mainly because I don't want to change its behavior too much to avoid regressions as previously
changing non-primary adapter settings returns success. And yes, part of the reasons include that
ChangeDisplaySettingsExW can then update non-primary adapter registry settings without actually
affecting display mode.

Thanks,
Zhiyi

On 5/27/20 11:11 AM, Ken Thomases wrote:
> Hi Zhiyi,
>
> Why do you need to fake success rather than return failure?  Is it so that ChangeDisplaySettingsExW() actually goes through with setting the registry settings?
>
> -Ken
> > On May 26, 2020, at 7:51 PM, Zhiyi Zhang  wrote:
> >
> > Hi Ken,
> >
> > Yes. There is enough to implement this properly. I am trying to introduce changes gradually
> > instead of one big patch. If passing non-primary adapters to ChangeDisplaySettingsExW() changes
> > the primary adapter's settings, it would cause test failures in between patches. I would like to
> > add the implementation to read/write multiple adapter settings to registry first, then to support
> > actually changing current display mode.
> >
> > Thanks,
> > Zhiyi
> >
> > On 5/27/20 4:16 AM, Ken Thomases wrote:
> > > Most of the infrastructure is there to implement this properly.  You just have to figure out which element of the "displays" array corresponds to the devname and use that instead of displays[0].
> > >
> > > -Ken
> > > > On May 26, 2020, at 3:35 AM, Zhiyi Zhang  wrote:
> > > >
> > > > Changing non-primary adapter settings is currently unsupported. Return
> > > > success for non-primary adapter settings changes so that the primary
> > > > adapter settings don't get changed unintentionally.
> > > > Signed-off-by: Zhiyi Zhang
> > > >
> > > > dlls/winemac.drv/display.c | 6 ++++++
> > > > 1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c
> > > > index 968b9277bba..402f54982a5 100644
> > > > --- a/dlls/winemac.drv/display.c
> > > > +++ b/dlls/winemac.drv/display.c
> > > > @@ -805,6 +805,12 @@ LONG CDECL macdrv_ChangeDisplaySettingsEx(LPCWSTR devname, LPDEVMODEW devmode,
> > > >        devmode = &default_mode;
> > > >    }
> > > > - if (lstrcmpiW(primary_adapter, devname))
> > > > - {
> > > > -
> > > >    FIXME("Changing non-primary adapter settings is currently unsupported.\n");
> > > > -
> > > >    return DISP_CHANGE_SUCCESSFUL;
> > > > - }
> > > > - if (macdrv_get_displays(&displays, &num_displays))
> > > >      return DISP_CHANGE_FAILED;
> > > >
> > > > -- 
> > > > 2.25.1

[Ken Thomases]

Signed-off-by: Ken Thomases