On Wed, 2019-03-06 at 13:39 +1000, Dave Airlie wrote: > On Wed, 6 Mar 2019 at 01:01, Erik Faye-Lund > <erik.faye-l...@collabora.com> 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
