[Mesa-dev] [PATCH] mesa/st: swap order of clear() and clear_with_quad()

[Marek Olšák]

On Wed, Nov 14, 2018, 10:36 AM Rob Clark <robdclark at gmail.com wrote:

> On Wed, Nov 14, 2018 at 10:13 AM Marek Olšák <maraeo at gmail.com> wrote:
> >
> >
> >
> > On Wed, Nov 14, 2018, 7:54 AM Rob Clark <robdclark at gmail.com wrote:
> >>
> >> On Tue, Nov 13, 2018 at 9:54 PM Marek Olšák <maraeo at gmail.com> wrote:
> >> >
> >> >
> >> >
> >> > On Tue, Nov 13, 2018, 6:00 PM Rob Clark <robdclark at gmail.com wrote:
> >> >>
> >> >> On Tue, Nov 13, 2018 at 5:25 PM Eric Anholt <eric at anholt.net> wrote:
> >> >> >
> >> >> > Rob Clark <robdclark at gmail.com> writes:
> >> >> >
> >> >> > > If we can't clear all the buffers with pctx->clear() (say, for
> example,
> >> >> > > because of ColorMask), push the buffers we *can* clear with
> pctx->clear()
> >> >> > > first.  Tilers want to see clears coming before draws to enable
> fast-
> >> >> > > paths, and clearing one of the attachments with a quad-draw first
> >> >> > > confuses that logic.
> >> >> >
> >> >> > Oh, nice!
> >> >> >
> >> >> > Reviewed-by: Eric Anholt <eric at anholt.net>
> >> >> >
> >> >> > Though it feels pretty silly that the ->clear() caller needs a
> >> >> > clear_with_quad implementation when the ->clear() implementation
> in the
> >> >> > driver also needs a clear_with_quad implementation for
> non-fast-cleared
> >> >> > buffers.  :/
> >> >>
> >> >> hmm, so perhaps one easy option is to change pctx->clear() to return
> a
> >> >> boolean, so driver can return false to ask the state tracker to do a
> >> >
> >> >
> >> > Ideally all pipe context functions should always return void to allow
> gallium multithreading.
> >>
> >> Hmm, that is a bit awkward..  and a pipe cap is a bit restrictive, ie.
> >> say you can fast-clear only certain color formats, or other weird
> >> combination of restrictions.
> >>
> >> I suppose a pctx->can_do_clear(..) which isn't multithreaded would be
> >> a bit more flexible than pipe caps.
> >
> >
> > can_do_clear(..) should be in pipe_screen. I wouldn't like to have
> context functions that return a value.
>
> that means we'd have to pass it framebuffer state, scissor state, and
> colormask.. which is a bit annoying and less of a trivial change to
> allow drivers to delete some code..
>

A driver can also set a flag in pipe_resource to indicate it's clearable.
Drivers still have to implement the quad clear for ignorant state trackers.

I would say that at least all desktop GPUs would want to use
u_threaded_context, so it doesn't make sense to add any queries into
pipe_context.

Marek


> BR,
> -R
>
> > Marek
> >
> >>
> >> > A pipe cap for colormasked and scissored clears would be better.
> >> >
> >> >
> >> >> clear_with_quad()..  maybe that would be a first step towards
> allowing
> >> >> driver to handle clears w/ colormask and possibly scissor (although
> >> >> for the later, plus
> >> >> glInvalidateFramebuffer()/glInvalidateSubFramebuffer(), I was
> thinking
> >> >> of pctx->invalidate_surface()/pctx->invalidate_sub_surface())
> >> >
> >> >
> >> > Pipe context already contains an invalidate function.
> >>
> >> yeah, but it is at the level of pipe_resource, which isn't really what
> >> you want if only a certain level/layer of a resource is invalidated.
> >>
> >> I'm still unsure about how I want to track this in the driver, the
> >> ideal case would be to track it in fd_surface, except that surfaces
> >> tend to be created transiently.  Still for glInvalidateFramebuffer()
> >> and friends, having an pctx->invalidate_surface() seems like the
> >> cleaner API.
> >>
> >> BR,
> >> -R
> >>
> >> >
> >> > Marek
> >> >
> >> >>
> >> >> But either way, I guess this patch is a simple stop-gap solution.
> >> >>
> >> >> BR,
> >> >> -R
> >> >> _______________________________________________
> >> >> mesa-dev mailing list
> >> >> mesa-dev at lists.freedesktop.org
> >> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181114/ad91f97a/attachment-0001.html>