[Mesa-dev] [PATCH] st/mesa: make unsupported renderbuffer formats always fail as FBO incomplete

[Marek Olšák]

instead of failing to allocate a renderbuffer. This also improves allocation
of MSAA renderbuffers (untested).

This also fixes piglit/get-renderbuffer-internalformat with non-renderable
formats.
---
 src/mesa/state_tracker/st_cb_fbo.c |   63 +++++++++++++++++++++++++++++++-----
 1 file changed, 55 insertions(+), 8 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_fbo.c b/src/mesa/state_tracker/st_cb_fbo.c
index 953295c..5a21c8e 100644
--- a/src/mesa/state_tracker/st_cb_fbo.c
+++ b/src/mesa/state_tracker/st_cb_fbo.c
@@ -72,8 +72,9 @@ st_renderbuffer_alloc_storage(struct gl_context * ctx,
    struct pipe_context *pipe = st->pipe;
    struct pipe_screen *screen = st->pipe->screen;
    struct st_renderbuffer *strb = st_renderbuffer(rb);
-   enum pipe_format format;
+   enum pipe_format format = PIPE_FORMAT_NONE;
    struct pipe_surface surf_tmpl;
+   unsigned nr_samples = 0;
 
    if (internalFormat == GL_RGBA16_SNORM && strb->software) {
       /* Special case for software accum buffers.  Otherwise, if the
@@ -84,12 +85,55 @@ st_renderbuffer_alloc_storage(struct gl_context * ctx,
       format = PIPE_FORMAT_R16G16B16A16_SNORM;
    }
    else {
-      format = st_choose_renderbuffer_format(screen, internalFormat,
-                                             rb->NumSamples);
-   }
+      /* Handle multisample renderbuffers first.
+       *
+       * From ARB_framebuffer_object:
+       *   If <samples> is zero, then RENDERBUFFER_SAMPLES is set to zero.
+       *   Otherwise <samples> represents a request for a desired minimum
+       *   number of samples. Since different implementations may support
+       *   different sample counts for multisampled rendering, the actual
+       *   number of samples allocated for the renderbuffer image is
+       *   implementation dependent.  However, the resulting value for
+       *   RENDERBUFFER_SAMPLES is guaranteed to be greater than or equal
+       *   to <samples> and no more than the next larger sample count supported
+       *   by the implementation.
+       *
+       * So let's find the supported number of samples closest to NumSamples.
+       * (NumSamples == 1) is treated the same as (NumSamples == 0).
+       */
+      if (rb->NumSamples > 1) {
+         unsigned i;
 
-   if (format == PIPE_FORMAT_NONE) {
-      return FALSE;
+         for (i = rb->NumSamples; i < ctx->Const.MaxSamples; i++) {
+            format = st_choose_renderbuffer_format(screen, internalFormat, i);
+
+            if (format != PIPE_FORMAT_NONE) {
+               rb->NumSamples = i;
+               nr_samples = i;
+               break;
+            }
+         }
+      } else {
+         format = st_choose_renderbuffer_format(screen, internalFormat, 0);
+      }
+
+      /* There is no supported renderable pipe format for internalformat.
+       * AllocStorage shouldn't fail because of an unsupported format,
+       * instead, unsupported formats should cause FBO incompleteness
+       * later, so let's set the closest sampler format we can get.
+       * This will also cause FBO incompleteness for unsupported sample
+       * counts, because the validation uses rb->NumSamples.
+       */
+      if (format == PIPE_FORMAT_NONE) {
+         format = st_choose_format(screen, internalFormat, GL_NONE, GL_NONE,
+                                   PIPE_TEXTURE_2D, 0,
+                                   PIPE_BIND_SAMPLER_VIEW);
+      }
+
+      assert(format != PIPE_FORMAT_NONE);
+      if (format == PIPE_FORMAT_NONE) {
+         return FALSE; /* shouldn't happen unless there's a bug somewhere */
+      }
    }
 
    /* init renderbuffer fields */
@@ -134,7 +178,7 @@ st_renderbuffer_alloc_storage(struct gl_context * ctx,
       template.depth0 = 1;
       template.array_size = 1;
       template.last_level = 0;
-      template.nr_samples = rb->NumSamples;
+      template.nr_samples = nr_samples;
       if (util_format_is_depth_or_stencil(format)) {
          template.bind = PIPE_BIND_DEPTH_STENCIL;
       }
@@ -455,9 +499,12 @@ st_validate_attachment(struct gl_context *ctx,
       format = st_mesa_format_to_pipe_format(linearFormat);
    }
 
+   /* Check Renderbuffer->NumSamples (which is what the user expects) and
+    * not pipe_resource::nr_samples (which might be zero if NumSamples is
+    * unsupported). */
    return screen->is_format_supported(screen, format,
                                       PIPE_TEXTURE_2D,
-                                      stObj->pt->nr_samples, bindings);
+                                      att->Renderbuffer->NumSamples, bindings);
 }
 
 
-- 
1.7.9.5