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

[Gert Wollny]

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