[Mesa-dev] [PATCH] virgl: Allow RGB32* textures only as buffer objects

[Ilia Mirkin]

On Sat, Jul 21, 2018 at 1:48 PM, Gert Wollny <gert.wollny at collabora.com> wrote:
> Am Samstag, den 21.07.2018, 11:33 -0400 schrieb Ilia Mirkin:
>>
>>
>> On Sat, Jul 21, 2018, 05:45 Gert Wollny <gert.wollny at collabora.com>
>> wrote:
>> > Am Freitag, den 20.07.2018, 12:31 -0400 schrieb Ilia Mirkin:
>> > >
>> > > > +   /* Allow 3-comp 32 bit texturs only for TBOs (needed for
>> > > > ARB_tbo_rgb32) */
>> > > > +   if ((format == PIPE_FORMAT_R32G32B32_FLOAT ||
>> > > > +       format == PIPE_FORMAT_R32G32B32_SINT ||
>> > > > +       format == PIPE_FORMAT_R32G32B32_UINT) &&
>> > > > +       target != PIPE_BUFFER)
>> > > > +      return FALSE;
>> > >
>> > > My personal recommendation would be to disallow *all* packed RGB
>> > > formats unless target == PIPE_BUFFER. (And even then --
>> > questionable,
>> > > except for RGB32, which is required.)
>> > >
>> > RGB8 and RGB16 are disabled/replaced by RGBX* on the host side, and
>> > R11G11B10 doesn't seem to make problems.
>>
>> No other driver supports these. What happens on the host side is
>> hidden from virgl.
> Not exactly, of course the host tells virgl what formats are supported,
> and we test using dEQP and piglit quite heavily to get things right, so
> I'm quite confident that whatever problem comes up, we will catch it.

The result is that you end up lying to mesa and the various helpers --
the user creates a RGB8 texture, which your gallium driver claims to
support. In reality, it's a RGBX texture on the host, but this
information is hidden from virgl and the state tracker and mesa core.
So now you have two problems:

1. You're hitting paths no one ever thought to be possible since "no
way a backend driver implements such weird formats" (I guarantee you
I've followed this line of thinking in the past, but I don't remember
precisely where).
2. You're lying to the state tracker about what the true format really
is (since you claim it's RGB8 but deep down inside, hidden away from
the guest, it's RGBX8). This will affect its various decisions.

I don't see why you're fighting so hard to keep the RGB8/16 formats...

  -ilia