Marek Olšák wrote:
Hi Brian,

On Mon, Jan 18, 2010 at 6:18 PM, Brian Paul <bri...@vmware.com <mailto:bri...@vmware.com>> wrote:

        The second patch enables hardware-accelerated CopyTex[Sub]Image
        in cases when the RGB and RGBA destination formats are used
        regardless of source formats. The idea is that if the
        colorbuffer is of RGB or RGBA format, the format conversion for
        copying between two different surfaces is basically done in
        texture units (using swizzles), therefore there is no reason to
        use software fallback. Without this patch, openarena fallbacks
        to software when bloom is enabled.


    Did you read the comment at line 1498?  I remember specifically
    adding this check to fix some issues when copying an RGBA
    framebuffer to a RGB texture.


The behavior of copying an RGBA framebuffer to an RGB texture is not changed by this patch, it is only generalized to support other formats. I agree the diff file is a little bit confusing. See the code before:

   else if (srcFormat == GL_RGBA &&
            dstLogicalFormat == GL_RGB) {
      /* Add a single special case to cope with RGBA->RGB transfers,
       * setting A to 1.0 to cope with situations where the RGB
       * destination is actually stored as RGBA.
       */
      return TGSI_WRITEMASK_XYZ; /* A ==> 1.0 */
   }
   else {

The code after:

   else if (dstLogicalFormat == GL_RGBA) {
      /* In theory, any format can be converted to GL_RGBA.
       */
      return TGSI_WRITEMASK_XYZW;
   }
   else if (dstLogicalFormat == GL_RGB) {
      /* Add a special case to cope with transfers into RGB,
       * setting A to 1.0 to cope with situations where the RGB
       * destination is actually stored as RGBA.
       */
      return TGSI_WRITEMASK_XYZ; /* A ==> 1.0 */
   }
   else {

Are you ok with that?

No.  But don't feel bad, I think my original code is broken too. :(

See the attached patch. It has a bunch of comments that explain what's going on.

If you can test this and it seems to solve your problem, I'll commit it.


BTW could you please push the first patch (and the second one if you agree)? My account hasn't been created yet. Thank you.

Yes, done.

-Brian
diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
index f01053c..3f927b6 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -31,6 +31,7 @@
 #include "main/convolve.h"
 #endif
 #include "main/enums.h"
+#include "main/fbobject.h"
 #include "main/formats.h"
 #include "main/image.h"
 #include "main/imports.h"
@@ -1368,33 +1369,64 @@ fallback_copy_texsubimage(GLcontext *ctx, GLenum target, GLint level,
 }
 
 
+
+/**
+ * If the format of the src renderbuffer and the format of the dest
+ * texture are compatible (in terms of blitting), return a TGSI writemask
+ * to be used during the blit.
+ * If the src/dest are incompatible, return 0.
+ */
 static unsigned
-compatible_src_dst_formats(const struct gl_renderbuffer *src,
+compatible_src_dst_formats(GLcontext *ctx,
+                           const struct gl_renderbuffer *src,
                            const struct gl_texture_image *dst)
 {
-   const GLenum srcFormat = _mesa_get_format_base_format(src->Format);
-   const GLenum dstLogicalFormat = _mesa_get_format_base_format(dst->TexFormat);
+   /* Get logical base formats for the src and dest.
+    * That is, use the user-requested formats and not the actual, device-
+    * chosen formats.
+    * For example, the user may have requested an A8 texture but the
+    * driver may actually be using an RGBA texture format.  When we
+    * copy/blit to that texture, we only want to copy the Alpha channel
+    * and not the RGB channels.
+    *
+    * Similarly, when the src FBO was created an RGB format may have been
+    * requested but the driver actually chose an RGBA format.  In that case,
+    * we don't want to copy the undefined Alpha channel to the dest texture
+    * (it should be 1.0).
+    */
+   const GLenum srcFormat = _mesa_base_fbo_format(ctx, src->InternalFormat);
+   const GLenum dstFormat = _mesa_base_tex_format(ctx, dst->InternalFormat);
 
-   if (srcFormat == dstLogicalFormat) {
+   /**
+    * XXX when we have red-only and red/green renderbuffers we'll need
+    * to add more cases here (or implement a general-purpose routine that
+    * queries the existance of the R,G,B,A channels in the src and dest).
+    */
+   if (srcFormat == dstFormat) {
       /* This is the same as matching_base_formats, which should
        * always pass, as it did previously.
        */
       return TGSI_WRITEMASK_XYZW;
    }
-   else if (srcFormat == GL_RGBA &&
-            dstLogicalFormat == GL_RGB) {
-      /* Add a single special case to cope with RGBA->RGB transfers,
-       * setting A to 1.0 to cope with situations where the RGB
-       * destination is actually stored as RGBA.
+   else if (srcFormat == GL_RGB && dstFormat == GL_RGBA) {
+      /* Make sure that A in the dest is 1.  The actual src format
+       * may be RGBA and have undefined A values.
+       */
+      return TGSI_WRITEMASK_XYZ;
+   }
+   else if (srcFormat == GL_RGBA && dstFormat == GL_RGB) {
+      /* Make sure that A in the dest is 1.  The actual dst format
+       * may be RGBA and will need A=1 to provide proper alpha values
+       * when sampled later.
        */
-      return TGSI_WRITEMASK_XYZ; /* A ==> 1.0 */
+      return TGSI_WRITEMASK_XYZ;
    }
    else {
       if (ST_DEBUG & DEBUG_FALLBACK)
          debug_printf("%s failed for src %s, dst %s\n",
                       __FUNCTION__, 
                       _mesa_lookup_enum_by_nr(srcFormat),
-                      _mesa_lookup_enum_by_nr(dstLogicalFormat));
+                      _mesa_lookup_enum_by_nr(dstFormat));
 
       /* Otherwise fail.
        */
@@ -1505,7 +1537,7 @@ st_copy_texsubimage(GLcontext *ctx,
    matching_base_formats =
       (_mesa_get_format_base_format(strb->Base.Format) ==
        _mesa_get_format_base_format(texImage->TexFormat));
-   format_writemask = compatible_src_dst_formats(&strb->Base, texImage);
+   format_writemask = compatible_src_dst_formats(ctx, &strb->Base, texImage);
 
    if (ctx->_ImageTransferState == 0x0) {
 
------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Reply via email to