[Mesa-dev] [RFC PATCH] mesa/st: don't prematurely optimize for render targets when on virgl

[Gert Wollny]

For three component textures virgl faces the problem that the host driver
may report that these can not be used as a render target, and when the
client requests such a texture a four-componet texture will be choosen
even if only a sampler view was requested. One example where this happens
is with the Intel i965 driver that doesn't support RGB32* as render target.
The result is that when allocating a GL_RGB32F and a GL_RGB32I texture, and
then glCopyImageSubData is called for these two texture, gallium will fail
with an assertion, because it reports a different per pixel bit count.

Therefore, when using the virgl driver, don't try to enable BIND_RENDER_TARGET
for RGB textures that were requested with only BIND_SAMPLER_VIEW.

Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
---

I'm aware that instead of using the device ID, I should probably add a new caps
flag, but apart from that I was wondering whether there may be better approaches
to achieve the same goal: The a texture is allocated with the internal format 
as closely as possible to the requested one. Especially it shouldn't change the
percieved pixel bit count. 

In fact, I was a bit surprised to see that the assertions regarding the 
different sizes was hit in st_copy_image:307 (swizzled_copy). It seems that 
there is some check missing that should redirect the copy in such a case.

Many thanks for any comments, 
Gert 

 src/mesa/state_tracker/st_format.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/mesa/state_tracker/st_format.c b/src/mesa/state_tracker/st_format.c
index 9ae796eca9..2d8ff756a9 100644
--- a/src/mesa/state_tracker/st_format.c
+++ b/src/mesa/state_tracker/st_format.c
@@ -2285,19 +2285,27 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum target,
 
    /* GL textures may wind up being render targets, but we don't know
     * that in advance.  Specify potential render target flags now for formats
-    * that we know should always be renderable.
+    * that we know should always be renderable, except when we are on virgl,
+    * we don't try this for three component textures, because the host might
+    * not support rendering to them, and then Gallium chooses a four component
+    * internal format and calls to e.g. glCopyImageSubData will fail for format
+    * that should be compatible.
     */
    bindings = PIPE_BIND_SAMPLER_VIEW;
    if (_mesa_is_depth_or_stencil_format(internalFormat))
       bindings |= PIPE_BIND_DEPTH_STENCIL;
-   else if (is_renderbuffer || internalFormat == 3 || internalFormat == 4 ||
-            internalFormat == GL_RGB || internalFormat == GL_RGBA ||
-            internalFormat == GL_RGB8 || internalFormat == GL_RGBA8 ||
+   else if (is_renderbuffer  ||
+            internalFormat == GL_RGBA ||
+            internalFormat == GL_RGBA8 ||
             internalFormat == GL_BGRA ||
-            internalFormat == GL_RGB16F ||
             internalFormat == GL_RGBA16F ||
-            internalFormat == GL_RGB32F ||
-            internalFormat == GL_RGBA32F)
+            internalFormat == GL_RGBA32F ||
+            ((st->pipe->screen->get_param(st->pipe->screen, PIPE_CAP_DEVICE_ID) != 0x1010) &&
+             (internalFormat == 3 || internalFormat == 4 ||
+              internalFormat == GL_RGB ||
+              internalFormat == GL_RGB8 ||
+              internalFormat == GL_RGB16F ||
+              internalFormat == GL_RGB32F )))
       bindings |= PIPE_BIND_RENDER_TARGET;
 
    /* GLES allows the driver to choose any format which matches
-- 
2.17.1