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 <srol...@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 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev