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

[Keith Whitwell]

On Mon, 2011-06-27 at 15:32 +0200, Marek Olšák wrote:
> 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.

I haven't looked at the code, maybe this is irrelevant for some reason,
but the alternative to doing rebuilds when this type of state changes is
to permit >1 compiled version of the shader to exist, parameterized in
different ways.  That way the ping-pong scenario you describe results in
swapping between shaders (which should be cheap), rather than
rebuilding.

> 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. 

Actually it sounds like you're describing the same idea here...

Keith