Detect screen resolution change errors

[Francois Gouget]

When I run the dlls/d3d9/device conformance test on Windows in VMware, I 
get a trace telling me that IDirect3D9_CreateDevice() failed and 
returned 0x8876086c. This seems to be because Windows has trouble 
changing the screen resolution when running in VMware (though it's 
perfectly capable of doing it when I go through the control panel).

But when I run the test in Wine I'm getting a test failure because:
  * IDirect3D9_CreateDevice() did not change the screen resolution
  * but it returned success so that the screen size checks fail

That's because XRandR fails to change the resolution (for whatever 
reason, it's irrelevant here). Wine detects that but does not report the 
information to the upper layers. So I have made a couple of patches:
  * the first one detects when XRandR, XVidMode or some low level 
routine fails to change the screen resolution, and propagates this so 
ChangeDisplaySettingsEx() returns an error
  * the second one modifies 
IWineD3DDeviceImpl_CreateAdditionalSwapChain() so it detects when 
ChangeDisplaySettingsEx() propagates that error upstream too

Now the dlls/d3d9/device test succeeds on my machine (all while loudly 
complaining something is wrong):
[...]
err:xrandr:X11DRV_XRandR_SetCurrentMode Resolution change not successful 
-- perhaps display has changed?
fixme:d3d9:IDirect3DDevice9Impl_CreateAdditionalSwapChain (0x1c4358) 
call to IWineD3DDevice_CreateAdditionalSwapChain failed
fixme:d3d9:IDirect3D9Impl_CreateDevice (0x171448) D3D Initialization 
failed for WineD3DDevice 0x174ee8
device.c:741:could not create device, IDirect3D9_CreateDevice returned 
0x8876086c
device.c: 245 tests executed (0 marked as todo, 0 failures), 0 skipped.

I have attached the relevant patches. Does this look ok to the DirectX 
gurus?

[H. Verbeet]

On 30/01/07, Francois Gouget  wrote:
> I have attached the relevant patches. Does this look ok to the DirectX
> gurus?

At least the wined3d part looks ok to me, although we should probably
be cleaning up the GL context as well, there and in the other places
where we return.

[Stefan Dösinger]

Am Mittwoch 31 Januar 2007 10:25 schrieb H. Verbeet:
> On 30/01/07, Francois Gouget  wrote:
> > I have attached the relevant patches. Does this look ok to the DirectX
> > gurus?
>
> At least the wined3d part looks ok to me, although we should probably
> be cleaning up the GL context as well, there and in the other places
> where we return.

For some reason I didn't get the original patch, or I accidentally deleted 
it(more likely). Note that I have patches to rework the context creation and 
initialization routines(for easier cleanup).

I do not know to what extend we want to catch screen res change failures. The 
resolution? I'd say yes. Color depth? Propably no because we can't change it 
anyway. Refresh rate? Don't think its important.

[Francois Gouget]

Stefan Dösinger wrote:
> Am Mittwoch 31 Januar 2007 10:25 schrieb H. Verbeet:
> > On 30/01/07, Francois Gouget  wrote:
> > > I have attached the relevant patches. Does this look ok to the DirectX
> > > gurus?
> >
> > At least the wined3d part looks ok to me, although we should probably
> > be cleaning up the GL context as well, there and in the other places
> > where we return.
>
> For some reason I didn't get the original patch, or I accidentally deleted 
> it(more likely).

Yo can find it there:
http://www.winehq.org/pipermail/wine-devel/2007-January/053725.html

[...]
> I do not know to what extend we want to catch screen res change failures. The 
> resolution? I'd say yes. Color depth? Propably no because we can't change it 
> anyway. Refresh rate? Don't think its important.

There's already some code dealing with color depth issues:

devmode.dmBitsPerPel = (bpp >= 24) ? 32 : bpp; /* Stupid XVidMode 
cannot change bpp */

But there's another place where we deal with these issues differently:

if(devmode.dmBitsPerPel == 24) devmode.dmBitsPerPel = 32;
     [...]
     ret = ChangeDisplaySettingsExW(NULL, &devmode, NULL, 
CDS_FULLSCREEN, NULL);
     if (ret != DISP_CHANGE_SUCCESSFUL) {
         if(devmode.dmDisplayFrequency != 0) {
             WARN("ChangeDisplaySettingsExW failed, trying without the 
refresh rate\n");
             devmode.dmFields &= ~DM_DISPLAYFREQUENCY;
             devmode.dmDisplayFrequency = 0;
             ret = ChangeDisplaySettingsExW(NULL, &devmode, NULL, 
CDS_FULLSCREEN, NULL) != DISP_CHANGE_SUCCESSFUL;
         }
         if(ret != DISP_CHANGE_SUCCESSFUL) {
             return DDERR_INVALIDMODE;
         }
     }

[Francois Gouget]

H. Verbeet wrote:
> On 30/01/07, Francois Gouget  wrote:
> > I have attached the relevant patches. Does this look ok to the DirectX
> > gurus?
>
> At least the wined3d part looks ok to me, although we should probably
> be cleaning up the GL context as well, there and in the other places
> where we return.

Is there some code that would show what cleaning the GL context entails? 
   I have looked at the other exit paths of CreateAdditionalSwapChain() 
but the only relevant one only does stuff if object->frontBuffer != NULL 
which cannot be the case there.

[Stefan Dösinger]

Am Freitag 02 Februar 2007 16:24 schrieb Francois Gouget:
> Is there some code that would show what cleaning the GL context entails?
>    I have looked at the other exit paths of CreateAdditionalSwapChain()
> but the only relevant one only does stuff if object->frontBuffer != NULL
> which cannot be the case there.

I have patches in my tree which change the context creation, and they also 
clean up properly in the case of an error(hopefully). I'll send them when 
things get going again after Alexandre's return.