[Mesa-dev] [Mesa-stable] [PATCH 2/2] meta: Write color values in to 'out' variables for all the draw buffers

[Anuj Phogat]

On Mon, May 19, 2014 at 3:52 PM, Chris Forbes <chrisf at ijw.co.nz> wrote:
> If you're going to do that, you'd really want to add draw buffer count
> to the cache key (and i guess this might be the point where you
> convert the blit shader cache to be a hashtable), to avoid recompiling
> all the time if the app does two blits with the same target but
> different draw buffer counts.
>
> This all seems like a huge amount of extra machinery to avoid using
> gl_FragColor and having the backend just take care of it, though. What
> do we actually gain from this?
>
Right, It doesn't look like worth doing it. I was avoiding 'gl_FragColor'
just because it's deprecated in GLSL 130. Using 'gl_FragColor" here
will work perfectly fine. But, seems like it won't work in fragment
shader for msaa blits because msaa blit shader makes use of non
vec4 output types. Although, blitting to multiple multisample buffers
is not a common use case, we'll have similar shader recompilation
problem due to changing draw buffers count.

For now, I'll go ahead and make changes to use 'gl_FragColor' for
non-multisample blits.
>
> On Tue, May 20, 2014 at 10:22 AM, Anuj Phogat <anuj.phogat at gmail.com> wrote:
>> On Mon, May 19, 2014 at 3:12 PM, Chris Forbes <chrisf at ijw.co.nz> wrote:
>>> On Tue, May 20, 2014 at 8:20 AM, Anuj Phogat <anuj.phogat at gmail.com> wrote:
>>>> @@ -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;
>>>
>>> You can't depend on the number of bound draw buffers here. These
>>> shaders get generated on first use, and cached for the life of the
>>> context.
>> Nice catch. I'll add a condition to recompile the shader if number of
>> draw buffers change.