[Mesa-dev] [RFC PATCH] mesa/st: don't prematurely optimize for render targets when on virgl
imirkin at alum.mit.edu
Tue Jul 10 14:47:52 UTC 2018
In practice, no one allocates GL_RGB textures. Only RGBX/RGBA. In part
due to this issue, an in part due to the hardware not being able to
support rendering to such textures (and usually not texturing either).
ARB_tbo_rgb32 is part of GLES 3.1 as I recall, but this only has to do
with buffer objects, so not renderable.
Separately, glCopyImageSubData has to work between compatible internal
formats, irrespective of what happens under the hood. If i965 can't
copy from a GL_RGB32F texture to a GL_RGB32I texture, that's a i965
On Tue, Jul 10, 2018 at 10:30 AM, Erik Faye-Lund
<erik.faye-lund at collabora.com> wrote:
> On 10. juli 2018 16:23, Gert Wollny wrote:
>> Am Dienstag, den 10.07.2018, 16:01 +0200 schrieb Erik Faye-Lund:
>>> On 10. juli 2018 15:42, Gert Wollny wrote:
>>>> For three component textures virgl faces the problem that the host
>>>> may report that these can not be used as a render target, and when
>>>> client requests such a texture a four-componet texture will be
>>>> even if only a sampler view was requested. One example where this
>>>> is with the Intel i965 driver that doesn't support RGB32* as render
>>>> 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
>>>> Therefore, when using the virgl driver, don't try to enable
>>>> 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
>>>> 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
>>>> Many thanks for any comments,
>>>> 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
>>>> 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
>>>> * 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;
>>> I don't think this is correct.
>> I'm also quite sure that this would just be a work-around-something ...
>>> The problem is that the spec defines GL_RGB et al as color-
>>> renderable, and in OpenGL any color-renderable texture can become a
>>> render-target at any point. So the driver *has* to be prepared for
>>> rendering to GL_RGB.
>>> The OpenGL 4.6 spec, section 9.4 "Framebuffer completeness" has this
>>> to say:
>>> "An internal format is color-renderable if it is RED, RG, RGB, RGBA,
>>> or one of the sized internal formats from table 8.12 whose “CR”
>>> (color-renderable) column is checked in that table"
>> Since this is also written in the 4.5 spec, and the Intel driver
>> advertises this version, but the test to attach such a texture to a
>> frame buffer device results in an incomplete framebuffer, the Intel
>> driver is not up to spec or so it would seem ...
> OpenGL has this other excape hatch, where it alllows drivers to say
> FRAMEBUFFER_UNSUPPORTED for basically any combination of attachments it
> doesn't like. Perhaps the Intel is using this as a way out? If so, we might
> have to be prepared to do something similar.
> I don't think Gallium has a way of rejecting framebuffers in this way,
> though. So if that's what's going on, we might have to introduce one.
>> I wonder at which point this was introduced, 3.3 also has it, and on
>> r600 these tests were not resulting in assertion failures, only some
>> textures (sRGB I think) were allocated on the host side as four
>> component, and the copy fallback path was used, but this should be
>> managable, since it didn't refer to the 32 bit variants that are needed
>> for ARB_texture_buffer_object_rgb32.
>>> So, all RGB formats must be color-renderable in OpenGL. For OpenGL
>>> ES, I think this is slightly different, where color-renderable
>>> guarantees for RGB-textures are extensions for at least ES 2.0 IIRC.
>>> So *perhaps* we could get away with something like this for that
>> A glipse over the GLES 3.1 spec gives me the impression that there we
>> don't need ARB_texture_buffer_object_rgb32, so for that goal we can can
>> just replace these formats by RGBX formats, but for Desktop OpenGL 4.0
>> is is needed.
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
More information about the mesa-dev