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?
To Vadim: Now that we have a GLSL pass for lowering output reads, are you okay with removing PIPE_SHADER_CAP_OUTPUT_READ? It makes the fallback for glClampColorARB really non-trivial. Marek On Tue, Feb 21, 2012 at 9:17 PM, Tilman Sauerbeck <[email protected]> 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? > > 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? _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
