[Mesa-dev] [PATCH 2/2] i965: Fix clears of layered framebuffers with mismatched layer counts.

Anuj Phogat anuj.phogat at gmail.com
Thu Jan 9 16:56:08 PST 2014


On Tue, Jan 7, 2014 at 7:33 AM, Paul Berry <stereotype441 at gmail.com> wrote:
> Previously, Mesa enforced the following rule (from
> ARB_geometry_shader4's list of criteria for framebuffer completeness):
>
>   * If any framebuffer attachment is layered, all attachments must have
>     the same layer count.  For three-dimensional textures, the layer count
>     is the depth of the attached volume.  For cube map textures, the layer
>     count is always six.  For one- and two-dimensional array textures, the
>     layer count is simply the number of layers in the array texture.
>     { FRAMEBUFFER_INCOMPLETE_LAYER_COUNT_ARB }
>
> However, when ARB_geometry_shader4 was adopted into GL 3.2, this rule
> was dropped; GL 3.2 permits different attachments to have different
> layer counts.  This patch brings Mesa in line with GL 3.2.
>
> In order to ensure that layered clears properly clear all layers, we
> now have to keep track of the maximum number of layers in a layered
> framebuffer.
>
> Fixes the following piglit tests in spec/!OpenGL 3.2/layered-rendering:
> - clear-color-all-types 1d_array mipmapped
> - clear-color-all-types 1d_array single_level
> - clear-color-mismatched-layer-count
> - framebuffer-layer-count-mismatch
> ---
>  src/mesa/drivers/common/meta.c                   |  4 ++--
>  src/mesa/drivers/dri/i965/brw_blorp_clear.cpp    |  8 ++++----
>  src/mesa/drivers/dri/i965/brw_clear.c            |  6 +++---
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |  2 +-
>  src/mesa/drivers/dri/i965/gen6_clip_state.c      |  2 +-
>  src/mesa/drivers/dri/i965/gen7_misc_state.c      |  2 +-
>  src/mesa/main/fbobject.c                         | 26 ++++++++++++------------
>  src/mesa/main/mtypes.h                           |  9 ++++----
>  8 files changed, 29 insertions(+), 30 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> index 7b41876..1294514 100644
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -2407,9 +2407,9 @@ _mesa_meta_glsl_Clear(struct gl_context *ctx, GLbitfield buffers)
>                        GL_DYNAMIC_DRAW_ARB);
>
>     /* draw quad(s) */
> -   if (fb->NumLayers > 0) {
> +   if (fb->MaxNumLayers > 0) {
>        unsigned layer;
> -      for (layer = 0; layer < fb->NumLayers; layer++) {
> +      for (layer = 0; layer < fb->MaxNumLayers; layer++) {
>           if (fb->_IntegerColor)
>              _mesa_Uniform1i(clear->IntegerLayerLocation, layer);
>           else
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> index 072ad55..c55108a 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> @@ -573,14 +573,14 @@ brw_blorp_clear_color(struct brw_context *brw, struct gl_framebuffer *fb,
>        if (rb == NULL)
>           continue;
>
> -      if (fb->NumLayers > 0) {
> +      if (fb->MaxNumLayers > 0) {
>           unsigned layer_multiplier =
>              (irb->mt->msaa_layout == INTEL_MSAA_LAYOUT_UMS ||
>               irb->mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS) ?
>              irb->mt->num_samples : 1;
> -         assert(fb->NumLayers * layer_multiplier ==
> -                irb->mt->level[irb->mt_level].depth);
> -         for (unsigned layer = 0; layer < fb->NumLayers; layer++) {
> +         unsigned num_layers =
> +            irb->mt->level[irb->mt_level].depth / layer_multiplier;
> +         for (unsigned layer = 0; layer < num_layers; layer++) {
>              if (!do_single_blorp_clear(brw, fb, rb, buf, partial_clear,
>                                         layer * layer_multiplier)) {
>                 return false;
> diff --git a/src/mesa/drivers/dri/i965/brw_clear.c b/src/mesa/drivers/dri/i965/brw_clear.c
> index 1cac996..fe68d9e 100644
> --- a/src/mesa/drivers/dri/i965/brw_clear.c
> +++ b/src/mesa/drivers/dri/i965/brw_clear.c
> @@ -181,9 +181,9 @@ brw_fast_clear_depth(struct gl_context *ctx)
>      */
>     intel_batchbuffer_emit_mi_flush(brw);
>
> -   if (fb->NumLayers > 0) {
> -      assert(fb->NumLayers == depth_irb->mt->level[depth_irb->mt_level].depth);
> -      for (unsigned layer = 0; layer < fb->NumLayers; layer++) {
> +   if (fb->MaxNumLayers > 0) {
> +      unsigned num_layers = depth_irb->mt->level[depth_irb->mt_level].depth;
> +      for (unsigned layer = 0; layer < num_layers; layer++) {
>           intel_hiz_exec(brw, mt, depth_irb->mt_level, layer,
>                          GEN6_HIZ_OP_DEPTH_CLEAR);
>        }
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 5236eda..6bd4a29 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -700,7 +700,7 @@ brw_update_renderbuffer_surfaces(struct brw_context *brw)
>        for (i = 0; i < ctx->DrawBuffer->_NumColorDrawBuffers; i++) {
>          if (intel_renderbuffer(ctx->DrawBuffer->_ColorDrawBuffers[i])) {
>             brw->vtbl.update_renderbuffer_surface(brw, ctx->DrawBuffer->_ColorDrawBuffers[i],
> -                                                  ctx->DrawBuffer->NumLayers > 0, i);
> +                                                  ctx->DrawBuffer->MaxNumLayers > 0, i);
>          } else {
>             brw->vtbl.update_null_renderbuffer_surface(brw, i);
>          }
> diff --git a/src/mesa/drivers/dri/i965/gen6_clip_state.c b/src/mesa/drivers/dri/i965/gen6_clip_state.c
> index 37a39b8..6cec0ff 100644
> --- a/src/mesa/drivers/dri/i965/gen6_clip_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_clip_state.c
> @@ -121,7 +121,7 @@ upload_clip_state(struct brw_context *brw)
>              dw2);
>     OUT_BATCH(U_FIXED(0.125, 3) << GEN6_CLIP_MIN_POINT_WIDTH_SHIFT |
>               U_FIXED(255.875, 3) << GEN6_CLIP_MAX_POINT_WIDTH_SHIFT |
> -             (fb->NumLayers > 0 ? 0 : GEN6_CLIP_FORCE_ZERO_RTAINDEX));
> +             (fb->MaxNumLayers > 0 ? 0 : GEN6_CLIP_FORCE_ZERO_RTAINDEX));
>     ADVANCE_BATCH();
>  }
>
> diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> index 4251949..8fb0eec 100644
> --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> @@ -81,7 +81,7 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
>        break;
>     }
>
> -   if (fb->NumLayers > 0 || !irb) {
> +   if (fb->MaxNumLayers > 0 || !irb) {
>        min_array_element = 0;
>     } else if (irb->mt->num_samples > 1) {
>        /* Convert physical layer to logical layer. */
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index c3f634f..7bc5b1b 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -877,8 +877,10 @@ _mesa_test_framebuffer_completeness(struct gl_context *ctx,
>     GLint fixedSampleLocations = -1;
>     GLint i;
>     GLuint j;
> -   bool layer_info_valid = false; /* Covers layer_count and layer_tex_target */
> -   GLuint layer_count = 0, att_layer_count;
> +   /* Covers max_layer_count, is_layered, and layer_tex_target */
> +   bool layer_info_valid = false;
> +   GLuint max_layer_count = 0, att_layer_count;
> +   bool is_layered;
>     GLenum layer_tex_target = 0;
>
>     assert(_mesa_is_user_fbo(fb));
> @@ -1064,26 +1066,24 @@ _mesa_test_framebuffer_completeness(struct gl_context *ctx,
>           att_layer_count = 0;
>        }
>        if (!layer_info_valid) {
> -         layer_count = att_layer_count;
> +         is_layered = att->Layered;
> +         max_layer_count = att_layer_count;
>           layer_tex_target = att_tex_target;
>           layer_info_valid = true;
> -      } else if (layer_count > 0 && layer_tex_target != att_tex_target) {
> +      } else if (max_layer_count > 0 && layer_tex_target != att_tex_target) {
>           fb->_Status = GL_FRAMEBUFFER_INCOMPLETE_LAYER_TARGETS;
>           fbo_incomplete(ctx, "layered framebuffer has mismatched targets", i);
>           return;
> -      } else if (layer_count != att_layer_count) {
> -         if (layer_count == 0 || att_layer_count == 0) {
> -            fb->_Status = GL_FRAMEBUFFER_INCOMPLETE_LAYER_TARGETS;
> -            fbo_incomplete(ctx, "framebuffer attachment layer mode is inconsistent", i);
> -         } else {
> -            fb->_Status = GL_FRAMEBUFFER_INCOMPLETE_LAYER_COUNT_ARB;
> -            fbo_incomplete(ctx, "framebuffer attachment layer count is inconsistent", i);
> -         }
> +      } else if (is_layered != att->Layered) {
> +         fb->_Status = GL_FRAMEBUFFER_INCOMPLETE_LAYER_TARGETS;
> +         fbo_incomplete(ctx, "framebuffer attachment layer mode is inconsistent", i);
>           return;
> +      } else if (att_layer_count > max_layer_count) {
> +         max_layer_count = att_layer_count;
>        }
>     }
>
> -   fb->NumLayers = layer_count;
> +   fb->MaxNumLayers = max_layer_count;
>
>     if (_mesa_is_desktop_gl(ctx) && !ctx->Extensions.ARB_ES2_compatibility) {
>        /* Check that all DrawBuffers are present */
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index a5294b0..0db7070 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -3007,12 +3007,11 @@ struct gl_framebuffer
>     struct gl_renderbuffer *_ColorReadBuffer;
>
>     /**
> -    * The number of layers in the framebuffer, or 0 if the framebuffer is not
> -    * layered.  For cube maps, this value is 6.  For cube map arrays, this
> -    * value is the "depth" value passed to TexImage3D (always a multiple of
> -    * 6).
> +    * The maximum number of layers in the framebuffer, or 0 if the framebuffer
> +    * is not layered.  For cube maps and cube map arrays, each cube face
> +    * counts as a layer.
>      */
> -   GLuint NumLayers;
> +   GLuint MaxNumLayers;
>
>     /** Delete this framebuffer */
>     void (*Delete)(struct gl_framebuffer *fb);
> --
> 1.8.5.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Both the patches are: Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>


More information about the mesa-dev mailing list