[Mesa-dev] About merging pipe-video to master

[Henri Verbeet]

2011/7/12 Christian König <deathsimple at vodafone.de>:
> it works with my available hardware (no piglit regressions). The changes
> to the winsys code is about making a bo optional, even when the reg
> informations says it isn't. This is useful for registers where only a
> subset of the bits needs to be informed about a bo relocation, and seems
> to work pretty fine as long as you don't touch those bits.
>
Well, ok, but I'd expect to find that information in the commit log.
As it is, 68cc6bc5d8b6986acc7f5780d705f4ae9be2a446 removes
REG_FLAG_NEED_BO, then e602ecf9ef2f66289bcb159fdbdce2c76e3c07c1 adds
it back without much of an explanation. Also, what subset is that?
After this patch both places that touch the register pass NULL for the
bo.

> +		// TODO get BLEND_CLAMP state from rasterizer state
Is this comment still accurate?

> +				color_info, ~S_0280A0_BLEND_CLAMP(1), NULL);
Did you mean to write "~C_0280A0_BLEND_CLAMP" there?

> On top of that it implements "clamp_fragment_color" also for the blender
> state, this is necessary because the blender will otherwise clamp the
> colour to [0,1] for unsigned and [-1,1] for signed buffers.
>
> This is another piece needed to get arb_color_buffer_float working
> correctly (without the need to recompile the shaders each time).
>
You should probably remove the existing code that does that in
r600_shader_from_tgsi() then, at least for r600. Either way, it sounds
like this is a mostly independent change from the rest of pipe-video
and should go to r600g through the regular way, probably through the
mailing list first.

> +        switch (res->usage) {
> +        case PIPE_USAGE_STREAM:
> +        case PIPE_USAGE_STAGING:
> +        case PIPE_USAGE_STATIC:
> +        case PIPE_USAGE_IMMUTABLE:
> +                return FALSE;
> +
> +        default:
> +                return TRUE;
> +        }
At the very least this has whitespace errors. Why do we want this?
Like the other change, the commit log for this change
(77217af40d67612d1f1089ca188393d27a8a038f) isn't very descriptive. If
it wasn't for the commit not being a merge commit, it would even be
ambiguous if "Merge fix" means merging a fix or fixing a merge.