On 05/06/2013 02:32 PM, Eric Anholt wrote:
Kenneth Graunke <kenn...@whitecape.org> writes:

On 04/30/2013 12:56 PM, Eric Anholt wrote:
Everyone was doing effectively the same thing, except for some funky code
reuse in Intel, and swrast mistakenly recomputing _BaseFormat instead of
using the texture's _BaseFormat.  swrast's sRGB handling is left in place,
though it should be done by using _mesa_get_render_format() at render time
instead (as-is, it will miss updates to GL_FRAMEBUFFER_SRGB).
---
   src/mesa/drivers/dri/intel/intel_fbo.c     |  6 ------
   src/mesa/drivers/dri/nouveau/nouveau_fbo.c | 18 ------------------
   src/mesa/main/fbobject.c                   |  7 +++++++
   src/mesa/state_tracker/st_cb_fbo.c         |  5 -----
   src/mesa/swrast/s_texrender.c              |  5 -----
   5 files changed, 7 insertions(+), 34 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c 
b/src/mesa/drivers/dri/intel/intel_fbo.c
index a3817eb..f037445 100644
--- a/src/mesa/drivers/dri/intel/intel_fbo.c
+++ b/src/mesa/drivers/dri/intel/intel_fbo.c
@@ -489,12 +489,6 @@ intel_renderbuffer_update_wrapper(struct intel_context 
*intel,
      struct intel_mipmap_tree *mt = intel_image->mt;
      int level = image->Level;

-   rb->Format = image->TexFormat;
-   rb->InternalFormat = image->InternalFormat;
-   rb->_BaseFormat = image->_BaseFormat;
-   rb->NumSamples = mt->num_samples;
-   rb->Width = image->Width2;
-   rb->Height = image->Height2;
      rb->Delete = intel_delete_renderbuffer;
      rb->AllocStorage = intel_nop_alloc_storage;

diff --git a/src/mesa/drivers/dri/nouveau/nouveau_fbo.c 
b/src/mesa/drivers/dri/nouveau/nouveau_fbo.c
index adead3d..a692051 100644
--- a/src/mesa/drivers/dri/nouveau/nouveau_fbo.c
+++ b/src/mesa/drivers/dri/nouveau/nouveau_fbo.c
@@ -247,21 +247,6 @@ nouveau_framebuffer_renderbuffer(struct gl_context *ctx, 
struct gl_framebuffer *
        context_dirty(ctx, FRAMEBUFFER);
   }

-static GLenum
-get_tex_format(struct gl_texture_image *ti)
-{
-       switch (ti->TexFormat) {
-       case MESA_FORMAT_ARGB8888:
-               return GL_RGBA8;
-       case MESA_FORMAT_XRGB8888:
-               return GL_RGB8;
-       case MESA_FORMAT_RGB565:
-               return GL_RGB5;
-       default:
-               return GL_NONE;
-       }
-}
-
   static void
   nouveau_render_texture(struct gl_context *ctx, struct gl_framebuffer *fb,
                       struct gl_renderbuffer_attachment *att)
@@ -271,9 +256,6 @@ nouveau_render_texture(struct gl_context *ctx, struct 
gl_framebuffer *fb,
                att->Texture->Image[att->CubeMapFace][att->TextureLevel];

        /* Update the renderbuffer fields from the texture. */
-       set_renderbuffer_format(rb, get_tex_format(ti));

I think the call to set_renderbuffer_format is still necessary, since it
does:

struct nouveau_surface *s = &to_nouveau_renderbuffer(rb)->surface;
s->cpp = ...;

and the core Mesa code doesn't do that.  Otherwise, this looks good.

I looked over the series for about an hour and didn't notice any other
large issues.  I'm really glad to see this cleaned up - it was really
awkward having the core Mesa code rely on driver hooks setting up the
renderbuffer wrapper.

For the series (assuming this nouveau thing gets fixed):
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>

The s->cpp and s->format were immediately getting set to the texture's
cpp and format by:

         nouveau_surface_ref(&to_nouveau_teximage(ti)->surface,
                             &to_nouveau_renderbuffer(rb)->surface);


You're right; I hadn't noticed that.  I withdraw the objection.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to