[Mesa-dev] [PATCH] gallivm: do clamping of border color correctly for all formats
Roland Scheidegger
sroland at vmware.com
Mon Aug 19 06:52:09 PDT 2013
Am 19.08.2013 12:45, schrieb Jose Fonseca:
> Looks alright to me.
>
> I just wonder if it would be possible to factor this logic into a separate function somehow.
Initially I wanted to factor out the logic for figuring out if
snorm/unorm clamping is necessary, because coord clamping for shadow ref
requires the same logic. However, I had to ditch that idea because some
of the formats require some specialized clamps essentially, and also
it's not really worth it for that because while shdaodw ref clamping
requires the same logic, only very very few formats are allowed there
hence none of the complicated logic is necessary there.
Could however still easily do a clamp_select_border_color function, or
alternatively could move the border color computation outside the fetch
loop and just precalculate it somewhere at the beginning
(build_sample_common), storing the clamped border color in the sample
build context, which would only leave the per-channel logic of doing
clamp/application of border color in fetch. Not sure though what's better.
Roland
>
> 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