* [PATCH libdrm] tests: fix MAKE_RGBA macro for 10bpp modes
@ 2017-11-23 18:59 Ilia Mirkin
2017-11-23 19:09 ` Ville Syrjälä
0 siblings, 1 reply; 5+ messages in thread
From: Ilia Mirkin @ 2017-11-23 18:59 UTC (permalink / raw)
To: dri-devel
We need to shift the values up, otherwise we'd end up with a negative
shift. This works for up-to 16-bit components, which is fine for now.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
---
tests/util/pattern.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index 41fb541b..2f9bb384 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -64,11 +64,17 @@ struct color_yuv {
.u = MAKE_YUV_601_U(r, g, b), \
.v = MAKE_YUV_601_V(r, g, b) }
+static inline uint32_t shiftcolor(const struct util_color_component *comp,
+ uint32_t value)
+{
+ return ((value << 8) >> (16 - comp->length)) << comp->offset;
+}
+
#define MAKE_RGBA(rgb, r, g, b, a) \
- ((((r) >> (8 - (rgb)->red.length)) << (rgb)->red.offset) | \
- (((g) >> (8 - (rgb)->green.length)) << (rgb)->green.offset) | \
- (((b) >> (8 - (rgb)->blue.length)) << (rgb)->blue.offset) | \
- (((a) >> (8 - (rgb)->alpha.length)) << (rgb)->alpha.offset))
+ (shiftcolor(&(rgb)->red, (r)) | \
+ shiftcolor(&(rgb)->green, (g)) | \
+ shiftcolor(&(rgb)->blue, (b)) | \
+ shiftcolor(&(rgb)->alpha, (a)))
#define MAKE_RGB24(rgb, r, g, b) \
{ .value = MAKE_RGBA(rgb, r, g, b, 0) }
--
2.13.6
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH libdrm] tests: fix MAKE_RGBA macro for 10bpp modes
2017-11-23 18:59 [PATCH libdrm] tests: fix MAKE_RGBA macro for 10bpp modes Ilia Mirkin
@ 2017-11-23 19:09 ` Ville Syrjälä
2017-11-23 20:14 ` Ilia Mirkin
0 siblings, 1 reply; 5+ messages in thread
From: Ville Syrjälä @ 2017-11-23 19:09 UTC (permalink / raw)
To: Ilia Mirkin; +Cc: dri-devel
On Thu, Nov 23, 2017 at 01:59:28PM -0500, Ilia Mirkin wrote:
> We need to shift the values up, otherwise we'd end up with a negative
> shift. This works for up-to 16-bit components, which is fine for now.
Shouldn't we actually replicate the high bits in the low bits?
>
> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> ---
> tests/util/pattern.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/tests/util/pattern.c b/tests/util/pattern.c
> index 41fb541b..2f9bb384 100644
> --- a/tests/util/pattern.c
> +++ b/tests/util/pattern.c
> @@ -64,11 +64,17 @@ struct color_yuv {
> .u = MAKE_YUV_601_U(r, g, b), \
> .v = MAKE_YUV_601_V(r, g, b) }
>
> +static inline uint32_t shiftcolor(const struct util_color_component *comp,
> + uint32_t value)
> +{
> + return ((value << 8) >> (16 - comp->length)) << comp->offset;
> +}
> +
> #define MAKE_RGBA(rgb, r, g, b, a) \
> - ((((r) >> (8 - (rgb)->red.length)) << (rgb)->red.offset) | \
> - (((g) >> (8 - (rgb)->green.length)) << (rgb)->green.offset) | \
> - (((b) >> (8 - (rgb)->blue.length)) << (rgb)->blue.offset) | \
> - (((a) >> (8 - (rgb)->alpha.length)) << (rgb)->alpha.offset))
> + (shiftcolor(&(rgb)->red, (r)) | \
> + shiftcolor(&(rgb)->green, (g)) | \
> + shiftcolor(&(rgb)->blue, (b)) | \
> + shiftcolor(&(rgb)->alpha, (a)))
>
> #define MAKE_RGB24(rgb, r, g, b) \
> { .value = MAKE_RGBA(rgb, r, g, b, 0) }
> --
> 2.13.6
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH libdrm] tests: fix MAKE_RGBA macro for 10bpp modes
2017-11-23 19:09 ` Ville Syrjälä
@ 2017-11-23 20:14 ` Ilia Mirkin
2017-11-24 13:38 ` Ville Syrjälä
0 siblings, 1 reply; 5+ messages in thread
From: Ilia Mirkin @ 2017-11-23 20:14 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: dri-devel@lists.freedesktop.org
On Thu, Nov 23, 2017 at 2:09 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Nov 23, 2017 at 01:59:28PM -0500, Ilia Mirkin wrote:
>> We need to shift the values up, otherwise we'd end up with a negative
>> shift. This works for up-to 16-bit components, which is fine for now.
>
> Shouldn't we actually replicate the high bits in the low bits?
Not entirely sure what you're proposing...
Ideally we wouldn't be lazy and pass e.g. 16-bit values to MAKE_RGBA
which would then shift down as necessary (and even there, you could
end up with off-by-1's maybe?). For e.g. 0xff, that should become
0x3ff but with my code will become 0x3fc. But for other values, it's
less clear what to do with the low bits. I figured it didn't really
matter.
Do you have a concrete proposal?
-ilia
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH libdrm] tests: fix MAKE_RGBA macro for 10bpp modes
2017-11-23 20:14 ` Ilia Mirkin
@ 2017-11-24 13:38 ` Ville Syrjälä
2017-11-24 15:23 ` Ilia Mirkin
0 siblings, 1 reply; 5+ messages in thread
From: Ville Syrjälä @ 2017-11-24 13:38 UTC (permalink / raw)
To: Ilia Mirkin; +Cc: dri-devel@lists.freedesktop.org
On Thu, Nov 23, 2017 at 03:14:46PM -0500, Ilia Mirkin wrote:
> On Thu, Nov 23, 2017 at 2:09 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Thu, Nov 23, 2017 at 01:59:28PM -0500, Ilia Mirkin wrote:
> >> We need to shift the values up, otherwise we'd end up with a negative
> >> shift. This works for up-to 16-bit components, which is fine for now.
> >
> > Shouldn't we actually replicate the high bits in the low bits?
>
> Not entirely sure what you're proposing...
>
> Ideally we wouldn't be lazy and pass e.g. 16-bit values to MAKE_RGBA
> which would then shift down as necessary (and even there, you could
> end up with off-by-1's maybe?). For e.g. 0xff, that should become
> 0x3ff but with my code will become 0x3fc.
Exactly the issue.
> But for other values, it's
> less clear what to do with the low bits. I figured it didn't really
> matter.
>
> Do you have a concrete proposal?
The usual thing would be just
(value) * 0x3ff / 0xff
or
((value) << 2) | ((value) >> 6)
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH libdrm] tests: fix MAKE_RGBA macro for 10bpp modes
2017-11-24 13:38 ` Ville Syrjälä
@ 2017-11-24 15:23 ` Ilia Mirkin
0 siblings, 0 replies; 5+ messages in thread
From: Ilia Mirkin @ 2017-11-24 15:23 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: dri-devel@lists.freedesktop.org
On Fri, Nov 24, 2017 at 8:38 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Nov 23, 2017 at 03:14:46PM -0500, Ilia Mirkin wrote:
>> On Thu, Nov 23, 2017 at 2:09 PM, Ville Syrjälä
>> <ville.syrjala@linux.intel.com> wrote:
>> > On Thu, Nov 23, 2017 at 01:59:28PM -0500, Ilia Mirkin wrote:
>> >> We need to shift the values up, otherwise we'd end up with a negative
>> >> shift. This works for up-to 16-bit components, which is fine for now.
>> >
>> > Shouldn't we actually replicate the high bits in the low bits?
>>
>> Not entirely sure what you're proposing...
>>
>> Ideally we wouldn't be lazy and pass e.g. 16-bit values to MAKE_RGBA
>> which would then shift down as necessary (and even there, you could
>> end up with off-by-1's maybe?). For e.g. 0xff, that should become
>> 0x3ff but with my code will become 0x3fc.
>
> Exactly the issue.
>
>> But for other values, it's
>> less clear what to do with the low bits. I figured it didn't really
>> matter.
>>
>> Do you have a concrete proposal?
>
> The usual thing would be just
>
> (value) * 0x3ff / 0xff
>
> or
>
> ((value) << 2) | ((value) >> 6)
The latter method is interesting, it slowly biases from rounding down
to rounding up as you go through the range. Clever. I guess I could
change my function to use
(value << 8) | value
and then use that to shift around. I'll send a v2.
-ilia
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-11-24 15:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-23 18:59 [PATCH libdrm] tests: fix MAKE_RGBA macro for 10bpp modes Ilia Mirkin
2017-11-23 19:09 ` Ville Syrjälä
2017-11-23 20:14 ` Ilia Mirkin
2017-11-24 13:38 ` Ville Syrjälä
2017-11-24 15:23 ` Ilia Mirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.