[Mesa-dev] [PATCH 3/3] r600g: implement fragment and vertex color clamp

[Marek Olšák]

On Mon, Jun 27, 2011 at 2:38 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 25.06.2011 00:22, schrieb Vadim Girlin:
>> On 06/24/2011 11:38 PM, Jerome Glisse wrote:
>>> On Fri, Jun 24, 2011 at 12:29 PM, Vadim Girlin<vadimgirlin at gmail.com>
>>> wrote:
>>>> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=38440
>>>>
>>>> Signed-off-by: Vadim Girlin<vadimgirlin at gmail.com>
>>>
>>> As discussed previously, there is better to handle this. I think best
>>> solution is to always add the instruction and to conditionally execute
>>> them thanks to the boolean constant. If this reveal to have a too big
>>> impact on shader, other solution i see is adding a cf block with those
>>> instructions and to enable or disable that block (cf_nop) and reupload
>>> shader that would avoid a rebuild.
>>
>> I know its not optimal to do a full rebuild, but rebuild is needed only
>> when the application will use the same shader in different clamping
>> states. It won't be a problem if the application doesn't change clamping
>> state or if it changes the state but uses each shader in one state only.
>> So assuming that typical app will not use one shader in both states, it
>> shouldn't be a problem. Is this assumption wrong? I'm not really sure
>> because I have no much experience in this. But if it's wrong then it's
>> probably better for performance to build and cache both versions.
> I tend to think you're right apps probably don't want to use the same
> shader both with and without clamping.

It still can be changed by st/mesa or by u_blitter and u_blit for
various reasons. IIRC, the OpenGL default is TRUE if the current
framebuffer is fixed-point including texture_snorm and FALSE
otherwise, so changing the framebuffer may change the clamp color
state. Besides that, the u_blitter and u_blit operations always
disable the clamping, so if a framebuffer is fixed-point and thus
clamp color state is TRUE (if not changed by an app), the driver may
receive state changes that turn the clamping on, off, on, off,... with
the blit operations turning it off and everything else turning it on.
The state might be changing pretty much all the time and doing full
shader rebuilds repeatedly may turn some apps into a slideshow.

Therefore we must ensure that a fragment shader is set/built as late
as possible, i.e. in draw_vbo. Each shader variant should be compiled
once at most and stored for later use. create_fs_state and
bind_fs_state should not do anything except copying the parameters.

Marek

>
>>
>> Also it seems last write to color output components is often done with
>> op2 instructions, so there is another optimization possible - to find
>> last op2 writes and set clamp bit instead of using additional
>> instructions. Probably it's not very hard to implement with this patch.
>> AFAICS it will be impossible with other suggested solutions.
>>
>
> Such optimization should probably be part of a more generic optimization
> pass.
>
> Roland
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>