[WineD3D] fix bug 4872

[Vitaliy Margolen]

Thursday, June 15, 2006, 1:23:37 AM, Raphael wrote:
>>Wednesday, June 14, 2006, 6:51:21 PM, Raphael wrote:
>>> On Thursday 15 June 2006 02:34, Raphael wrote:
>>>> Hi,
>>>>
>>>> first patch since a long time :)
>>>>
>>>> Changelog:
>>>>  -  rename WineD3D_(Create|Release)FakeGLContext to WineD3D_(Create|
>>>> Release)FakeGLContextOnNeed
>>>>   - and if a GL context is already active (and was not a fake context)
>>>> returns simili fake context with current GL context infos
>>>>   - cleanup not more needed checks
>>>>
>>>> fix #4872: make IWineD3DImpl_CheckDeviceType working if a GL context is
>>>> already active
>>
>>> Use this patch instead, i let some stupid (and buggy) debug code
>>
>>I think I counted at least 5 indentation stiles in your patch. I understand
>>staying consistent with the code's style and using your own stile. But 5?!

> No i only see two problems, and made by use of xemacs
> (as i have broken my kate install). Anyway i plan to cleanup
> code in file in a next patch

No you have:
1. 2 spaces
2. 3 spaces
3. 4 spaces
4. spaces no tabs
5. spaces with tabs

And just formatting change patches are not accepted. So you better have it right
the first rime.


>>>  }
>>You forgot LEAVE_GL();

> NO, LEAVE_GL() is done on ReleaseFakeGLContextOnNeed.
> to permit between two calls to can make X/GLX calls safely
So where are you disposing of it then? I can't find the place that will delete
this context on device delete.


>>> +  case WINED3DFMT_A4R4G4B4:
>>> +    if (4 == rb && 4 == gb && 4 == bb && 4 == ab) return TRUE;
>>> +    break;
>>Has nothing to do with context.
>  ??
>>> +  case WINED3DFMT_A8:
>>> +    if (8 == ab) return TRUE;
>>> +    break;
>>Same here
>  ??
These changes has nothing to do with what stated in the changelog. Please make a
separate patch for them.

>>>      if (Adapter >= IWineD3D_GetAdapterCount(iface)) {
>>> -        TRACE("(%p) Failed: Atapter (%u) higher than supported
>>> adapters (%u) returning WINED3DERR_INVALIDCALL\n", This,
>>> Adapter, IWineD3D_GetAdapterCount(iface));
>>> +     TRACE_(d3d_caps)("[FAILED: Atapter (%u) higher than 
>>> supported adapters (%u)]\n", Adapter,
>>> IWineD3D_GetAdapterCount(iface));
>>>          return WINED3DERR_INVALIDCALL;
>>>      }
>>This one as well. And I haven't seen any traces so far that used square
>>brackets. Now we will!

> Have you ever seen D3D traces ?

> I only made same as IWineD3DImpl_CheckDeviceFormat
> (for coherency)

> Anyway displaying [OK] or [FAILED] for checks results seems a good 
> idea (and easy to find in logs)

Please don't add this sort of stuff. You are correct as-is d3d traces look horrid
and impossible to find anything. Adding one more type of "screaming at you"
output will not make it more readable. It's only used in one place, so I don't
see why would you accent this error out of any other related stuff?

Vitaliy