Bug 41263 - [r600g] glCopyTexImage2D selects a texture format that involves fallback to software
Summary: [r600g] glCopyTexImage2D selects a texture format that involves fallback to s...
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: git
Hardware: x86 (IA32) Linux (All)
: medium normal
Assignee: mesa-dev
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-27 08:27 UTC by Simon Farnsworth
Modified: 2011-09-30 08:43 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
git patch that fixes problem by ensuring InternalFormat is set (1.34 KB, patch)
2011-09-29 03:16 UTC, Simon Farnsworth
Details | Splinter Review

Description Simon Farnsworth 2011-09-27 08:27:35 UTC
In my compositor, on AMD G-T56N (Fusion, Evergreen) I create a texture using GLX_EXT_texture_from_pixmap, where the source application is using Xv textured video on evergreen to write to its backing pixmap (found with XCompositeNamePixmap).

To have a stable copy of this output for compositing from, I immediately copy the texture to a separate texture whose backing store is owned by the compositor, using code like the following:

glBindFramebuffer( GL_FRAMEBUFFER, framebuffer );

glFramebufferTexture2D( GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, source_texture, 0 );

glBindTexture( GL_TEXTURE_2D, dest_texture );
glCopyTexImage2D( GL_TEXTURE_2D, 0, GL_RGBA, 0, 0, width, height, 0 );
glBindFramebuffer( GL_FRAMEBUFFER, 0 );

Where framebuffer is an FBO that I've created earlier with         glGenFramebuffers( 1, &framebuffer ), source_texture is the texture bound to the video pixmap, and dest_texture is my private texture for the compositor to use.

The glCopyTexImage2D call is incredibly slow - it's falling back to a CPU-side copy. Tracing in with GDB shows me that, in st_copy_texsubimage (st_cb_texture.c:1463), src_format is PIPE_FORMAT_B8G8R8A8_UNORM and dest_format is PIPE_FORMAT_R8G8B8A8_UNORM (st_cb_texture.c:1523).

As a result, Gallium can neither do a straight copy of the data, nor can it find a suitable format_writemask to do the swizzle, so it falls back to software.

Either Gallium and r600g needs to learn how to do swizzled copies to make this work, or whichever parts of the chain (highest level in Mesa is copyteximage() at teximage.c:2744) choose the swizzled format need to learn to Not Do That.
Comment 1 Simon Farnsworth 2011-09-27 08:31:26 UTC
Forgot to mention - I'm looking at Mesa git, as of:

commit 4c84fbea9d496567d706468113d63cd8f0faeb7f
Author: Brian Paul <brianp@vmware.com>
Date:   Mon Sep 26 20:44:09 2011 -0600

    mesa: fix indentation in mipmap.c (3 spaces)

I can update or apply test patches as required.
Comment 2 Simon Farnsworth 2011-09-28 10:07:53 UTC
I think it may be core Mesa at fault.

main/teximage.c:2779

         gl_format texFormat = _mesa_choose_texture_format(ctx, texObj,
                                                           target, level,
                                                           internalFormat,
                                                           GL_NONE, GL_NONE);

is called when internalFormat is GL_RGB. Gallium, not unreasonably, treats this as "choose a nice fast GL_RGB style format", and chooses a reversed format to that of the source. The result is a slow software copy, not a fast hardware copy.

So, two possibilities present themselves to me:

1) Gallium should learn to do swizzled hardware blits with a textured quad.
2) Core Mesa should give Gallium more information about the source, so that it can choose a matching format if possible.
Comment 3 Simon Farnsworth 2011-09-28 10:28:33 UTC
And I have confirmation that it's about choice of texture formats: the following patch "fixes" the bug for me (no doubt by adding hundreds more for other people).

diff --git a/src/mesa/state_tracker/st_format.c b/src/mesa/state_tracker/st_format.c
index 6eb8a50..a310762 100644
--- a/src/mesa/state_tracker/st_format.c
+++ b/src/mesa/state_tracker/st_format.c
@@ -616,10 +616,10 @@ struct format_mapping
       0
 
 #define DEFAULT_RGB_FORMATS \
+      PIPE_FORMAT_B8G8R8A8_UNORM, \
       PIPE_FORMAT_B8G8R8X8_UNORM, \
       PIPE_FORMAT_X8R8G8B8_UNORM, \
       PIPE_FORMAT_X8B8G8R8_UNORM, \
-      PIPE_FORMAT_B8G8R8A8_UNORM, \
       PIPE_FORMAT_A8R8G8B8_UNORM, \
       PIPE_FORMAT_A8B8G8R8_UNORM, \
       PIPE_FORMAT_B5G6R5_UNORM, \
Comment 4 Marek Olšák 2011-09-28 16:24:23 UTC
st/mesa can do swizzled copies just fine. Something else is failing in st_cb_texture.c. See line 1476 in that file. Both those ifs fail. Can you take a look and see why?
Comment 5 Simon Farnsworth 2011-09-29 02:21:41 UTC
I've now updated mesa to git revision e112287474e225969fb10e6bc744d9c48a853fc6

I added the following to that file, to clue me in when software fallbacks happen (with apologies for line wrapping):


commit 550cf15bb2ba564a927bd4bfba5a85623636caef
Author: Simon Farnsworth <simon.farnsworth@onelan.co.uk>
Date:   Tue Sep 27 14:42:12 2011 +0100

    Debug patch

diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
index e744a9f..3208ac6 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -1619,6 +1619,22 @@ st_copy_texsubimage(struct gl_context *ctx,
    }
 
    if (use_fallback) {
+       debug_printf("%s: software fallback ctx->_ImageTransferState %d matching_base_formats %d src_format 0x%x dest_format 0x%x do_flip %d format_writemask 0x%x texBaseFormat 0x%x sampler_supported %d render_supported %d\n",
+                    __FUNCTION__,
+                    ctx->_ImageTransferState,
+                    matching_base_formats,
+                    src_format,
+                    dest_format,
+                    do_flip,
+                    format_writemask,
+                    texBaseFormat,
+                    screen->is_format_supported(screen, src_format,
+                                                PIPE_TEXTURE_2D, sample_count,
+                                                PIPE_BIND_SAMPLER_VIEW) &&
+                    screen->is_format_supported(screen, dest_format,
+                                                PIPE_TEXTURE_2D, 0,
+                                                PIPE_BIND_RENDER_TARGET)
+           );
       /* software fallback */
       fallback_copy_texsubimage(ctx, target, level,
                                 strb, stImage, texBaseFormat,


The output I get is

st_copy_texsubimage: software fallback ctx->_ImageTransferState 0 matching_base_formats 0 src_format 0x1 dest_format 0x2 do_flip 0 format_writemask 0x0 texBaseFormat 0x1907 sampler_supported 1 render_supported 164514680

I can't use the first path (identical formats), as src_format != dest_format, and matching_base_formats is false.

compatible_src_dst_formats has returned 0, so I can't use the textured quad path.

I therefore fall back to software unexpectedly.
Comment 6 Simon Farnsworth 2011-09-29 02:44:17 UTC
I enabled DEBUG_FALLBACK, and got the following extra information:


2011-09-29T10:42:53+01:00 layout[6969] info: compatible_src_dst_formats failed for src GL_FALSE, dst GL_RGB

I presumably need to work out why the source FBO claims to be format "GL_FALSE"?
Comment 7 Simon Farnsworth 2011-09-29 03:07:17 UTC
I'm now looking at st_render_texture in st_cb_fbo.c:336. I don't see how InternalFormat is set in that function - I could of course be simply missing a function it's calling that sets it.
Comment 8 Simon Farnsworth 2011-09-29 03:10:04 UTC
That indeed appeared to be the problem. Patch will be attached shortly.
Comment 9 Simon Farnsworth 2011-09-29 03:16:20 UTC
Created attachment 51747 [details] [review]
git patch that fixes problem by ensuring InternalFormat is set

This is the patch that fixes it for me - I've assumed that InternalFormat is safe to access.
Comment 10 Marek Olšák 2011-09-29 06:47:20 UTC
Thanks. Please send the patch to mesa-dev@lists.freedesktop.org
Comment 11 Marek Olšák 2011-09-30 08:43:31 UTC
The patch has been committed. Closing.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.