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

[Marek Olšák]

If some formats are not supported as render targets, I recommend that
they are not supported for texturing either. (radeonsi doesn't support
unaligned 3-channel formats either. It only supports aligned formats,
such as R8G8B8X8 and R32G32B32X32.)

If you can't support RGBX formats as render targets, you can still
expose them as long as you support RGBX for texturing. Everything will
be correct except for blending where DST_ALPHA will read the value
from memory instead of returning 1. (and you can fix the blend state to
use ONE instead)

Marek

On Tue, Jul 10, 2018 at 9:42 AM, Gert Wollny <gert.wollny at collabora.com> wrote:
> 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
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev