[Mesa-dev] [PATCH 2/4] i965: be more specific about FBO completeness errors

Iago Toral itoral at igalia.com
Mon Nov 19 09:26:18 UTC 2018


On Mon, 2018-11-19 at 10:21 +0100, Iago Toral wrote:
> I was about to write about this too. I think the patch is not
> correct for a couple of reasons:
> 
> 1.  GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS is not defined for OpenGL,
> only for GL ES. The GL definition has the _EXT suffix and is included
> with a specific extension, so I don't think we can use it like this.
> 
> 2. According to the OpenGL ES 3.2 spec:
> 
> "Note that the error token FRAMEBUFFER_INCOMPLETE_DIMENSIONS is 
> included in the API for OpenGL ES 2.0 compatibility, but cannot be
> generated by an OpenGL ES 3.0 or later implementation."
> 
> Maybe we should revert this patch?

Well, not revert it maybe, but amend the case where we return
incomplete dimensions and just return GL_FRAMEBUFFER_UNSUPPORTED
instead? Would that be enought for EXT_texture_sRGB_R8?

Iago

> Iago
> 
> On Mon, 2018-11-19 at 11:00 +0200, andrey simiklit wrote:
> > Hello,
> > 
> > I tried to compile latest mesa master this morning but faced with a
> > compilation error:
> >    intel_fbo.c: In function ‘intel_validate_framebuffer’:
> >    intel_fbo.c:696:25: error:
> > ‘GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS’
> > undeclared (first use in this function); did you mean
> > ‘GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS_EXT’?
> >          fbo_incomplete(fb, GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS,
> >                             ^
> >    intel_fbo.c:642:21: note: in definition of macro
> > ‘fbo_incomplete’
> >           fb->_Status =
> > error_id;                                                 \
> >                         ^~~~~~~~
> >    intel_fbo.c:696:25: note: each undeclared identifier is reported
> > only once for each function it appears in
> >          fbo_incomplete(fb, GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS,
> >                             ^
> >    intel_fbo.c:642:21: note: in definition of macro
> > ‘fbo_incomplete’
> >           fb->_Status =
> > error_id;                                                 \
> > 
> > This small change helped me but I can't say for sure that it is
> > right:
> > diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c
> > b/src/mesa/drivers/dri/i965/intel_fbo.c
> > index febd1aca98..5d870c65f8 100644
> > --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> > +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> > @@ -693,7 +693,7 @@ intel_validate_framebuffer(struct gl_context
> > *ctx, struct gl_framebuffer *fb)
> >               d_depth != s_depth ||
> >               depthRb->mt_level != stencilRb->mt_level ||
> >              depthRb->mt_layer != stencilRb->mt_layer) {
> > -           fbo_incomplete(fb,
> > GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS,
> > +           fbo_incomplete(fb,
> > GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS_EXT,
> >                             "FBO incomplete: depth and stencil must
> > match in"
> >                             "width, height, depth, LOD and
> > layer\n");
> >          }
> > 
> > Thanks,
> > Andrii.
> > 
> > On Thu, Nov 15, 2018 at 8:01 PM Gert Wollny <
> > gert.wollny at collabora.com> wrote:
> > > The driver was returning GL_FRAMEBUFFER_UNSUPPORTED for all cases
> > > of an
> > > incomplete fbo, be a bit more specific about this following the
> > > description
> > > of glCheckFramebufferStatus.
> > > 
> > > This helps to keeps dEQP happy when adding EXT_texture_sRGB_R8
> > > support.
> > > 
> > > Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> > > ---
> > >  src/mesa/drivers/dri/i965/intel_fbo.c | 42 +++++++++++++++------
> > > ------
> > >  1 file changed, 23 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c
> > > b/src/mesa/drivers/dri/i965/intel_fbo.c
> > > index 2bbbc34114..d657fdf8e4 100644
> > > --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> > > +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> > > @@ -629,7 +629,7 @@ intel_render_texture(struct gl_context * ctx,
> > >  }
> > > 
> > > 
> > > -#define fbo_incomplete(fb, ...) do
> > > {                             
> > >             \
> > > +#define fbo_incomplete(fb, error_id, ...) do
> > > {                   
> > >                       \
> > >        static GLuint msg_id =
> > > 0;                                   
> > >            \
> > >        if (unlikely(ctx->Const.ContextFlags &
> > > GL_CONTEXT_FLAG_DEBUG_BIT)) {    \
> > >           _mesa_gl_debug(ctx,
> > > &msg_id,                             
> > >            \
> > > @@ -639,7 +639,7 @@ intel_render_texture(struct gl_context * ctx,
> > >                          __VA_ARGS__);                           
> > >   
> > >            \
> > >        }                                                         
> > >   
> > >            \
> > >        DBG(__VA_ARGS__);                                         
> > >   
> > >            \
> > > -      fb->_Status =
> > > GL_FRAMEBUFFER_UNSUPPORTED;                   
> > >            \
> > > +      fb->_Status =
> > > error_id;                                     
> > >            \
> > >     } while (0)
> > > 
> > >  /**
> > > @@ -693,7 +693,7 @@ intel_validate_framebuffer(struct gl_context
> > > *ctx, struct gl_framebuffer *fb)
> > >               d_depth != s_depth ||
> > >               depthRb->mt_level != stencilRb->mt_level ||
> > >              depthRb->mt_layer != stencilRb->mt_layer) {
> > > -           fbo_incomplete(fb,
> > > +           fbo_incomplete(fb,
> > > GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS,
> > >                             "FBO incomplete: depth and stencil
> > > must
> > > match in"
> > >                             "width, height, depth, LOD and
> > > layer\n");
> > >          }
> > > @@ -705,7 +705,7 @@ intel_validate_framebuffer(struct gl_context
> > > *ctx, struct gl_framebuffer *fb)
> > >           */
> > >          if (depthRb->mt_level != stencilRb->mt_level ||
> > >              depthRb->mt_layer != stencilRb->mt_layer) {
> > > -           fbo_incomplete(fb,
> > > +           fbo_incomplete(fb, GL_FRAMEBUFFER_UNSUPPORTED,
> > >                             "FBO incomplete: depth image
> > > level/layer %d/%d != "
> > >                             "stencil image %d/%d\n",
> > >                             depthRb->mt_level,
> > > @@ -715,13 +715,14 @@ intel_validate_framebuffer(struct
> > > gl_context
> > > *ctx, struct gl_framebuffer *fb)
> > >          }
> > >        } else {
> > >          if (!brw->has_separate_stencil) {
> > > -           fbo_incomplete(fb, "FBO incomplete: separate stencil
> > > "
> > > -                           "unsupported\n");
> > > +           fbo_incomplete(fb, GL_FRAMEBUFFER_UNSUPPORTED,
> > > +                      "FBO incomplete: separate stencil
> > > unsupported\n");
> > >          }
> > >          if (stencil_mt->format != MESA_FORMAT_S_UINT8) {
> > > -           fbo_incomplete(fb, "FBO incomplete: separate stencil
> > > is
> > > %s "
> > > -                           "instead of S8\n",
> > > -                           _mesa_get_format_name(stencil_mt-
> > > > format));
> > > 
> > > +           fbo_incomplete(fb,
> > > GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT,
> > > +                      "FBO incomplete: separate stencil is %s "
> > > +                      "instead of S8\n",
> > > +                      _mesa_get_format_name(stencil_mt-
> > > >format));
> > >          }
> > >          if (devinfo->gen < 7 &&
> > > !intel_renderbuffer_has_hiz(depthRb)) {
> > >             /* Before Gen7, separate depth and stencil buffers
> > > can
> > > be used
> > > @@ -730,8 +731,8 @@ intel_validate_framebuffer(struct gl_context
> > > *ctx, struct gl_framebuffer *fb)
> > >              *     [DevSNB]: This field must be set to the same
> > > value (enabled
> > >              *     or disabled) as Hierarchical Depth Buffer
> > > Enable.
> > >              */
> > > -           fbo_incomplete(fb, "FBO incomplete: separate stencil
> > > "
> > > -                           "without HiZ\n");
> > > +           fbo_incomplete(fb, GL_FRAMEBUFFER_UNSUPPORTED,
> > > +                          "FBO incomplete: separate stencil
> > > without HiZ\n");
> > >          }
> > >        }
> > >     }
> > > @@ -749,29 +750,32 @@ intel_validate_framebuffer(struct
> > > gl_context
> > > *ctx, struct gl_framebuffer *fb)
> > >         */
> > >        rb = fb->Attachment[i].Renderbuffer;
> > >        if (rb == NULL) {
> > > -        fbo_incomplete(fb, "FBO incomplete: attachment without "
> > > -                        "renderbuffer\n");
> > > +        fbo_incomplete(fb,
> > > GL_FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT,
> > > +                       "FBO incomplete: attachment without "
> > > +                       "renderbuffer\n");
> > >          continue;
> > >        }
> > > 
> > >        if (fb->Attachment[i].Type == GL_TEXTURE) {
> > >          if (rb->TexImage->Border) {
> > > -           fbo_incomplete(fb, "FBO incomplete: texture with
> > > border\n");
> > > +           fbo_incomplete(fb, GL_FRAMEBUFFER_UNSUPPORTED,
> > > +                      "FBO incomplete: texture with border\n");
> > >             continue;
> > >          }
> > >        }
> > > 
> > >        irb = intel_renderbuffer(rb);
> > >        if (irb == NULL) {
> > > -        fbo_incomplete(fb, "FBO incomplete: software rendering "
> > > -                        "renderbuffer\n");
> > > +        fbo_incomplete(fb, GL_FRAMEBUFFER_UNSUPPORTED,
> > > +                   "FBO incomplete: software rendering
> > > renderbuffer\n");
> > >          continue;
> > >        }
> > > 
> > >        if (!brw_render_target_supported(brw, rb)) {
> > > -        fbo_incomplete(fb, "FBO incomplete: Unsupported HW "
> > > -                        "texture/renderbuffer format attached:
> > > %s\n",
> > > -                       
> > > _mesa_get_format_name(intel_rb_format(irb)));
> > > +        fbo_incomplete(fb, GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT,
> > > +                   "FBO incomplete: Unsupported HW "
> > > +                   "texture/renderbuffer format attached: %s\n",
> > > +                   _mesa_get_format_name(intel_rb_format(irb)));
> > >        }
> > >     }
> > >  }
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > > 



More information about the mesa-dev mailing list