<div dir="ltr">I think an indicator for float can double as indicating the size is multiplied by 8. All float formats I am familiar with take a mulitple of 8 bits, including an 8-bit format that is sometimes used in CG.<div><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Oct 29, 2018 at 2:18 AM Maarten Lankhorst <<a href="mailto:maarten.lankhorst@linux.intel.com">maarten.lankhorst@linux.intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Op 29-10-18 om 09:16 schreef Siarhei Siamashka:<br>
> On Wed, 03 Oct 2018 10:11:46 +0100<br>
> Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>> wrote:<br>
><br>
>> Quoting Maarten Lankhorst (2018-08-01 13:41:33)<br>
>>> diff --git a/pixman/pixman.h b/pixman/pixman.h<br>
>>> index 509ba5e534a8..c9bf4faa80d4 100644<br>
>>> --- a/pixman/pixman.h<br>
>>> +++ b/pixman/pixman.h<br>
>>> @@ -647,19 +647,24 @@ struct pixman_indexed<br>
>>>   * sample implementation allows only packed RGB and GBR<br>
>>>   * representations for data to simplify software rendering,<br>
>>>   */<br>
>>> -#define PIXMAN_FORMAT(bpp,type,a,r,g,b)        (((bpp) << 24) |  \<br>
>>> +#define PIXMAN_FORMAT_PACKC(val) ((val) <= 10 ? (val) : \<br>
>>> +                                (val) == 32 ? 11 : 0)<br>
>>> +#define PIXMAN_FORMAT_UNPACKC(val) ((val) <= 10 ? (val) : \<br>
>>> +                                   (val) == 11 ? 32 : 0)  <br>
>> We have 4bits, why not reserve 0xf for float32? Certainly you<br>
>> want space for<br>
>><br>
>> #define PIXMAN_FORMAT_PACKED_FLOAT16 0xb<br>
>> #define PIXMAN_FORMAT_PACKED_FLOAT32 0xc<br>
>> #define PIXMAN_FORMAT_PACKED_FLOAT64 0xd<br>
>><br>
>> I just wonder if we may have 12bpc one day as well, so leaving 0xc clear<br>
>> has some some appeal.<br>
> We could probably also try to do something like this:<br>
><br>
> /*<br>
>  * While the protocol is generous in format support, the<br>
>  * sample implementation allows only packed RGB and GBR<br>
>  * representations for data to simplify software rendering,<br>
>  *<br>
>  * Bit 23 selects the granularity of channel color depth<br>
>  *  0: 1-bit granularity (allows up to 15 bits per channel)<br>
>  *  1: 1-byte granularity (allows up to 120 bits per channel)<br>
>  */<br>
> #define PIXMAN_FORMAT(bpp,type,a,r,g,b)       \<br>
>       (((bpp) << 24) |  \<br>
>       ((type) << 16) | \<br>
>       (((a) < 16) ? ((a) << 12) : (((a / 8) << 12) | (1 << 23))) | \<br>
>       (((r) < 16) ? ((r) << 8)  : (((r / 8) << 8)  | (1 << 23))) | \<br>
>       (((g) < 16) ? ((g) << 4)  : (((g / 8) << 4)  | (1 << 23))) | \<br>
>       (((b) < 16) ? ((b))       : (((b / 8))       | (1 << 23))))<br>
><br>
> /* 0 for 1-bit granularity and 3 for 1-byte granularity */<br>
> #define PIXMAN_FORMAT_CHANDEPTH_SHIFT(f) \<br>
>       ((((f) >> 23) & 1) | ((((f) >> 23) & 1) << 1))<br>
I would use 2 bits then, 6 is still plenty for the rest of the type.<br>
<br>
Perhaps make a separate PIXMAN_FORMAT_TYPE_RGBA_FLOAT and PIXMAN_FORMAT_TYPE_RGB_FLOAT?<br>
<br>
> #define PIXMAN_FORMAT_A(f) \<br>
>       ((((f) >> 12) & 0x0f) << PIXMAN_FORMAT_CHANDEPTH_SHIFT(f))<br>
> #define PIXMAN_FORMAT_R(f) \<br>
>       ((((f) >>  8) & 0x0f) << PIXMAN_FORMAT_CHANDEPTH_SHIFT(f))<br>
> #define PIXMAN_FORMAT_G(f) \<br>
>       ((((f) >>  4) & 0x0f) << PIXMAN_FORMAT_CHANDEPTH_SHIFT(f))<br>
> #define PIXMAN_FORMAT_B(f) \<br>
>       ((((f)      ) & 0x0f) << PIXMAN_FORMAT_CHANDEPTH_SHIFT(f))<br>
><br>
> #define PIXMAN_FORMAT_BPP(f)  (((uint32_t)(f)) >> 24)<br>
> #define PIXMAN_FORMAT_TYPE(f) (((f) >> 16) & 0x7f)<br>
> #define PIXMAN_FORMAT_RGB(f)  (((f)      ) & 0xfff)<br>
> #define PIXMAN_FORMAT_VIS(f)  (((f)      ) & 0xffff)<br>
> #define PIXMAN_FORMAT_DEPTH(f)        (PIXMAN_FORMAT_A(f) +   \<br>
>                                PIXMAN_FORMAT_R(f) +   \<br>
>                                PIXMAN_FORMAT_G(f) +   \<br>
>                                PIXMAN_FORMAT_B(f))<br>
><br>
><br>
> Right now the format type occupies 8 bits. The highest bit could be<br>
> repurposed as a flag for storing channel depth as bytes instead<br>
> of bits. This way 16-bit and 64-bit per color component will be<br>
> also supported. The limitation of this approach is that the depth<br>
> of every color component has to be a multiple of 8 bits if any of<br>
> them is 16 bits or larger.<br>
><br>
> I don't feel very comfortable about the fact that some applications<br>
> are using the PIXMAN_FORMAT_A/R/G/B macros and this code gets compiled<br>
> as part of these application binaries.<br>
><br>
> Are there any other interesting >32bpp formats that we may need<br>
> to support in the future?<br>
><br>
Doubt it, pixman doesn't have that accuracy currently, but F16 might be interesting at some point..<br>
<br>
~Maarten<br>
<br>
_______________________________________________<br>
Pixman mailing list<br>
<a href="mailto:Pixman@lists.freedesktop.org" target="_blank">Pixman@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/pixman" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/pixman</a><br>
</blockquote></div>