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
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.wol...@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;
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
API.
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.

Best,
Gert
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

Reply via email to