[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