[Mesa-dev] [PATCH 2/6] mesa: wire up InvalidateSubFramebuffer

[Rob Clark]

On Tue, Dec 11, 2018 at 6:16 PM Ian Romanick <idr at freedesktop.org> wrote:
>
> On 12/11/18 2:50 PM, Rob Clark wrote:
> > Signed-off-by: Rob Clark <robdclark at gmail.com>
> > ---
> >  src/mesa/main/dd.h       |  3 +++
> >  src/mesa/main/fbobject.c | 34 +++++++++++++++++++++++++++++++++-
> >  2 files changed, 36 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> > index 1214eeaa474..c7112677223 100644
> > --- a/src/mesa/main/dd.h
> > +++ b/src/mesa/main/dd.h
> > @@ -786,6 +786,9 @@ struct dd_function_table {
> >                             GLbitfield mask, GLenum filter);
> >     void (*DiscardFramebuffer)(struct gl_context *ctx, struct gl_framebuffer *fb,
> >                                struct gl_renderbuffer_attachment *att);
> > +   void (*DiscardSubFramebuffer)(struct gl_context *ctx, struct gl_framebuffer *fb,
> > +                                 struct gl_renderbuffer_attachment *att, GLint x,
> > +                                 GLint y, GLsizei width, GLsizei height);
>
> After looking at the rest of the series... I wonder if some higher layer
> should be responsible for detecting the case where the subrect size of
> the DiscardSubFramebuffer is the size of the entire attachment (which
> may be different than the renderable size of the framebuffer) and call
> DiscardFramebuffer instead.  It seems like many implementations won't do
> anything for DiscardSubFramebuffer but will do something for
> DiscardFramebuffer.

Fair point, the DiscardSubFramebuffer() part could be harder or less
useful for other drivers.. smashing in something like this would be
trival:

----------------
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 04fd7b8b943..f5b7562c9d7 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -4706,6 +4706,17 @@ discard_sub_framebuffer(struct gl_context *ctx,
struct gl_framebuffer *fb,
    if (!ctx->Driver.DiscardSubFramebuffer)
       return;

+   /* In the trivial case, turn it into discard_framebuffer(), on the
+    * premise that drivers are more likely to implement the simpler
+    * interface:
+    */
+   if (x == 0 && y == 0 &&
+       width >= fb->Width &&
+       height >= fb->Height) {
+      discard_framebuffer(ctx, fb, numAttachments, attachments);
+      return;
+   }
+
    for (int i = 0; i < numAttachments; i++) {
       struct gl_renderbuffer_attachment *att =
             get_fb_attachment(ctx, fb, attachments[i]);
----------------

BR,
-R

>
> Maybe just leave a note in discard_sub_framebuffer so that the first
> person working on a driver that would benefit from this will implement it?
>
> >
> >     /**
> >      * \name Functions for GL_ARB_sample_locations
> > diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> > index f931e8f76b1..8ef5eb747c0 100644
> > --- a/src/mesa/main/fbobject.c
> > +++ b/src/mesa/main/fbobject.c
> > @@ -4699,12 +4699,41 @@ discard_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> >     }
> >  }
> >
> > +static void
> > +discard_sub_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> > +                        GLsizei numAttachments, const GLenum *attachments,
> > +                        GLint x, GLint y, GLsizei width, GLsizei height)
> > +{
> > +   GLint i;
> > +
> > +   if (!ctx->Driver.DiscardSubFramebuffer)
> > +      return;
> > +
> > +   for (i = 0; i < numAttachments; i++) {
> > +      struct gl_renderbuffer_attachment *att =
> > +            get_fb_attachment(ctx, fb, attachments[i]);
> > +
> > +      if (!att)
> > +         continue;
> > +
> > +      ctx->Driver.DiscardSubFramebuffer(ctx, fb, att, x, y, width, height);
> > +   }
> > +}
> > +
> >  void GLAPIENTRY
> >  _mesa_InvalidateSubFramebuffer_no_error(GLenum target, GLsizei numAttachments,
> >                                          const GLenum *attachments, GLint x,
> >                                          GLint y, GLsizei width, GLsizei height)
> >  {
> > -   /* no-op */
> > +   struct gl_framebuffer *fb;
> > +   GET_CURRENT_CONTEXT(ctx);
> > +
> > +   fb = get_framebuffer_target(ctx, target);
> > +   if (!fb)
> > +      return;
> > +
> > +   discard_sub_framebuffer(ctx, fb, numAttachments, attachments,
> > +                           x, y, width, height);
> >  }
> >
> >
> > @@ -4727,6 +4756,9 @@ _mesa_InvalidateSubFramebuffer(GLenum target, GLsizei numAttachments,
> >     invalidate_framebuffer_storage(ctx, fb, numAttachments, attachments,
> >                                    x, y, width, height,
> >                                    "glInvalidateSubFramebuffer");
> > +
> > +   discard_sub_framebuffer(ctx, fb, numAttachments, attachments,
> > +                           x, y, width, height);
> >  }
> >
> >
> >
>