ok lets try this yet again

I am sorry everyone for the spam =) 
been awhile since I developed stuff, been so used to designing things? =)

here in all its glorry is the diff -u context.c context.c.new > diff.txt file

I couldnt get the -c option to work so here is just the -u

--- context.c??? 2008-06-26 13:52:57.000000000 -0400
+++ context.c.change??? 2008-06-26 14:32:48.000000000 -0400
@@ -116,13 +116,12 @@
???? int iPixelFormat=0;
???? short redBits, greenBits, blueBits, alphaBits, colorBits;
???? short depthBits=0, stencilBits=0;
-
???? int i = 0;
???? int nCfgs = This->adapter->nCfgs;
-??? WineD3D_PixelFormat *cfgs = This->adapter->cfgs;
?
-??? TRACE("ColorFormat=%s, DepthStencilFormat=%s, auxBuffers=%d, numSamples=%d, pbuffer=%d, findCompatible=%d\n",
-????????? debug_d3dformat(ColorFormat), debug_d3dformat(DepthStencilFormat), auxBuffers, numSamples, pbuffer, findCompatible);
+??? WineD3D_PixelFormat *cfgs = This->adapter->cfgs;
+??? BOOL exactDepthMatch = FALSE;? /*Changed june 23,08 */?? 
+??? PIXELFORMATDESCRIPTOR pfd;???? /*Changed june 23,08 */
?
???? if(!getColorBits(ColorFormat, &redBits, &greenBits, &blueBits, &alphaBits, &colorBits)) {
???????? ERR("Unable to get color bits for format %s (%#x)!\n", debug_d3dformat(ColorFormat), ColorFormat);
@@ -145,84 +144,91 @@
?
???? DepthStencilFormat = WINED3DFMT_D24S8;
?
-??? if(DepthStencilFormat) {
-??????? getDepthStencilBits(DepthStencilFormat, &depthBits, &stencilBits);
-??? }
+/* Changed Section June 24,08 */
+
+#if 0
+??? if(DepthStencilFormat) 
+????? {???????????????????? 
+??? getDepthStencilBits(DepthStencilFormat, &depthBits, &stencilBits);
+??? } 
+#endif
+
+/* Just call getDepthStencilBits as the above IF will always in this case be true */
+
+??? getDepthStencilBits(DepthStencilFormat, &depthBits, &stencilBits);
?
???? /* Find a pixel format which EXACTLY matches our requirements (except for depth) */
-??? for(i=0; i<nCfgs; i++) {
-??????? BOOL exactDepthMatch = TRUE;
+??? for(i=0; i<nCfgs; i++) 
+????? {
???????? cfgs = &This->adapter->cfgs[i];
-
+??????????????????? 
???????? /* For now only accept RGBA formats. Perhaps some day we will
????????? * allow floating point formats for pbuffers. */
???????? if(cfgs->iPixelType != WGL_TYPE_RGBA_ARB)
-??????????? continue;
+?????????? continue;
?
???????? /* In window mode (!pbuffer) we need a window drawable format and double buffering. */
???????? if(!pbuffer && !(cfgs->windowDrawable && cfgs->doubleBuffer))
-??????????? continue;
+?????????? continue;
?
-??????? /* We like to have aux buffers in backbuffer mode */
+?????? /* We like to have aux buffers in backbuffer mode */
???????? if(auxBuffers && !cfgs->auxBuffers)
-??????????? continue;
+?????????? continue;
?
???????? /* In pbuffer-mode we need a pbuffer-capable format but we don't want double buffering */
???????? if(pbuffer && (!cfgs->pbufferDrawable || cfgs->doubleBuffer))
-??????????? continue;
+???????????? continue;
?
-??????? if(cfgs->redSize != redBits)
-??????????? continue;
-??????? if(cfgs->greenSize != greenBits)
-??????????? continue;
-??????? if(cfgs->blueSize != blueBits)
-??????????? continue;
-??????? if(cfgs->alphaSize != alphaBits)
-??????????? continue;
-
-??????? /* We try to locate a format which matches our requirements exactly. In case of
-???????? * depth it is no problem to emulate 16-bit using e.g. 24-bit, so accept that. */
-??????? if(cfgs->depthSize < depthBits)
-??????????? continue;
-??????? else if(cfgs->depthSize > depthBits)
-??????????? exactDepthMatch = FALSE;
+??????? if ((cfgs->redSize != redBits) || (cfgs->greenSize != greenBits) || (cfgs->blueSize != blueBits) || (cfgs->alphaSize != alphaBits))
+???????????? continue;
?
???????? /* In all cases make sure the number of stencil bits matches our requirements
????????? * even when we don't need stencil because it could affect performance EXCEPT
????????? * on cards which don't offer depth formats without stencil like the i915 drivers
????????? * on Linux. */
-??????? if(stencilBits != cfgs->stencilSize && !(This->adapter->brokenStencil && stencilBits <= cfgs->stencilSize))
-??????????? continue;
+??????? if((stencilBits != cfgs->stencilSize) && !((This->adapter->brokenStencil && stencilBits) <= cfgs->stencilSize))
+???????????? continue;
?
???????? /* Check multisampling support */
???????? if(cfgs->numSamples != numSamples)
-??????????? continue;
+???????????? continue;
?
-??????? /* When we have passed all the checks then we have found a format which matches our
-???????? * requirements. Note that we only check for a limit number of capabilities right now,
-???????? * so there can easily be a dozen of pixel formats which appear to be the 'same' but
-???????? * can still differ in things like multisampling, stereo, SRGB and other flags.
-???????? */
+??????? /* We try to locate a format which matches our requirements exactly. In case of
+???????? * depth it is no problem to emulate 16-bit using e.g. 24-bit, so accept that. */
+???????? if (cfgs->depthSize !=? depthBits)
+??????????? continue;
?
???????? /* Exit the loop as we have found a format :) */
-??????? if(exactDepthMatch) {
+??????? if (exactDepthMatch) 
+?????????? {
+??????????? TRACE("Exact Depth Match\n");
???????????? iPixelFormat = cfgs->iPixelFormat;
???????????? break;
-??????? } else if(!iPixelFormat) {
+?????????? } 
+??????? if (!iPixelFormat) 
+????????? {
???????????? /* In the end we might end up with a format which doesn't exactly match our depth
????????????? * requirements. Accept the first format we found because formats with higher iPixelFormat
????????????? * values tend to have more extended capabilities (e.g. multisampling) which we don't need. */
+
+??????????? TRACE("Emulating %d\n",cfgs->iPixelFormat);
???????????? iPixelFormat = cfgs->iPixelFormat;
-??????? }
+??????????? break; /* Added June 24,08 */
+????????? }
???? }
?
???? /* When findCompatible is set and no suitable format was found, let ChoosePixelFormat choose a pixel format in order not to crash. */
-??? if(!iPixelFormat && !findCompatible) {
+
+#if 0??? 
+if (!iPixelFormat && !findCompatible) 
+????? {
???????? ERR("Can't find a suitable iPixelFormat\n");
???????? return FALSE;
-??? } else if(!iPixelFormat) {
-??????? PIXELFORMATDESCRIPTOR pfd;
-
+????? } 
+#endif
+????? 
+??? if (!iPixelFormat) 
+????? {
???????? TRACE("Falling back to ChoosePixelFormat as we weren't able to find an exactly matching pixel format\n");
???????? /* PixelFormat selection */
???????? ZeroMemory(&pfd, sizeof(pfd));
@@ -235,16 +241,17 @@
???????? pfd.cDepthBits = depthBits;
???????? pfd.cStencilBits = stencilBits;
???????? pfd.iLayerType = PFD_MAIN_PLANE;
-
+??????? 
???????? iPixelFormat = ChoosePixelFormat(hdc, &pfd);
-??????? if(!iPixelFormat) {
-??????????? /* If this happens something is very wrong as ChoosePixelFormat barely fails */
-??????????? ERR("Can't find a suitable iPixelFormat\n");
-??????????? return FALSE;
-??????? }
-??? }
+??????? if (!iPixelFormat) 
+????????? {
+?????????? /* If this happens something is very wrong as ChoosePixelFormat barely fails */
+?????????? ERR("Can't find a suitable iPixelFormat\n");
+?????????? return FALSE;
+????????? }
+????? }
?
-??? TRACE("Found iPixelFormat=%d for ColorFormat=%s, DepthStencilFormat=%s\n", iPixelFormat, debug_d3dformat(ColorFormat), debug_d3dformat(DepthStencilFormat));
+??? TRACE("Found iPixelFormat\n");
???? return iPixelFormat;
?}
?
@@ -372,6 +379,7 @@
?
???????? /* If we still don't have a pixel format, something is very wrong as ChoosePixelFormat barely fails */
???????? if(!iPixelFormat) {
+??????????? TRACE("Choose Pixel Format Failed\n");
???????????? ERR("Can't find a suitable iPixelFormat\n");
???????????? return FALSE;
???????? }

Better?

Chris

[James Hawkins]

On Thu, Jun 26, 2008 at 1:38 PM,   wrote:
> I am sorry everyone for the spam =)
> been awhile since I developed stuff, been so used to designing things  =)
>
> here in all its glorry is the diff -u context.c context.c.new > diff.txt
> file
>
> I couldnt get the -c option to work so here is just the -u
>
> --- context.c    2008-06-26 13:52:57.000000000 -0400
> +++ context.c.change    2008-06-26 14:32:48.000000000 -0400
> @@ -116,13 +116,12 @@
>      int iPixelFormat=0;
>      short redBits, greenBits, blueBits, alphaBits, colorBits;
>      short depthBits=0, stencilBits=0;
> -
>
> int i = 0;
>    int nCfgs = This->adapter->nCfgs;
> -
>
> WineD3D_PixelFormat *cfgs = This->adapter->cfgs;
> -
>
> TRACE("ColorFormat=%s, DepthStencilFormat=%s, auxBuffers=%d,
>
> numSamples=%d, pbuffer=%d, findCompatible=%d\n",
> -
>      debug_d3dformat(ColorFormat),
>
> debug_d3dformat(DepthStencilFormat), auxBuffers, numSamples, pbuffer,
> findCompatible);
> -
>
> WineD3D_PixelFormat *cfgs = This->adapter->cfgs;
> -
>
> BOOL exactDepthMatch = FALSE;  /*Changed june 23,08 */
> -
>
> PIXELFORMATDESCRIPTOR pfd;     /*Changed june 23,08 */
>
> if(!getColorBits(ColorFormat, &redBits, &greenBits, &blueBits,
>
> &alphaBits, &colorBits)) {
>          ERR("Unable to get color bits for format %s (%#x)!\n",
> debug_d3dformat(ColorFormat), ColorFormat);
> @@ -145,84 +144,91 @@
>  DepthStencilFormat = WINED3DFMT_D24S8;
>
> - if(DepthStencilFormat) {
> -
>    getDepthStencilBits(DepthStencilFormat, &depthBits, &stencilBits);
> - }
>
> +/* Changed Section June 24,08 */
> -
>
> +#if 0
> - if(DepthStencilFormat)
> -
>  {
> - getDepthStencilBits(DepthStencilFormat, &depthBits, &stencilBits);
> - }
>
> +#endif
> -
>
> +/* Just call getDepthStencilBits as the above IF will always in this case
> be true */
> -
> -
>
> getDepthStencilBits(DepthStencilFormat, &depthBits, &stencilBits);
>
> /* Find a pixel format which EXACTLY matches our requirements (except
>
> for depth) */
> - for(i=0; i<nCfgs; i++) {
> -
>    BOOL exactDepthMatch = TRUE;
> - for(i=0; i<nCfgs; i++)
> -
>  {
>    cfgs = &This->adapter->cfgs[i];
> -
> -
>    /* For now only accept RGBA formats. Perhaps some day we will
>     * allow floating point formats for pbuffers. */
>    if(cfgs->iPixelType != WGL_TYPE_RGBA_ARB)
> -
>        continue;
> -
>       continue;
>
>    /* In window mode (!pbuffer) we need a window drawable format and
>
> double buffering. */
>          if(!pbuffer && !(cfgs->windowDrawable && cfgs->doubleBuffer))
> -
>        continue;
> -
>       continue;
>
> -
>    /* We like to have aux buffers in backbuffer mode */
> -
>   /* We like to have aux buffers in backbuffer mode */
>    if(auxBuffers && !cfgs->auxBuffers)
> -
>        continue;
> -
>       continue;
>
>    /* In pbuffer-mode we need a pbuffer-capable format but we don't
>
> want double buffering */
>          if(pbuffer && (!cfgs->pbufferDrawable || cfgs->doubleBuffer))
> -
>        continue;
> -
>         continue;
>
> -
>    if(cfgs->redSize != redBits)
> -
>        continue;
> -
>    if(cfgs->greenSize != greenBits)
> -
>        continue;
> -
>    if(cfgs->blueSize != blueBits)
> -
>        continue;
> -
>    if(cfgs->alphaSize != alphaBits)
> -
>        continue;
> -
> -
>    /* We try to locate a format which matches our requirements
>
> exactly. In case of
> -
>     * depth it is no problem to emulate 16-bit using e.g. 24-bit, so
>
> accept that. */
> -
>    if(cfgs->depthSize < depthBits)
> -
>        continue;
> -
>    else if(cfgs->depthSize > depthBits)
> -
>        exactDepthMatch = FALSE;
> -
>    if ((cfgs->redSize != redBits) || (cfgs->greenSize != greenBits) ||
>
> (cfgs->blueSize != blueBits) || (cfgs->alphaSize != alphaBits))
> -
>         continue;
>
>    /* In all cases make sure the number of stencil bits matches our
>
> requirements
>           * even when we don't need stencil because it could affect
> performance EXCEPT
>           * on cards which don't offer depth formats without stencil like
> the i915 drivers
>           * on Linux. */
> -
>    if(stencilBits != cfgs->stencilSize &&
>
> !(This->adapter->brokenStencil && stencilBits <= cfgs->stencilSize))
> -
>        continue;
> -
>    if((stencilBits != cfgs->stencilSize) &&
>
> !((This->adapter->brokenStencil && stencilBits) <= cfgs->stencilSize))
> -
>         continue;
>
>    /* Check multisampling support */
>    if(cfgs->numSamples != numSamples)
> -
>        continue;
> -
>         continue;
>
> -
>    /* When we have passed all the checks then we have found a format
>
> which matches our
> -
>     * requirements. Note that we only check for a limit number of
>
> capabilities right now,
> -
>     * so there can easily be a dozen of pixel formats which appear to
>
> be the 'same' but
> -
>     * can still differ in things like multisampling, stereo, SRGB and
>
> other flags.
> -
>     */
> -
>    /* We try to locate a format which matches our requirements
>
> exactly. In case of
> -
>     * depth it is no problem to emulate 16-bit using e.g. 24-bit, so
>
> accept that. */
> -
>     if (cfgs->depthSize !=  depthBits)
> -
>        continue;
>
>    /* Exit the loop as we have found a format :) */
> -
>    if(exactDepthMatch) {
> -
>    if (exactDepthMatch)
> -
>       {
> -
>        TRACE("Exact Depth Match\n");
>        iPixelFormat = cfgs->iPixelFormat;
>        break;
> -
>    } else if(!iPixelFormat) {
> -
>       }
> -
>    if (!iPixelFormat)
> -
>      {
>        /* In the end we might end up with a format which doesn't
>
> exactly match our depth
>               * requirements. Accept the first format we found because
> formats with higher iPixelFormat
>               * values tend to have more extended capabilities (e.g.
> multisampling) which we don't need. */
> -
> -
>        TRACE("Emulating %d\n",cfgs->iPixelFormat);
>        iPixelFormat = cfgs->iPixelFormat;
> -
>    }
> -
>        break; /* Added June 24,08 */
> -
>      }
>
> }
>
> /* When findCompatible is set and no suitable format was found, let
>
> ChoosePixelFormat choose a pixel format in order not to crash. */
> - if(!iPixelFormat && !findCompatible) {
> -
>
> +#if 0
> +if (!iPixelFormat && !findCompatible)
> -
>  {
>    ERR("Can't find a suitable iPixelFormat\n");
>    return FALSE;
> - } else if(!iPixelFormat) {
> -
>    PIXELFORMATDESCRIPTOR pfd;
> -
> -
>  }
>
> +#endif
> -
> - if (!iPixelFormat)
> -
>  {
>    TRACE("Falling back to ChoosePixelFormat as we weren't able to find
>
> an exactly matching pixel format\n");
>          /* PixelFormat selection */
>          ZeroMemory(&pfd, sizeof(pfd));
> @@ -235,16 +241,17 @@
>          pfd.cDepthBits = depthBits;
>          pfd.cStencilBits = stencilBits;
>          pfd.iLayerType = PFD_MAIN_PLANE;
> -
> -
>    iPixelFormat = ChoosePixelFormat(hdc, &pfd);
> -
>    if(!iPixelFormat) {
> -
>        /* If this happens something is very wrong as ChoosePixelFormat
>
> barely fails */
> -
>        ERR("Can't find a suitable iPixelFormat\n");
> -
>        return FALSE;
> -
>    }
> - }
> -
>    if (!iPixelFormat)
> -
>      {
> -
>       /* If this happens something is very wrong as ChoosePixelFormat
>
> barely fails */
> -
>       ERR("Can't find a suitable iPixelFormat\n");
> -
>       return FALSE;
> -
>      }
> -
>  }
>
> - TRACE("Found iPixelFormat=%d for ColorFormat=%s,
>
> DepthStencilFormat=%s\n", iPixelFormat, debug_d3dformat(ColorFormat),
> debug_d3dformat(DepthStencilFormat));
> - TRACE("Found iPixelFormat\n");
> return iPixelFormat;
>
> }
>
> @@ -372,6 +379,7 @@
>      /* If we still don't have a pixel format, something is very wrong
>
> as ChoosePixelFormat barely fails */
>          if(!iPixelFormat) {
> -
>        TRACE("Choose Pixel Format Failed\n");
>        ERR("Can't find a suitable iPixelFormat\n");
>        return FALSE;
>    }
>
>
> Better?

Not really.  You made a lot of superfluous changes (line spacing,
formatting, etc) that make it hard to see what you actually changed.

[Juan Lang]

As others have said, there are plenty of unnecessary changes, but you
probably need some pointing at them, so here goes:
> --- context.c    2008-06-26 13:52:57.000000000 -0400
> +++ context.c.change    2008-06-26 14:32:48.000000000 -0400
> @@ -116,13 +116,12 @@
>      int iPixelFormat=0;
>      short redBits, greenBits, blueBits, alphaBits, colorBits;
>      short depthBits=0, stencilBits=0;
> -

Unneeded.
>  int i = 0;
>  int nCfgs = This->adapter->nCfgs;
> -
>
> WineD3D_PixelFormat *cfgs = This->adapter->cfgs;
> -
>
> TRACE("ColorFormat=%s, DepthStencilFormat=%s, auxBuffers=%d,
>
> numSamples=%d, pbuffer=%d, findCompatible=%d\n",
> -
>      debug_d3dformat(ColorFormat),
>
> debug_d3dformat(DepthStencilFormat), auxBuffers, numSamples, pbuffer,
> findCompatible);

Why did you remove the trace?
> - WineD3D_PixelFormat *cfgs = This->adapter->cfgs;
> - BOOL exactDepthMatch = FALSE;  /*Changed june 23,08 */
> - PIXELFORMATDESCRIPTOR pfd;     /*Changed june 23,08 */

Don't put comments on the change date in the code, git history will
tell us all we need to know about that.
>  if(!getColorBits(ColorFormat, &redBits, &greenBits, &blueBits,
>
> &alphaBits, &colorBits)) {
>          ERR("Unable to get color bits for format %s (%#x)!\n",
> debug_d3dformat(ColorFormat), ColorFormat);
> @@ -145,84 +144,91 @@
>  DepthStencilFormat = WINED3DFMT_D24S8;
>
> - if(DepthStencilFormat) {
> -
>    getDepthStencilBits(DepthStencilFormat, &depthBits, &stencilBits);
> - }
>
> +/* Changed Section June 24,08 */
> -
>
> +#if 0
> - if(DepthStencilFormat)
> -
>  {
> - getDepthStencilBits(DepthStencilFormat, &depthBits, &stencilBits);
> - }
>
> +#endif
> -
>
> +/* Just call getDepthStencilBits as the above IF will always in this case
> be true */
> -
> - getDepthStencilBits(DepthStencilFormat, &depthBits, &stencilBits);

While you're right that this if is unnecessary,
1) putting a redundant copy if an #if 0 block is really wasteful, as
it doesn't give future developers any hint as to when, if ever, the
block should be removed.
2) The comment is also unnecessary, as "the above IF" won't refer to
anything once it's removed.
Just remove the redundant if and be done with it.

Also, this change is really a no-op, since the if is guaranteed to be
true.  As such it should probably be a separate patch from any other
change.
>  /* Find a pixel format which EXACTLY matches our requirements (except
>
> for depth) */
> - for(i=0; i<nCfgs; i++) {
> -
>    BOOL exactDepthMatch = TRUE;
> - for(i=0; i<nCfgs; i++)
> -
>  {

Why did you re-indent the brace?  Just leave it where it is, please.
>      cfgs = &This->adapter->cfgs[i];
> -
> -

Here you've added a bunch of white space.  Please don't do that.
>      /* For now only accept RGBA formats. Perhaps some day we will
>       * allow floating point formats for pbuffers. */
>      if(cfgs->iPixelType != WGL_TYPE_RGBA_ARB)
> -
>        continue;
> -
>       continue;

Another whitespace-only change.
>      /* In window mode (!pbuffer) we need a window drawable format and
>
> double buffering. */
>          if(!pbuffer && !(cfgs->windowDrawable && cfgs->doubleBuffer))
> -
>        continue;
> -
>       continue;

and again..
> -
>    /* We like to have aux buffers in backbuffer mode */
> -
>   /* We like to have aux buffers in backbuffer mode */

and again..
>      if(auxBuffers && !cfgs->auxBuffers)
> -
>        continue;
> -
>       continue;

and again.. getting the picture?
>      /* In pbuffer-mode we need a pbuffer-capable format but we don't
>
> want double buffering */
>          if(pbuffer && (!cfgs->pbufferDrawable || cfgs->doubleBuffer))
> -
>        continue;
> -
>         continue;

more..
> -
>    if(cfgs->redSize != redBits)
> -
>        continue;
> -
>    if(cfgs->greenSize != greenBits)
> -
>        continue;
> -
>    if(cfgs->blueSize != blueBits)
> -
>        continue;
> -
>    if(cfgs->alphaSize != alphaBits)
> -
>        continue;
> -
> -
>    /* We try to locate a format which matches our requirements
>
> exactly. In case of
> -
>     * depth it is no problem to emulate 16-bit using e.g. 24-bit, so
>
> accept that. */
> -
>    if(cfgs->depthSize < depthBits)
> -
>        continue;
> -
>    else if(cfgs->depthSize > depthBits)
> -
>        exactDepthMatch = FALSE;
> -
>    if ((cfgs->redSize != redBits) || (cfgs->greenSize != greenBits) ||
>
> (cfgs->blueSize != blueBits) || (cfgs->alphaSize != alphaBits))
> -
>         continue;
>
>    /* In all cases make sure the number of stencil bits matches our
>
> requirements
>           * even when we don't need stencil because it could affect
> performance EXCEPT
>           * on cards which don't offer depth formats without stencil like
> the i915 drivers
>           * on Linux. */
> -
>    if(stencilBits != cfgs->stencilSize &&
>
> !(This->adapter->brokenStencil && stencilBits <= cfgs->stencilSize))
> -
>        continue;
> -
>    if((stencilBits != cfgs->stencilSize) &&
>
> !((This->adapter->brokenStencil && stencilBits) <= cfgs->stencilSize))
> -
>         continue;

more..
>      /* Check multisampling support */
>      if(cfgs->numSamples != numSamples)
> -
>        continue;
> -
>         continue;

yet more..
> -
>    /* When we have passed all the checks then we have found a format
>
> which matches our
> -
>     * requirements. Note that we only check for a limit number of
>
> capabilities right now,
> -
>     * so there can easily be a dozen of pixel formats which appear to
>
> be the 'same' but
> -
>     * can still differ in things like multisampling, stereo, SRGB and
>
> other flags.
> -
>     */
> -
>    /* We try to locate a format which matches our requirements
>
> exactly. In case of
> -
>     * depth it is no problem to emulate 16-bit using e.g. 24-bit, so
>
> accept that. */
> -
>     if (cfgs->depthSize !=  depthBits)
> -
>        continue;
>
>    /* Exit the loop as we have found a format :) */
> -
>    if(exactDepthMatch) {
> -
>    if (exactDepthMatch)
> -
>       {

more...
> -
>        TRACE("Exact Depth Match\n");
>        iPixelFormat = cfgs->iPixelFormat;
>        break;
> -
>    } else if(!iPixelFormat) {
> -
>       }
> -
>    if (!iPixelFormat)

and again.  Removing the else is silly, as the if branch breaks,
guaranteeing we can only get here if the first if was not true.
> -
>      {
>        /* In the end we might end up with a format which doesn't
>
> exactly match our depth
>               * requirements. Accept the first format we found because
> formats with higher iPixelFormat
>               * values tend to have more extended capabilities (e.g.
> multisampling) which we don't need. */
> -
> -
>        TRACE("Emulating %d\n",cfgs->iPixelFormat);
>        iPixelFormat = cfgs->iPixelFormat;
> -
>    }
> -
>        break; /* Added June 24,08 */

Again, don't put a changelog in the code.
> -
>      }

More whitespace change.
>  }
>
>  /* When findCompatible is set and no suitable format was found, let
>
> ChoosePixelFormat choose a pixel format in order not to crash. */
> - if(!iPixelFormat && !findCompatible) {
> -
>
> +#if 0

If you want to remove something, just remove it.  Don't add #if 0 around it.
> +if (!iPixelFormat && !findCompatible)
> -
>  {
>    ERR("Can't find a suitable iPixelFormat\n");
>    return FALSE;
> - } else if(!iPixelFormat) {
> -
>    PIXELFORMATDESCRIPTOR pfd;
> -
> -
>  }
>
> +#endif
> -
> - if (!iPixelFormat)
> -
>  {
>    TRACE("Falling back to ChoosePixelFormat as we weren't able to find
>
> an exactly matching pixel format\n");
>          /* PixelFormat selection */
>          ZeroMemory(&pfd, sizeof(pfd));
> @@ -235,16 +241,17 @@
>          pfd.cDepthBits = depthBits;
>          pfd.cStencilBits = stencilBits;
>          pfd.iLayerType = PFD_MAIN_PLANE;
> -
> -

More whitespace changes..
>      iPixelFormat = ChoosePixelFormat(hdc, &pfd);
> -
>    if(!iPixelFormat) {
> -
>        /* If this happens something is very wrong as ChoosePixelFormat
>
> barely fails */
> -
>        ERR("Can't find a suitable iPixelFormat\n");
> -
>        return FALSE;
> -
>    }
> - }
> -
>    if (!iPixelFormat)
> -
>      {
> -
>       /* If this happens something is very wrong as ChoosePixelFormat
>
> barely fails */

And again..
> -
>       ERR("Can't find a suitable iPixelFormat\n");
> -
>       return FALSE;
> -
>      }
> -
>  }
>
> - TRACE("Found iPixelFormat=%d for ColorFormat=%s,
>
> DepthStencilFormat=%s\n", iPixelFormat, debug_d3dformat(ColorFormat),
> debug_d3dformat(DepthStencilFormat));
> - TRACE("Found iPixelFormat\n");

Why did you change that?
>  return iPixelFormat;
>
> }
>
> @@ -372,6 +379,7 @@
>      /* If we still don't have a pixel format, something is very wrong
>
> as ChoosePixelFormat barely fails */
>          if(!iPixelFormat) {
> -
>        TRACE("Choose Pixel Format Failed\n");
>        ERR("Can't find a suitable iPixelFormat\n");
>        return FALSE;
>    }

Clean up the unnecessary stuff and we'll have another look.  Thanks,
--Juan

[Detlef Riekenberg]

On Do, 2008-06-26 at 14:38 -0400, [e-mail] wrote:

I have no Idea about the touched code, but still some more hints:

You must use your real name
> - WineD3D_PixelFormat *cfgs = This->adapter->cfgs;
> - BOOL exactDepthMatch = FALSE;  /*Changed june 23,08 */
> - PIXELFORMATDESCRIPTOR pfd;     /*Changed june 23,08 */

Why do you never see similar comments in any Open Source Project?
- It's useless in the source
- That and more informations are available in the
  software control system (wine uses git)
> +/* Changed Section June 24,08 */
> -

See Above
> +#if 0
> - if(DepthStencilFormat)
> -
>  {                     
> - getDepthStencilBits(DepthStencilFormat, &depthBits,
>
> &stencilBits);
> - }
>
> +#endif

You add unused sourcecode here. Why?

Another example, what you missed is the indention in Wine: 
The same as in the rest of the File. Default is 4 Space

Please read our Wiki ( http://wiki.winehq.org/Developers ),
before sending patches.

Thanks for helping Wine