Re: ok lets try this yet again
On Do, 2008-06-26 at 14:38 -0400, [EMAIL PROTECTED] 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
--
By by ... Detlef
Re: ok lets try this yet again
As others have said, there are plenty of unnecessary changes, but you
probably need some pointing at them, so here goes:
> --- context.c2008-06-26 13:52:57.0 -0400
> +++ context.c.change2008-06-26 14:32:48.0 -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 -BOOL exactDepthMatch = TRUE;
> +for(i=0; 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;
> +
Re: ok lets try this yet again
On Thu, Jun 26, 2008 at 1:38 PM, <[EMAIL PROTECTED]> 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.c2008-06-26 13:52:57.0 -0400
> +++ context.c.change2008-06-26 14:32:48.0 -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 -BOOL exactDepthMatch = TRUE;
> +for(i=0; 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.
> - */
>
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.0 -0400
+++ context.c.change??? 2008-06-26 14:32:48.0 -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; iadapter->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 :) */
-??? i
