Re: wined3d/d3d9: Set the initial scissor rect to the window size

[H. Verbeet]

2008/8/21 Rico Schüller :
> - /* Set the scissor rect values */
> - scissor.left=0;
> - scissor.right=ThisDevice->ddraw_width;
> - scissor.top=0;
> - scissor.bottom=ThisDevice->ddraw_height;
> - IWineD3DDevice_SetScissorRect(device, &scissor);
> -

Are you sure you shouldn't be using the primary swapchain's backbuffer
dimensions?

wined3d/d3d9: Set the initial scissor rect to the window size

[Rico Schüller]

H. Verbeet schrieb:
> 2008/8/21 Rico Schüller :
> > - /* Set the scissor rect values */
> > - scissor.left=0;
> > - scissor.right=ThisDevice->ddraw_width;
> > - scissor.top=0;
> > - scissor.bottom=ThisDevice->ddraw_height;
> > - IWineD3DDevice_SetScissorRect(device, &scissor);
> > -
>
> Are you sure you shouldn't be using the primary swapchain's backbuffer
> dimensions?

I'm not sure what exactly you mean. But the attached patch works for me 
(on wine and windows). Could you have a look at it, please. Probably I 
should improve the patch a bit.

Cheers
Rico

diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c
index b8fc3c6..c0fac4c 100644
--- a/dlls/d3d9/tests/visual.c
+++ b/dlls/d3d9/tests/visual.c
@@ -169,8 +169,8 @@ static IDirect3DDevice9 *init_d3d9(void)
     present_parameters.Windowed = FALSE;
     present_parameters.hDeviceWindow = create_window();
     present_parameters.SwapEffect = D3DSWAPEFFECT_DISCARD;
-    present_parameters.BackBufferWidth = 640;
-    present_parameters.BackBufferHeight = 480;
+    present_parameters.BackBufferWidth = 1024;
+    present_parameters.BackBufferHeight = 768;
     present_parameters.BackBufferFormat = D3DFMT_A8R8G8B8;
     present_parameters.EnableAutoDepthStencil = TRUE;
     present_parameters.AutoDepthStencilFormat = D3DFMT_D24S8;
@@ -197,7 +197,8 @@ static IDirect3DDevice9 *init_d3d9(void)

/* Check for the default scissor rect size */
     IDirect3DDevice9_GetScissorRect(device_ptr, &scissor);
-    ok(scissor.right == 640 && scissor.bottom == 480 && scissor.top == 0 && scissor.left == 0, "Scissorrect missmatch\n");
+    ok(scissor.right == 1024 && scissor.bottom == 768 && scissor.top == 0 && scissor.left == 0, "Scissorrect missmatch\n");
+exit(0);

return device_ptr;
 }

wined3d/d3d9: Set the initial scissor rect to the window size

[Henri Verbeet]

2008/8/22 Rico Schüller :
> H. Verbeet schrieb:
> > 2008/8/21 Rico Schüller :
> > > - /* Set the scissor rect values */
> > > - scissor.left=0;
> > > - scissor.right=ThisDevice->ddraw_width;
> > > - scissor.top=0;
> > > - scissor.bottom=ThisDevice->ddraw_height;
> > > - IWineD3DDevice_SetScissorRect(device, &scissor);
> > > -
> >
> > Are you sure you shouldn't be using the primary swapchain's backbuffer
> > dimensions?
>
> I'm not sure what exactly you mean. But the attached patch works for me (on
> wine and windows). Could you have a look at it, please. Probably I should
> improve the patch a bit.

I'm not sure what that patch is supposed to do.

ddraw_width and ddraw_height are the width and height of the current
screen. For fullscreen applications that's the same as the backbuffer,
but for windowed applications the backbuffer is smaller. I think it
would make more sense for the initial scissor rect to be the size of
the backbuffer than the size of the screen. Also keep in mind that a
device might have multiple swapchains, not all of which might have the
same backbuffer dimensions.

wined3d/d3d9: Set the initial scissor rect to the window size

[Stefan Dösinger]

> > Are you sure you shouldn't be using the primary swapchain's
>
> backbuffer
> > dimensions?
>
> I'm not sure what exactly you mean.

Something like this:

IWineD3DSwapChain *swapchain;
IWineD3DSurface *backbuffer;
D3DSURFACE_DESC desc;

IWineD3DDevice_GetSwapChain(device, 0, &swapchain)
IWineD3DSwapChain_GetBackBuffer(swapchain, &backbuffer);
IWineD3DSurface_GetSurfaceDesc(backbuffer, &desc);
IWineD3DSurface_Release(backbuffer);
IWineD3DSwapChain_Release(swapchain);

scissorrect = {0, 0, desc.Width, desc.Height};

Just pseudocode, please look up the exact names of the methods and members.
Maybe there's a IWineD3DDevice_GetBackBuffer method which spares you the
extra GetSwapChain call. Don't forget the release calls.

The difference between your original patch and this can be seen in Windowed
mode. I recommend to put the appropriate test into dlls/d3d9/tests/device.c
and create a Windowed device.

wined3d/d3d9: Set the initial scissor rect to the window size

[Rico Schüller]

Stefan Dösinger schrieb:
> > > Are you sure you shouldn't be using the primary swapchain's
> >
> > backbuffer
> > > dimensions?
> >
> > I'm not sure what exactly you mean.
>
> Something like this:
>
> IWineD3DSwapChain *swapchain;
> IWineD3DSurface *backbuffer;
> D3DSURFACE_DESC desc;
>
> IWineD3DDevice_GetSwapChain(device, 0, &swapchain)
> IWineD3DSwapChain_GetBackBuffer(swapchain, &backbuffer);
> IWineD3DSurface_GetSurfaceDesc(backbuffer, &desc);
> IWineD3DSurface_Release(backbuffer);
> IWineD3DSwapChain_Release(swapchain);
>
> scissorrect = {0, 0, desc.Width, desc.Height};
>
> Just pseudocode, please look up the exact names of the methods and members.
> Maybe there's a IWineD3DDevice_GetBackBuffer method which spares you the
> extra GetSwapChain call. Don't forget the release calls.
>
> The difference between your original patch and this can be seen in Windowed
> mode. I recommend to put the appropriate test into dlls/d3d9/tests/device.c
> and create a Windowed device.

Here is the patch which checks the default scissorrect. I don't know if 
the resolutions are to big (1024x768). And I don't know if the stuff 
with the second swapchain (swapchain1) is correct. It produces this 
error in wine: "err:d3d:IWineD3DDeviceImpl_SetupFullscreenWindow 
(0x1e7370): Want to change the window parameters of HWND 0x20034, but 
another style is stored for restoration afterwards".

So comments are welcome.

Cheers
Rico

diff --git a/dlls/d3d9/tests/device.c b/dlls/d3d9/tests/device.c
index 3a56298..4e7a305 100644
--- a/dlls/d3d9/tests/device.c
+++ b/dlls/d3d9/tests/device.c
@@ -2051,6 +2051,103 @@ static void test_set_material(void)
     if(d3d9) IDirect3D9_Release(d3d9);
 }

+static void test_scissor_size(void)
+{
+    IDirect3D9 *d3d9_ptr = 0;
+    int i;
+    static const struct {
+        int winx; int winy; int backx; int backy; int backswap1x; int backswap1y; BOOL window;
+    } scts[] = { /* scissor tests */
+        {800, 600, 640, 480, 1024, 768, TRUE},
+        {800, 600, 640, 480, 1024, 768, FALSE},
+        {640, 480, 800, 600, 1024, 768, TRUE},
+        {640, 480, 800, 600, 1024, 768, FALSE},
+
+        {1024, 768, 640, 480, 800, 600, TRUE},
+        {1024, 768, 640, 480, 800, 600, FALSE},
+        {640, 480, 1024, 768, 800, 600, TRUE},
+        {640, 480, 1024, 768, 800, 600, FALSE},
+
+        {1024, 768, 800, 600, 640, 480, TRUE},
+        {1024, 768, 800, 600, 640, 480, FALSE},
+        {800, 600, 1024, 768, 640, 480, TRUE},
+        {800, 600, 1024, 768, 640, 480, FALSE},
+    };
+
+    d3d9_ptr = pDirect3DCreate9(D3D_SDK_VERSION);
+    ok(d3d9_ptr != NULL, "Failed to create IDirect3D9 object\n");
+    if (!d3d9_ptr){
+        skip("Failed to create IDirect3D9 object\n");
+        return;
+    }
+
+    for(i=0; i<sizeof(scts)/sizeof(scts[0]); i++) {
+        IDirect3DDevice9 *device_ptr = 0;
+        D3DPRESENT_PARAMETERS present_parameters;
+        HRESULT hr;
+        WNDCLASS wc = {0};
+        HWND hwnd = 0;
+        IDirect3DSwapChain9 *swapchain1;
+        RECT scissorrect;
+
+        wc.lpfnWndProc = DefWindowProc;
+        wc.lpszClassName = "d3d9_test_wc";
+        RegisterClass(&wc);
+
+        hwnd = CreateWindow("d3d9_test_wc", "d3d9_test",
+                        WS_MAXIMIZE | WS_VISIBLE | WS_CAPTION , 0, 0, scts[i].winx, scts[i].winy, 0, 0, 0, 0);
+
+        ZeroMemory(&present_parameters, sizeof(present_parameters));
+        present_parameters.Windowed = scts[i].window;
+        present_parameters.hDeviceWindow = hwnd;
+        present_parameters.SwapEffect = D3DSWAPEFFECT_DISCARD;
+        present_parameters.BackBufferWidth = scts[i].backx;
+        present_parameters.BackBufferHeight = scts[i].backy;
+        present_parameters.BackBufferFormat = D3DFMT_A8R8G8B8;
+        present_parameters.EnableAutoDepthStencil = TRUE;
+        present_parameters.AutoDepthStencilFormat = D3DFMT_D24S8;
+
+        hr = IDirect3D9_CreateDevice(d3d9_ptr, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, present_parameters.hDeviceWindow, D3DCREATE_HARDWARE_VERTEXPROCESSING, &present_parameters, &device_ptr);
+        if(FAILED(hr)) {
+            present_parameters.AutoDepthStencilFormat = D3DFMT_D16;
+            hr = IDirect3D9_CreateDevice(d3d9_ptr, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, present_parameters.hDeviceWindow, D3DCREATE_HARDWARE_VERTEXPROCESSING, &present_parameters, &device_ptr);
+            if(FAILED(hr)) {
+                hr = IDirect3D9_CreateDevice(d3d9_ptr, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, present_parameters.hDeviceWindow, D3DCREATE_SOFTWARE_VERTEXPROCESSING, &present_parameters, &device_ptr);
+            }
+        }
+        ok(hr == D3D_OK || hr == D3DERR_NOTAVAILABLE, "IDirect3D_CreateDevice returned: %08x\n", hr);
+
+        if (!device_ptr)
+        {
+            DestroyWindow(hwnd);
+            skip("Creating the device failed\n");
+            goto err_out;
+        }
+
+        present_parameters.BackBufferWidth = scts[i].backswap1x;
+        present_parameters.BackBufferHeight = scts[i].backswap1y;
+        IDirect3DDevice9_CreateAdditionalSwapChain(device_ptr, &present_parameters, &swapchain1);
+
+        /* Check for the default scissor rect size */
+        IDirect3DDevice9_GetScissorRect(device_ptr, &scissorrect);
+        ok(scissorrect.right == scts[i].backx && scissorrect.bottom == scts[i].backy && scissorrect.top == 0 && scissorrect.left == 0, "Scissorrect missmatch (%d, %d) should be (%d, %d)\n", scissorrect.right, scissorrect.bottom, scts[i].backx, scts[i].backy);
+
+        if(swapchain1) IDirect3DSwapChain9_Release(swapchain1);
+        if(device_ptr) {
+            ULONG ref;
+
+            ref = IDirect3DDevice9_Release(device_ptr);
+            DestroyWindow(hwnd);
+            ok(ref == 0, "The device was not properly freed: refcount %u\n", ref);
+        }
+    }
+
+err_out:
+    if(d3d9_ptr) IDirect3D9_Release(d3d9_ptr);
+    return;
+}
+
+
 START_TEST(device)
 {
     HMODULE d3d9_handle = LoadLibraryA( "d3d9.dll" );
@@ -2081,5 +2178,6 @@ START_TEST(device)
         test_lights();
         test_set_stream_source();
         test_set_material();
+        test_scissor_size();
     }
 }
diff --git a/dlls/wined3d/stateblock.c b/dlls/wined3d/stateblock.c
index e8ff6c1..e953470 100644
--- a/dlls/wined3d/stateblock.c
+++ b/dlls/wined3d/stateblock.c
@@ -1037,6 +1037,11 @@ static HRESULT  WINAPI IWineD3DStateBlockImpl_InitStartupStateBlock(IWineD3DStat
         DWORD d;
     } tmpfloat;
     unsigned int i;
+    IWineD3DSwapChain *swapchain;
+    IWineD3DSurface *backbuffer;
+    WINED3DSURFACE_DESC desc = {0};
+    UINT width, height;
+    RECT scissorrect;

/* Note this may have a large overhead but it should only be executed
        once, in order to initialize the complete state of the device and
@@ -1239,6 +1244,22 @@ static HRESULT  WINAPI IWineD3DStateBlockImpl_InitStartupStateBlock(IWineD3DStat
         This->textures[i]         = NULL;
     }

+    /* Set the default scissor rect values */
+    desc.Width = &width;
+    desc.Height = &height;
+
+    IWineD3DDevice_GetSwapChain(device, 0, &swapchain);
+    IWineD3DSwapChain_GetBackBuffer(swapchain, 0, WINED3DBACKBUFFER_TYPE_MONO, &backbuffer);
+    IWineD3DSurface_GetDesc(backbuffer, &desc);
+    IWineD3DSurface_Release(backbuffer);
+    IWineD3DSwapChain_Release(swapchain);
+
+    scissorrect.left = 0;
+    scissorrect.right = width;
+    scissorrect.top = 0;
+    scissorrect.bottom = height;
+    IWineD3DDevice_SetScissorRect(device, &scissorrect);
+
     TRACE("-----------------------> Device defaults now set up...\n");
     return WINED3D_OK;
 }

wined3d/d3d9: Set the initial scissor rect to the window size

[Henri Verbeet]

2008/8/23 Rico Schüller :
> Here is the patch which checks the default scissorrect. I don't know if the
> resolutions are to big (1024x768). And I don't know if the stuff with the
> second swapchain (swapchain1) is correct. It produces this error in wine:
> "err:d3d:IWineD3DDeviceImpl_SetupFullscreenWindow (0x1e7370): Want to change
> the window parameters of HWND 0x20034, but another style is stored for
> restoration afterwards".

Devices like the Eee PC only support up to 800x480, but in that case
creating the device should simply fail, and skip the test, so in that
regard the test should be fine. The error that gets printed in
probably a flaw in our handling of multiple swapchains. As long as the
test doesn't fail because of it it's probably not a concern for now.
You should check the return values for CreateAdditionallSwapChain and
GetScissorRect though.

wined3d/d3d9: Set the initial scissor rect to the window size

[Rico Schüller]

Henri Verbeet schrieb:
> 2008/8/23 Rico Schüller :
> > Here is the patch which checks the default scissorrect. I don't know if the
> > resolutions are to big (1024x768). And I don't know if the stuff with the
> > second swapchain (swapchain1) is correct. It produces this error in wine:
> > "err:d3d:IWineD3DDeviceImpl_SetupFullscreenWindow (0x1e7370): Want to change
> > the window parameters of HWND 0x20034, but another style is stored for
> > restoration afterwards".
>
> Devices like the Eee PC only support up to 800x480, but in that case
> creating the device should simply fail, and skip the test, so in that
> regard the test should be fine. The error that gets printed in
> probably a flaw in our handling of multiple swapchains. As long as the
> test doesn't fail because of it it's probably not a concern for now.
> You should check the return values for CreateAdditionallSwapChain and
> GetScissorRect though.

Thanks for the answer. I'll add a check for the return values.

wined3d/d3d9: Set the initial scissor rect to the window size

[Stefan Dösinger]

I think you don't have to test a 2nd swapchain here. The initial scissor rectangle is set in CreateDevice, so an extra call to CreateAdditionalSwapchain afterwards shouldn't have any effect on this. I am mainly concerned by the open source drivers here which do not always like messing with OpenGL contexts too much. I am afraid we'll get complaints that the test crashes or even causes a kernel panic :-/.

Does "WINED3DSURFACE_DESC desc = {0};" set all the pointers in WINED3DSURFACEDESC to NULL? I am not sure about this. (At some point we may also want to de-pointerize WINED3DSURFACEDESC like we've done with the other structures)

wined3d/d3d9: Set the initial scissor rect to the window size

[Henri Verbeet]

2008/8/23 Stefan Dösinger :
> Does "WINED3DSURFACE_DESC desc = {0};" set all the pointers in WINED3DSURFACEDESC to NULL? I am not sure about this. (At some point we may also want to de-pointerize WINED3DSURFACEDESC like we've done with the other structures)

It initializes the struct with zeroes.