[Mesa-dev] [RFC] llvmpipe texture coordinate rounding

[Jeff Muizelaar]

On Feb 14, 2014, at 1:00 PM, Roland Scheidegger <sroland at vmware.com> wrote:

> Am 14.02.2014 18:07, schrieb Jeff Muizelaar:
> 
> I'll need to take another look and run some tests, though I've got some
> quick comments:
> 
> 
> @@ -1031,16 +1082,28 @@ lp_build_sample_image_linear(struct
> lp_build_sample_context *bld,
>       s = lp_build_mul_imm(&bld->coord_bld, s, 256);
>       if (dims >= 2)
>          t = lp_build_mul_imm(&bld->coord_bld, t, 256);
>       if (dims >= 3)
>          r = lp_build_mul_imm(&bld->coord_bld, r, 256);
>    }
> 
>    /* convert float to int */
> +   half = lp_build_const_vec(bld->gallivm, bld->coord_bld.type, 0.5);
> +   s = lp_build_add(&bld->coord_bld, s, half);
> +   s = LLVMBuildFPToSI(builder, s, i32_vec_type, "");
> +   if (dims >= 2) {
> +      t = lp_build_add(&bld->coord_bld, t, half);
> +      t = LLVMBuildFPToSI(builder, t, i32_vec_type, "");
> +   }
> +   if (dims >= 3) {
> +      r = lp_build_add(&bld->coord_bld, r, half);
> +      r = LLVMBuildFPToSI(builder, r, i32_vec_type, "");
> +   }
> +
>    s = LLVMBuildFPToSI(builder, s, i32_vec_type, "");
>    if (dims >= 2)
>       t = LLVMBuildFPToSI(builder, t, i32_vec_type, "");
>    if (dims >= 3)
>       r = LLVMBuildFPToSI(builder, r, i32_vec_type, "");
> This looks quite incorrect you're converting the s/t/r coords twice from
> float to int.

Yep. I forgot to remove this second hunk.

>    /* subtract 0.5 (add -128) */
>    i32_c128 = lp_build_const_int_vec(bld->gallivm, i32.type, -128);
> 
> 
> Also, the add looks iffy as it won't work correctly if the coords are
> negative, since the FPToSI is of course trunc, not floor.

I think it will be ok because the REPEAT case avoids negative coord before converting to int and the other cases clamp to 0.

> Maybe instead of using add + fptosi should just use lp_build_iround
> (which is just one sse instruction too on x86 though if you're targeting
> another arch it will definitely be more code at least unless someone
> adds an intrinsic for it if the cpu even has one). Might not matter
> though depending on address mode…

Yeah, that might be a better idea.

> 
> And I might be missing something why do you think the new repeat code is
> faster? Though that might also depend on arch_rounding being available
> and such but at first looks it seems slightly more complex to me.

The current code converts integer and fractional parts to integer separately. It also does the subtract 0.5 in floating point instead of integer arithmetic (-128).

-Jeff
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140214/9f66ac55/attachment.html>