[PATCH] Fix OFFSET_[XYZ] macro in shader.h
Subject and patch comment says all. Cheers, -- Yang Zhao http://yangman.ca
2009/3/11 Matthias Hopf
-#define OFFSET_X(x) (x) +#define OFFSET_X(x) (((x) >= 0) ? ((x) & 0x0f) : (((-(x)-1) & 0x0f) ^ 0x1f))
Shouldn't
#define OFFSET_X(x) ((x) & 0x1f) /* 5 bit signed two's complement -16..15 */
be enough?...
Hm, you're right. I wrote this pretty early in the morning, and apparently didn't think it through enough. Thanks, -- Yang Zhao http://yangman.ca -- To unsubscribe, e-mail: radeonhd+unsubscribe@opensuse.org For additional commands, e-mail: radeonhd+help@opensuse.org
On Mar 11, 09 08:13:54 -0700, Yang Zhao wrote:
2009/3/11 Matthias Hopf
: -#define OFFSET_X(x) (x) +#define OFFSET_X(x) (((x) >= 0) ? ((x) & 0x0f) : (((-(x)-1) & 0x0f) ^ 0x1f))
Shouldn't
#define OFFSET_X(x) ((x) & 0x1f) /* 5 bit signed two's complement -16..15 */
be enough?...
Hm, you're right. I wrote this pretty early in the morning, and apparently didn't think it through enough.
Not exactly. Your version is the only one 100% correct, and I'm *really*
unsure whether we should go for the 100% correct or the more readable
version here :-]
Anyway, committing the updated version now, if you think we should go
for the 100% correct way, we can change later ;)
CU
Matthias
--
Matthias Hopf
2009/3/11 Matthias Hopf
On Mar 11, 09 08:13:54 -0700, Yang Zhao wrote:
2009/3/11 Matthias Hopf
: -#define OFFSET_X(x) (x) +#define OFFSET_X(x) (((x) >= 0) ? ((x) & 0x0f) : (((-(x)-1) & 0x0f) ^ 0x1f))
Shouldn't
#define OFFSET_X(x) ((x) & 0x1f) /* 5 bit signed two's complement -16..15 */
be enough?...
Hm, you're right. I wrote this pretty early in the morning, and apparently didn't think it through enough.
Not exactly. Your version is the only one 100% correct, and I'm *really* unsure whether we should go for the 100% correct or the more readable version here :-]
Anyway, committing the updated version now, if you think we should go for the 100% correct way, we can change later ;)
Ack, I *just* noticed that the specification actually calls for S3.1 fixed-point float instead of signed int(5). I felt something was off when the value is 5 bits but has the range [-8,8) (This is what happens when you hack at 1:30 in the morning, after switching to DST just days prior.) Suggestions on the sanest way to handle this? -- Yang Zhao http://yangman.ca -- To unsubscribe, e-mail: radeonhd+unsubscribe@opensuse.org For additional commands, e-mail: radeonhd+help@opensuse.org
On Mar 11, 09 08:31:13 -0700, Yang Zhao wrote:
Ack, I *just* noticed that the specification actually calls for S3.1 fixed-point float instead of signed int(5). I felt something was off when the value is 5 bits but has the range [-8,8)
Ugh.
Suggestions on the sanest way to handle this?
Using a float input - and AFAIR S3.1 means sign + magnitude:
#define ABS(x) ((x) < 0 ? -(x) : (x))
#define OFFSET_X(x) (((x) < 0 ? 0x10 : 0) | ((ABS(x)*2) & 0x0f)) /* S3.1 fix point */
What do you think?
I also think that we should comment these macros. Probably best to do in
r600_demo first, the difference is small (I wonder why there is any,
apart from the still broken license header in r600_demo).
Matthias
--
Matthias Hopf
2009/3/11 Matthias Hopf
On Mar 11, 09 08:31:13 -0700, Yang Zhao wrote:
Ack, I *just* noticed that the specification actually calls for S3.1 fixed-point float instead of signed int(5)... ... Suggestions on the sanest way to handle this?
Using a float input - and AFAIR S3.1 means sign + magnitude:
#define ABS(x) ((x) < 0 ? -(x) : (x)) #define OFFSET_X(x) (((x) < 0 ? 0x10 : 0) | ((ABS(x)*2) & 0x0f)) /* S3.1 fix point */
I think what we want is shifted two's complement, since the expected range is [-8, 8), which, with just 1 bit to the right of decimal point, gives [-8.0, 7.5] in 0.5 increments. So, #define OFFSET_X(x) ((int)((x) * 2) & 0x1f) /* S3.1 fixed point, two's complement*/ The small test code I wrote seems to be behaving sanely for the values that are exactly representable, and rounding towards 0 for those that aren't. I'll verify that this behaves as expected on the shader hardware sometime later today. Cheers, -- Yang Zhao http://yangman.ca -- To unsubscribe, e-mail: radeonhd+unsubscribe@opensuse.org For additional commands, e-mail: radeonhd+help@opensuse.org
On Mar 11, 09 09:22:17 -0700, Yang Zhao wrote:
I think what we want is shifted two's complement, since the expected range is [-8, 8), which, with just 1 bit to the right of decimal point, gives [-8.0, 7.5] in 0.5 increments.
So,
#define OFFSET_X(x) ((int)((x) * 2) & 0x1f) /* S3.1 fixed point, two's complement*/
The small test code I wrote seems to be behaving sanely for the values that are exactly representable, and rounding towards 0 for those that aren't.
I'll verify that this behaves as expected on the shader hardware sometime later today.
Yes, please, because in that case the description S3.1 would IMHO be
wrong. Because it means 1 sign bit, and 3.1 value bits.
CU
Matthias
--
Matthias Hopf
2009/3/11 Matthias Hopf
On Mar 11, 09 09:22:17 -0700, Yang Zhao wrote:
I think what we want is shifted two's complement, since the expected range is [-8, 8), which, with just 1 bit to the right of decimal point, gives [-8.0, 7.5] in 0.5 increments.
This seems to indeed be the correct input encoding; patch attached.
Yes, please, because in that case the description S3.1 would IMHO be wrong. Because it means 1 sign bit, and 3.1 value bits.
AFAIK there's no industry-wide accepted notation for describing fixed-point value, at least not for the Sx.y form. I thought I saw the notation explained in one of the earlier documentations, but I seem to have been imagining things. In any case, as long as the reference is consistent within itself, we should be OK. Cheers, -- Yang Zhao http://yangman.ca
On Mar 11, 09 19:56:34 -0700, Yang Zhao wrote:
2009/3/11 Matthias Hopf
: On Mar 11, 09 09:22:17 -0700, Yang Zhao wrote:
I think what we want is shifted two's complement, since the expected range is [-8, 8), which, with just 1 bit to the right of decimal point, gives [-8.0, 7.5] in 0.5 increments.
This seems to indeed be the correct input encoding; patch attached.
Thanks. Pushed.
CU
Matthias
--
Matthias Hopf
participants (2)
-
Matthias Hopf
-
Yang Zhao