On Sat 11 Jun 2016, Jason Ekstrand wrote:
> This commit switches clear colors to use #if's instead of a C if. This
> lets us properly handle SNB where the clear color field doesn't exist.
> ---
> src/intel/isl/isl_surface_state.c | 44
> +++++++++++++++++++--------------------
> 1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/src/intel/isl/isl_surface_state.c
> b/src/intel/isl/isl_surface_state.c
> index db90936..aa720d8 100644
> --- a/src/intel/isl/isl_surface_state.c
> +++ b/src/intel/isl/isl_surface_state.c
> @@ -372,31 +372,31 @@ isl_genX(surf_fill_state_s)(const struct isl_device
> *dev, void *state,
> }
> #endif
>
> - if (GEN_GEN <= 8) {
> - /* Prior to Sky Lake, we only have one bit for the clear color which
> - * gives us 0 or 1 in whatever the surface's format happens to be.
> - */
> - if (isl_format_has_int_channel(info->view->format)) {
> - for (unsigned i = 0; i < 4; i++) {
> - assert(info->clear_color.u32[i] == 0 ||
> - info->clear_color.u32[i] == 1);
> - }
> - } else {
> - for (unsigned i = 0; i < 4; i++) {
> - assert(info->clear_color.f32[i] == 0.0f ||
> - info->clear_color.f32[i] == 1.0f);
> - }
> +#if GEN_GEN >= 9
> + s.RedClearColor = info->clear_color.u32[0];
> + s.GreenClearColor = info->clear_color.u32[1];
> + s.BlueClearColor = info->clear_color.u32[2];
> + s.AlphaClearColor = info->clear_color.u32[3];
The gen9 block looks good.
> +#elif GEN_GEN >= 7
> + /* Prior to Sky Lake, we only have one bit for the clear color which
> + * gives us 0 or 1 in whatever the surface's format happens to be.
> + */
> + if (isl_format_has_int_channel(info->view->format)) {
> + for (unsigned i = 0; i < 4; i++) {
> + assert(info->clear_color.u32[i] == 0 ||
> + info->clear_color.u32[i] == 1);
> }
> - s.RedClearColor = info->clear_color.u32[0] != 0;
> - s.GreenClearColor = info->clear_color.u32[1] != 0;
> - s.BlueClearColor = info->clear_color.u32[2] != 0;
> - s.AlphaClearColor = info->clear_color.u32[3] != 0;
> } else {
> - s.RedClearColor = info->clear_color.u32[0];
> - s.GreenClearColor = info->clear_color.u32[1];
> - s.BlueClearColor = info->clear_color.u32[2];
> - s.AlphaClearColor = info->clear_color.u32[3];
> + for (unsigned i = 0; i < 4; i++) {
> + assert(info->clear_color.f32[i] == 0.0f ||
> + info->clear_color.f32[i] == 1.0f);
> + }
> }
> + s.RedClearColor = info->clear_color.u32[0] != 0;
> + s.GreenClearColor = info->clear_color.u32[1] != 0;
> + s.BlueClearColor = info->clear_color.u32[2] != 0;
> + s.AlphaClearColor = info->clear_color.u32[3] != 0;
This block looks broken, both pre- and post-patch. Is this code actually
used anywhere in the Vulkan driver? I expect not, as the driver doesn't
use the CCS yet.
In the for loop, the code asserts clear_color.f32[i] is 0.0f or 1.0f.
I believe the assignment expressions should also use f32, not u32. That
is,
s.RedClearColor = info->clear_color.f32[0] != 0.0f;
s.GreenClearColor = info->clear_color.f32[1] != 0.0f;
s.BlueClearColor = info->clear_color.f32[2] != 0.0f;
s.AlphaClearColor = info->clear_color.f32[3] != 0.0f;
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
