Bug 11931

Summary:	Mesa: banding in rendering when using blending for transparency, asm statement issue?
Product:	Mesa	Reporter:	Brice Goglin <brice.goglin>
Component:	Other	Assignee:	mesa-dev
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	medium
Version:	6.5
Hardware:	Other
OS:	All
URL:	http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=423739
Whiteboard:
i915 platform:		i915 features:

Description Brice Goglin 2007-08-10 09:41:48 UTC

This bug has been reported by Dan Torop on the Debian BTS 3 months ago. Here's a summary of what he said. See Debian bug URL above for details, he did a lot of testing.

When using blending for transparency, banding occurs in the output and colors have a greenish cast.

There's a example code at http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=faded.c;att=1;bug=423739
It should display a 50% gray screen, but it displays greenish bands depending on mesa version and whether or not DRI. 

In the end, there's a small patch against an asm statement in spantmp2.h which fixes the problem in Debian on r200 in mesa 6.5.2 and 6.5.3
http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=45;filename=read_rgba.patch;att=1;bug=423739

But:
- Is the "0" usage in "asm" statements a gcc-specific extension?  If
so, would this break x86 r200 DRI on non-gcc compilers, if any such
are used?
- Lots of other DRI drivers use spantmp2.h, why wouldn't this have
shown up broken in these by now, if this were really the problem?

Here's the reasoning behind the patch, spelled out in case it is utterly wrong:

The patch is to make the 32-bit color buffer READ_RGBA macro, in its
x86-assembly variant, know that the input parameter of the "bswap"
instruction resides in the same register as the output parameter
(change the "r" to a "0" for the input).

Looking at the assembly code of r200ReadRGBAPixels_ARGB8888 (generated
by dri/r200/r200_span.c via dri/common/spantmp2.h) the patched code
generates something like:

mov    %eax,0xffffffd0(%ebp)
bswap  %eax
ror    $0x8,%eax

while the unpatched code, when compiled -O2, loses the initial "mov"
instruction and hence the %eax register is actually never even
initialized...  This would also explain why turning off register-move
optimizations is another way to fix the problem.

Which follows from the gcc docs:

Only a number in the constraint can guarantee that one operand will be
in the same place as another.  The mere fact that `foo' is the value of
both operands is not enough to guarantee that they will be in the same
place in the generated assembler code.

Comment 1 Roland Scheidegger 2007-08-12 04:30:10 UTC

(In reply to comment #0)
> In the end, there's a small patch against an asm statement in spantmp2.h which
> fixes the problem in Debian on r200 in mesa 6.5.2 and 6.5.3
> http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=45;filename=read_rgba.patch;att=1;bug=423739
> 
> But:
> - Is the "0" usage in "asm" statements a gcc-specific extension?  If
> so, would this break x86 r200 DRI on non-gcc compilers, if any such
> are used?
Isn't the whole usage of inline asm gcc-specific anyway?

> - Lots of other DRI drivers use spantmp2.h, why wouldn't this have
> shown up broken in these by now, if this were really the problem?
The problem would only show up if compiled with x86 optimizations, and only in swrast fallbacks, which are rare in "normal" apps.

> Here's the reasoning behind the patch, spelled out in case it is utterly wrong:
> 
> The patch is to make the 32-bit color buffer READ_RGBA macro, in its
> x86-assembly variant, know that the input parameter of the "bswap"
> instruction resides in the same register as the output parameter
> (change the "r" to a "0" for the input).
> 
> Looking at the assembly code of r200ReadRGBAPixels_ARGB8888 (generated
> by dri/r200/r200_span.c via dri/common/spantmp2.h) the patched code
> generates something like:
> 
> mov    %eax,0xffffffd0(%ebp)
> bswap  %eax
> ror    $0x8,%eax
> 
> while the unpatched code, when compiled -O2, loses the initial "mov"
> instruction and hence the %eax register is actually never even
> initialized...  This would also explain why turning off register-move
> optimizations is another way to fix the problem.
> 
> Which follows from the gcc docs:
> 
> Only a number in the constraint can guarantee that one operand will be
> in the same place as another.  The mere fact that `foo' is the value of
> both operands is not enough to guarantee that they will be in the same
> place in the generated assembler code.

When I last looked at this, it seemed pretty reasonable, but I'm no expert at inline assembly. I'm going to commit it now.

Comment 2 Brice Goglin 2007-08-13 09:37:40 UTC

Thanks for fixing this so quickly!

Could we get this fix in mesa_7_0_branch too?

Comment 3 Brian Paul 2007-08-13 13:17:49 UTC

Done (applied to 7.0 branch).

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.