This looks fine to me. We should probably also do this for snorm formats. I don't care if that's part of this or in a separate patch. --Jason
Reviewed-by: Jason Ekstrand <jason.ekstr...@intel.com> On Wed, Jan 14, 2015 at 11:46 AM, Neil Roberts <n...@linux.intel.com> wrote: > When converting to a format that has fewer bits the previous code was just > shifting off the bits. This doesn't provide very accurate results. For > example > when converting from 8 bits to 5 bits it is equivalent to doing this: > > x * 32 / 256 > > This works as if it's taking a value from a range where 256 represents 1.0 > and > scaling it down to a range where 32 represents 1.0. However this is not > correct because it is actually 255 and 31 that represent 1.0. > > We can do better with a formula like this: > > (x * 31 + 127) / 255 > > The +127 is to make it round correctly. > > The new code has a special case to use uint64_t when the result of the > multiplication would overflow an unsigned int. This function is inline and > only ever called with constant values so hopefully the if statements will > be > folded. > > The main incentive to do this is to make the CPU conversion path pick the > same > values as the hardware would if it did the conversion. This fixes failures > with the ‘texsubimage pbo’ test when using the patches from here: > > http://lists.freedesktop.org/archives/mesa-dev/2015-January/074312.html > > v2: Use 64-bit arithmetic when src_bits+dst_bits > 32 > --- > > Ok, this time I've actually run it through Piglit before sending it. > It doesn't cause any regressions at least on IvyBridge. > > src/mesa/main/format_utils.h | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/src/mesa/main/format_utils.h b/src/mesa/main/format_utils.h > index b588695..b49415c 100644 > --- a/src/mesa/main/format_utils.h > +++ b/src/mesa/main/format_utils.h > @@ -96,10 +96,19 @@ _mesa_half_to_unorm(uint16_t x, unsigned dst_bits) > static inline unsigned > _mesa_unorm_to_unorm(unsigned x, unsigned src_bits, unsigned dst_bits) > { > - if (src_bits < dst_bits) > + if (src_bits < dst_bits) { > return EXTEND_NORMALIZED_INT(x, src_bits, dst_bits); > - else > - return x >> (src_bits - dst_bits); > + } else { > + unsigned src_half = (1 << (src_bits - 1)) - 1; > + > + if (src_bits + dst_bits > sizeof(x) * 8) { > + assert(src_bits + dst_bits <= sizeof(uint64_t) * 8); > + return (((uint64_t) x * MAX_UINT(dst_bits) + src_half) / > + MAX_UINT(src_bits)); > + } else { > + return (x * MAX_UINT(dst_bits) + src_half) / MAX_UINT(src_bits); > + } > + } > } > > static inline unsigned > -- > 1.9.3 > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
