[virglrenderer-devel] [PATCH 2/3] vrend: If available use glCopyImageSubData to execute memcopy like blits

[Gurchetan Singh]

On Tue, Jun 26, 2018 at 11:11 PM Gert Wollny <gert.wollny at collabora.com> wrote:
>
> Am Dienstag, den 26.06.2018, 12:46 -0700 schrieb Gurchetan Singh:
> > On Tue, Jun 26, 2018 at 9:51 AM Gert Wollny <gert.wollny at collabora.co
> > m> wrote:
> > >
> > > Am Dienstag, den 26.06.2018, 09:27 -0700 schrieb Gurchetan Singh:
> > > ...
> > > > > I have to admid, I kind of ignored this, but now that you point
> > > > > it
> > > > > out: the reason why I came up with using glCopyImageSubData is
> > > > > because that's what is actually used by the tests I'm trying to
> > > > > fix
> > > > > with these patches, and these are from the GLES31 set. So if
> > > > > glCopyImageSubData is not available on the host, one can not
> > > > > expect
> > > > > these tests to pass, right?  In any case, does the hardware
> > > > > you're
> > > > > using not expose one of the *_copy_image extensions?
> > > >
> > > > I found this relevant snippet in es31fCopyImageTests.cpp:
> > > >
> > > >         if (!isES32 && !ctxInfo-
> > > > > isExtensionSupported("GL_EXT_copy_image"))
> > > >
> > > >                 throw tcu::NotSupportedError("Extension
> > > > GL_EXT_copy_image not supported.", "", __FILE__, __LINE__);
> > > >
> > > > Does the guest advertise glCopyImageSubData when we shouldn't?
> > >
> > > When I run Qemu with
> > >
> > >   MESA_EXTENSION_OVERRIDE=-GL_ARB_copy_image
> > >
> > > then GL_*_copy_image is not advertised  in the guest (as it should
> > > be), so having a code path in the host that uses this extenstion
> > > when evailable shouldn't pose a problem, we only have to make sure
> > > that the there are no regressions with the old code path.
> >
> > That's odd -- we don't ever query for GL_ARB_copy_image and send it
> > via the capability set (usually caps->v1.bset).  How does the guest
> > know this capability exists on the host?
> I was wrong, I was running with the master virglrenderer/mesa trees and
> they never advertise this extension.
>
> In the experimental one its is forced to get guest OpenGL 4.3, and
> disabling it there on the host doesn't change anything in the guest.
> I'll have to add this CAP to the patch and also send a patch to mesa.
>
> >
> > > In st_cb_copyimage.c get_canonocal_format the various format names
> > > are
> >
> > mapped to these PIPE_FORMAT_* types, here I check whether the the
> > current format is one of those that is returned by this function,
> > that's why I thought the name would be apropriated.
> >
> > So you're trying to return the formats which get_canonical_format can
> > possibly return, correct?
> Not exactly, I'm looking at whether the format is one that is returned
> from "canonical_format_from_bits", here you have for each bitcount
> exactly one return value,

Why do you have:

PIPE_FORMAT_G8R8_UNORM,
PIPE_FORMAT_R8G8B8_UINT,
PIPE_FORMAT_A8B8G8R8_UNORM,
PIPE_FORMAT_R16G16B16_UINT,
PIPE_FORMAT_R32G32B32_UINT

when they can't be returned by canonical_format_from_bits?

> and if they are equal then either the formats
> were equal to begin with, or glCopyImageSubData was called (At least
> that is my assumption).

Does pipe->blit() expect a pure memcpy (like glCopyImageSubData
performs -- R32 to RGBA8 pure copy) or a per-channel memcpy (R32 to
RGBA8 only fills in R component)?  It's not completely clear from
reading pipe_context.h or struct pipe_blit_info:

   /* Optimal hardware path for blitting pixels.
    * Scaling, format conversion, up- and downsampling (resolve) are allowed.
    */
   void (*blit)(struct pipe_context *pipe,
                const struct pipe_blit_info *info);

glCopyImageSubData is completely different:

"CopyImageSubData does not perform general-purpose conversions
    such as scaling, resizing, blending, color-space, or format
    conversions. "

How does Gallium differentiate between the two for drivers?

> In both cases I can call glCopyImageSubData on
> the host (given the other restrictions apply)
>> > It looks we're missing PIPE_FORMAT_B8G8R8A8_UNORM.
> I don't think so, because "see above".
>
> > Also, from swizzled_copy in st_cb_copyimage.c it seems that the
> > memcopy blit should work for cases (info->src.format !=
> > info->dst.format), from this comment:
> >
> > /* BGRA8 -> R32 is realized as BGRA8 -> RGBA8        */
> I think this refers to non glCopyImageSubData blits.





> >
> > (There's still a lot failures with
> > dEQP-GLES31.functional.copy_image.mixed* and
> > dEQP-GLES31.functional.copy_image.compressed*).
> One step at a time ;) First I have to figure out the regression Dave
> sees and I've seen on Intel but not on R600.
>
> Best,
> Gert