[virglrenderer-devel] [PATCH 4/5] add a cap for TGSI precise modifiers

[Erik Faye-Lund]

On 12. juli 2018 14:50, Erik Faye-Lund wrote:
> On 12. juli 2018 14:35, Gert Wollny wrote:
>> Am Donnerstag, den 12.07.2018, 09:55 +0200 schrieb Erik Faye-Lund:
>>> This way, mesa can know that the TGSI-parser groks the _PRECISE
>>> modifier, and it doesn't need to cull them any more.
>>>
>>> Signed-off-by: Erik Faye-Lund <erik.faye-lund at collabora.com>
>>> ---
>>>   src/virgl_hw.h       | 1 +
>>>   src/vrend_renderer.c | 2 +-
>>>   2 files changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/virgl_hw.h b/src/virgl_hw.h
>>> index 44c7108..bbd4d38 100644
>>> --- a/src/virgl_hw.h
>>> +++ b/src/virgl_hw.h
>>> @@ -215,6 +215,7 @@ enum virgl_formats {
>>>   #define VIRGL_CAP_NONE 0
>>>   #define VIRGL_CAP_TGSI_INVARIANT       (1 << 0)
>>>   #define VIRGL_CAP_TEXTURE_VIEW         (1 << 1)
>>> +#define VIRGL_CAP_TGSI_PRECISE         (1 << 2)
>> We should agree on the numbers here, Dave's experimental tree uses
>> (1<<2) already for compute shaders, and I added (1<<3) for copy_image,
>> so (1 << 4) would be the next one.
>>
> I don't think we need to. We could just go first-come-first-serve, and 
> let the merge-to-master point decide the number.
>
> Synchronizing this seems like more work than just rebasing. Besides, 
> it only solves half of the problem; we have a similar issue with the 
> ordering of other caps, and I don't think there's anything else that 
> really works than merge-point.
>
> Maybe we could make it a bit easier to deal with by not having the 
> bitflags in the caps, but an enum instead.
> Something like this:
>
> ---8<---
> enum virgl_cap {
>     VIRGL_CAP_TGSI_INVARIANT = 0,
>     VIRGL_CAP_TEXTURE_VIEW,
>     VIRGL_CAP_TEXTURE_PRECISE
> }
>
> inline bool virgl_has_cap(union virgl_caps caps, enum virgl_cap cap)
> {
>     return caps.v2.capability_bits & (1 << (int)cap);
> }
> ---8<---
>
> We still need to keep the order up-to-date, but that might be a bit 
> easier than having to change the values as well... But on the other 
> hand, it's a lot of churn for a kinda hypothetical gain...

Thinking about this a bit more (it is rather painful to not be able to 
easily add caps in any order), it'd have been a lot more neat if the 
caps were not bitflags, but string-lists instead. That's a lot easier to 
gracefully extend. Similarly, I think it'd been nice to have simple 
key-value properties for all constants we want.

Oh well, I guess this ship has kinda sailed...