The way I see it, pipe_surface is a render target view for
pipe_framebuffer_state. The fact that other functions also accept
pipe_surface is a historic artifact. resource_copy_region used to accept 2
pipe_surfaces before it was changed to 2 pipe_resources + parameters.
Somebody might decide to change invalidate_subresource to take
pipe_resource in the future.
It seems to me that you really want e.g. invalidate_framebuffer(ctx,
PIPE_FRAMEBUFFER_COLOR0 | PIPE_FRAMEBUFFER_COLOR1), not pipe_surface.
I'm not too crazy about this, just pointing out the way I understand the
conventions.
Marek
On Fri, Dec 14, 2018 at 3:51 PM Rob Clark <robdcl...@gmail.com> wrote:
> I don't mind renaming it to invalidate_subsurface() if you prefer
> that, but I really prefer that it takes a surface rather than a
> resource... pipe_resource is the wrong level to do this.
>
> Even though surfaces are transient, tracking the invalidate state in
> the surface works out much easier in the driver. And I don't want to
> have to map resource+params back to a surface. If another driver
> wants to track this at the resource level, it is easier for them to go
> from surface to resource+params, compared to the opposite direction.
>
> BR,
> -R
>
>
> On Fri, Dec 14, 2018 at 3:38 PM Marek Olšák <mar...@gmail.com> wrote:
> >
> > Can you please call it invalidate_subresource and inline relevant
> pipe_surface variables inside the parameters?
> >
> > Thanks,
> > Marek
> >
> > On Wed, Dec 12, 2018 at 10:48 AM Rob Clark <robdcl...@gmail.com> wrote:
> >>
> >> A new API to implement glInvalidateFramebuffer() and friends. It is
> >> similar to invalidate_resource() but can be used to invalidate a
> >> specific layer/level, so it is suitable to use for user FBOs in addition
> >> to window system framebuffer.
> >>
> >> Signed-off-by: Rob Clark <robdcl...@gmail.com>
> >> ---
> >> src/gallium/docs/source/context.rst | 18 ++++++++++++++++++
> >> src/gallium/include/pipe/p_context.h | 18 ++++++++++++++++++
> >> 2 files changed, 36 insertions(+)
> >>
> >> diff --git a/src/gallium/docs/source/context.rst
> b/src/gallium/docs/source/context.rst
> >> index 20d0df79312..9f3cd49ee31 100644
> >> --- a/src/gallium/docs/source/context.rst
> >> +++ b/src/gallium/docs/source/context.rst
> >> @@ -740,7 +740,25 @@ of the buffer is not a multiple of the page size,
> changing the commit state of
> >> the last (partial) page requires a box that ends at the end of the
> buffer
> >> (i.e., box->x + box->width == buffer->width0).
> >>
> >> +.. _invalidate_surface:
> >>
> >> +invalidate_surface
> >> +%%%%%%%%%%%%%%%%%%
> >> +
> >> +This optional function marks a surface, or a portion of a surface, as
> invalid,
> >> +for example to implement glInvalidateFramebuffer() (and friends). It
> is
> >> +useful in particular for tiler GPUs, as a way to avoid unnecessary
> transfers
> >> +between system memory and tile buffer.
> >> +
> >> +For example, invalidating a surface after rendering, but before flush,
> can
> >> +be used to avoid tile to system memory transfer (for example, if zs
> can be
> >> +discarded). And an invalidate after flush before rendering can be
> used to
> >> +avoid a system memory to tile buffer transfer.
> >> +
> >> +Invalidating a partial surface can also be used to avoid unnecessary
> transfer
> >> +from system memory to tile buffer in the case of a scissored clear
> (which is
> >> +implemented via ->draw_vbo() by the state tracker) by invalidating the
> >> +scissored region.
> >>
> >> .. _pipe_transfer:
> >>
> >> diff --git a/src/gallium/include/pipe/p_context.h
> b/src/gallium/include/pipe/p_context.h
> >> index e07b76d4f03..4dfc87faff6 100644
> >> --- a/src/gallium/include/pipe/p_context.h
> >> +++ b/src/gallium/include/pipe/p_context.h
> >> @@ -803,6 +803,24 @@ struct pipe_context {
> >> void (*invalidate_resource)(struct pipe_context *ctx,
> >> struct pipe_resource *resource);
> >>
> >> + /**
> >> + * Like ->invalidate_resource, but can invalidate a specific layer
> and level
> >> + * of a resource, and optionally a specific sub-region of the
> resource (if
> >> + * region is not NULL).
> >> + *
> >> + * If the backing surf->texture has just a single layer and level
> (like
> >> + * window system buffers), and region is NULL, it is equivalent to
> >> + * ->invalidate_resource().
> >> + *
> >> + * \param ctx pipe context
> >> + * \param surf surface to invalidate
> >> + * \param region NULL to invalidate whole surface, otherwise
> specifies which
> >> + * portion of the surface is invalidated
> >> + */
> >> + void (*invalidate_surface)(struct pipe_context *ctx,
> >> + struct pipe_surface *surf,
> >> + const struct pipe_box *region);
> >> +
> >> /**
> >> * Return information about unexpected device resets.
> >> */
> >> --
> >> 2.19.2
> >>
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
