[Mesa-dev] [PATCH] gallivm: do clamping of border color correctly for all formats

Jose Fonseca jfonseca at vmware.com
Mon Aug 19 03:45:36 PDT 2013


Looks alright to me.

I just wonder if it would be possible to factor this logic into a separate function somehow.

Jose

----- Original Message -----
> From: Roland Scheidegger <sroland at vmware.com>
> 
> Turns out it is actually very complicated to figure out what a format really
> is wrt range, as using channel information for determining unorm/snorm etc.
> doesn't work for a bunch of cases - namely compressed, subsampled, other.
> Also while here add clamping for uint/sint as well - d3d10 doesn't actually
> need this (can only use ld with these formats hence no border) and we could
> do this outside the shader for GL easily (due to the fixed texture/sampler
> relation) do it here do just so I can forget about it.
> ---
>  src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c |  162
>  ++++++++++++++++++---
>  1 file changed, 144 insertions(+), 18 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> index 76de006..f61c6c5 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> @@ -42,6 +42,7 @@
>  #include "util/u_math.h"
>  #include "util/u_format.h"
>  #include "util/u_cpu_detect.h"
> +#include "util/u_format_rgb9e5.h"
>  #include "lp_bld_debug.h"
>  #include "lp_bld_type.h"
>  #include "lp_bld_const.h"
> @@ -206,33 +207,158 @@ lp_build_sample_texel_soa(struct
> lp_build_sample_context *bld,
>                                    lp_build_const_int32(bld->gallivm, chan));
>              LLVMValueRef border_chan_vec =
>                 lp_build_broadcast_scalar(&bld->float_vec_bld, border_chan);
> +            LLVMValueRef min_clamp = NULL;
> +            LLVMValueRef max_clamp = NULL;
>  
>              if (!bld->texel_type.floating) {
>                 border_chan_vec = LLVMBuildBitCast(builder, border_chan_vec,
>                                                    bld->texel_bld.vec_type,
>                                                    "");
>              }
> -            else {
> -               /*
> -                * For normalized format need to clamp border color
> (technically
> -                * probably should also quantize the data). Really sucks
> doing this
> -                * here but can't avoid at least for now since this is part
> of
> -                * sampler state and texture format is part of sampler_view
> state.
> -                */
> +            /*
> +             * For normalized format need to clamp border color (technically
> +             * probably should also quantize the data). Really sucks doing
> this
> +             * here but can't avoid at least for now since this is part of
> +             * sampler state and texture format is part of sampler_view
> state.
> +             * (Could definitely do it outside per-sample loop but llvm
> should
> +             * take care of that.)
> +             * GL expects also expects clamping for uint/sint formats too so
> +             * do that as well (d3d10 can't end up here with uint/sint since
> it
> +             * only supports them with ld).
> +             */
> +            if (format_desc->layout == UTIL_FORMAT_LAYOUT_PLAIN) {
>                 unsigned chan_type = format_desc->channel[chan_s].type;
>                 unsigned chan_norm = format_desc->channel[chan_s].normalized;
> -               if (chan_type == UTIL_FORMAT_TYPE_SIGNED && chan_norm) {
> -                  LLVMValueRef clamp_min;
> -                  clamp_min = lp_build_const_vec(bld->gallivm,
> bld->texel_type, -1.0F);
> -                  border_chan_vec = lp_build_clamp(&bld->texel_bld,
> border_chan_vec,
> -                                                   clamp_min,
> -                                                   bld->texel_bld.one);
> +               unsigned pure_int =
> format_desc->channel[chan_s].pure_integer;
> +               if (chan_type == UTIL_FORMAT_TYPE_SIGNED) {
> +                  if (chan_norm) {
> +                     min_clamp = lp_build_const_vec(bld->gallivm,
> bld->texel_type, -1.0F);
> +                     max_clamp = bld->texel_bld.one;
> +                  }
> +                  else if (pure_int) {
> +                     /*
> +                      * Border color was stored as int, hence need min/max
> clamp
> +                      * only if chan has less than 32 bits..
> +                      */
> +                     unsigned chan_size = format_desc->channel[chan_s].size
> < 32;
> +                     if (chan_size < 32) {
> +                        min_clamp = lp_build_const_int_vec(bld->gallivm,
> bld->texel_type,
> +                                                           0 - (1 <<
> (chan_size - 1)));
> +                        max_clamp = lp_build_const_int_vec(bld->gallivm,
> bld->texel_type,
> +                                                           (1 << (chan_size
> - 1)) - 1);
> +                     }
> +                  }
> +                  /* TODO: no idea about non-pure, non-normalized! */
>                 }
> -               else if (chan_type == UTIL_FORMAT_TYPE_UNSIGNED && chan_norm)
> {
> -                  border_chan_vec = lp_build_clamp(&bld->texel_bld,
> border_chan_vec,
> -                                                   bld->texel_bld.zero,
> -                                                   bld->texel_bld.one);
> +               else if (chan_type == UTIL_FORMAT_TYPE_UNSIGNED) {
> +                  if (chan_norm) {
> +                     min_clamp = bld->texel_bld.zero;
> +                     max_clamp = bld->texel_bld.one;
> +                  }
> +                  /*
> +                   * Need a ugly hack here, because we don't have
> Z32_FLOAT_X8X24
> +                   * we use Z32_FLOAT_S8X24 to imply sampling depth
> component
> +                   * and ignoring stencil, which will blow up here if we try
> to
> +                   * do a uint clamp in a float texel build...
> +                   * And even if we had that format, mesa st also thinks
> using z24s8
> +                   * means depth sampling ignoring stencil.
> +                   */
> +                  else if (pure_int &&
> +
> !util_format_is_depth_and_stencil(format_desc->format))
> {
> +                     /*
> +                      * Border color was stored as uint, hence never need
> min
> +                      * clamp, and only need max clamp if chan has less than
> 32 bits.
> +                      */
> +                     unsigned chan_size = format_desc->channel[chan_s].size
> < 32;
> +                     if (chan_size < 32) {
> +                        max_clamp = lp_build_const_int_vec(bld->gallivm,
> bld->texel_type,
> +                                                           (1 << chan_size)
> - 1);
> +                     }
> +                     /* TODO: no idea about non-pure, non-normalized! */
> +                  }
> +               }
> +               else if (chan_type == UTIL_FORMAT_TYPE_FIXED) {
> +                  /* TODO: I have no idea what clamp this would need if any!
> */
>                 }
> -               /* not exactly sure about all others but I think should be
> ok? */
> +            }
> +            else {
> +               /* cannot figure this out from format description */
> +               if (format_desc->layout == UTIL_FORMAT_LAYOUT_S3TC) {
> +                  /* s3tc is always unorm */
> +                  min_clamp = bld->texel_bld.zero;
> +                  max_clamp = bld->texel_bld.one;
> +               }
> +               else if (format_desc->layout == UTIL_FORMAT_LAYOUT_RGTC ||
> +                        format_desc->layout == UTIL_FORMAT_LAYOUT_ETC) {
> +                  switch (format_desc->format) {
> +                  case PIPE_FORMAT_RGTC1_UNORM:
> +                  case PIPE_FORMAT_RGTC2_UNORM:
> +                  case PIPE_FORMAT_LATC1_UNORM:
> +                  case PIPE_FORMAT_LATC2_UNORM:
> +                  case PIPE_FORMAT_ETC1_RGB8:
> +                     min_clamp = bld->texel_bld.zero;
> +                     max_clamp = bld->texel_bld.one;
> +                     break;
> +                  case PIPE_FORMAT_RGTC1_SNORM:
> +                  case PIPE_FORMAT_RGTC2_SNORM:
> +                  case PIPE_FORMAT_LATC1_SNORM:
> +                  case PIPE_FORMAT_LATC2_SNORM:
> +                     min_clamp = lp_build_const_vec(bld->gallivm,
> bld->texel_type, -1.0F);
> +                     max_clamp = bld->texel_bld.one;
> +                     break;
> +                  default:
> +                     assert(0);
> +                     break;
> +                  }
> +               }
> +               /*
> +                * all others from subsampled/other group, though we don't
> care
> +                * about yuv (and should not have any from zs here)
> +                */
> +               else if (format_desc->colorspace !=
> UTIL_FORMAT_COLORSPACE_YUV){
> +                  switch (format_desc->format) {
> +                  case PIPE_FORMAT_R8G8_B8G8_UNORM:
> +                  case PIPE_FORMAT_G8R8_G8B8_UNORM:
> +                  case PIPE_FORMAT_G8R8_B8R8_UNORM:
> +                  case PIPE_FORMAT_R8G8_R8B8_UNORM:
> +                  case PIPE_FORMAT_R1_UNORM: /* doesn't make sense but ah
> well */
> +                     min_clamp = bld->texel_bld.zero;
> +                     max_clamp = bld->texel_bld.one;
> +                     break;
> +                  case PIPE_FORMAT_R8G8Bx_SNORM:
> +                     min_clamp = lp_build_const_vec(bld->gallivm,
> bld->texel_type, -1.0F);
> +                     max_clamp = bld->texel_bld.one;
> +                     break;
> +                     /*
> +                      * Note smallfloat formats usually don't need clamping
> +                      * (they still have infinite range) however this is not
> +                      * true for r11g11b10 and r9g9b9e5, which can't
> represent
> +                      * negative numbers (and additionally r9g9b9e5 can't
> represent
> +                      * very large numbers). d3d10 seems happy without
> clamping in
> +                      * this case, but gl spec is pretty clear: "for
> floating
> +                      * point and integer formats, border values are clamped
> to
> +                      * the representable range of the format" so do that
> here.
> +                      */
> +                  case PIPE_FORMAT_R11G11B10_FLOAT:
> +                     min_clamp = bld->texel_bld.zero;
> +                     break;
> +                  case PIPE_FORMAT_R9G9B9E5_FLOAT:
> +                     min_clamp = bld->texel_bld.zero;
> +                     max_clamp = lp_build_const_vec(bld->gallivm,
> bld->texel_type,
> +                                                    MAX_RGB9E5);
> +                     break;
> +                  default:
> +                     assert(0);
> +                     break;
> +                  }
> +               }
> +            }
> +            if (min_clamp) {
> +               border_chan_vec = lp_build_max(&bld->texel_bld,
> border_chan_vec,
> +                                              min_clamp);
> +            }
> +            if (max_clamp) {
> +               border_chan_vec = lp_build_min(&bld->texel_bld,
> border_chan_vec,
> +                                              max_clamp);
>              }
>              texel_out[chan] = lp_build_select(&bld->texel_bld, use_border,
>                                                border_chan_vec,
>                                                texel_out[chan]);
> --
> 1.7.9.5
> 


More information about the mesa-dev mailing list