Re: [Mesa-dev] [virglrenderer-devel] virglrenderer on gles, format conversion

2019-03-14 Thread Erik Faye-Lund
On Wed, 2019-03-06 at 13:39 +1000, Dave Airlie wrote:
> On Wed, 6 Mar 2019 at 01:01, Erik Faye-Lund
>  wrote:
> > When we try to do a vrend transfer from a renderable texture, we
> > end up
> > using glReadPixels to read the data. However, OpenGL ES has
> > restrictions on what formats are allowed to be used for
> > glReadPixels.
> > In partcular, only GL_UNSIGNED_BYTE + GL_RGBA are guaranteed to be
> > supported (for OpenGL ES 2.0; for OpenGL ES 3.x there's some other
> > variants for integer and float textures - it seems we can rely on
> > the
> > destination format having at least as much range/precision as the
> > source format).
> > 
> > But vrend transfers expect no format conversion at all, they want
> > to
> > read the data verbatim.
> > 
> > I thought the obvious choice was to modify virglrenderer to convert
> > back from RGBA to the source format. After a typing out a lot of
> > code,
> > I realized that even though we have util_format_translate in
> > u_format.h, the actual implementation was deleted in d01f462[1].
> > 
> > OK, so that leaves me with a couple of choices:
> > 
> > 1) Restore the format conversion code in virglrenderer.
> > 2) Use some 3rd party pixel-format conversion code instead.
> > 3) Do format-conversion back to the desired format in Mesa.
> > 
> > I gave 1) a go by reverting the removal, but this conflicted pretty
> > badly due to the 3 years of development that has happened since. It
> > might be possible to continue this effort, but I also don't really
> > like
> > the idea of crash-prone format converion code in virglrenderer. You
> > know, for security reasons; virglrenderer runs inside the virtual
> > machine manager, which makes it pretty high priviledge code. If we
> > don't do 1), I have sent a MR[2] that removes the rest of the
> > format
> > conversion, so others won't be led astray in the future.
> 
> It's likely we should just pull that in anyways, and start from a
> clean base if you decide that is the best path.
> 

OK, I'll untag this with WIP soon. A review would also be nice :)

> I agree I don't think this could should be in the host layer if we
> can avoid it.
> 
> > I have relatively low expectations that we'd find a good fit for 3)
> > as
> > we have a very specific and *big* set of formats we need to
> > support.
> > Format conversion across APIs is often a big pain-point in my
> > experience.
> > 
> > As for 3), it's not quite as easily said as done, because we
> > currently
> > only have protocol commands for reading a format verbatim. So I
> > guess
> > we'd have to add a new format, and inform the Mesa driver that
> > we're
> > limited in what formats we support. With this approach, we don't
> > need
> > conversion code both in mesa *and* in virglrenderer, and we don't
> > have
> > to actively maintain a fork of the pixel-format code separately.
> > 
> > I'm currently leaning towards 3) for now, but I would like input
> > from
> > the list, as I feel this is a rather important directional choice.
> 
> I think we'd need to define a list of formats we can readback, and
> then somehow convince the guest mesa to do the conversions for us, it
> should have all the necessary code already shouldn't it?
> 

Yes, or at least almost. I got this approach working:

https://gitlab.freedesktop.org/virgl/virglrenderer/merge_requests/191

https://gitlab.freedesktop.org/kusma/mesa/tree/virgl-readback-formats

I'm pretty happy with how this worked out.


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [virglrenderer-devel] virglrenderer on gles, format conversion

2019-03-05 Thread Dave Airlie
On Wed, 6 Mar 2019 at 01:01, Erik Faye-Lund
 wrote:
>
> When we try to do a vrend transfer from a renderable texture, we end up
> using glReadPixels to read the data. However, OpenGL ES has
> restrictions on what formats are allowed to be used for glReadPixels.
> In partcular, only GL_UNSIGNED_BYTE + GL_RGBA are guaranteed to be
> supported (for OpenGL ES 2.0; for OpenGL ES 3.x there's some other
> variants for integer and float textures - it seems we can rely on the
> destination format having at least as much range/precision as the
> source format).
>
> But vrend transfers expect no format conversion at all, they want to
> read the data verbatim.
>
> I thought the obvious choice was to modify virglrenderer to convert
> back from RGBA to the source format. After a typing out a lot of code,
> I realized that even though we have util_format_translate in
> u_format.h, the actual implementation was deleted in d01f462[1].
>
> OK, so that leaves me with a couple of choices:
>
> 1) Restore the format conversion code in virglrenderer.
> 2) Use some 3rd party pixel-format conversion code instead.
> 3) Do format-conversion back to the desired format in Mesa.
>
> I gave 1) a go by reverting the removal, but this conflicted pretty
> badly due to the 3 years of development that has happened since. It
> might be possible to continue this effort, but I also don't really like
> the idea of crash-prone format converion code in virglrenderer. You
> know, for security reasons; virglrenderer runs inside the virtual
> machine manager, which makes it pretty high priviledge code. If we
> don't do 1), I have sent a MR[2] that removes the rest of the format
> conversion, so others won't be led astray in the future.

It's likely we should just pull that in anyways, and start from a
clean base if you decide that is the best path.

I agree I don't think this could should be in the host layer if we can avoid it.

>
> I have relatively low expectations that we'd find a good fit for 3) as
> we have a very specific and *big* set of formats we need to support.
> Format conversion across APIs is often a big pain-point in my
> experience.
>
> As for 3), it's not quite as easily said as done, because we currently
> only have protocol commands for reading a format verbatim. So I guess
> we'd have to add a new format, and inform the Mesa driver that we're
> limited in what formats we support. With this approach, we don't need
> conversion code both in mesa *and* in virglrenderer, and we don't have
> to actively maintain a fork of the pixel-format code separately.
>
> I'm currently leaning towards 3) for now, but I would like input from
> the list, as I feel this is a rather important directional choice.

I think we'd need to define a list of formats we can readback, and
then somehow convince the guest mesa to do the conversions for us, it
should have all the necessary code already shouldn't it?

> I'm also intererested in if someone has some alternative approaches.
> Perhaps we could use some lower-level API (GBM? Some sort of PBuffer or
> EGLimage?) to create a lockable, linear surface of the same format,
> blit to that and read the result from there? Does something like this
> exist? If so, can we depend on this?

I'm not sure there's a nice low level generic way to do it, it's
likely we'd have to do something else for nvidia driver anyways.

Dave.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev