Bug 8489

(In reply to comment #0)
> I get blue patches in textures after the following commit:
> 
> try to use a 8888 texture format which will result in only a memcopy in mesa's
> texstore functions whenever possible for r200 and r300. r200 can use hw formats
> argb8888, rgba8888 and abgr8888 (or the opposite on big endian), r300 can use
> argb8888, bgra8888, rgba8888 and abgr8888 regardless of endian, as it supports
> free component swizzling.
I guess I'm responsible for this...

> The blue patches seem to only occur on objects drawn using arb fragment and
> vertex programs. The hardware is r300. Unfortunately I don't have a small test
> that shows this.
I can't see any reason why using arb vp/fp would make a difference. I assume
this is on little endian (x86)? If you could run glean's pixelFormats test, this
should show what combinations of src and internal format are failing.

(In reply to comment #1)
> (In reply to comment #0)
> > I get blue patches in textures after the following commit:
> > 
> > try to use a 8888 texture format which will result in only a memcopy in mesa's
> > texstore functions whenever possible for r200 and r300. r200 can use hw formats
> > argb8888, rgba8888 and abgr8888 (or the opposite on big endian), r300 can use
> > argb8888, bgra8888, rgba8888 and abgr8888 regardless of endian, as it supports
> > free component swizzling.
> I guess I'm responsible for this...
> 
> > The blue patches seem to only occur on objects drawn using arb fragment and
> > vertex programs. The hardware is r300. Unfortunately I don't have a small test
> > that shows this.
> I can't see any reason why using arb vp/fp would make a difference. I assume
> this is on little endian (x86)? If you could run glean's pixelFormats test, this
> should show what combinations of src and internal format are failing.
> 

Sorry, I forgot to mention this is on x86_64 with r300. I'll run the
pixelFormats test now and post the results soon. Hopefully this test will run
successfully. Doing the complete glean testsuite causes a crash on r300, but
that's another issue.

(In reply to comment #2)
> (In reply to comment #1)
> > (In reply to comment #0)
> > > I get blue patches in textures after the following commit:
> > > 
> > > try to use a 8888 texture format which will result in only a memcopy in mesa's
> > > texstore functions whenever possible for r200 and r300. r200 can use hw
formats
> > > argb8888, rgba8888 and abgr8888 (or the opposite on big endian), r300 can use
> > > argb8888, bgra8888, rgba8888 and abgr8888 regardless of endian, as it supports
> > > free component swizzling.
> > I guess I'm responsible for this...
> > 
> > > The blue patches seem to only occur on objects drawn using arb fragment and
> > > vertex programs. The hardware is r300. Unfortunately I don't have a small test
> > > that shows this.
> > I can't see any reason why using arb vp/fp would make a difference. I assume
> > this is on little endian (x86)? If you could run glean's pixelFormats test, this
> > should show what combinations of src and internal format are failing.
> > 
> 
> Sorry, I forgot to mention this is on x86_64 with r300. I'll run the
> pixelFormats test now and post the results soon. Hopefully this test will run
> successfully. Doing the complete glean testsuite causes a crash on r300, but
> that's another issue.

This is odd. Glean reports that every pixelFormats test has passed, but the
commit I identified is defiantly the source of the bug. I reversed that commit
on the current mesa HEAD and the bug disappears...

We could add a new mode to the pixelFormats test to use a simple fragment
program when drawing the test polygon.  That might be interesting.

(In reply to comment #3)
> > Sorry, I forgot to mention this is on x86_64 with r300. I'll run the
> > pixelFormats test now and post the results soon. Hopefully this test will run
> > successfully. Doing the complete glean testsuite causes a crash on r300, but
> > that's another issue.
> 
> This is odd. Glean reports that every pixelFormats test has passed, but the
> commit I identified is defiantly the source of the bug. I reversed that commit
> on the current mesa HEAD and the bug disappears...
I assume this is native 64bit? I'm not sure, but maybe pixels might end up
swapped somehow, which the glean pixelFormats test would not detect (at least
not if they are only swapped pair-wise).

(In reply to comment #5)
> (In reply to comment #3)
> > > Sorry, I forgot to mention this is on x86_64 with r300. I'll run the
> > > pixelFormats test now and post the results soon. Hopefully this test will run
> > > successfully. Doing the complete glean testsuite causes a crash on r300, but
> > > that's another issue.
> > 
> > This is odd. Glean reports that every pixelFormats test has passed, but the
> > commit I identified is defiantly the source of the bug. I reversed that commit
> > on the current mesa HEAD and the bug disappears...
> I assume this is native 64bit? I'm not sure, but maybe pixels might end up
> swapped somehow, which the glean pixelFormats test would not detect (at least
> not if they are only swapped pair-wise).
> 

Yes, it's all native 64bit. I'm using a GPL licensed engine, actually a
modified Quake 3 that I modified some more, so it's all compiled natively for
x86_64.

(In reply to comment #4)
> We could add a new mode to the pixelFormats test to use a simple fragment
> program when drawing the test polygon.  That might be interesting.
> 

Might be a good idea. I'll happily test with it if you provide a patch to add
this test.

OK, update your Glean tpixelformats.cpp file and set USE_FRAG_PROG to 1 before
compiling.

(In reply to comment #8)
> OK, update your Glean tpixelformats.cpp file and set USE_FRAG_PROG to 1 before
> compiling.
> 

I just tried that test, and I got a lot of failures. I tested after I reversed
the commit I mentioned earlier though, so perhaps the underlying problem is
somewhere else, and this commit just "exposes" it. I'll attach the glean log.

Created attachment 7235 [details]
glean pixelFormats test log

(In reply to comment #9)
> (In reply to comment #8)
> > OK, update your Glean tpixelformats.cpp file and set USE_FRAG_PROG to 1 before
> > compiling.
> > 
> 
> I just tried that test, and I got a lot of failures. I tested after I reversed
> the commit I mentioned earlier though, so perhaps the underlying problem is
> somewhere else, and this commit just "exposes" it. I'll attach the glean log.

I just checked, and I seem to get the same results (lots of failures) from
glean before and after the commit, however the bug only shows up after the
commit... Really strange.

Just checking to see if anyone has any ideas on this? The activity has been idle
for a while, and this is a pretty serious bug.

After quickly looking at glean log, i think that the whole point is we never
get correct value for the A channel. This might be the cause of what you are
experiencing.

Would an incorrect value in the alpha channel cause the blue patches though? I
suppose it's possible, for example, in the zfill pass, the alpha channel is
used in an important way...

Actually, this might be a cause of another related bug I'm experiencing...
Regardless, the alpha bug needs to be fixed, and maybe this will fix the blue
patches bug (this bug) and my other bug; no bug report for the other bug yet,
I'll see if it still exists after this bug is fixed.

Do you have any idea why the alpha channel is not set, or is set incorrectly?
I'll happily test any patches for drm or mesa and report the results.

I think we might actually be dealing with two bugs here. The alpha bug (before
and after the commit) and the blue patches bug (only after the commit.)

Sorry to keep bumping this, but it is a rather serious bug (the alpha bug) and
the blue patches bug is also pretty bad...

I think I should reassign this to the r300 developers, since I think this is
specific to r300.

Has anyone got any idea why the alpha channel isn't being correctly written with fragment programs, and why I get blue patches after the commit I mentioned earlier?  I'm not sure about the first, but the later could be the texture format is incorrectly selected? As far as I can tell, that's all the commit does; selects a different texture format based on a few things...  I really like to see these bugs fixed, but there hasn't been any real activity on this bug for a while now. :( 

(In reply to comment #17)
> Has anyone got any idea why the alpha channel isn't being correctly written
with fragment programs, and why I get blue patches after the commit I mentioned
earlier?  I'm not sure about the first, but the later could be the texture
format is incorrectly selected? As far as I can tell, that's all the commit
does; selects a different texture format based on a few things...  I really like
to see these bugs fixed, but there hasn't been any real activity on this bug for
a while now. :( 
There is almost no such thing as incorrect selection of the internal texture
format - the driver is really free to choose almost whatever it wants. The patch
really just tries to select a format which will result in a memcopy from the
user-supplied format instead of requiring conversion (since the r300 chip can
handle the necessary swizzling on its own later).
Could you try with a r300Choose8888TexFormat function which always returns
&_mesa_texformat_rgba8888, ...rgba8888_rev, ...argb8888 and ...argb8888_rev
respectively? btw does your app always use the src format of GL_RGBA and the src
type GL_UNSIGNED_BYTE (like almost everyone does)?

As far as I can tell, yes, the renderer only uses GL_UNSIGNED_BYTE, but it uses
GL_RGB and GL_RGBA.

I can't be 100% sure since I didn't write the renderer. It's a modified Quake 3
renderer from the GPL release. Tr3b did all of the modification for per pixel
lighting and other fun things, I just did arbfp/vp support.

Anyway, I'll try what you said about r300Choose8888TexFormat later today when
I'm hopefully feeling better. (Got the flu. :()

This seems to be caused by an rgba vs argb problem. Bugzilla might destroy the
formatting here, but you should still get the idea. If I have
_dri_texformat_rgba8888 everything works fine (no blue patches) but if I have
_dri_texformat_argb8888 then I get blue patches...

I'm sure there is some higher-level reason why this doesn't work, but I'm not
sure what that is...

diff --git a/src/mesa/drivers/dri/r300/r300_tex.c
b/src/mesa/drivers/dri/r300/r300_tex.c index 6348ba1..06edbe7 100644
--- a/src/mesa/drivers/dri/r300/r300_tex.c
+++ b/src/mesa/drivers/dri/r300/r300_tex.c
@@ -385,7 +385,7 @@ #endif
 		case GL_UNSIGNED_SHORT_5_6_5_REV:
 			return _dri_texformat_rgb565;
 		default:
-			return do32bpt ? _dri_texformat_argb8888 :
+			return do32bpt ? _dri_texformat_rgba8888 :
 			    _dri_texformat_rgb565;
 		}
 

(In reply to comment #20)
> This seems to be caused by an rgba vs argb problem. Bugzilla might destroy the
> formatting here, but you should still get the idea. If I have
> _dri_texformat_rgba8888 everything works fine (no blue patches) but if I have
> _dri_texformat_argb8888 then I get blue patches...
> 
> I'm sure there is some higher-level reason why this doesn't work, but I'm not
> sure what that is...
> 
> diff --git a/src/mesa/drivers/dri/r300/r300_tex.c
> b/src/mesa/drivers/dri/r300/r300_tex.c index 6348ba1..06edbe7 100644
> --- a/src/mesa/drivers/dri/r300/r300_tex.c
> +++ b/src/mesa/drivers/dri/r300/r300_tex.c
> @@ -385,7 +385,7 @@ #endif
>  		case GL_UNSIGNED_SHORT_5_6_5_REV:
>  			return _dri_texformat_rgb565;
>  		default:
> -			return do32bpt ? _dri_texformat_argb8888 :
> +			return do32bpt ? _dri_texformat_rgba8888 :
>  			    _dri_texformat_rgb565;
>  		}
So, if you change the r300Choose8888TexFormat function it doesn't make a
difference at all, only if you change that GL_RGB case? This is really weird, it
would seem to indicate that the internal conversion from rgb, unsigned byte to
argb8888 (which is indeed more optimized than to rgba8888) would not work
correctly, but in this case you should get an error with glean's pixelFormats
test, so that can't really be.
That all doesn't make sense to me. Someone more familiar with that driver needs
to look into it.
As for the all-incorrect alpha values in glean with frag progs, I think the
reason for that could be a bug in the r300 frag prog parser, as color and alpha
sources are fully independant some special handling is needed there increasing
complexity and the likelyhood of bugs...

Yeah, it just seems to be the GL_RGB case, not the r300Choose8888TexFormat
function. Something strange is going on here. It's probably in the conversion
code, like you said.

Who would I contact regarding the fragment program alpha issue? Which is really
a separate bug to this one. I'd also like to contact someone about this bug, so
I guess that would be someone who knows the r300 texture code.

Ok your bug doesn't seems to be related with pixelformat. It's likely
a synchronization problem btw the RGB and A pipeline (which are seperate
on r300/r400) like Roland said. Could you provide the fragment program
you are using ?

I've put the arb programs online at http://z3ro.name/arb. lighting_d_omni is the "main" one. Maybe you'll spot something horribly wrong there and it will be a 2 second fix. :) 

I just tried the same code on an x86 computer, with an r300 (although a different model to my x86_64 computer) and the blue patches bug does not occur.  Perhaps it's some obscure x86_64 bug that no one has caught? 

I did some testing on x86 again, and it seems that the bug does indeed affect x86 now; So either I screwed up the initial testing on x86, or something changed that makes this bug also affect x86 now.  I'll have to do some more investigation. My current idea is to dump the texture it and color values for each texture as it goes through Mesa and the R300 driver, comparing them along the way to figure out where the blue patches come from. I haven't done anything yet, though. 

Is this still an issue?

Closing due to lack of feedback.

Mass version move, cvs -> git