[Mesa-dev] [PATCH] st/mesa: disallow mixed formats in a FBO

[Marek Olšák]

New patch here:

    gallium: add a CAP for mixed colorbuffer format support

    Some GPUs can't do it (I think most of DX9 ones), so they should have
    the option not to allow it.

diff --git a/src/gallium/include/pipe/p_defines.h
b/src/gallium/include/pipe/p_defines.h
index 1c1a8f2..0f0ca39 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -463,7 +463,8 @@ enum pipe_cap {
    PIPE_CAP_SHADER_STENCIL_EXPORT,
    PIPE_CAP_TGSI_INSTANCEID,
    PIPE_CAP_VERTEX_ELEMENT_INSTANCE_DIVISOR,
-   PIPE_CAP_FRAGMENT_COLOR_CLAMP_CONTROL
+   PIPE_CAP_FRAGMENT_COLOR_CLAMP_CONTROL,
+   PIPE_CAP_MIXED_COLORBUFFER_FORMATS,
 };

 /* Shader caps not specific to any single stage */
diff --git a/src/mesa/state_tracker/st_cb_fbo.c
b/src/mesa/state_tracker/st_cb_fbo.c
index 0df0428..7ffee90 100644
--- a/src/mesa/state_tracker/st_cb_fbo.c
+++ b/src/mesa/state_tracker/st_cb_fbo.c
@@ -529,6 +529,9 @@ st_validate_framebuffer(struct gl_context *ctx, struct
gl_framebuffer
    const struct gl_renderbuffer_attachment *stencil =
          &fb->Attachment[BUFFER_STENCIL];
    GLuint i;
+   enum pipe_format first_format = PIPE_FORMAT_NONE;
+   boolean mixed_formats =
+         screen->get_param(screen, PIPE_CAP_MIXED_COLORBUFFER_FORMATS) !=
0;

    if (depth->Type && stencil->Type && depth->Type != stencil->Type) {
       fb->_Status = GL_FRAMEBUFFER_UNSUPPORTED_EXT;
@@ -562,13 +565,33 @@ st_validate_framebuffer(struct gl_context *ctx, struct
gl_framebuff
       return;
    }
    for (i = 0; i < ctx->Const.MaxColorAttachments; i++) {
+      struct gl_renderbuffer_attachment *att =
+            &fb->Attachment[BUFFER_COLOR0 + i];
+      enum pipe_format format;
+
       if (!st_validate_attachment(ctx,
                                   screen,
-                                 &fb->Attachment[BUFFER_COLOR0 + i],
+                                 att,
                                  PIPE_BIND_RENDER_TARGET)) {
         fb->_Status = GL_FRAMEBUFFER_UNSUPPORTED_EXT;
         return;
       }
+
+      if (!mixed_formats) {
+         /* Disallow mixed formats. */
+         if (att->Type != GL_NONE) {
+            format = st_renderbuffer(att->Renderbuffer)->surface->format;
+         } else {
+            continue;
+         }
+
+         if (first_format == PIPE_FORMAT_NONE) {
+            first_format = format;
+         } else if (format != first_format) {
+            fb->_Status = GL_FRAMEBUFFER_UNSUPPORTED_EXT;
+            return;
+         }
+      }
    }
 }

Marek


On Mon, Mar 28, 2011 at 11:27 AM, Christoph Bumiller <
e0425955 at student.tuwien.ac.at> wrote:

> On 28.03.2011 02:51, Marek Olšák wrote:
> > Some GPUs can't do it (I think most of DX9 ones).
> >
> > We should later decide how Gallium will expose this feature.
> How about you just add a PIPE_CAP_FRAMEBUFFE_MIXED_FORMATS and
> PIPE_CAP_FRAMEBUFFER_MIXED_DIMENSIONS ?
> And if those aren't present you only expose GL_EXT_framebuffer_object
> and not the ARB version.
>
> Or alternatively a pipe_context.validate_framebuffer_configuration where
> more advanced cards can just return TRUE ?
>
> I'd rather avoid breaking applications that use this feature for an
> undefined amount of time.
>
> > ---
> >  src/mesa/state_tracker/st_cb_fbo.c |   21 ++++++++++++++++++++-
> >  1 files changed, 20 insertions(+), 1 deletions(-)
> >
> > diff --git a/src/mesa/state_tracker/st_cb_fbo.c
> b/src/mesa/state_tracker/st_cb_fbo.c
> > index 0df0428..e1057b1 100644
> > --- a/src/mesa/state_tracker/st_cb_fbo.c
> > +++ b/src/mesa/state_tracker/st_cb_fbo.c
> > @@ -529,6 +529,7 @@ st_validate_framebuffer(struct gl_context *ctx,
> struct gl_framebuffer *fb)
> >     const struct gl_renderbuffer_attachment *stencil =
> >           &fb->Attachment[BUFFER_STENCIL];
> >     GLuint i;
> > +   enum pipe_format first_format = PIPE_FORMAT_NONE;
> >
> >     if (depth->Type && stencil->Type && depth->Type != stencil->Type) {
> >        fb->_Status = GL_FRAMEBUFFER_UNSUPPORTED_EXT;
> > @@ -562,13 +563,31 @@ st_validate_framebuffer(struct gl_context *ctx,
> struct gl_framebuffer *fb)
> >        return;
> >     }
> >     for (i = 0; i < ctx->Const.MaxColorAttachments; i++) {
> > +      struct gl_renderbuffer_attachment *att =
> > +            &fb->Attachment[BUFFER_COLOR0 + i];
> > +      enum pipe_format format;
> > +
> >        if (!st_validate_attachment(ctx,
> >                                    screen,
> > -                               &fb->Attachment[BUFFER_COLOR0 + i],
> > +                               att,
> >                                 PIPE_BIND_RENDER_TARGET)) {
> >        fb->_Status = GL_FRAMEBUFFER_UNSUPPORTED_EXT;
> >        return;
> >        }
> > +
> > +      /* Disallow mixed formats. This may be revisited later. */
> > +      if (att->Type != GL_NONE) {
> > +         format = st_renderbuffer(att->Renderbuffer)->surface->format;
> > +      } else {
> > +         continue;
> > +      }
> > +
> > +      if (first_format == PIPE_FORMAT_NONE) {
> > +         first_format = format;
> > +      } else if (format != first_format) {
> > +         fb->_Status = GL_FRAMEBUFFER_UNSUPPORTED_EXT;
> > +         return;
> > +      }
> >     }
> >  }
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110328/88c60ed3/attachment-0001.htm>