[Mesa-dev] [PATCH 1/3] st/mesa: do vertex and fragment color clamping in shaders

[Tilman Sauerbeck]

Marek Olšák [2012-02-21 21:36]:
> Hi Tilman,
> 
> Thanks for the info. I didn't consider outputs to be readable, sorry.
> A quick fix would be to move the outputs to temps. There is a GLSL pass
> for that and it can be enabled by reporting
> PIPE_SHADER_CAP_OUTPUT_READ --> 0 in r600_pipe.c. Can you try that and
> see if it helps?

Confirmed; disabling PIPE_SHADER_CAP_OUTPUT_READ fixes the problem.

Thanks,
Tilman

> On Tue, Feb 21, 2012 at 9:17 PM, Tilman Sauerbeck <tilman at code-monkey.de> wrote:
> > Tilman Sauerbeck [2012-02-12 11:31]:
> >> Marek Olšák [2012-01-23 13:32]:
> >> > For ARB_color_buffer_float. Most hardware can't do it and st/mesa is
> >> > the perfect place for a fallback.
> >>
> >> This breaks lighting in Heroes of Newerth on my rv730:
> >>   http://files.code-monkey.de/frag_color_clamp_bad.png (after patch)
> >>   http://files.code-monkey.de/frag_color_clamp_good.png (before patch)
> >> I can provide a trace file that shows the problem (3.5GB, so will take
> >> a while to upload).
> >
> > The problematic fragment shader looks like this:
> >
> > void main() {
> >  gl_FragColor = vec4(0.0);
> >  gl_FragColor += texture2D(...);
> >  gl_FragColor += texture2D(...);
> >  gl_FragColor += texture2D(...);
> >  gl_FragColor += texture2D(...);
> >  gl_FragColor *= 0.25;
> >  gl_FragColor *= v_vColor; // varying vec4
> > }
> >
> > Before this patch landed, mesa would generate the following TGSI for
> > that shader:
> >
> >  0: MOV TEMP[0].x, -CONST[1].xxxx
> >  1: MOV TEMP[0].y, -CONST[1].xxxx
> >  2: ADD TEMP[0].xy, IN[0].xyyy, TEMP[0].xyyy
> >  3: TEX TEMP[0], TEMP[0].xyyy, SAMP[0], 2D
> >  4: MOV OUT[0], TEMP[0]
> >  5: MOV TEMP[0].x, -CONST[1].xxxx
> >  6: MOV TEMP[0].y, CONST[1].xxxx
> >  7: ADD TEMP[0].xy, IN[0].xyyy, TEMP[0].xyyy
> >  8: TEX TEMP[0], TEMP[0].xyyy, SAMP[0], 2D
> >  9: ADD OUT[0], OUT[0], TEMP[0]
> >  10: ADD TEMP[0].xy, IN[0].xyyy, CONST[1].xxxx
> >  11: TEX TEMP[0], TEMP[0].xyyy, SAMP[0], 2D
> >  12: ADD OUT[0], OUT[0], TEMP[0]
> >  13: MOV TEMP[0].x, CONST[1].xxxx
> >  14: MOV TEMP[0].y, -CONST[1].xxxx
> >  15: ADD TEMP[0].xy, IN[0].xyyy, TEMP[0].xyyy
> >  16: TEX TEMP[0], TEMP[0].xyyy, SAMP[0], 2D
> >  17: ADD TEMP[0], OUT[0], TEMP[0]
> >  18: MUL TEMP[0], TEMP[0], IMM[0].xxxx
> >  19: MUL OUT[0], TEMP[0], IN[1]
> >  20: END
> >
> > With this patch, instructions 4, 9, 12 and 19 are replaced by their
> > _SAT variants.
> >
> > If I hack the code to only use saturate in the final instruction
> > (MUL -> MUL_SAT), the game looks okay.
> >
> > I'm wondering whether the current behaviour is correct in that it uses
> > saturate ops for the intermediate results as well.
> >
> > Issue 24 of the ARB_color_buffer_float says:
> >
> >> 24. Does this extension interact with the ARB_fragment_program or
> >>     ARB_fragment_shader extensions?
> >>
> >>     RESOLVED:  Yes.  The only interaction is that the fragment color
> >>     clamp enable determines if the final color(s) produced by the
> >>     fragment program/shader has its components clamped to [0,1].
> >>
> >>     However, the fragment color clamp enable affects only the final
> >>     result; it does NOT affect any computations performed during
> >>     program execution. Note that the same clamping can be done
> >>     explicitly in a fragment program or shader.
> >>     ARB_fragment_program provides the "_SAT" opcode suffix to clamp
> >>     instruction results to [0,1].
> >
> > If I'm understanding this language correctly, _and_ it applies to GLSL
> > too then it seems like Mesa should only use saturation in the final
> > operation that's writing to gl_FragColor, but not in the earlier ones?

-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120221/5d97be68/attachment.pgp>