Re: [WineD3D] fix bug 4872

[Raphael]

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
>
> Regards,
> Raphael

Use this patch instead, i let some stupid (and buggy) debug code

Regards,
Raphael

[WineD3D] fix bug 4872

[Vitaliy Margolen]

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
> >
> > Regards,
> > Raphael
> Use this patch instead, i let some stupid (and buggy) debug code
> Regards,
> Raphael

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?!
> @@ -139,22 +145,35 @@
>         }
> } else {

-     if (ctx.ref >> 0) ret = &ctx;
+      if (ctx.ref >> 0) ret = &ctx;
> -
>  else {
> -
>    ret = &ctx;
> -
>    /* refill infos as current context may be changed by application between calls */
> -
>    ctx.ref = 0; 
> -
>    ctx.glCtx = gl_current_ctx;
> -
>    ENTER_GL();
> -
>    ctx.display = glXGetCurrentDisplay();
> -
>    /* others WineD3D_Context attributes don't need to be provided for local use */
> -
>    ctx.visInfo = NULL;
> -
>    ctx.drawable = (Drawable) NULL;
> -
>  }
>
> }
>
> if (NULL != ret) InterlockedIncrement(&ret->ref);
>   return ret;
>
> }

You forgot LEAVE_GL();
> - 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
> - case WINED3DFMT_A4R4G4B4:
> - case WINED3DFMT_X4R4G4B4:
> case WINED3DFMT_R5G6B5:
> case WINED3DFMT_R3G3B2:
> - case WINED3DFMT_A8:
> case WINED3DFMT_A8P8:
> case WINED3DFMT_P8:
>
> -return TRUE;
> - return TRUE;
> default:
> ERR("unsupported format %s\n", debug_d3dformat(Format));
> break;
>
> @@ -1226,7 +1254,7 @@
>      int nCfgs = 0;
>      int it;
> - WARN_(d3d_caps)("(%p)-> (STUB) (Adptr:%d, DevType:(%x,%s), AdptFmt:(%x,%s), RendrTgtFmt:(%x,%s), DepthStencilFmt:(%x,%s))\n",
> - TRACE_(d3d_caps)("(%p)-> (STUB) (Adptr:%d, DevType:(%x,%s), AdptFmt:(%x,%s), RendrTgtFmt:(%x,%s), DepthStencilFmt:(%x,%s)) ",
>        This, Adapter,
>        DeviceType, debug_d3ddevicetype(DeviceType),
>        AdapterFormat, debug_d3dformat(AdapterFormat),

And here.
> @@ -1234,12 +1262,12 @@
>             DepthStencilFormat, debug_d3dformat(DepthStencilFormat));
>  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!

Vitaliy Margolen