[Mesa-dev] [PATCH 4/6] mesa/st: wire up DiscardSubFramebuffer

[Rob Clark]

On Tue, Dec 11, 2018 at 6:08 PM Roland Scheidegger <sroland at vmware.com> wrote:
>
> Am 11.12.18 um 23:50 schrieb Rob Clark:
> > Signed-off-by: Rob Clark <robdclark at gmail.com>
> > ---
> >  src/gallium/include/pipe/p_context.h | 11 +++++++++++
> >  src/mesa/state_tracker/st_cb_fbo.c   | 26 ++++++++++++++++++++++++++
> >  2 files changed, 37 insertions(+)
> >
> > diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
> > index d4e9179b78a..eb52c7e9a4e 100644
> > --- a/src/gallium/include/pipe/p_context.h
> > +++ b/src/gallium/include/pipe/p_context.h
> > @@ -811,6 +811,17 @@ struct pipe_context {
> >     void (*invalidate_surface)(struct pipe_context *ctx,
> >                                struct pipe_surface *surf);
> >
> > +   /**
> > +    * Invalidate a portion of a surface.  This is used to
> > +    *
> > +    * (1) implement glInvalidateSubFramebuffer() and friends
> > +    * (2) as a hint before a scissored clear (which is turned into draw_vbo()
> > +    *     that the cleared rect can be discarded
> > +    */
> > +   void (*invalidate_sub_surface)(struct pipe_context *ctx,
> > +                                  struct pipe_surface *surf,
> > +                                  const struct pipe_scissor_state *rect);
> > +
>
> The same applies as to the previous change. Additionally, I think we
> really don't need 3 functions essentially doing the same thing. Ok I
> could see that maybe there's value passing in the surface directly
> (rather than mip levels, layers), but surely
> invalidate_surface/invalidate_sub_surface looks like overkill to me.
> Maybe pass in a NULL pointer for rect if you want to clear everything?

Will splitup gallium+docs and mesa/st, as you mentioned..

I guess rect==NULL is ok by me for avoiding invalidate_surface vs
invalidate_sub_surface

I'm not a huge fan of keeping this at the pipe_resource level and
passing layer/level/etc since in the driver it works out well to track
this at fd_surface rather than fd_resource level, and I don't want to
have to try and recover the surface from resource+params.

BR,
-R


>
> Roland
>
>
> >     /**
> >      * Return information about unexpected device resets.
> >      */
> > diff --git a/src/mesa/state_tracker/st_cb_fbo.c b/src/mesa/state_tracker/st_cb_fbo.c
> > index 3ece1d4a9de..50c27ea51d9 100644
> > --- a/src/mesa/state_tracker/st_cb_fbo.c
> > +++ b/src/mesa/state_tracker/st_cb_fbo.c
> > @@ -774,6 +774,31 @@ st_discard_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> >        st->pipe->invalidate_surface(st->pipe, psurf);
> >  }
> >
> > +static void
> > +st_discard_sub_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> > +                           struct gl_renderbuffer_attachment *att, GLint x,
> > +                           GLint y, GLsizei width, GLsizei height)
> > +{
> > +   struct st_context *st = st_context(ctx);
> > +   struct pipe_surface *psurf;
> > +
> > +   if (!att->Renderbuffer)
> > +      return;
> > +
> > +   psurf = st_renderbuffer(att->Renderbuffer)->surface;
> > +
> > +   if (st->pipe->invalidate_sub_surface) {
> > +      struct pipe_scissor_state rect;
> > +
> > +      rect.minx = x;
> > +      rect.maxx = x + width - 1;
> > +      rect.miny = y;
> > +      rect.maxy = y + height - 1;
> > +
> > +      st->pipe->invalidate_sub_surface(st->pipe, psurf, &rect);
> > +   }
> > +}
> > +
> >  /**
> >   * Called via glDrawBuffer.  We only provide this driver function so that we
> >   * can check if we need to allocate a new renderbuffer.  Specifically, we
> > @@ -952,6 +977,7 @@ st_init_fbo_functions(struct dd_function_table *functions)
> >     functions->FinishRenderTexture = st_finish_render_texture;
> >     functions->ValidateFramebuffer = st_validate_framebuffer;
> >     functions->DiscardFramebuffer = st_discard_framebuffer;
> > +   functions->DiscardSubFramebuffer = st_discard_sub_framebuffer;
> >
> >     functions->DrawBufferAllocate = st_DrawBufferAllocate;
> >     functions->ReadBuffer = st_ReadBuffer;
> >
>