May 2011 - Wine-devel - list.winehq.org

D3D performance debugging report
by Stefan Dösinger 02 May '11

02 May '11

Hi, I spent a few hours debugging wined3d performance today. No, I found no magic fix for the slowness, just some semi-usable data. First I wrote a hacky patch to avoid redundant FBO applications. This gave a tiny, tiny performance increase, see http://www.winehq.org/pipermail/wine- devel/2011-April/089832.html. The main investigation concerned redundant shader applications. The aim was to find out how many of our glBindProgramARB calls are re-binding the same program, and how much this costs. Depending on the game between 20% to 90% of all BindProgram calls are redundant. I'll attach my debug hack so others can test their own apps. I used ARB shaders for testing because they can apply vertex and fragment programs separately. This brings up two questions: (a) How much does this cost (b) Why does this happen The costs: In my draw overhead tester hacking out the redundant apply calls improved performance a lot, from about 101 fps to 157 fps. The biggest part of that are the GL calls. Without them but the remaining shader logic I get 144 fps. Unfortunately this does not translate to any performance gains in real apps. I tried to filter out the redundant apply calls in the simplest way possible: Track the current value per wined3d_context and check before calling glBindProgramARB. This gave the 144 fps in the draw overhead tester, but no measurable increase in any other apps(I tested StarCraft 2, HL2, Team Fortress 2, World in Conflict and a few others) Given the amount of redundant apply calls and the cost of them in the draw overlay tester I have expected at least some improvement. Certainly not a 50% performance increase(the draw overlay tester performs no shader changes at all in the draw loop), but at least a 2-3% gain. So far I have no explanation why I didn't see that. But why do those redundant apply calls happen? It seems like the state dirtification comes all the way from the stream sources and/or vertex declaration. STREAMSRC is linked to VDECL, which is linked to VERTEXSHADER, which in turn reapplies the pixel shader. This means redundant vertex and pixel shader applications. Separating those states will be a major challenge. The vdecl<->vshader link shouldn't be needed any more, except in rare cases where GL_ARB_vertex_array_bgra is not supported and the application switches one attribute from D3DDECL_D3DCOLOR to a non-d3dcolor attribute. If the vertex shader changes we still have to reparse the vertex declaraion and reapply the stream sources because the vshader determines the stream numbers. Maybe we can reduce the number of times this happens by ordering stream usages and indices to make sure shaders with compatible input get the same stream ordering. vdecl and streamsrc are pretty related. If the vdecl is changed we have to reapply the stream sources. The other way around shouldn't cause problems though. There's no need to reapply every stream except the changed ones and there's no need to reapply the vertex shader. The vertex and pixel shader are linked for a few reasons: The shader backend API offers only a function to set both. Basic GLSL only offers a function to set both at once(GL_ARB_separate_shader_objects changes that). And even in ARB the pixel shader input may require some changes in the vertex shader output to get Shader Model 3.0 varyings right. The shader backend API can be changed, but it has to be done in a way that doesn't hurt GLSL without ARB_separate_shader_objects. If we have classic GLSL we have to keep the link. With ARB we can conditionally reapply the vertex shader if the ps_input_signature is changed. To complicate matters there are additional states that affect the shaders, like fog, textures, clipping. We don't keep track of those dependencies. So it's a lot of work to clean up these state dependencies and we don't know how much it'll gain us :-( Stefan

4 8

Re: [PATCH 1/2] d3d9/tests: Added a test for resetting an invisible, inactive window to fullscreen mode.
by Henri Verbeet 02 May '11

02 May '11

On 2 May 2011 13:18, John Edmonds <pocketcookies2(a)gmail.com> wrote: > + /*Pull all currently available messages. We don't care about messages generated after this loop exits.*/ > + while(PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) > + { > + TranslateMessage(&msg); > + DispatchMessage(&msg); > + } You should probably use flush_events() for that.

1 0

Re: [PATCH 2/2] wined3d: Turn off message filtering temporarily in the Reset() method to allow certain messages (e.g. WM_ACT
by Marvin 02 May '11

02 May '11

Hi, While running your changed tests on Windows, 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 http://testbot.winehq.org/JobDetails.pl?Key=10678 Your paranoid android. === WXPPROSP3 (32 bit device) === device: unhandled exception c0000005 at 0040320F === W2K3R2SESP2 (32 bit device) === device: unhandled exception c0000005 at 0040320F === WVISTAADM (32 bit device) === device: unhandled exception c0000005 at 0040320F === W2K8SE (32 bit device) === device: unhandled exception c0000005 at 0040320F === W7PRO (32 bit device) === device: unhandled exception c0000005 at 0040320F === W7PROX64 (32 bit device) === device: unhandled exception c0000005 at 0040320F === W7PROX64 (64 bit device) === Timeout

1 0

Re: wininet: Use test.winehq.org for post tests
by Marvin 02 May '11

02 May '11

Hi, While running your changed tests on Windows, 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 http://testbot.winehq.org/JobDetails.pl?Key=10676 Your paranoid android. === WVISTAADM (32 bit http) === Failure running script in VM: The specified guest user must be logged in interactively to perform this operation

1 0

Re: [PATCH 1/5] mmdevapi: Add InitializeDriver() method to drivers
by Alexandre Julliard 02 May '11

02 May '11

Andrew Eikum <aeikum(a)codeweavers.com> writes: > @@ -72,11 +73,19 @@ static BOOL load_driver(const WCHAR *name) > } > > #define LDFC(n) do { drvs.p##n = (void*)GetProcAddress(drvs.module, #n);\ > - if(!drvs.p##n) return FALSE; } while(0); > + if(!drvs.p##n) { FreeLibrary(drvs.module); return FALSE; } } while(0); > + LDFC(InitializeDriver); > LDFC(GetEndpointIDs); > LDFC(GetAudioEndpoint); > #undef LDFC > > + hr = drvs.pInitializeDriver(); > + if(FAILED(hr)){ > + TRACE("Driver initialization failed: %08x\n", hr); > + FreeLibrary(drvs.module); > + return hr; > + } If you are going to call it on load, you could just as well do the init in the driver on DLL_PROCESS_ATTACH and avoid the extra method. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [1/5] cmd: WCMD_delete: simplify logic, avoid assigning variable to itself
by Henri Verbeet 02 May '11

02 May '11

On 2 May 2011 02:23, Dan Kegel <dank(a)kegel.com> wrote: > - if (attribute && !negate) { > - stillOK = stillOK; > - } else if (!attribute && negate) { > - stillOK = stillOK; > - } else { > + if (attribute == negate) > stillOK = FALSE; > - } If you just write that as "if (!attribute == !negate)" you avoid both the assumption that TRUE is defined to 1 and the ugly "!= 0"'s. Or you could build actual attribute masks first and compare those instead, which may even be readable.

1 0

re: comctl32: realloc or free and alloc
by Dan Kegel 02 May '11

02 May '11

Alexey wrote: > i currently digg in comctl32 to find why my app fails. I found that > string conversation AtoW in some places silently fails. The problem is > that the destination for string is just a fresh pointer (not NULL). > Str_SetPtrAtoW check if it is NULL pointer and if not it trys to > ReAlloc. There was no Alloc before so ReAlloc returns NULL The code looks like it assumes that pointer is always managed by Alloc/ReAlloc/Free. In what context is the destination a fresh, non-Alloc'd pointer? Perhaps that's where the bug lies. Your proposed solution would pass a non-Alloc'd pointer to Free, which doesn't seem good. - Dan

4 9

Re: [PATCH 4/5] winecoreaudio.drv: Implement IAudioSessionControl::GetState
by Ken Thomases 01 May '11

01 May '11

Hi, On Apr 29, 2011, at 3:50 PM, Andrew Eikum wrote: > + LIST_FOR_EACH_ENTRY(client, &This->session->clients, ACImpl, entry){ > + OSSpinLockLock(&client->lock); > + if(client->playing == StatePlaying){ > + *state = AudioSessionStateActive; I think the session should be considered active if a client is in StateInTransition, too. Regards, Ken

1 0

Re: ddraw: Add a few tests for Setcooperativelevel
by Marvin 01 May '11

01 May '11

Hi, While running your changed tests on Windows, 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 http://testbot.winehq.org/JobDetails.pl?Key=10649 Your paranoid android. === WNT4WSSP6 (32 bit ddrawmodes) === ddrawmodes.c:800: Test failed: SetCooperativelevel(DDSCL_SETFOCUSWINDOW) returned: 80070057 ddrawmodes.c:803: Test failed: SetCooperativelevel(DDSCL_CREATEDEVICEWINDOW | DDSCL_EXCLUSIVE | DDSCL_FULLSCREEN) returned: 80070057 ddrawmodes.c:828: Test failed: SetCooperativelevel(DDSCL_SETFOCUSWINDOW) returned: 80070057 === W7PROX64 (64 bit ddrawmodes) === Timeout

1 0