On Mon, May 19, 2014 at 2:45 PM, Matt Turner <matts...@gmail.com> wrote:
> On Mon, May 19, 2014 at 1:20 PM, Anuj Phogat <anuj.pho...@gmail.com> wrote:
>> _mesa_meta_setup_blit_shader() currently generates a fragment shader
>> which, irrespective of the number of draw buffers, writes the color
>> to only one output variable. Current shader rely on an undefined
>> behavior and possibly works by chance.
>>
>> From OpenGL 4.0 spec, page 256:
>> "If a fragment shader writes to gl_FragColor, DrawBuffers specifies a
>> set of draw buffers into which the single fragment color defined by
>> gl_FragColor is written. If a fragment shader writes to gl_FragData,
>> or a user-defined varying out variable, DrawBuffers specifies a set
>> of draw buffers into which each of the multiple output colors defined
>> by these variables are separately written. If a fragment shader writes
>> to none of gl_FragColor, gl_FragData, nor any user defined varying out
>> variables, the values of the fragment colors following shader execution
>> are undefined, and may differ for each fragment color."
>>
>> OpenGL 4.4 spec, page 463, added an additional line in this section:
>> "If some, but not all user-defined output variables are written, the
>> values of fragment colors corresponding to unwritten variables are
>> similarly undefined."
>>
>> Cc: <mesa-sta...@lists.freedesktop.org>
>> Signed-off-by: Anuj Phogat <anuj.pho...@gmail.com>
>> ---
>> src/mesa/drivers/common/meta.c | 23 ++++++++++++++++++-----
>> 1 file changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
>> index 87609b4..4897cd9 100644
>> --- a/src/mesa/drivers/common/meta.c
>> +++ b/src/mesa/drivers/common/meta.c
>> @@ -247,7 +247,8 @@ _mesa_meta_setup_blit_shader(struct gl_context *ctx,
>> struct blit_shader *shader = choose_blit_shader(target, table);
>> const char *vs_input, *vs_output, *fs_input, *fs_output;
>> const char *vs_preprocess = "", *fs_preprocess = "";
>> - const char *fs_output_decl = "";
>> + const char *fs_output_decl = "", *for_loop = "";
>> + const int draw_buf_count = ctx->DrawBuffer->_NumColorDrawBuffers;
>>
>> if (ctx->Const.GLSLVersion < 130) {
>> vs_input = "attribute";
>> @@ -255,12 +256,23 @@ _mesa_meta_setup_blit_shader(struct gl_context *ctx,
>> fs_preprocess = "#extension GL_EXT_texture_array : enable";
>> fs_output = "gl_FragColor";
>> } else {
>> - vs_preprocess = fs_preprocess = "#version 130";
>> + vs_preprocess = "#version 130";
>> vs_input = fs_input = "in";
>> vs_output = "out";
>> - fs_output = "out_color";
>> - fs_output_decl = "out vec4 out_color;";
>> shader->func = "texture";
>> + if (draw_buf_count > 1) {
>> + fs_preprocess = ralloc_asprintf(mem_ctx,
>> + "#version 130\n"
>> + "#define NUM_DRAW_BUFS %d",
>> + draw_buf_count);
>> + fs_output = "out_color[i]";
>> + fs_output_decl = "out vec4 out_color[NUM_DRAW_BUFS];";
>> + for_loop = " for (int i = 0; i < NUM_DRAW_BUFS; i++)\n ";
>> + } else {
>> + fs_preprocess = "#version 130";
>> + fs_output = "out_color";
>> + fs_output_decl = "out vec4 out_color;";
>> + }
>
> It's safe to emit a loop with only one iterations. The compiler will
> happily optimize that (it's going to unroll all of these loops
> anyway). Emitting GLSL code for the for loop unconditionally seems
> like it would clean this up some.
>
I wasn't sure if that'll generate extra instructions for one
iteration. I'll clean it
up before pushing.
> With the comments for these two patches addressed, both of these are
>
> Reviewed-by: Matt Turner <matts...@gmail.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
