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

[Tilman Sauerbeck]

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?

Regards,
Tilman

-- 
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/36c97df6/attachment.pgp>